Hi Jean, > From: Jean-Philippe Brucker < jean-phili...@linaro.org> > Sent: Wednesday, June 17, 2020 10:39 PM > > [+ Will and Robin] > > Hi Yi, > > On Thu, Jun 11, 2020 at 05:15:21AM -0700, Liu Yi L wrote: > > IOMMUs that support nesting translation needs report the capability > > info to userspace, e.g. the format of first level/stage paging structures. > > > > Cc: Kevin Tian <kevin.t...@intel.com> > > CC: Jacob Pan <jacob.jun....@linux.intel.com> > > Cc: Alex Williamson <alex.william...@redhat.com> > > Cc: Eric Auger <eric.au...@redhat.com> > > Cc: Jean-Philippe Brucker <jean-phili...@linaro.org> > > Cc: Joerg Roedel <j...@8bytes.org> > > Cc: Lu Baolu <baolu...@linux.intel.com> > > Signed-off-by: Liu Yi L <yi.l....@intel.com> > > Signed-off-by: Jacob Pan <jacob.jun....@linux.intel.com> > > --- > > @Jean, Eric: as nesting was introduced for ARM, but looks like no > > actual user of it. right? So I'm wondering if we can reuse > > DOMAIN_ATTR_NESTING to retrieve nesting info? how about your opinions? > > Sure, I think we could rework the getters for DOMAIN_ATTR_NESTING since they > aren't used, but we do need to keep the setters as is. > > Before attaching a domain, VFIO sets DOMAIN_ATTR_NESTING if userspace > requested a VFIO_TYPE1_NESTING_IOMMU container. This is necessary for the > SMMU driver to know how to attach later, but at that point we don't know > whether > the SMMU does support nesting (since the domain isn't attached to any > endpoint). > During attach, the SMMU driver adapts to the SMMU's capabilities, and may well > fallback to one stage if the SMMU doesn't support nesting.
got you. so even VFIO sets DOMAIN_ATTR_NESTING successfully, it doesn't mean the nesting will be used. yeah, it's a little bit different with VT-d side. intel iommu driver will fail ATT_NESTING setting if it found not all iommu units in the system are nesting capable. > VFIO should check after attaching that the nesting attribute held, by calling > iommu_domain_get_attr(NESTING). At the moment it does not, and since your > 03/15 patch does that with additional info, I agree with reusing > DOMAIN_ATTR_NESTING instead of adding DOMAIN_ATTR_NESTING_INFO. > > However it requires changing the get_attr(NESTING) implementations in both > SMMU > drivers as a precursor of this series, to avoid breaking > VFIO_TYPE1_NESTING_IOMMU on Arm. Since we haven't yet defined the > nesting_info structs for SMMUv2 and v3, I suppose we could return an empty > struct > iommu_nesting_info for now? got you. I think it works. So far, I didn't see any getter for ATTR_NESTING, once SMMU drivers return empty struct iommu_nesting_info, VFIO won't fail. will do it when switching to reuse ATTR_NESTING for getting nesting info. > > > > include/linux/iommu.h | 1 + > > include/uapi/linux/iommu.h | 34 ++++++++++++++++++++++++++++++++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h index > > 78a26ae..f6e4b49 100644 > > --- a/include/linux/iommu.h > > +++ b/include/linux/iommu.h > > @@ -126,6 +126,7 @@ enum iommu_attr { > > DOMAIN_ATTR_FSL_PAMUV1, > > DOMAIN_ATTR_NESTING, /* two stages of translation */ > > DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, > > + DOMAIN_ATTR_NESTING_INFO, > > DOMAIN_ATTR_MAX, > > }; > > > > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h > > index 303f148..02eac73 100644 > > --- a/include/uapi/linux/iommu.h > > +++ b/include/uapi/linux/iommu.h > > @@ -332,4 +332,38 @@ struct iommu_gpasid_bind_data { > > }; > > }; > > > > +struct iommu_nesting_info { > > + __u32 size; > > + __u32 format; > > What goes into format? And flags? This structure needs some documentation. format will be the same with the definition of @format in iommu_gpasid_bind_data. flags is reserved for future extension. will add description in next version. :-) struct iommu_gpasid_bind_data { __u32 argsz; #define IOMMU_GPASID_BIND_VERSION_1 1 __u32 version; #define IOMMU_PASID_FORMAT_INTEL_VTD 1 __u32 format; #define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */ __u64 flags; __u64 gpgd; __u64 hpasid; __u64 gpasid; __u32 addr_width; __u8 padding[12]; /* Vendor specific data */ union { struct iommu_gpasid_bind_data_vtd vtd; } vendor; }; Regards, Yi Liu > Thanks, > Jean > > > + __u32 features; > > +#define IOMMU_NESTING_FEAT_SYSWIDE_PASID (1 << 0) > > +#define IOMMU_NESTING_FEAT_BIND_PGTBL (1 << 1) > > +#define IOMMU_NESTING_FEAT_CACHE_INVLD (1 << 2) > > + __u32 flags; > > + __u8 data[]; > > +}; > > + > > +/* > > + * @flags: VT-d specific flags. Currently reserved for future > > + * extension. > > + * @addr_width: The output addr width of first level/stage translation > > + * @pasid_bits: Maximum supported PASID bits, 0 represents no PASID > > + * support. > > + * @cap_reg: Describe basic capabilities as defined in VT-d > > capability > > + * register. > > + * @cap_mask: Mark valid capability bits in @cap_reg. > > + * @ecap_reg: Describe the extended capabilities as defined in VT-d > > + * extended capability register. > > + * @ecap_mask: Mark the valid capability bits in @ecap_reg. > > + */ > > +struct iommu_nesting_info_vtd { > > + __u32 flags; > > + __u16 addr_width; > > + __u16 pasid_bits; > > + __u64 cap_reg; > > + __u64 cap_mask; > > + __u64 ecap_reg; > > + __u64 ecap_mask; > > +}; > > + > > #endif /* _UAPI_IOMMU_H */ > > -- > > 2.7.4 > > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu