| Message ID | 1571920483-3382-15-git-send-email-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | intel_iommu: expose Shared Virtual Addressing to VM | expand |
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.tian@intel.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Peter Xu <peterx@redhat.com> > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Yi Sun <yi.y.sun@linux.intel.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Liu Yi L <yi.l.liu@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? > + 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. > +#endif > +}; > +typedef struct IOMMUCTXPASIDBindData IOMMUCTXPASIDBindData; > + > struct IOMMUCTXEventData { > IOMMUCTXEvent event; > uint64_t length;
> From: David Gibson > Sent: Tuesday, November 5, 2019 12:02 AM > To: Liu, Yi L <yi.l.liu@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.tian@intel.com> > > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > > Cc: Peter Xu <peterx@redhat.com> > > Cc: Eric Auger <eric.auger@redhat.com> > > Cc: Yi Sun <yi.y.sun@linux.intel.com> > > Cc: David Gibson <david@gibson.dropbear.id.au> > > Signed-off-by: Liu Yi L <yi.l.liu@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
On Wed, Nov 06, 2019 at 12:22:46PM +0000, Liu, Yi L wrote: > > From: David Gibson > > Sent: Tuesday, November 5, 2019 12:02 AM > > To: Liu, Yi L <yi.l.liu@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.tian@intel.com> > > > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > > > Cc: Peter Xu <peterx@redhat.com> > > > Cc: Eric Auger <eric.auger@redhat.com> > > > Cc: Yi Sun <yi.y.sun@linux.intel.com> > > > Cc: David Gibson <david@gibson.dropbear.id.au> > > > Signed-off-by: Liu Yi L <yi.l.liu@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. To my understanding this should not be needed because VFIO doesn't work with non-linux after all (as said)... while... > > > > + 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. ... I'm not sure but maybe we need this here because I _think_ vt-d should even work on Windows. However instead of __linux__ over *data, should you cover the whole IOMMUCTXPASIDBindData?
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__ + 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; +#endif +}; +typedef struct IOMMUCTXPASIDBindData IOMMUCTXPASIDBindData; + struct IOMMUCTXEventData { IOMMUCTXEvent event; uint64_t length;
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.tian@intel.com> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> Cc: Peter Xu <peterx@redhat.com> Cc: Eric Auger <eric.auger@redhat.com> Cc: Yi Sun <yi.y.sun@linux.intel.com> Cc: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> --- hw/vfio/pci.c | 39 +++++++++++++++++++++++++++++++++++++++ include/hw/iommu/iommu.h | 11 +++++++++++ 2 files changed, 50 insertions(+)