From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43332) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wt2vI-0000kJ-St for qemu-devel@nongnu.org; Fri, 06 Jun 2014 18:47:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wt2v9-0003Vw-UA for qemu-devel@nongnu.org; Fri, 06 Jun 2014 18:47:44 -0400 Received: from mail-pb0-f49.google.com ([209.85.160.49]:35984) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wt2v9-0003Vf-MM for qemu-devel@nongnu.org; Fri, 06 Jun 2014 18:47:35 -0400 Received: by mail-pb0-f49.google.com with SMTP id jt11so3035863pbb.36 for ; Fri, 06 Jun 2014 15:47:34 -0700 (PDT) Message-ID: <539244FF.7000007@ozlabs.ru> Date: Sat, 07 Jun 2014 08:47:27 +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> In-Reply-To: <1402073878.14174.12.camel@ul30vt.home> 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 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. > 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