| Message ID | 1585542301-84087-4-git-send-email-yi.l.liu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | intel_iommu: expose Shared Virtual Addressing to VMs | expand |
Yi, On 3/30/20 6:24 AM, Liu Yi L wrote: > VFIO needs to check VFIO_TYPE1_NESTING_IOMMU support with Kernel before > further using it. e.g. requires to check IOMMU UAPI version. > > 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> > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > --- > hw/vfio/common.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 0b3593b..c276732 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -1157,12 +1157,21 @@ static void vfio_put_address_space(VFIOAddressSpace *space) > static int vfio_get_iommu_type(VFIOContainer *container, > Error **errp) > { > - int iommu_types[] = { VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU, > + int iommu_types[] = { VFIO_TYPE1_NESTING_IOMMU, > + VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU, > VFIO_SPAPR_TCE_v2_IOMMU, VFIO_SPAPR_TCE_IOMMU }; > - int i; > + int i, version; > > for (i = 0; i < ARRAY_SIZE(iommu_types); i++) { > if (ioctl(container->fd, VFIO_CHECK_EXTENSION, iommu_types[i])) { > + if (iommu_types[i] == VFIO_TYPE1_NESTING_IOMMU) { > + version = ioctl(container->fd, VFIO_CHECK_EXTENSION, > + VFIO_NESTING_IOMMU_UAPI); > + if (version < IOMMU_UAPI_VERSION) { > + info_report("IOMMU UAPI incompatible for nesting"); > + continue; > + } > + } This means that by default VFIO_TYPE1_NESTING_IOMMU wwould be chosen. I don't think this what we want. On ARM this would mean that for a standard VFIO assignment without vIOMMU, SL will be used instead of FL. This may not be harmless. For instance, in "[RFC v6 09/24] vfio: Force nested if iommu requires it", I use nested only if I detect we have a vSMMU. Otherwise I keep the legacy VFIO_TYPE1v2_IOMMU. Thanks Eric > return iommu_types[i]; > } > } > @@ -1278,6 +1287,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as, > } > > switch (container->iommu_type) { > + case VFIO_TYPE1_NESTING_IOMMU: > case VFIO_TYPE1v2_IOMMU: > case VFIO_TYPE1_IOMMU: > { >
Eric, > From: Auger Eric <eric.auger@redhat.com> > Sent: Monday, March 30, 2020 5:36 PM > To: Liu, Yi L <yi.l.liu@intel.com>; qemu-devel@nongnu.org; > Subject: Re: [PATCH v2 03/22] vfio: check VFIO_TYPE1_NESTING_IOMMU support > > Yi, > > On 3/30/20 6:24 AM, Liu Yi L wrote: > > VFIO needs to check VFIO_TYPE1_NESTING_IOMMU support with Kernel before > > further using it. e.g. requires to check IOMMU UAPI version. > > > > 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> > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > > --- > > hw/vfio/common.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > > index 0b3593b..c276732 100644 > > --- a/hw/vfio/common.c > > +++ b/hw/vfio/common.c > > @@ -1157,12 +1157,21 @@ static void > vfio_put_address_space(VFIOAddressSpace *space) > > static int vfio_get_iommu_type(VFIOContainer *container, > > Error **errp) > > { > > - int iommu_types[] = { VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU, > > + int iommu_types[] = { VFIO_TYPE1_NESTING_IOMMU, > > + VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU, > > VFIO_SPAPR_TCE_v2_IOMMU, VFIO_SPAPR_TCE_IOMMU }; > > - int i; > > + int i, version; > > > > for (i = 0; i < ARRAY_SIZE(iommu_types); i++) { > > if (ioctl(container->fd, VFIO_CHECK_EXTENSION, iommu_types[i])) { > > + if (iommu_types[i] == VFIO_TYPE1_NESTING_IOMMU) { > > + version = ioctl(container->fd, VFIO_CHECK_EXTENSION, > > + VFIO_NESTING_IOMMU_UAPI); > > + if (version < IOMMU_UAPI_VERSION) { > > + info_report("IOMMU UAPI incompatible for nesting"); > > + continue; > > + } > > + } > This means that by default VFIO_TYPE1_NESTING_IOMMU wwould be chosen. I > don't think this what we want. On ARM this would mean that for a > standard VFIO assignment without vIOMMU, SL will be used instead of FL. > This may not be harmless. > > For instance, in "[RFC v6 09/24] vfio: Force nested if iommu requires > it", I use nested only if I detect we have a vSMMU. Otherwise I keep the > legacy VFIO_TYPE1v2_IOMMU. > Good point. I also replied in your patch. Regards, Yi Liu
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 0b3593b..c276732 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -1157,12 +1157,21 @@ static void vfio_put_address_space(VFIOAddressSpace *space) static int vfio_get_iommu_type(VFIOContainer *container, Error **errp) { - int iommu_types[] = { VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU, + int iommu_types[] = { VFIO_TYPE1_NESTING_IOMMU, + VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU, VFIO_SPAPR_TCE_v2_IOMMU, VFIO_SPAPR_TCE_IOMMU }; - int i; + int i, version; for (i = 0; i < ARRAY_SIZE(iommu_types); i++) { if (ioctl(container->fd, VFIO_CHECK_EXTENSION, iommu_types[i])) { + if (iommu_types[i] == VFIO_TYPE1_NESTING_IOMMU) { + version = ioctl(container->fd, VFIO_CHECK_EXTENSION, + VFIO_NESTING_IOMMU_UAPI); + if (version < IOMMU_UAPI_VERSION) { + info_report("IOMMU UAPI incompatible for nesting"); + continue; + } + } return iommu_types[i]; } } @@ -1278,6 +1287,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as, } switch (container->iommu_type) { + case VFIO_TYPE1_NESTING_IOMMU: case VFIO_TYPE1v2_IOMMU: case VFIO_TYPE1_IOMMU: {