Re: [Qemu-devel] [PATCH qemu v5 1/2] memory/iommu/vfio: Define add_vfio_group() callback

[Alex Williamson]

On Fri, 1 Dec 2017 08:56:42 +1100
Alexey Kardashevskiy <address@hidden> wrote:

> On 05/10/17 16:50, Alexey Kardashevskiy wrote:
> > The new callback will be called when a new VFIO IOMMU group is added.
> > 
> > This should cause no behavioral change, the next patch will.
> > 
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>  
> 
> 
> What about this one,  any conclusion yet?

Patchew reported that if failed to build and you never replied with an
explanation, so I haven't looked at it.

> > ---
> > 
> > This could be at the higher level - in MemoryRegionOps, makes sense?
> > 
> > ---
> >  include/exec/memory.h |  4 ++++
> >  hw/vfio/common.c      | 15 +++++++++++++++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index 5ed4042f87..64e0b4fc96 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -210,6 +210,10 @@ typedef struct IOMMUMemoryRegionClass {
> >                                  IOMMUNotifierFlag new_flags);
> >      /* Set this up to provide customized IOMMU replay function */
> >      void (*replay)(IOMMUMemoryRegion *iommu, IOMMUNotifier *notifier);
> > +
> > +    /* Notifies IOMMUMR about attached VFIO IOMMU group */
> > +    void (*add_vfio_group)(IOMMUMemoryRegion *iommu_mr, int vfio_kvm_fd,
> > +                           int groupfd);

If there's an "add" why is there no remove?

Clearly a vfio specific callback is pretty much a failure as far as
abstraction and layering is concerned.

> >  } IOMMUMemoryRegionClass;
> >  
> >  typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index 7b2924c0ef..9e861e0393 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -481,6 +481,21 @@ static void vfio_listener_region_add(MemoryListener 
> > *listener,
> >          VFIOGuestIOMMU *giommu;
> >          IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
> >  
> > +#ifdef CONFIG_KVM
> > +        if (kvm_enabled()) {
> > +            VFIOGroup *group;
> > +            IOMMUMemoryRegionClass *imrc =
> > +                IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
> > +
> > +            QLIST_FOREACH(group, &container->group_list, container_next) {
> > +                if (imrc->add_vfio_group) {
> > +                    imrc->add_vfio_group(iommu_mr, vfio_kvm_device_fd,
> > +                                         group->fd);
> > +                }
> > +            }
> > +        }
> > +#endif
> > +
> >          trace_vfio_listener_region_add_iommu(iova, end);
> >          /*
> >           * FIXME: For VFIO iommu types which have KVM acceleration to
> >   
> 
>