Re: [Qemu-devel] [RESEND PATCH 5/6] vfio/pci: add notify framework based on IOMMUObject

[Liu, Yi L]

Hi Eric,

On Tue, Nov 14, 2017 at 11:23:03AM +0100, Auger Eric wrote:
> Hi Yi L,
> 
> On 03/11/2017 13:01, Liu, Yi L wrote:
> > This patch introduce a notify framework for IOMMUObject.iommu_notifiers.
> > Introduce VFIOGuestIOMMUObject is to link VFIO Container and the new
> > IOMMUObject notififiers.
> notifiers
> > 
> > VFIOGuestIOMMUObject instance is allocated when device is assigned and
> > meanwhile vIOMMU is exposed to guest.
> > 
> > If there is IOMMUObject behind the device AddressSpace(a.ka vIOMMU exposed).
> > The VFIOGuestIOMMUObject instance would be allocated and inserted to the
> > VFIOContainer.giommu_object_list.
> > 
> > Signed-off-by: Liu, Yi L <address@hidden>
> > ---
> >  hw/vfio/pci.c                 | 39 ++++++++++++++++++++++++++++++++++++++-
> >  include/hw/vfio/vfio-common.h |  8 ++++++++
> >  2 files changed, 46 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index c977ee3..5b77c7e 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -2642,6 +2642,8 @@ static void vfio_realize(PCIDevice *pdev, Error 
> > **errp)
> >      VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> >      VFIODevice *vbasedev_iter;
> >      VFIOGroup *group;
> > +    AddressSpace *as;
> > +    IOMMUObject *iommu;
> >      char *tmp, group_path[PATH_MAX], *group_name;
> >      Error *err = NULL;
> >      ssize_t len;
> > @@ -2694,7 +2696,8 @@ static void vfio_realize(PCIDevice *pdev, Error 
> > **errp)
> >  
> >      trace_vfio_realize(vdev->vbasedev.name, groupid);
> >  
> > -    group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), 
> > errp);
> > +    as = pci_device_iommu_address_space(pdev);
> > +    group = vfio_get_group(groupid, as, errp);
> >      if (!group) {
> >          goto error;
> >      }
> > @@ -2877,6 +2880,17 @@ static void vfio_realize(PCIDevice *pdev, Error 
> > **errp)
> >      vfio_register_req_notifier(vdev);
> >      vfio_setup_resetfn_quirk(vdev);
> >  
> > +    iommu = address_space_iommu_get(as);
> > +    if (iommu != NULL) {
> > +        VFIOGuestIOMMUObject *giommu;
> nit: void line needed

accepted.

> > +        giommu = g_malloc0(sizeof(*giommu));
> > +        giommu->iommu = iommu;
> > +        giommu->container = group->container;
> > +        QLIST_INSERT_HEAD(&group->container->giommu_object_list,
> There is no QLIST_INIT anywhere for container's giommu_object_list?

yes, need to add. thx for the catching.

Thanks,
Yi L

> 
> Thanks
> 
> Eric
> > +                          giommu,
> > +                          giommu_next);
> > +    }
> > +
> >      return;
> >  
> >  out_teardown:
> > @@ -2907,6 +2921,28 @@ static void vfio_instance_finalize(Object *obj)
> >      vfio_put_group(group);
> >  }
> >  
> > +static void vfio_release_iommu_object(PCIDevice *pdev)
> > +{
> > +    VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> > +    AddressSpace *as;
> > +    IOMMUObject *iommu;
> > +
> > +    as = pci_device_iommu_address_space(pdev);
> > +    iommu = address_space_iommu_get(as);
> > +    if (iommu != NULL) {
> > +        VFIOGuestIOMMUObject *giommu, *tmp;
> > +        VFIOGroup *group;
> > +        group = vdev->vbasedev.group;
> > +
> > +        QLIST_FOREACH_SAFE(giommu,
> > +                           &group->container->giommu_object_list,
> > +                           giommu_next, tmp) {
> > +            QLIST_REMOVE(giommu, giommu_next);
> > +            g_free(giommu);
> > +        }
> > +    }
> > +    return;
> > +}
> >  static void vfio_exitfn(PCIDevice *pdev)
> >  {
> >      VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> > @@ -2915,6 +2951,7 @@ static void vfio_exitfn(PCIDevice *pdev)
> >      vfio_unregister_err_notifier(vdev);
> >      pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
> >      vfio_disable_interrupts(vdev);
> > +    vfio_release_iommu_object(pdev);
> >      if (vdev->intx.mmap_timer) {
> >          timer_free(vdev->intx.mmap_timer);
> >      }
> > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> > index 702a085..e4963cc 100644
> > --- a/include/hw/vfio/vfio-common.h
> > +++ b/include/hw/vfio/vfio-common.h
> > @@ -88,6 +88,7 @@ typedef struct VFIOContainer {
> >       * future
> >       */
> >      QLIST_HEAD(, VFIOGuestIOMMUMR) giommu_mr_list;
> > +    QLIST_HEAD(, VFIOGuestIOMMUObject) giommu_object_list;
> >      QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
> >      QLIST_HEAD(, VFIOGroup) group_list;
> >      QLIST_ENTRY(VFIOContainer) next;
> > @@ -101,6 +102,13 @@ typedef struct VFIOGuestIOMMUMR {
> >      QLIST_ENTRY(VFIOGuestIOMMUMR) giommu_next;
> >  } VFIOGuestIOMMUMR;
> >  
> > +typedef struct VFIOGuestIOMMUObject {
> > +    VFIOContainer *container;
> > +    IOMMUObject *iommu;
> > +    IOMMUNotifier n;
> > +    QLIST_ENTRY(VFIOGuestIOMMUObject) giommu_next;
> > +} VFIOGuestIOMMUObject;
> > +
> >  typedef struct VFIOHostDMAWindow {
> >      hwaddr min_iova;
> >      hwaddr max_iova;
> >