[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v2 09/22] vfio/pci: add iommu_context notifier for pasid alloc/
From: |
David Gibson |
Subject: |
Re: [RFC v2 09/22] vfio/pci: add iommu_context notifier for pasid alloc/free |
Date: |
Tue, 29 Oct 2019 13:15:44 +0100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Thu, Oct 24, 2019 at 08:34:30AM -0400, Liu Yi L wrote:
> This patch adds pasid alloc/free notifiers for vfio-pci. It is
> supposed to be fired by vIOMMU. VFIO then sends PASID allocation
> or free request to host.
>
> Cc: Kevin Tian <address@hidden>
> Cc: Jacob Pan <address@hidden>
> Cc: Peter Xu <address@hidden>
> Cc: Eric Auger <address@hidden>
> Cc: Yi Sun <address@hidden>
> Cc: David Gibson <address@hidden>
> Signed-off-by: Liu Yi L <address@hidden>
> ---
> hw/vfio/common.c | 9 ++++++
> hw/vfio/pci.c | 81
> ++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/iommu/iommu.h | 15 +++++++++
> 3 files changed, 105 insertions(+)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index d418527..e6ad21c 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1436,6 +1436,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
> if (QLIST_EMPTY(&container->group_list)) {
> VFIOAddressSpace *space = container->space;
> VFIOGuestIOMMU *giommu, *tmp;
> + VFIOIOMMUContext *giommu_ctx, *ctx;
>
> QLIST_REMOVE(container, next);
>
> @@ -1446,6 +1447,14 @@ static void vfio_disconnect_container(VFIOGroup *group)
> g_free(giommu);
> }
>
> + QLIST_FOREACH_SAFE(giommu_ctx, &container->iommu_ctx_list,
> + iommu_ctx_next, ctx) {
> + iommu_ctx_notifier_unregister(giommu_ctx->iommu_ctx,
> + &giommu_ctx->n);
> + QLIST_REMOVE(giommu_ctx, iommu_ctx_next);
> + g_free(giommu_ctx);
> + }
> +
> trace_vfio_disconnect_container(container->fd);
> close(container->fd);
> g_free(container);
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 12fac39..8721ff6 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2699,11 +2699,80 @@ static void
> vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
> vdev->req_enabled = false;
> }
>
> +static void vfio_register_iommu_ctx_notifier(VFIOPCIDevice *vdev,
> + IOMMUContext *iommu_ctx,
> + IOMMUCTXNotifyFn fn,
> + IOMMUCTXEvent event)
> +{
> + VFIOContainer *container = vdev->vbasedev.group->container;
> + VFIOIOMMUContext *giommu_ctx;
> +
> + giommu_ctx = g_malloc0(sizeof(*giommu_ctx));
> + giommu_ctx->container = container;
> + giommu_ctx->iommu_ctx = iommu_ctx;
> + QLIST_INSERT_HEAD(&container->iommu_ctx_list,
> + giommu_ctx,
> + iommu_ctx_next);
> + iommu_ctx_notifier_register(iommu_ctx,
> + &giommu_ctx->n,
> + fn,
> + event);
> +}
> +
> +static void vfio_iommu_pasid_alloc_notify(IOMMUCTXNotifier *n,
> + IOMMUCTXEventData *event_data)
> +{
> + VFIOIOMMUContext *giommu_ctx = container_of(n, VFIOIOMMUContext, n);
> + VFIOContainer *container = giommu_ctx->container;
> + IOMMUCTXPASIDReqDesc *pasid_req =
> + (IOMMUCTXPASIDReqDesc *) event_data->data;
> + struct vfio_iommu_type1_pasid_request req;
> + unsigned long argsz;
> + int pasid;
> +
> + argsz = sizeof(req);
> + req.argsz = argsz;
> + req.flag = VFIO_IOMMU_PASID_ALLOC;
> + req.min_pasid = pasid_req->min_pasid;
> + req.max_pasid = pasid_req->max_pasid;
> +
> + pasid = ioctl(container->fd, VFIO_IOMMU_PASID_REQUEST, &req);
> + if (pasid < 0) {
> + error_report("%s: %d, alloc failed", __func__, -errno);
> + }
> + pasid_req->alloc_result = pasid;
Altering the event data from the notifier doesn't make sense. By
definition there can be multiple notifiers on the chain, so in that
case which one is responsible for updating the writable field?
> +}
> +
> +static void vfio_iommu_pasid_free_notify(IOMMUCTXNotifier *n,
> + IOMMUCTXEventData *event_data)
> +{
> + VFIOIOMMUContext *giommu_ctx = container_of(n, VFIOIOMMUContext, n);
> + VFIOContainer *container = giommu_ctx->container;
> + IOMMUCTXPASIDReqDesc *pasid_req =
> + (IOMMUCTXPASIDReqDesc *) event_data->data;
> + struct vfio_iommu_type1_pasid_request req;
> + unsigned long argsz;
> + int ret = 0;
> +
> + argsz = sizeof(req);
> + req.argsz = argsz;
> + req.flag = VFIO_IOMMU_PASID_FREE;
> + req.pasid = pasid_req->pasid;
> +
> + ret = ioctl(container->fd, VFIO_IOMMU_PASID_REQUEST, &req);
> + if (ret != 0) {
> + error_report("%s: %d, pasid %u free failed",
> + __func__, -errno, (unsigned) pasid_req->pasid);
> + }
> + pasid_req->free_result = ret;
Same problem here.
> +}
> +
> static void vfio_realize(PCIDevice *pdev, Error **errp)
> {
> VFIOPCIDevice *vdev = PCI_VFIO(pdev);
> VFIODevice *vbasedev_iter;
> VFIOGroup *group;
> + IOMMUContext *iommu_context;
> char *tmp, *subsys, group_path[PATH_MAX], *group_name;
> Error *err = NULL;
> ssize_t len;
> @@ -3000,6 +3069,18 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> vfio_register_req_notifier(vdev);
> vfio_setup_resetfn_quirk(vdev);
>
> + iommu_context = pci_device_iommu_context(pdev);
> + if (iommu_context) {
> + vfio_register_iommu_ctx_notifier(vdev,
> + iommu_context,
> + vfio_iommu_pasid_alloc_notify,
> + IOMMU_CTX_EVENT_PASID_ALLOC);
> + vfio_register_iommu_ctx_notifier(vdev,
> + iommu_context,
> + vfio_iommu_pasid_free_notify,
> + IOMMU_CTX_EVENT_PASID_FREE);
> + }
> +
> return;
>
> out_teardown:
> diff --git a/include/hw/iommu/iommu.h b/include/hw/iommu/iommu.h
> index c22c442..4352afd 100644
> --- a/include/hw/iommu/iommu.h
> +++ b/include/hw/iommu/iommu.h
> @@ -31,10 +31,25 @@
> typedef struct IOMMUContext IOMMUContext;
>
> enum IOMMUCTXEvent {
> + IOMMU_CTX_EVENT_PASID_ALLOC,
> + IOMMU_CTX_EVENT_PASID_FREE,
> IOMMU_CTX_EVENT_NUM,
> };
> typedef enum IOMMUCTXEvent IOMMUCTXEvent;
>
> +union IOMMUCTXPASIDReqDesc {
> + struct {
> + uint32_t min_pasid;
> + uint32_t max_pasid;
> + int32_t alloc_result; /* pasid allocated for the alloc request */
> + };
> + struct {
> + uint32_t pasid; /* pasid to be free */
> + int free_result;
> + };
> +};
Apart from theproblem with writable fields, using a big union for
event data is pretty ugly. If you need this different information for
the different events, it might make more sense to have a separate
notifier chain with a separate call interface for each event type,
rather than trying to multiplex them together.
> +typedef union IOMMUCTXPASIDReqDesc IOMMUCTXPASIDReqDesc;
> +
> struct IOMMUCTXEventData {
> IOMMUCTXEvent event;
> uint64_t length;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
- [RFC v2 00/22] intel_iommu: expose Shared Virtual Addressing to VM, Liu Yi L, 2019/10/24
- [RFC v2 01/22] update-linux-headers: Import iommu.h, Liu Yi L, 2019/10/24
- [RFC v2 06/22] hw/pci: modify pci_setup_iommu() to set PCIIOMMUOps, Liu Yi L, 2019/10/24
- [RFC v2 09/22] vfio/pci: add iommu_context notifier for pasid alloc/free, Liu Yi L, 2019/10/24
- Re: [RFC v2 09/22] vfio/pci: add iommu_context notifier for pasid alloc/free,
David Gibson <=
- [RFC v2 10/22] intel_iommu: add virtual command capability support, Liu Yi L, 2019/10/24
- [RFC v2 05/22] vfio/common: add iommu_ctx_notifier in container, Liu Yi L, 2019/10/24
- [RFC v2 03/22] intel_iommu: modify x-scalable-mode to be string option, Liu Yi L, 2019/10/24
- [RFC v2 04/22] hw/iommu: introduce IOMMUContext, Liu Yi L, 2019/10/24
- [RFC v2 07/22] hw/pci: introduce pci_device_iommu_context(), Liu Yi L, 2019/10/24
- [RFC v2 08/22] intel_iommu: provide get_iommu_context() callback, Liu Yi L, 2019/10/24
- [RFC v2 02/22] header update VFIO/IOMMU vSVA APIs against 5.4.0-rc3+, Liu Yi L, 2019/10/24
- [RFC v2 11/22] intel_iommu: process pasid cache invalidation, Liu Yi L, 2019/10/24