Re: [Qemu-ppc] [PATCH qemu v13 15/16] vfio: Move iova_pgsizes from container to guest IOMMU

[Alexey Kardashevskiy]

On 03/03/2016 10:22 PM, David Gibson wrote:
> On Tue, Mar 01, 2016 at 08:10:40PM +1100, Alexey Kardashevskiy wrote:
> > The page size is an attribute of an IOMMU, not a container as a container
> > may contain more just one IOMMU.
> >
> > This moves iova_pgsizes from VFIOContainer to VFIOGuestIOMMU.
> > The following patch will use this.
> >
> > This removes iova_pgsizes from Type1 IOMMU as it is not used there anyway
> > and when it will get guest visible IOMMU, it will use VFIOGuestIOMMU's
> > iova_pgsizes.
> >
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>
>
> Hmm.  This makes an important semantic change which.. I'm not sure is
> wrong, but certainly isn't adequately addressed in your commit
> message.
>
> The current iova_pgsizes is populated with information about the
> *host* IOMMU, whereas you're replacing it with information about the
> *guest* IOMMU.


Ah, did not realize that. Then it should be not a move but an additional 
giommu->iova_pgsizes. And this probably answers todo#1 in 16/16 about page 
masks.



> > ---
> >   hw/vfio/common.c              | 16 ++++------------
> >   include/hw/vfio/vfio-common.h |  2 +-
> >   2 files changed, 5 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index f2a03e0..42ef1eb 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -313,9 +313,9 @@ out:
> >       rcu_read_unlock();
> >   }
> >
> > -static hwaddr vfio_container_granularity(VFIOContainer *container)
> > +static hwaddr vfio_container_granularity(VFIOGuestIOMMU *giommu)
> >   {
> > -    return (hwaddr)1 << ctz64(container->iova_pgsizes);
> > +    return (hwaddr)1 << ctz64(giommu->iova_pgsizes);
> >   }
> >
> >   static hwaddr vfio_iommu_page_mask(MemoryRegion *mr)
> > @@ -392,12 +392,13 @@ static void vfio_listener_region_add(VFIOMemoryListener 
> > *vlistener,
> >               section->offset_within_address_space;
> >           giommu->container = container;
> >           giommu->n.notify = vfio_iommu_map_notify;
> > +        giommu->iova_pgsizes = 
> > section->mr->iommu_ops->get_page_sizes(section->mr);
> >           QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
> >
> >           memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> >           giommu->iommu->iommu_ops->vfio_notify(section->mr, true);
> >           memory_region_iommu_replay(giommu->iommu, &giommu->n,
> > -                                   vfio_container_granularity(container),
> > +                                   vfio_container_granularity(giommu),
> >                                      false);
> >
> >           return;
> > @@ -743,14 +744,8 @@ static int vfio_connect_container(VFIOGroup *group, 
> > AddressSpace *as)
> >           container->min_iova = 0;
> >           container->max_iova = (hwaddr)-1;
> >
> > -        /* Assume just 4K IOVA page size */
> > -        container->iova_pgsizes = 0x1000;
> >           info.argsz = sizeof(info);
> >           ret = ioctl(fd, VFIO_IOMMU_GET_INFO, &info);
> > -        /* Ignore errors */
> > -        if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
> > -            container->iova_pgsizes = info.iova_pgsizes;
> > -        }
> >       } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> >                  ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
> >           struct vfio_iommu_spapr_tce_info info;
> > @@ -811,9 +806,6 @@ static int vfio_connect_container(VFIOGroup *group, 
> > AddressSpace *as)
> >           }
> >           container->min_iova = info.dma32_window_start;
> >           container->max_iova = container->min_iova + info.dma32_window_size - 
> > 1;
> > -
> > -        /* Assume just 4K IOVA pages for now */
> > -        container->iova_pgsizes = 0x1000;
> >       } else {
> >           error_report("vfio: No available IOMMU models");
> >           ret = -EINVAL;
> > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> > index bcbc5cb..48a1d7f 100644
> > --- a/include/hw/vfio/vfio-common.h
> > +++ b/include/hw/vfio/vfio-common.h
> > @@ -80,7 +80,6 @@ typedef struct VFIOContainer {
> >        * future
> >        */
> >       hwaddr min_iova, max_iova;
> > -    uint64_t iova_pgsizes;
> >       QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
> >       QLIST_HEAD(, VFIOGroup) group_list;
> >       QLIST_ENTRY(VFIOContainer) next;
> > @@ -90,6 +89,7 @@ typedef struct VFIOGuestIOMMU {
> >       VFIOContainer *container;
> >       MemoryRegion *iommu;
> >       hwaddr offset_within_address_space;
> > +    uint64_t iova_pgsizes;
> >       Notifier n;
> >       QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
> >   } VFIOGuestIOMMU;


--
Alexey