In this patch, QEMU firstly gets iommu info from kernel to check the
supported capabilities by a VFIO_IOMMU_TYPE1_NESTING iommu. And inits
HostIOMMUContet instance.

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>
Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
---
 hw/vfio/common.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 5f3534d..44b142c 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1226,10 +1226,89 @@ static int vfio_host_iommu_ctx_pasid_free(HostIOMMUContext *iommu_ctx,
     return 0;
 }
 
+/**
+ * Get iommu info from host. Caller of this funcion should free
+ * the memory pointed by the returned pointer stored in @info
+ * after a successful calling when finished its usage.
+ */
+static int vfio_get_iommu_info(VFIOContainer *container,
+                         struct vfio_iommu_type1_info **info)
+{
+
+    size_t argsz = sizeof(struct vfio_iommu_type1_info);
+
+    *info = g_malloc0(argsz);
+
+retry:
+    (*info)->argsz = argsz;
+
+    if (ioctl(container->fd, VFIO_IOMMU_GET_INFO, *info)) {
+        g_free(*info);
+        *info = NULL;
+        return -errno;
+    }
+
+    if (((*info)->argsz > argsz)) {
+        argsz = (*info)->argsz;
+        *info = g_realloc(*info, argsz);
+        goto retry;
+    }
+
+    return 0;
+}
+
+static struct vfio_info_cap_header *
+vfio_get_iommu_info_cap(struct vfio_iommu_type1_info *info, uint16_t id)
+{
+    struct vfio_info_cap_header *hdr;
+    void *ptr = info;
+
+    if (!(info->flags & VFIO_IOMMU_INFO_CAPS)) {
+        return NULL;
+    }
+
+    for (hdr = ptr + info->cap_offset; hdr != ptr; hdr = ptr + hdr->next) {
+        if (hdr->id == id) {
+            return hdr;
+        }
+    }
+
+    return NULL;
+}
+
+static int vfio_get_nesting_iommu_cap(VFIOContainer *container,
+                   struct vfio_iommu_type1_info_cap_nesting *cap_nesting)
+{
+    struct vfio_iommu_type1_info *info;
+    struct vfio_info_cap_header *hdr;
+    struct vfio_iommu_type1_info_cap_nesting *cap;
+    int ret;
+
+    ret = vfio_get_iommu_info(container, &info);
+    if (ret) {
+        return ret;
+    }
+
+    hdr = vfio_get_iommu_info_cap(info,
+                        VFIO_IOMMU_TYPE1_INFO_CAP_NESTING);
+    if (!hdr) {
+        g_free(info);
+        return -errno;
+    }
+
+    cap = container_of(hdr,
+                struct vfio_iommu_type1_info_cap_nesting, header);
+    *cap_nesting = *cap;
+
+    g_free(info);
+    return 0;
+}
+
 static int vfio_init_container(VFIOContainer *container, int group_fd,
                                Error **errp)
 {
     int iommu_type, ret;
+    uint64_t flags = 0;
 
     iommu_type = vfio_get_iommu_type(container, errp);
     if (iommu_type < 0) {
@@ -1257,6 +1336,26 @@ static int vfio_init_container(VFIOContainer *container, int group_fd,
         return -errno;
     }
 
+    if (iommu_type == VFIO_TYPE1_NESTING_IOMMU) {
+        struct vfio_iommu_type1_info_cap_nesting nesting = {
+                                         .nesting_capabilities = 0x0,
+                                         .stage1_formats = 0, };
+
+        ret = vfio_get_nesting_iommu_cap(container, &nesting);
+        if (ret) {
+            error_setg_errno(errp, -ret,
+                             "Failed to get nesting iommu cap");
+            return ret;
+        }
+
+        flags |= (nesting.nesting_capabilities & VFIO_IOMMU_PASID_REQS) ?
+                 HOST_IOMMU_PASID_REQUEST : 0;
+        host_iommu_ctx_init(&container->iommu_ctx,
+                            sizeof(container->iommu_ctx),
+                            TYPE_VFIO_HOST_IOMMU_CONTEXT,
+                            flags);
+    }
+
     container->iommu_type = iommu_type;
     return 0;
 }
-- 
2.7.4

Hi Yi,

On 3/30/20 6:24 AM, Liu Yi L wrote:
> In this patch, QEMU firstly gets iommu info from kernel to check the
> supported capabilities by a VFIO_IOMMU_TYPE1_NESTING iommu. And inits
> HostIOMMUContet instance.
> 
> 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>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> ---
>  hw/vfio/common.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 5f3534d..44b142c 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1226,10 +1226,89 @@ static int vfio_host_iommu_ctx_pasid_free(HostIOMMUContext *iommu_ctx,
>      return 0;
>  }
>  
> +/**
> + * Get iommu info from host. Caller of this funcion should free
> + * the memory pointed by the returned pointer stored in @info
> + * after a successful calling when finished its usage.
> + */
> +static int vfio_get_iommu_info(VFIOContainer *container,
> +                         struct vfio_iommu_type1_info **info)
> +{
> +
> +    size_t argsz = sizeof(struct vfio_iommu_type1_info);
> +
> +    *info = g_malloc0(argsz);
> +
> +retry:
> +    (*info)->argsz = argsz;
> +
> +    if (ioctl(container->fd, VFIO_IOMMU_GET_INFO, *info)) {
> +        g_free(*info);
> +        *info = NULL;
> +        return -errno;
> +    }
> +
> +    if (((*info)->argsz > argsz)) {
> +        argsz = (*info)->argsz;
> +        *info = g_realloc(*info, argsz);
> +        goto retry;
> +    }
> +
> +    return 0;
> +}
> +
> +static struct vfio_info_cap_header *
> +vfio_get_iommu_info_cap(struct vfio_iommu_type1_info *info, uint16_t id)
> +{
> +    struct vfio_info_cap_header *hdr;
> +    void *ptr = info;
> +
> +    if (!(info->flags & VFIO_IOMMU_INFO_CAPS)) {
> +        return NULL;
> +    }
> +
> +    for (hdr = ptr + info->cap_offset; hdr != ptr; hdr = ptr + hdr->next) {
> +        if (hdr->id == id) {
> +            return hdr;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static int vfio_get_nesting_iommu_cap(VFIOContainer *container,
> +                   struct vfio_iommu_type1_info_cap_nesting *cap_nesting)
> +{
> +    struct vfio_iommu_type1_info *info;
> +    struct vfio_info_cap_header *hdr;
> +    struct vfio_iommu_type1_info_cap_nesting *cap;
> +    int ret;
> +
> +    ret = vfio_get_iommu_info(container, &info);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    hdr = vfio_get_iommu_info_cap(info,
> +                        VFIO_IOMMU_TYPE1_INFO_CAP_NESTING);
> +    if (!hdr) {
> +        g_free(info);
> +        return -errno;
> +    }
> +
> +    cap = container_of(hdr,
> +                struct vfio_iommu_type1_info_cap_nesting, header);
> +    *cap_nesting = *cap;
> +
> +    g_free(info);
> +    return 0;
> +}
> +
>  static int vfio_init_container(VFIOContainer *container, int group_fd,
>                                 Error **errp)
>  {
>      int iommu_type, ret;
> +    uint64_t flags = 0;
>  
>      iommu_type = vfio_get_iommu_type(container, errp);
>      if (iommu_type < 0) {
> @@ -1257,6 +1336,26 @@ static int vfio_init_container(VFIOContainer *container, int group_fd,
>          return -errno;
>      }
>  
> +    if (iommu_type == VFIO_TYPE1_NESTING_IOMMU) {
> +        struct vfio_iommu_type1_info_cap_nesting nesting = {
> +                                         .nesting_capabilities = 0x0,
> +                                         .stage1_formats = 0, };
> +
> +        ret = vfio_get_nesting_iommu_cap(container, &nesting);
> +        if (ret) {
> +            error_setg_errno(errp, -ret,
> +                             "Failed to get nesting iommu cap");
> +            return ret;
> +        }
> +
> +        flags |= (nesting.nesting_capabilities & VFIO_IOMMU_PASID_REQS) ?
> +                 HOST_IOMMU_PASID_REQUEST : 0;
I still don't get why you can't transform your iommu_ctx into a  pointer
and do
container->iommu_ctx = g_new0(HostIOMMUContext, 1);
then
host_iommu_ctx_init(container->iommu_ctx, flags);

looks something similar to (hw/vfio/common.c). You may not even need to
use a derived VFIOHostIOMMUContext object (As only VFIO does use that
object)? Only the ops do change, no new field?
        region->mem = g_new0(MemoryRegion, 1);
        memory_region_init_io(region->mem, obj, &vfio_region_ops,
                              region, name, region->size);

Thanks

Eric

> +        host_iommu_ctx_init(&container->iommu_ctx,
> +                            sizeof(container->iommu_ctx),
> +                            TYPE_VFIO_HOST_IOMMU_CONTEXT,
> +                            flags);
> +    }
> +
>      container->iommu_type = iommu_type;
>      return 0;
>  }
> 

Hi Eric,

> From: Auger Eric <eric.auger@redhat.com>
> Sent: Wednesday, April 1, 2020 3:51 PM
> To: Liu, Yi L <yi.l.liu@intel.com>; qemu-devel@nongnu.org;
> Subject: Re: [PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container
> 
> Hi Yi,
> 
> On 3/30/20 6:24 AM, Liu Yi L wrote:
> > In this patch, QEMU firstly gets iommu info from kernel to check the
> > supported capabilities by a VFIO_IOMMU_TYPE1_NESTING iommu. And inits
> > HostIOMMUContet instance.
> >
> > 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>
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > ---
> >  hw/vfio/common.c | 99
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 99 insertions(+)
> >
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c index
> > 5f3534d..44b142c 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -1226,10 +1226,89 @@ static int
> vfio_host_iommu_ctx_pasid_free(HostIOMMUContext *iommu_ctx,
> >      return 0;
> >  }
> >
> > +/**
> > + * Get iommu info from host. Caller of this funcion should free
> > + * the memory pointed by the returned pointer stored in @info
> > + * after a successful calling when finished its usage.
> > + */
> > +static int vfio_get_iommu_info(VFIOContainer *container,
> > +                         struct vfio_iommu_type1_info **info) {
> > +
> > +    size_t argsz = sizeof(struct vfio_iommu_type1_info);
> > +
> > +    *info = g_malloc0(argsz);
> > +
> > +retry:
> > +    (*info)->argsz = argsz;
> > +
> > +    if (ioctl(container->fd, VFIO_IOMMU_GET_INFO, *info)) {
> > +        g_free(*info);
> > +        *info = NULL;
> > +        return -errno;
> > +    }
> > +
> > +    if (((*info)->argsz > argsz)) {
> > +        argsz = (*info)->argsz;
> > +        *info = g_realloc(*info, argsz);
> > +        goto retry;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static struct vfio_info_cap_header *
> > +vfio_get_iommu_info_cap(struct vfio_iommu_type1_info *info, uint16_t
> > +id) {
> > +    struct vfio_info_cap_header *hdr;
> > +    void *ptr = info;
> > +
> > +    if (!(info->flags & VFIO_IOMMU_INFO_CAPS)) {
> > +        return NULL;
> > +    }
> > +
> > +    for (hdr = ptr + info->cap_offset; hdr != ptr; hdr = ptr + hdr->next) {
> > +        if (hdr->id == id) {
> > +            return hdr;
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +static int vfio_get_nesting_iommu_cap(VFIOContainer *container,
> > +                   struct vfio_iommu_type1_info_cap_nesting
> > +*cap_nesting) {
> > +    struct vfio_iommu_type1_info *info;
> > +    struct vfio_info_cap_header *hdr;
> > +    struct vfio_iommu_type1_info_cap_nesting *cap;
> > +    int ret;
> > +
> > +    ret = vfio_get_iommu_info(container, &info);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +
> > +    hdr = vfio_get_iommu_info_cap(info,
> > +                        VFIO_IOMMU_TYPE1_INFO_CAP_NESTING);
> > +    if (!hdr) {
> > +        g_free(info);
> > +        return -errno;
> > +    }
> > +
> > +    cap = container_of(hdr,
> > +                struct vfio_iommu_type1_info_cap_nesting, header);
> > +    *cap_nesting = *cap;
> > +
> > +    g_free(info);
> > +    return 0;
> > +}
> > +
> >  static int vfio_init_container(VFIOContainer *container, int group_fd,
> >                                 Error **errp)  {
> >      int iommu_type, ret;
> > +    uint64_t flags = 0;
> >
> >      iommu_type = vfio_get_iommu_type(container, errp);
> >      if (iommu_type < 0) {
> > @@ -1257,6 +1336,26 @@ static int vfio_init_container(VFIOContainer
> *container, int group_fd,
> >          return -errno;
> >      }
> >
> > +    if (iommu_type == VFIO_TYPE1_NESTING_IOMMU) {
> > +        struct vfio_iommu_type1_info_cap_nesting nesting = {
> > +                                         .nesting_capabilities = 0x0,
> > +                                         .stage1_formats = 0, };
> > +
> > +        ret = vfio_get_nesting_iommu_cap(container, &nesting);
> > +        if (ret) {
> > +            error_setg_errno(errp, -ret,
> > +                             "Failed to get nesting iommu cap");
> > +            return ret;
> > +        }
> > +
> > +        flags |= (nesting.nesting_capabilities & VFIO_IOMMU_PASID_REQS) ?
> > +                 HOST_IOMMU_PASID_REQUEST : 0;
> I still don't get why you can't transform your iommu_ctx into a  pointer and do
> container->iommu_ctx = g_new0(HostIOMMUContext, 1);
> then
> host_iommu_ctx_init(container->iommu_ctx, flags);
> 
> looks something similar to (hw/vfio/common.c). You may not even need to use a
> derived VFIOHostIOMMUContext object (As only VFIO does use that object)? Only
> the ops do change, no new field?
>         region->mem = g_new0(MemoryRegion, 1);
>         memory_region_init_io(region->mem, obj, &vfio_region_ops,
>                               region, name, region->size);

In this way, the vfio hook can easily get the VFIOContainer from
HostIOMMUContext when call in the hook provided by vfio. e.g. the
one below.

+static int vfio_host_iommu_ctx_pasid_alloc(HostIOMMUContext *iommu_ctx,
+                                           uint32_t min, uint32_t max,
+                                           uint32_t *pasid)
+{
+    VFIOContainer *container = container_of(iommu_ctx,
+                                            VFIOContainer, iommu_ctx);
 
Regards,
Yi Liu

Hi Yi,

On 4/6/20 9:12 AM, Liu, Yi L wrote:
> Hi Eric,
> 
>> From: Auger Eric <eric.auger@redhat.com>
>> Sent: Wednesday, April 1, 2020 3:51 PM
>> To: Liu, Yi L <yi.l.liu@intel.com>; qemu-devel@nongnu.org;
>> Subject: Re: [PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container
>>
>> Hi Yi,
>>
>> On 3/30/20 6:24 AM, Liu Yi L wrote:
>>> In this patch, QEMU firstly gets iommu info from kernel to check the
>>> supported capabilities by a VFIO_IOMMU_TYPE1_NESTING iommu. And inits
>>> HostIOMMUContet instance.
>>>
>>> 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>
>>> Cc: Alex Williamson <alex.williamson@redhat.com>
>>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
>>> ---
>>>  hw/vfio/common.c | 99
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 99 insertions(+)
>>>
>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c index
>>> 5f3534d..44b142c 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -1226,10 +1226,89 @@ static int
>> vfio_host_iommu_ctx_pasid_free(HostIOMMUContext *iommu_ctx,
>>>      return 0;
>>>  }
>>>
>>> +/**
>>> + * Get iommu info from host. Caller of this funcion should free
>>> + * the memory pointed by the returned pointer stored in @info
>>> + * after a successful calling when finished its usage.
>>> + */
>>> +static int vfio_get_iommu_info(VFIOContainer *container,
>>> +                         struct vfio_iommu_type1_info **info) {
>>> +
>>> +    size_t argsz = sizeof(struct vfio_iommu_type1_info);
>>> +
>>> +    *info = g_malloc0(argsz);
>>> +
>>> +retry:
>>> +    (*info)->argsz = argsz;
>>> +
>>> +    if (ioctl(container->fd, VFIO_IOMMU_GET_INFO, *info)) {
>>> +        g_free(*info);
>>> +        *info = NULL;
>>> +        return -errno;
>>> +    }
>>> +
>>> +    if (((*info)->argsz > argsz)) {
>>> +        argsz = (*info)->argsz;
>>> +        *info = g_realloc(*info, argsz);
>>> +        goto retry;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static struct vfio_info_cap_header *
>>> +vfio_get_iommu_info_cap(struct vfio_iommu_type1_info *info, uint16_t
>>> +id) {
>>> +    struct vfio_info_cap_header *hdr;
>>> +    void *ptr = info;
>>> +
>>> +    if (!(info->flags & VFIO_IOMMU_INFO_CAPS)) {
>>> +        return NULL;
>>> +    }
>>> +
>>> +    for (hdr = ptr + info->cap_offset; hdr != ptr; hdr = ptr + hdr->next) {
>>> +        if (hdr->id == id) {
>>> +            return hdr;
>>> +        }
>>> +    }
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>> +static int vfio_get_nesting_iommu_cap(VFIOContainer *container,
>>> +                   struct vfio_iommu_type1_info_cap_nesting
>>> +*cap_nesting) {
>>> +    struct vfio_iommu_type1_info *info;
>>> +    struct vfio_info_cap_header *hdr;
>>> +    struct vfio_iommu_type1_info_cap_nesting *cap;
>>> +    int ret;
>>> +
>>> +    ret = vfio_get_iommu_info(container, &info);
>>> +    if (ret) {
>>> +        return ret;
>>> +    }
>>> +
>>> +    hdr = vfio_get_iommu_info_cap(info,
>>> +                        VFIO_IOMMU_TYPE1_INFO_CAP_NESTING);
>>> +    if (!hdr) {
>>> +        g_free(info);
>>> +        return -errno;
>>> +    }
>>> +
>>> +    cap = container_of(hdr,
>>> +                struct vfio_iommu_type1_info_cap_nesting, header);
>>> +    *cap_nesting = *cap;
>>> +
>>> +    g_free(info);
>>> +    return 0;
>>> +}
>>> +
>>>  static int vfio_init_container(VFIOContainer *container, int group_fd,
>>>                                 Error **errp)  {
>>>      int iommu_type, ret;
>>> +    uint64_t flags = 0;
>>>
>>>      iommu_type = vfio_get_iommu_type(container, errp);
>>>      if (iommu_type < 0) {
>>> @@ -1257,6 +1336,26 @@ static int vfio_init_container(VFIOContainer
>> *container, int group_fd,
>>>          return -errno;
>>>      }
>>>
>>> +    if (iommu_type == VFIO_TYPE1_NESTING_IOMMU) {
>>> +        struct vfio_iommu_type1_info_cap_nesting nesting = {
>>> +                                         .nesting_capabilities = 0x0,
>>> +                                         .stage1_formats = 0, };
>>> +
>>> +        ret = vfio_get_nesting_iommu_cap(container, &nesting);
>>> +        if (ret) {
>>> +            error_setg_errno(errp, -ret,
>>> +                             "Failed to get nesting iommu cap");
>>> +            return ret;
>>> +        }
>>> +
>>> +        flags |= (nesting.nesting_capabilities & VFIO_IOMMU_PASID_REQS) ?
>>> +                 HOST_IOMMU_PASID_REQUEST : 0;
>> I still don't get why you can't transform your iommu_ctx into a  pointer and do
>> container->iommu_ctx = g_new0(HostIOMMUContext, 1);
>> then
>> host_iommu_ctx_init(container->iommu_ctx, flags);
>>
>> looks something similar to (hw/vfio/common.c). You may not even need to use a
>> derived VFIOHostIOMMUContext object (As only VFIO does use that object)? Only
>> the ops do change, no new field?
>>         region->mem = g_new0(MemoryRegion, 1);
>>         memory_region_init_io(region->mem, obj, &vfio_region_ops,
>>                               region, name, region->size);
> 
> In this way, the vfio hook can easily get the VFIOContainer from
> HostIOMMUContext when call in the hook provided by vfio. e.g. the
> one below.
OK I get it. However in memory_region_init_io(), you also pass the
owner, eg. region so I think you could do the same. no?

Thanks

Eric
> 
> +static int vfio_host_iommu_ctx_pasid_alloc(HostIOMMUContext *iommu_ctx,
> +                                           uint32_t min, uint32_t max,
> +                                           uint32_t *pasid)
> +{
> +    VFIOContainer *container = container_of(iommu_ctx,
> +                                            VFIOContainer, iommu_ctx);
>  
> Regards,
> Yi Liu
> 

Hi Eric,

> From: Auger Eric <eric.auger@redhat.com>
> Sent: Monday, April 6, 2020 6:20 PM
> Subject: Re: [PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container
> 
> Hi Yi,
> 
> On 4/6/20 9:12 AM, Liu, Yi L wrote:
> > Hi Eric,
> >
> >> From: Auger Eric <eric.auger@redhat.com>
> >> Sent: Wednesday, April 1, 2020 3:51 PM
> >> To: Liu, Yi L <yi.l.liu@intel.com>; qemu-devel@nongnu.org;
> >> Subject: Re: [PATCH v2 09/22] vfio/common: init HostIOMMUContext
> >> per-container
> >>
> >> Hi Yi,
> >>
> >> On 3/30/20 6:24 AM, Liu Yi L wrote:
> >>> In this patch, QEMU firstly gets iommu info from kernel to check the
> >>> supported capabilities by a VFIO_IOMMU_TYPE1_NESTING iommu. And
> >>> inits HostIOMMUContet instance.
> >>>
> >>> 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>
> >>> Cc: Alex Williamson <alex.williamson@redhat.com>
> >>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> >>> ---
> >>>  hw/vfio/common.c | 99
> >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 99 insertions(+)
> >>>
> >>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c index
> >>> 5f3534d..44b142c 100644
> >>> --- a/hw/vfio/common.c
> >>> +++ b/hw/vfio/common.c
> >>> @@ -1226,10 +1226,89 @@ static int
> >> vfio_host_iommu_ctx_pasid_free(HostIOMMUContext *iommu_ctx,
> >>>      return 0;
> >>>  }
> >>>
> >>> +/**
> >>> + * Get iommu info from host. Caller of this funcion should free
> >>> + * the memory pointed by the returned pointer stored in @info
> >>> + * after a successful calling when finished its usage.
> >>> + */
> >>> +static int vfio_get_iommu_info(VFIOContainer *container,
> >>> +                         struct vfio_iommu_type1_info **info) {
> >>> +
> >>> +    size_t argsz = sizeof(struct vfio_iommu_type1_info);
> >>> +
> >>> +    *info = g_malloc0(argsz);
> >>> +
> >>> +retry:
> >>> +    (*info)->argsz = argsz;
> >>> +
> >>> +    if (ioctl(container->fd, VFIO_IOMMU_GET_INFO, *info)) {
> >>> +        g_free(*info);
> >>> +        *info = NULL;
> >>> +        return -errno;
> >>> +    }
> >>> +
> >>> +    if (((*info)->argsz > argsz)) {
> >>> +        argsz = (*info)->argsz;
> >>> +        *info = g_realloc(*info, argsz);
> >>> +        goto retry;
> >>> +    }
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +
> >>> +static struct vfio_info_cap_header * vfio_get_iommu_info_cap(struct
> >>> +vfio_iommu_type1_info *info, uint16_t
> >>> +id) {
> >>> +    struct vfio_info_cap_header *hdr;
> >>> +    void *ptr = info;
> >>> +
> >>> +    if (!(info->flags & VFIO_IOMMU_INFO_CAPS)) {
> >>> +        return NULL;
> >>> +    }
> >>> +
> >>> +    for (hdr = ptr + info->cap_offset; hdr != ptr; hdr = ptr + hdr->next) {
> >>> +        if (hdr->id == id) {
> >>> +            return hdr;
> >>> +        }
> >>> +    }
> >>> +
> >>> +    return NULL;
> >>> +}
> >>> +
> >>> +static int vfio_get_nesting_iommu_cap(VFIOContainer *container,
> >>> +                   struct vfio_iommu_type1_info_cap_nesting
> >>> +*cap_nesting) {
> >>> +    struct vfio_iommu_type1_info *info;
> >>> +    struct vfio_info_cap_header *hdr;
> >>> +    struct vfio_iommu_type1_info_cap_nesting *cap;
> >>> +    int ret;
> >>> +
> >>> +    ret = vfio_get_iommu_info(container, &info);
> >>> +    if (ret) {
> >>> +        return ret;
> >>> +    }
> >>> +
> >>> +    hdr = vfio_get_iommu_info_cap(info,
> >>> +                        VFIO_IOMMU_TYPE1_INFO_CAP_NESTING);
> >>> +    if (!hdr) {
> >>> +        g_free(info);
> >>> +        return -errno;
> >>> +    }
> >>> +
> >>> +    cap = container_of(hdr,
> >>> +                struct vfio_iommu_type1_info_cap_nesting, header);
> >>> +    *cap_nesting = *cap;
> >>> +
> >>> +    g_free(info);
> >>> +    return 0;
> >>> +}
> >>> +
> >>>  static int vfio_init_container(VFIOContainer *container, int group_fd,
> >>>                                 Error **errp)  {
> >>>      int iommu_type, ret;
> >>> +    uint64_t flags = 0;
> >>>
> >>>      iommu_type = vfio_get_iommu_type(container, errp);
> >>>      if (iommu_type < 0) {
> >>> @@ -1257,6 +1336,26 @@ static int vfio_init_container(VFIOContainer
> >> *container, int group_fd,
> >>>          return -errno;
> >>>      }
> >>>
> >>> +    if (iommu_type == VFIO_TYPE1_NESTING_IOMMU) {
> >>> +        struct vfio_iommu_type1_info_cap_nesting nesting = {
> >>> +                                         .nesting_capabilities = 0x0,
> >>> +                                         .stage1_formats = 0, };
> >>> +
> >>> +        ret = vfio_get_nesting_iommu_cap(container, &nesting);
> >>> +        if (ret) {
> >>> +            error_setg_errno(errp, -ret,
> >>> +                             "Failed to get nesting iommu cap");
> >>> +            return ret;
> >>> +        }
> >>> +
> >>> +        flags |= (nesting.nesting_capabilities & VFIO_IOMMU_PASID_REQS) ?
> >>> +                 HOST_IOMMU_PASID_REQUEST : 0;
> >> I still don't get why you can't transform your iommu_ctx into a
> >> pointer and do
> >> container->iommu_ctx = g_new0(HostIOMMUContext, 1);
> >> then
> >> host_iommu_ctx_init(container->iommu_ctx, flags);
> >>
> >> looks something similar to (hw/vfio/common.c). You may not even need
> >> to use a derived VFIOHostIOMMUContext object (As only VFIO does use
> >> that object)? Only the ops do change, no new field?
> >>         region->mem = g_new0(MemoryRegion, 1);
> >>         memory_region_init_io(region->mem, obj, &vfio_region_ops,
> >>                               region, name, region->size);
> >
> > In this way, the vfio hook can easily get the VFIOContainer from
> > HostIOMMUContext when call in the hook provided by vfio. e.g. the one
> > below.
> OK I get it. However in memory_region_init_io(), you also pass the owner, eg.
> region so I think you could do the same. no?
Hmm, I can add it. But I've no idea about the proper owner for it so far.
any suggestion?

Regards,
Yi Liu