From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47898) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2QzA-0000Qr-1D for qemu-devel@nongnu.org; Tue, 09 Jun 2015 17:23:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2Qz9-00010t-0j for qemu-devel@nongnu.org; Tue, 09 Jun 2015 17:23:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50098) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2Qz8-00010n-Rf for qemu-devel@nongnu.org; Tue, 09 Jun 2015 17:23:02 -0400 Message-ID: <1433884981.4927.143.camel@redhat.com> From: Alex Williamson Date: Tue, 09 Jun 2015 15:23:01 -0600 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v9 08/18] vfio: improve vfio_get_group to support adding group without devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chen Fan Cc: izumi.taku@jp.fujitsu.com, qemu-devel@nongnu.org On Tue, 2015-06-09 at 11:37 +0800, Chen Fan wrote: > Pre-adding all affected groups for aer devices, it could > ensure the affected groups are owned in VM. > > Signed-off-by: Chen Fan > --- > hw/vfio/common.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index b1045da..4230f83 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -793,8 +793,15 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) > > QLIST_FOREACH(group, &vfio_group_list, next) { > if (group->groupid == groupid) { > + if (as && !group->container) { > + if (vfio_connect_container(group, as)) { > + error_report("vfio: failed to setup container for group %d", > + groupid); > + return NULL; > + } > + } > /* Found it. Now is it already in the right context? */ > - if (group->container->space->as == as) { > + if (!as || group->container->space->as == as) { > return group; > } else { > error_report("vfio: group %d used in multiple address spaces", > @@ -828,7 +835,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) > group->groupid = groupid; > QLIST_INIT(&group->device_list); > > - if (vfio_connect_container(group, as)) { > + if (as && vfio_connect_container(group, as)) { > error_report("vfio: failed to setup container for group %d", groupid); > goto close_fd_exit; > } > @@ -859,7 +866,9 @@ void vfio_put_group(VFIOGroup *group) > } > > vfio_kvm_device_del_group(group); > - vfio_disconnect_container(group); > + if (group->container) { > + vfio_disconnect_container(group); > + } > QLIST_REMOVE(group, next); > trace_vfio_put_group(group->fd); > close(group->fd); It's an interesting idea, but should we instead separate the container from vfio_get/put_group() to be done as a separate step? Using the AddressSpace pointer is a bit obfuscated. Also, this gets you the group, but the vfio_group_get_external_user() interface used by vfio-pci to do a bus reset requires that the group is active, with an iommu. If it didn't, we could potentially be resetting a non-viable group, with devices still owned by the host or not isolated by the iommu. So you're a step closer, but if we go back to the example of a dual port NIC with isolated functions, QEMU may be able to get the group for the second port, it may even be a viable group, but if the second port doesn't eventually get connected to the VM, the bus reset won't be allowed.