qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/3 QEMU v4] vfio on ppc64


From: Alex Williamson
Subject: Re: [Qemu-devel] [PATCH 0/3 QEMU v4] vfio on ppc64
Date: Mon, 22 Apr 2013 12:39:26 -0600

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
> 






reply via email to

[Prev in Thread] Current Thread [Next in Thread]