Re: [Qemu-devel] [Qemu-ppc] [PATCH] RFC: vfio-powerpc: added VFIO support (v2)

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 2012-07-12 at 20:54 +0000, Blue Swirl wrote:
> On Thu, Jul 12, 2012 at 8:52 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > It literally does the following:
> >
> > 1. POWERPC IOMMU support (the kernel counterpart is required)
> >
> > 2. The patch assumes that IOAPIC calls are going to be replaced
> > with something generic. I have something in my local git but it's
> > too early, we need to extend PCIINTxRoute first.
> >
> > 3. vfio_get_group() made public. I want to open IOMMU group from
> > the sPAPR code to have everything I need for VFIO on sPAPR and
> > avoid ugly workarounds with finilizing PHB setup on sPAPR.
> >
> > 4. Change sPAPR PHB to scan the PCI bus which is used for
> > the IOMMU-VFIO group. Now it is enough to add the following to
> > the QEMU command line to get VFIO up with all the devices from
> > IOMMU group with id=3:
> > -device spapr-pci-host-bridge,busname=E1000E,buid=0x3,iommu=3,\
> > mem_win_addr=0x230000000000,io_win_addr=0x240000000000,msi_win_addr=0x250000000000
> >
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> >  hw/ppc/Makefile.objs |    3 ++
> >  hw/spapr.h           |    4 ++
> >  hw/spapr_iommu.c     |   87 ++++++++++++++++++++++++++++++++++++++
> >  hw/spapr_pci.c       |  115 +++++++++++++++++++++++++++++++++++++++++++++++---
> >  hw/spapr_pci.h       |    5 +++
> >  hw/vfio_pci.c        |   28 +++++++++++-
> >  hw/vfio_pci.h        |    2 +
> >  7 files changed, 237 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index f573a95..c46a049 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -25,4 +25,7 @@ obj-$(CONFIG_FDT) += ../device_tree.o
> >  # Xilinx PPC peripherals
> >  obj-y += xilinx_ethlite.o
> >
> > +# VFIO PCI device assignment
> > +obj-$(CONFIG_VFIO_PCI) += vfio_pci.o
> > +
> >  obj-y := $(addprefix ../,$(obj-y))
> > diff --git a/hw/spapr.h b/hw/spapr.h
> > index b37f337..9dca704 100644
> > --- a/hw/spapr.h
> > +++ b/hw/spapr.h
> > @@ -340,4 +340,8 @@ int spapr_dma_dt(void *fdt, int node_off, const char *propname,
> >  int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname,
> >                        DMAContext *dma);
> >
> > +void spapr_vfio_init_dma(int fd, uint32_t liobn,
> > +                         uint64_t *dma32_window_start,
> > +                         uint64_t *dma32_window_size);
> > +
> >  #endif /* !defined (__HW_SPAPR_H__) */
> > diff --git a/hw/spapr_iommu.c b/hw/spapr_iommu.c
> > index 50c288d..0a194e8 100644
> > --- a/hw/spapr_iommu.c
> > +++ b/hw/spapr_iommu.c
> > @@ -16,6 +16,8 @@
> >   * You should have received a copy of the GNU Lesser General Public
> >   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> >   */
> > +#include <sys/ioctl.h>
> > +
> >  #include "hw.h"
> >  #include "kvm.h"
> >  #include "qdev.h"
> > @@ -23,6 +25,7 @@
> >  #include "dma.h"
> >
> >  #include "hw/spapr.h"
> > +#include "hw/linux-vfio.h"
> >
> >  #include <libfdt.h>
> >
> > @@ -183,6 +186,86 @@ static int put_tce_emu(target_ulong liobn, target_ulong ioba, target_ulong tce)
> >      return 0;
> >  }
> >
> > +/* -------- API for POWERPC IOMMU -------- */
> > +
> > +#define POWERPC_IOMMU           2
> > +
> > +struct tce_iommu_info {
> 
> CamelCase.
> 
> > +    __u32 argsz;
> > +    __u32 dma32_window_start;
> > +    __u32 dma32_window_size;
> 
> Please use uint32_t.

These should eventually be included from a kernel header file.  I assume
that's the reason for the non-qemu-isms.

> > +};
> > +
> > +#define POWERPC_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
> > +
> > +struct tce_iommu_dma_map {
> > +    __u32 argsz;
> 
> The structure may or may not be padded here since there's no
> QEMU_PACKED attribute. If possible, just rearrange the fields.

I'm hoping Alexey adds a __u32 flags here, which solves that problem as
well.  Thanks,

Alex