RE: [PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container

2020-04-07 Thread Liu, Yi L 
Hi Eric,

> From: Auger Eric 
> 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 
> >> Sent: Wednesday, April 1, 2020 3:51 PM
> >> To: Liu, Yi L ; 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 
> >>> Cc: Jacob Pan 
> >>> Cc: Peter Xu 
> >>> Cc: Eric Auger 
> >>> Cc: Yi Sun 
> >>> Cc: David Gibson 
> >>> Cc: Alex Williamson 
> >>> Signed-off-by: Liu Yi L 
> >>> ---
> >>>  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;
> >>> +
> >>

Re: [PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container

2020-04-06 Thread Auger Eric 
Hi Yi,

On 4/6/20 9:12 AM, Liu, Yi L wrote:
> Hi Eric,
> 
>> From: Auger Eric 
>> Sent: Wednesday, April 1, 2020 3:51 PM
>> To: Liu, Yi L ; 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 
>>> Cc: Jacob Pan 
>>> Cc: Peter Xu 
>>> Cc: Eric Auger 
>>> Cc: Yi Sun 
>>> Cc: David Gibson 
>>> Cc: Alex Williamson 
>>> Signed-off-by: Liu Yi L 
>>> ---
>>>  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 = {
>>> +

RE: [PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container

2020-04-06 Thread Liu, Yi L 
Hi Eric,

> From: Auger Eric 
> Sent: Wednesday, April 1, 2020 3:51 PM
> To: Liu, Yi L ; 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 
> > Cc: Jacob Pan 
> > Cc: Peter Xu 
> > Cc: Eric Auger 
> > Cc: Yi Sun 
> > Cc: David Gibson 
> > Cc: Alex Williamson 
> > Signed-off-by: Liu Yi L 
> > ---
> >  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;
> >

Re: [PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container

2020-04-01 Thread Auger Eric 
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 
> Cc: Jacob Pan 
> Cc: Peter Xu 
> Cc: Eric Auger 
> Cc: Yi Sun 
> Cc: David Gibson 
> Cc: Alex Williamson 
> Signed-off-by: Liu Yi L 
> ---
>  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;
>  }
>

[PATCH v2 09/22] vfio/common: init HostIOMMUContext per-container

2020-03-29 Thread Liu Yi L 
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 
Cc: Jacob Pan 
Cc: Peter Xu 
Cc: Eric Auger 
Cc: Yi Sun 
Cc: David Gibson 
Cc: Alex Williamson 
Signed-off-by: Liu Yi L 
---
 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