Re: [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu

[Liu, Yi L]

Hi Peter,

Some updates on it.

> -----Original Message-----
> From: Peter Xu [mailto:address@hidden
> Sent: Thursday, April 27, 2017 5:34 PM
> To: address@hidden
> Cc: Lan, Tianyu <address@hidden>; Paolo Bonzini <address@hidden>;
> Tian, Kevin <address@hidden>; Liu, Yi L <address@hidden>;
> address@hidden; Jason Wang <address@hidden>; David Gibson
> <address@hidden>; Alex Williamson <address@hidden>
> Subject: [RFC PATCH 8/8] iommu: introduce hw/core/iommu
> 
> Time to consider a common stuff for IOMMU. Let's start from an common IOMMU
> object (which should be inlayed in custom IOMMU implementations) and a 
> notifier
> mechanism.
> 
> Let VT-d IOMMU be the first user.
> 
> An example to use this per-iommu notifier:
> 
>   (when registering)
>   iommu = address_space_iommu_get(pci_device_iommu_address_space(dev));
>   notifier = iommu_notifier_register(iommu, IOMMU_EVENT_SVM_PASID, func);
>   ...
>   (when notify)
>   IOMMUEvent event = { .type = IOMMU_EVENT_SVM_PASID ... };
>   iommu_notify(iommu, &event);
>   ...
>   (when releasing)
>   iommu_notifier_unregister(notifier);
>   notifier = NULL;
> 
> Signed-off-by: Peter Xu <address@hidden>
> ---

[...]

> +#include "qemu/osdep.h"
> +#include "hw/core/iommu.h"
> +
> +IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
> +                                       IOMMUNotifyFn fn,
> +                                       uint64_t event_mask) {
> +    IOMMUNotifier *notifier = g_new0(IOMMUNotifier, 1);

For this part, I think may need to consider to alloc the memory in a
similar way with IOMMUMRNotifier. The notifier surely needs to
be connect with vfio container so that it could manipulate
pIOMMU through vfio IOCTL.

I'm thinking of adding a new struct VFIOGuestIOMMUObject which
is similar to strcut VFIOGuestIOMMU. And have the original struct
VFIOGuestIOMMU modified to be VFIOGuestIOMMUMR.

Then there would be following definition in
"include\hw\vfio\vfio-common.h":

typedef struct VFIOGuestIOMMUObject {
    VFIOContainer *container;
    IOMMUObject *iommu;
    IOMMUNotifier n; // n is for non-MemoryRegion related events, e.g. pasid 
table binding
    QLIST_ENTRY(VFIOGuestIOMMUObject) giommu_next;
} VFIOGuestIOMMUObject;

typedef struct VFIOGuestIOMMUMR {
    VFIOContainer *container;
    MemoryRegion *iommu;
    hwaddr iommu_offset;
    IOMMUNotifier n;
    QLIST_ENTRY(VFIOGuestIOMMUMR) giommu_next;
} VFIOGuestIOMMUMR;

How about your opinion?

Thanks,
Yi L