From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41031) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ayRwO-000118-MY for qemu-devel@nongnu.org; Thu, 05 May 2016 18:40:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ayRwC-0003xo-Re for qemu-devel@nongnu.org; Thu, 05 May 2016 18:40:11 -0400 Date: Thu, 5 May 2016 16:39:41 -0600 From: Alex Williamson Message-ID: <20160505163941.7628431c@t450s.home> In-Reply-To: <1462344751-28281-2-git-send-email-aik@ozlabs.ru> References: <1462344751-28281-1-git-send-email-aik@ozlabs.ru> <1462344751-28281-2-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH qemu v16 01/19] vfio: Delay DMA address space listener release List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Alexander Graf , David Gibson , Paolo Bonzini On Wed, 4 May 2016 16:52:13 +1000 Alexey Kardashevskiy wrote: > This postpones VFIO container deinitialization to let region_del() > callbacks (called via vfio_listener_release) do proper clean up > while the group is still attached to the container. Any mappings within the container should clean themselves up when the container is deprivleged by removing the last group in the kernel. Is the issue that that doesn't happen, which would be a spapr vfio kernel bug, or that our QEMU side structures get all out of whack if we let that happen? > > Signed-off-by: Alexey Kardashevskiy > --- > hw/vfio/common.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index fe5ec6a..0b40262 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -921,23 +921,31 @@ static void vfio_disconnect_container(VFIOGroup *group) > { > VFIOContainer *container = group->container; > > - if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER, &container->fd)) { > - error_report("vfio: error disconnecting group %d from container", > - group->groupid); > - } > - > QLIST_REMOVE(group, container_next); > + > + if (QLIST_EMPTY(&container->group_list)) { > + VFIOGuestIOMMU *giommu; > + > + vfio_listener_release(container); > + > + QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) { > + memory_region_unregister_iommu_notifier(&giommu->n); > + } > + } > + > group->container = NULL; > + if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER, &container->fd)) { > + error_report("vfio: error disconnecting group %d from container", > + group->groupid); > + } > > if (QLIST_EMPTY(&container->group_list)) { > VFIOAddressSpace *space = container->space; > VFIOGuestIOMMU *giommu, *tmp; > > - vfio_listener_release(container); > QLIST_REMOVE(container, next); > > QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) { > - memory_region_unregister_iommu_notifier(&giommu->n); > QLIST_REMOVE(giommu, giommu_next); > g_free(giommu); > } I'm not spotting why this is a 2-pass process vs simply moving the existing QLIST_EMPTY cleanup above the ioctl. Thanks, Alex