> From: Peter Xu <peterx@xxxxxxxxxx> > Sent: Wednesday, February 12, 2020 3:30 AM > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Subject: Re: [RFC v3 11/25] vfio: get stage-1 pasid formats from Kernel > > On Wed, Jan 29, 2020 at 04:16:42AM -0800, Liu, Yi L wrote: > > From: Liu Yi L <yi.l.liu@xxxxxxxxx> > > > > VFIO checks IOMMU UAPI version when it finds Kernel supports > > VFIO_TYPE1_NESTING_IOMMU. It is enough for UAPI compatibility check. > > However, IOMMU UAPI may support multiple stage-1 pasid formats in a > > specific UAPI version, which is highly possible since IOMMU UAPI > > supports stage-1 formats across all IOMMU vendors. > > So VFIO needs to get the supported formats from Kernel and tell > > vIOMMU. Let vIOMMU select proper format when setup dual stage DMA > > translation. > > > > This patch gets the stage-1 pasid format from kernel by using IOCTL > > VFIO_IOMMU_GET_INFO and pass the supported format to vIOMMU by the > > DualStageIOMMUObject instance which has been registered to vIOMMU. > > > > This patch referred some code from Shameer Kolothum. > > https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg03759.html > > > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx> > > Cc: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx> > > Cc: Peter Xu <peterx@xxxxxxxxxx> > > Cc: Eric Auger <eric.auger@xxxxxxxxxx> > > Cc: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> > > Cc: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> > > Cc: Alex Williamson <alex.williamson@xxxxxxxxxx> > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx> > > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx> > > --- > > hw/iommu/dual_stage_iommu.c | 5 ++- > > hw/vfio/common.c | 85 > ++++++++++++++++++++++++++++++++++++- > > include/hw/iommu/dual_stage_iommu.h | 10 ++++- > > 3 files changed, 97 insertions(+), 3 deletions(-) > > > > diff --git a/hw/iommu/dual_stage_iommu.c > b/hw/iommu/dual_stage_iommu.c > > index be4179d..d5a7168 100644 > > --- a/hw/iommu/dual_stage_iommu.c > > +++ b/hw/iommu/dual_stage_iommu.c > > @@ -48,9 +48,12 @@ int ds_iommu_pasid_free(DualStageIOMMUObject > > *dsi_obj, uint32_t pasid) } > > > > void ds_iommu_object_init(DualStageIOMMUObject *dsi_obj, > > - DualStageIOMMUOps *ops) > > + DualStageIOMMUOps *ops, > > + DualStageIOMMUInfo *uinfo) > > { > > dsi_obj->ops = ops; > > + > > + dsi_obj->uinfo.pasid_format = uinfo->pasid_format; > > } > > > > void ds_iommu_object_destroy(DualStageIOMMUObject *dsi_obj) diff > > --git a/hw/vfio/common.c b/hw/vfio/common.c index fc1723d..a07824b > > 100644 > > --- a/hw/vfio/common.c > > +++ b/hw/vfio/common.c > > @@ -1182,10 +1182,84 @@ static int vfio_get_iommu_type(VFIOContainer > > *container, static struct DualStageIOMMUOps vfio_ds_iommu_ops = { }; > > > > +static int vfio_get_iommu_info(VFIOContainer *container, > > + struct vfio_iommu_type1_info **info) > > Better comment on the function to remember to free(*info) after use for the > callers. Will do. 😊 > > > +{ > > + > > + size_t argsz = sizeof(struct vfio_iommu_type1_info); > > + > > Nit: extra newline. accepted. 😊 > > + > > + *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_format(VFIOContainer *container, > > + uint32_t *pasid_format) { > > + struct vfio_iommu_type1_info *info; > > + struct vfio_info_cap_header *hdr; > > + struct vfio_iommu_type1_info_cap_nesting *cap; > > + > > + if (vfio_get_iommu_info(container, &info)) { > > + return -errno; > > Should return the retcode from vfio_get_iommu_info. yes , it is. thx for catching it. > > + } > > + > > + 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); > > + *pasid_format = cap->pasid_format; > > + > > + g_free(info); > > + return 0; > > +} > > + > > static int vfio_init_container(VFIOContainer *container, int group_fd, > > Error **errp) { > > int iommu_type, ret; > > + uint32_t format; > > + DualStageIOMMUInfo uinfo; > > > > iommu_type = vfio_get_iommu_type(container, errp); > > if (iommu_type < 0) { > > @@ -1214,7 +1288,16 @@ static int vfio_init_container(VFIOContainer > *container, int group_fd, > > } > > > > if (iommu_type == VFIO_TYPE1_NESTING_IOMMU) { > > - ds_iommu_object_init(&container->dsi_obj, &vfio_ds_iommu_ops); > > + if (vfio_get_nesting_iommu_format(container, &format)) { > > + error_setg_errno(errp, errno, > > + "Failed to get nesting iommu format"); > > + return -errno; > > Same here, you might want to return the retcode from > vfio_get_nesting_iommu_format()? will do it. 😊 Thanks for your comments, I'll address them in next version. Regards, Yi Liu
