From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39938) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UULeN-0007vi-5J for qemu-devel@nongnu.org; Mon, 22 Apr 2013 14:39:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UULeL-0006MM-DM for qemu-devel@nongnu.org; Mon, 22 Apr 2013 14:39:39 -0400 Message-ID: <1366655966.2918.414.camel@bling.home> From: Alex Williamson Date: Mon, 22 Apr 2013 12:39:26 -0600 In-Reply-To: <1366617757-6604-1-git-send-email-aik@ozlabs.ru> References: <1366617757-6604-1-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/3 QEMU v4] vfio on ppc64 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, David Gibson On Mon, 2013-04-22 at 18:02 +1000, Alexey Kardashevskiy wrote: > Yes, we are still tuning this stuff for us :) Yay! > Changes: > > * new "spapr-pci-vfio-host-bridge" device is introduced > (inherits from spapr-pci-host-bridge); > > * device#1->group->container->device#2,.. creation scheme is changed to > group->container->devices scheme (on ppc64); > > * removed vfio_iommu_spapr_tce_dma_map/vfio_iommu_spapr_tce_dma_unmap > as they are not really necessary now and it does not look like we will > need them in the nearest future. > > Comments? Not happy with this... :( > Alexey Kardashevskiy (3): > vfio: make some of VFIO API public Exports and conditionalizes vfio_get_group for no good reason other than to get a pointer to a group that's not connected to a container. Not only is the bool parameter ugly, it has a poor interface - call it once with "false" and get an unconnected group, call it for the same group with "true", and still get back an unconnected group. I'd say vfio_get_group should be split to have group and container connect separate, but I don't think you even need that... > vfio: add support for spapr platform (powerpc64 server) ...because this patch ignores the extensibility of vfio_connect_container. There is absolutely no reason for vfio_container_spapr_alloc to exist. It duplicates most of vfio_connect_container, apparently using the attempt to reuse containers as an excuse, even though this should happily fall through unless the host code is broken. vfio supports containers capable of multiple groups, it does not assume it or require it. > spapr vfio: add spapr-pci-vfio-host-bridge to support vfio And obviously this is the consumer of all that, which really just wants some parameters from the iommu info ioctl and a way to call map and unmap for dma. What if instead of all of the above, we simply: 1) export vfio_dma_map/vfio_dma_unmap 2) add spapr support to vfio_connect_container (skip the info and enable chunks) 2) add to vfio.c and export: VFIOContainer *vfio_spapr_php_init(int groupid, struct vfio_iommu_spapr_tce_info *info) { vfio_get_group(groupid); // including connect ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, info); ioctl(container->fd, VFIO_IOMMU_ENABLE); return group->container; } Isn't that all you really need? Thanks, Alex > hw/misc/vfio.c | 94 ++++++++++++++++++++-- > hw/ppc/spapr_iommu.c | 151 +++++++++++++++++++++++++++-------- > hw/ppc/spapr_pci.c | 186 +++++++++++++++++++++++++++++++++++++++++-- > include/hw/misc/vfio.h | 20 +++++ > include/hw/pci-host/spapr.h | 12 +++ > include/hw/ppc/spapr.h | 3 + > trace-events | 4 + > 7 files changed, 422 insertions(+), 48 deletions(-) > create mode 100644 include/hw/misc/vfio.h >