LinuxLists.cc - [PATCH v1 5/8] vfio/type1: Report 1st-level/stage-1 format to userspace

2020-03-22 12:27:34

Subject: [PATCH v1 5/8] vfio/type1: Report 1st-level/stage-1 format to userspace

From: Liu Yi L <[email protected]>

VFIO exposes IOMMU nesting translation (a.k.a dual stage translation)
capability to userspace. Thus applications like QEMU could support
vIOMMU with hardware's nesting translation capability for pass-through
devices. Before setting up nesting translation for pass-through devices,
QEMU and other applications need to learn the supported 1st-lvl/stage-1
translation structure format like page table format.

Take vSVA (virtual Shared Virtual Addressing) as an example, to support
vSVA for pass-through devices, QEMU setup nesting translation for pass-
through devices. The guest page table are configured to host as 1st-lvl/
stage-1 page table. Therefore, guest format should be compatible with
host side.

This patch reports the supported 1st-lvl/stage-1 page table format on the
current platform to userspace. QEMU and other alike applications should
use this format info when trying to setup IOMMU nesting translation on
host IOMMU.

Cc: Kevin Tian <[email protected]>
CC: Jacob Pan <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Signed-off-by: Liu Yi L <[email protected]>
---
drivers/vfio/vfio_iommu_type1.c | 56 +++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 1 +
2 files changed, 57 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 9aa2a67..82a9e0b 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -2234,11 +2234,66 @@ static int vfio_iommu_type1_pasid_free(struct vfio_iommu *iommu,
return ret;
}

+static int vfio_iommu_get_stage1_format(struct vfio_iommu *iommu,
+ u32 *stage1_format)
+{
+ struct vfio_domain *domain;
+ u32 format = 0, tmp_format = 0;
+ int ret;
+
+ mutex_lock(&iommu->lock);
+ if (list_empty(&iommu->domain_list)) {
+ mutex_unlock(&iommu->lock);
+ return -EINVAL;
+ }
+
+ list_for_each_entry(domain, &iommu->domain_list, next) {
+ if (iommu_domain_get_attr(domain->domain,
+ DOMAIN_ATTR_PASID_FORMAT, &format)) {
+ ret = -EINVAL;
+ format = 0;
+ goto out_unlock;
+ }
+ /*
+ * format is always non-zero (the first format is
+ * IOMMU_PASID_FORMAT_INTEL_VTD which is 1). For
+ * the reason of potential different backed IOMMU
+ * formats, here we expect to have identical formats
+ * in the domain list, no mixed formats support.
+ * return -EINVAL to fail the attempt of setup
+ * VFIO_TYPE1_NESTING_IOMMU if non-identical formats
+ * are detected.
+ */
+ if (tmp_format && tmp_format != format) {
+ ret = -EINVAL;
+ format = 0;
+ goto out_unlock;
+ }
+
+ tmp_format = format;
+ }
+ ret = 0;
+
+out_unlock:
+ if (format)
+ *stage1_format = format;
+ mutex_unlock(&iommu->lock);
+ return ret;
+}
+
static int vfio_iommu_info_add_nesting_cap(struct vfio_iommu *iommu,
struct vfio_info_cap *caps)
{
struct vfio_info_cap_header *header;
struct vfio_iommu_type1_info_cap_nesting *nesting_cap;
+ u32 formats = 0;
+ int ret;
+
+ ret = vfio_iommu_get_stage1_format(iommu, &formats);
+ if (ret) {
+ pr_warn("Failed to get stage-1 format\n");
+ return ret;
+ }

header = vfio_info_cap_add(caps, sizeof(*nesting_cap),
VFIO_IOMMU_TYPE1_INFO_CAP_NESTING, 1);
@@ -2254,6 +2309,7 @@ static int vfio_iommu_info_add_nesting_cap(struct vfio_iommu *iommu,
/* nesting iommu type supports PASID requests (alloc/free) */
nesting_cap->nesting_capabilities |= VFIO_IOMMU_PASID_REQS;
}
+ nesting_cap->stage1_formats = formats;

return 0;
}
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index ed9881d..ebeaf3e 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -763,6 +763,7 @@ struct vfio_iommu_type1_info_cap_nesting {
struct vfio_info_cap_header header;
#define VFIO_IOMMU_PASID_REQS (1 << 0)
__u32 nesting_capabilities;
+ __u32 stage1_formats;
};

#define VFIO_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
--
2.7.4

2020-03-22 16:46:59

Hi Jean, Eric,

> From: Liu, Yi L <[email protected]>
> Sent: Thursday, April 9, 2020 8:47 PM
> Subject: RE: [PATCH v1 5/8] vfio/type1: Report 1st-level/stage-1 format to
> userspace
>
[...]
> > > >>
> > > >> Yes I don't think an u32 is going to cut it for Arm :( We need to
> > > >> describe all sorts
> > of
> > > >> capabilities for page and PASID tables (granules, GPA size, ASID/PASID size,
> HW
> > > >> access/dirty, etc etc.) Just saying "Arm stage-1 format" wouldn't mean
> much. I
> > > >> guess we could have a secondary vendor capability for these?
> > > >
> > > > Actually, I'm wondering if we can define some formats to stands for a set of
> > > > capabilities. e.g. VTD_STAGE1_FORMAT_V1 which may indicates the 1st
> level
> > > > page table related caps (aw, a/d, SRE, EA and etc.). And vIOMMU can parse
> > > > the capabilities.
> > >
> > > But eventually do we really need all those capability getters? I mean
> > > can't we simply rely on the actual call to VFIO_IOMMU_BIND_GUEST_PGTBL()
> > > to detect any mismatch? Definitively the error handling may be heavier
> > > on userspace but can't we manage.
> >
> > I think we need to present these capabilities at boot time, long before
> > the guest triggers a bind(). For example if the host SMMU doesn't support
> > 16-bit ASID, we need to communicate that to the guest using vSMMU ID
> > registers or PROBE properties. Otherwise a bind() will succeed, but if the
> > guest uses 16-bit ASIDs in its CD, DMA will result in C_BAD_CD events
> > which we'll inject into the guest, for no apparent reason from their
> > perspective.
> >
> > In addition some VMMs may have fallbacks if shared page tables are not
> > available. They could fall back to a MAP/UNMAP interface, or simply not
> > present a vIOMMU to the guest.
> >
>
> Based on the comments, I think it would be a need to report iommu caps
> in detail. So I guess iommu uapi needs to provide something alike vfio
> cap chain in iommu uapi. Please feel free let me know your thoughts. :-)

Consider more, I guess it may be better to start simpler. Cap chain suits
the case in which there are multiple caps. e.g. some vendor iommu driver
may want to report iommu capabilities via multiple caps. Actually, in VT-d
side, the host IOMMU capability could be reported in a single cap structure.
I'm not sure about ARM side. Will there be multiple iommu_info_caps for ARM?

> In vfio, we can define a cap as below:
>
> struct vfio_iommu_type1_info_cap_nesting {
> struct vfio_info_cap_header header;
> __u64 iommu_model;
> #define VFIO_IOMMU_PASID_REQS (1 << 0)
> #define VFIO_IOMMU_BIND_GPASID (1 << 1)
> #define VFIO_IOMMU_CACHE_INV (1 << 2)
> __u32 nesting_capabilities;
> __u32 pasid_bits;
> #define VFIO_IOMMU_VENDOR_SUB_CAP (1 << 3)
> __u32 flags;
> __u32 data_size;
> __u8 data[]; /*iommu info caps defined by iommu uapi */
> };
>

If iommu vendor driver only needs one cap structure to report hw
capability, then I think we needn't implement cap chain in iommu
uapi. The @data[] field could be determined by the @iommu_model
and @flags fields. This would be easier. thoughts?

> VFIO needs new iommu APIs to ask iommu driver whether PASID/bind_gpasid/
> cache_inv/bind_gpasid_table is available or not and also the pasid
> bits. After that VFIO will ask iommu driver about the iommu_cap_info
> and fill in the @data[] field.
>
> iommu uapi:
> struct iommu_info_cap_header {
> __u16 id; /* Identifies capability */
> __u16 version; /* Version specific to the capability ID */
> __u32 next; /* Offset of next capability */
> };
>
> #define IOMMU_INFO_CAP_INTEL_VTD 1
> struct iommu_info_cap_intel_vtd {
> struct iommu_info_cap_header header;
> __u32 vaddr_width; /* VA addr_width*/
> __u32 ipaddr_width; /* IPA addr_width, input of SL page table */
> /* same definition with @flags instruct iommu_gpasid_bind_data_vtd */
> __u64 flags;
> };
>
> #define IOMMU_INFO_CAP_ARM_SMMUv3 2
> struct iommu_info_cap_arm_smmuv3 {
> struct iommu_info_cap_header header;
> ...
> };

Regards,
Yi Liu