From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50923) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtQMX-0000ff-89 for qemu-devel@nongnu.org; Sat, 07 Jun 2014 19:49:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WtQMP-0001YZ-Hx for qemu-devel@nongnu.org; Sat, 07 Jun 2014 19:49:25 -0400 Received: from mail-pd0-f170.google.com ([209.85.192.170]:46797) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtQMP-0001YS-9Q for qemu-devel@nongnu.org; Sat, 07 Jun 2014 19:49:17 -0400 Received: by mail-pd0-f170.google.com with SMTP id g10so3882621pdj.1 for ; Sat, 07 Jun 2014 16:49:16 -0700 (PDT) Message-ID: <5393A4F6.50508@ozlabs.ru> Date: Sun, 08 Jun 2014 09:49:10 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1402025693-25096-1-git-send-email-aik@ozlabs.ru> <1402025693-25096-5-git-send-email-aik@ozlabs.ru> <1402073878.14174.12.camel@ul30vt.home> <539244FF.7000007@ozlabs.ru> In-Reply-To: <539244FF.7000007@ozlabs.ru> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v8 4/4] vfio: Enable for SPAPR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Gavin Shan , Alexander Graf On 06/07/2014 08:47 AM, Alexey Kardashevskiy wrote: > On 06/07/2014 02:57 AM, Alex Williamson wrote: >> On Fri, 2014-06-06 at 13:34 +1000, Alexey Kardashevskiy wrote: >>> This turns the sPAPR support on and enables VFIO container use >>> in the kernel. >>> >>> This extends vfio_connect_container to support VFIO_SPAPR_TCE_IOMMU type >>> in the host kernel. >>> >>> This registers a memory listener which sPAPR IOMMU will notify when >>> executing H_PUT_TCE/etc DMA calls. The listener then will notify the host >>> kernel about DMA map/unmap operation via VFIO_IOMMU_MAP_DMA/ >>> VFIO_IOMMU_UNMAP_DMA ioctls. >>> >>> This executes VFIO_IOMMU_ENABLE ioctl to make sure that the IOMMU is free >>> of mappings and can be exclusively given to the user. At the moment SPAPR >>> is the only platform requiring this call to be implemented. >>> >>> Note that the host kernel function implementing VFIO_IOMMU_DISABLE >>> is called automatically when container's fd is closed so there is >>> no need to call it explicitly from QEMU. This may change in the future. >> >> So you're saying we rely on a behavior that may change in the future... >> The kernel must do cleanup, it can never rely on userspace to do the >> right thing or we have a bug. Does that mean we can remove the "may >> change in the future" part of this comment or is that directed at QEMU's >> behavior and not the kernel's? > > > I wanted to say that if/when we support PCI hotplug or dynamic IOMMU group > reconfiguration (we might be able to do so in POWER9 or later, do not know > details), we will call DISABLE explicitly. Any better? === Note that the host kernel function implementing VFIO_IOMMU_DISABLE is called automatically when container's fd is closed so there is no need to call it explicitly from QEMU. We may need to call VFIO_IOMMU_DISABLE explicitely in the future for some sort of dynamic reconfiguration (PCI hotplug or dynamic IOMMU group management). === Thanks! > > >> A comment in the code where we setup the >> release handler or call ENABLE would be a good idea too. Thanks, > > > > >> >> Alex >> >>> Signed-off-by: Alexey Kardashevskiy >>> --- >>> Changes: >>> v8: >>> * added note about VFIO_IOMMU_DISABLE in the commit log >>> >>> v7: >>> * added more details in commit log >>> >>> v5: >>> * multiple returns converted to gotos >>> >>> v4: >>> * fixed format string to use %m which is a glibc extension: >>> "Print output of strerror(errno). No argument is required." >>> --- >>> hw/misc/vfio.c | 28 ++++++++++++++++++++++++++++ >>> 1 file changed, 28 insertions(+) >>> >>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c >>> index bb77934..78d2045 100644 >>> --- a/hw/misc/vfio.c >>> +++ b/hw/misc/vfio.c >>> @@ -3650,6 +3650,34 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as) >>> >>> container->iommu_data.type1.initialized = true; >>> >>> + } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) { >>> + ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd); >>> + if (ret) { >>> + error_report("vfio: failed to set group container: %m"); >>> + ret = -errno; >>> + goto free_container_exit; >>> + } >>> + >>> + ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU); >>> + if (ret) { >>> + error_report("vfio: failed to set iommu for container: %m"); >>> + ret = -errno; >>> + goto free_container_exit; >>> + } >>> + >>> + ret = ioctl(fd, VFIO_IOMMU_ENABLE); >>> + if (ret) { >>> + error_report("vfio: failed to enable container: %m"); >>> + ret = -errno; >>> + goto free_container_exit; >>> + } >>> + >>> + container->iommu_data.type1.listener = vfio_memory_listener; >>> + container->iommu_data.release = vfio_listener_release; >>> + >>> + memory_listener_register(&container->iommu_data.type1.listener, >>> + container->space->as); >>> + >>> } else { >>> error_report("vfio: No available IOMMU models"); >>> ret = -EINVAL; >> >> >> > > -- Alexey