> From: David Gibson > Sent: Tuesday, November 5, 2019 12:02 AM > To: Liu, Yi L <yi.l....@intel.com> > Subject: Re: [RFC v2 14/22] vfio/pci: add iommu_context notifier for pasid > bind/unbind > > On Thu, Oct 24, 2019 at 08:34:35AM -0400, Liu Yi L wrote: > > This patch adds notifier for pasid bind/unbind. VFIO registers this > > notifier to listen to the dual-stage translation (a.k.a. nested > > translation) configuration changes and propagate to host. Thus vIOMMU > > is able to set its translation structures to host. > > > > Cc: Kevin Tian <kevin.t...@intel.com> > > Cc: Jacob Pan <jacob.jun....@linux.intel.com> > > Cc: Peter Xu <pet...@redhat.com> > > Cc: Eric Auger <eric.au...@redhat.com> > > Cc: Yi Sun <yi.y....@linux.intel.com> > > Cc: David Gibson <da...@gibson.dropbear.id.au> > > Signed-off-by: Liu Yi L <yi.l....@intel.com> > > --- > > hw/vfio/pci.c | 39 +++++++++++++++++++++++++++++++++++++++ > > include/hw/iommu/iommu.h | 11 +++++++++++ > > 2 files changed, 50 insertions(+) > > > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > > index 8721ff6..012b8ed 100644 > > --- a/hw/vfio/pci.c > > +++ b/hw/vfio/pci.c > > @@ -2767,6 +2767,41 @@ static void > vfio_iommu_pasid_free_notify(IOMMUCTXNotifier *n, > > pasid_req->free_result = ret; > > } > > > > +static void vfio_iommu_pasid_bind_notify(IOMMUCTXNotifier *n, > > + IOMMUCTXEventData *event_data) > > +{ > > +#ifdef __linux__ > > Is hw/vfio/pci.c even built on non-linux hosts?
I'm not quite sure. It's based a comment from RFC v1. I think it could somehow prevent compiling issue when doing code porting. So I added it. If it's impossible to build on non-linux hosts per your experience, I can remove it to make things simple. > > + VFIOIOMMUContext *giommu_ctx = container_of(n, VFIOIOMMUContext, n); > > + VFIOContainer *container = giommu_ctx->container; > > + IOMMUCTXPASIDBindData *pasid_bind = > > + (IOMMUCTXPASIDBindData *) event_data->data; > > + struct vfio_iommu_type1_bind *bind; > > + struct iommu_gpasid_bind_data *bind_data; > > + unsigned long argsz; > > + > > + argsz = sizeof(*bind) + sizeof(*bind_data); > > + bind = g_malloc0(argsz); > > + bind->argsz = argsz; > > + bind->bind_type = VFIO_IOMMU_BIND_GUEST_PASID; > > + bind_data = (struct iommu_gpasid_bind_data *) &bind->data; > > + *bind_data = *pasid_bind->data; > > + > > + if (pasid_bind->flag & IOMMU_CTX_BIND_PASID) { > > + if (ioctl(container->fd, VFIO_IOMMU_BIND, bind) != 0) { > > + error_report("%s: pasid (%llu:%llu) bind failed: %d", __func__, > > + bind_data->gpasid, bind_data->hpasid, -errno); > > + } > > + } else if (pasid_bind->flag & IOMMU_CTX_UNBIND_PASID) { > > + if (ioctl(container->fd, VFIO_IOMMU_UNBIND, bind) != 0) { > > + error_report("%s: pasid (%llu:%llu) unbind failed: %d", > > __func__, > > + bind_data->gpasid, bind_data->hpasid, -errno); > > + } > > + } > > + > > + g_free(bind); > > +#endif > > +} > > + > > static void vfio_realize(PCIDevice *pdev, Error **errp) > > { > > VFIOPCIDevice *vdev = PCI_VFIO(pdev); > > @@ -3079,6 +3114,10 @@ static void vfio_realize(PCIDevice *pdev, Error > > **errp) > > iommu_context, > > vfio_iommu_pasid_free_notify, > > IOMMU_CTX_EVENT_PASID_FREE); > > + vfio_register_iommu_ctx_notifier(vdev, > > + iommu_context, > > + vfio_iommu_pasid_bind_notify, > > + IOMMU_CTX_EVENT_PASID_BIND); > > } > > > > return; > > diff --git a/include/hw/iommu/iommu.h b/include/hw/iommu/iommu.h > > index 4352afd..4f21aa1 100644 > > --- a/include/hw/iommu/iommu.h > > +++ b/include/hw/iommu/iommu.h > > @@ -33,6 +33,7 @@ typedef struct IOMMUContext IOMMUContext; > > enum IOMMUCTXEvent { > > IOMMU_CTX_EVENT_PASID_ALLOC, > > IOMMU_CTX_EVENT_PASID_FREE, > > + IOMMU_CTX_EVENT_PASID_BIND, > > IOMMU_CTX_EVENT_NUM, > > }; > > typedef enum IOMMUCTXEvent IOMMUCTXEvent; > > @@ -50,6 +51,16 @@ union IOMMUCTXPASIDReqDesc { > > }; > > typedef union IOMMUCTXPASIDReqDesc IOMMUCTXPASIDReqDesc; > > > > +struct IOMMUCTXPASIDBindData { > > +#define IOMMU_CTX_BIND_PASID (1 << 0) > > +#define IOMMU_CTX_UNBIND_PASID (1 << 1) > > + uint32_t flag; > > +#ifdef __linux__ > > + struct iommu_gpasid_bind_data *data; > > Embedding a linux specific structure in the notification message seems > dubious to me. Just similar as your above comment in this thread. If we don't want to add it there, then here it is also unnecessary. @Eric, do you think it is still necessary to add the __linux__ marco here? Thanks, Yi Liu