Hi Jason, On Tue, Aug 27, 2024 at 12:51:36PM -0300, Jason Gunthorpe wrote: > From: Nicolin Chen <nicolinc@xxxxxxxxxx> > > For virtualization cases the IDR/IIDR/AIDR values of the actual SMMU > instance need to be available to the VMM so it can construct an > appropriate vSMMUv3 that reflects the correct HW capabilities. > > For userspace page tables these values are required to constrain the valid > values within the CD table and the IOPTEs. > > The kernel does not sanitize these values. If building a VMM then > userspace is required to only forward bits into a VM that it knows it can > implement. Some bits will also require a VMM to detect if appropriate > kernel support is available such as for ATS and BTM. > > Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 24 ++++++++++++++ > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 ++ > include/uapi/linux/iommufd.h | 35 +++++++++++++++++++++ > 3 files changed, 61 insertions(+) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > index c2021e821e5cb6..ec2fcdd4523a26 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -2288,6 +2288,29 @@ static bool arm_smmu_enforce_cache_coherency(struct iommu_domain *domain) > return ret; > } > > +static void *arm_smmu_hw_info(struct device *dev, u32 *length, u32 *type) > +{ > + struct arm_smmu_master *master = dev_iommu_priv_get(dev); > + struct iommu_hw_info_arm_smmuv3 *info; > + u32 __iomem *base_idr; > + unsigned int i; > + > + info = kzalloc(sizeof(*info), GFP_KERNEL); > + if (!info) > + return ERR_PTR(-ENOMEM); > + > + base_idr = master->smmu->base + ARM_SMMU_IDR0; > + for (i = 0; i <= 5; i++) > + info->idr[i] = readl_relaxed(base_idr + i); > + info->iidr = readl_relaxed(master->smmu->base + ARM_SMMU_IIDR); > + info->aidr = readl_relaxed(master->smmu->base + ARM_SMMU_AIDR); > + > + *length = sizeof(*info); > + *type = IOMMU_HW_INFO_TYPE_ARM_SMMUV3; > + > + return info; > +} > + > struct arm_smmu_domain *arm_smmu_domain_alloc(void) > { > struct arm_smmu_domain *smmu_domain; > @@ -3467,6 +3490,7 @@ static struct iommu_ops arm_smmu_ops = { > .identity_domain = &arm_smmu_identity_domain, > .blocked_domain = &arm_smmu_blocked_domain, > .capable = arm_smmu_capable, > + .hw_info = arm_smmu_hw_info, > .domain_alloc_paging = arm_smmu_domain_alloc_paging, > .domain_alloc_sva = arm_smmu_sva_domain_alloc, > .domain_alloc_user = arm_smmu_domain_alloc_user, > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > index 45882f65bfcad0..4b05c81b181a82 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > @@ -80,6 +80,8 @@ > #define IIDR_REVISION GENMASK(15, 12) > #define IIDR_IMPLEMENTER GENMASK(11, 0) > > +#define ARM_SMMU_AIDR 0x1C > + > #define ARM_SMMU_CR0 0x20 > #define CR0_ATSCHK (1 << 4) > #define CR0_CMDQEN (1 << 3) > diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h > index 4dde745cfb7e29..83b6e1cd338d8f 100644 > --- a/include/uapi/linux/iommufd.h > +++ b/include/uapi/linux/iommufd.h > @@ -484,15 +484,50 @@ struct iommu_hw_info_vtd { > __aligned_u64 ecap_reg; > }; > > +/** > + * struct iommu_hw_info_arm_smmuv3 - ARM SMMUv3 hardware information > + * (IOMMU_HW_INFO_TYPE_ARM_SMMUV3) > + * > + * @flags: Must be set to 0 > + * @__reserved: Must be 0 > + * @idr: Implemented features for ARM SMMU Non-secure programming interface > + * @iidr: Information about the implementation and implementer of ARM SMMU, > + * and architecture version supported > + * @aidr: ARM SMMU architecture version > + * > + * For the details of @idr, @iidr and @aidr, please refer to the chapters > + * from 6.3.1 to 6.3.6 in the SMMUv3 Spec. > + * > + * User space should read the underlying ARM SMMUv3 hardware information for > + * the list of supported features. > + * > + * Note that these values reflect the raw HW capability, without any insight if > + * any required kernel driver support is present. Bits may be set indicating the > + * HW has functionality that is lacking kernel software support, such as BTM. If > + * a VMM is using this information to construct emulated copies of these > + * registers it should only forward bits that it knows it can support. > + * > + * In future, presence of required kernel support will be indicated in flags. > + */ > +struct iommu_hw_info_arm_smmuv3 { > + __u32 flags; > + __u32 __reserved; > + __u32 idr[6]; > + __u32 iidr; > + __u32 aidr; > +}; There is a ton of information here, I think we might need to santitze the values for what user space needs to know (that's why I was asking about qemu) also SMMU_IDR4 is implementation define, not sure if we can unconditionally expose it to userspace. Thanks, Mostafa > + > /** > * enum iommu_hw_info_type - IOMMU Hardware Info Types > * @IOMMU_HW_INFO_TYPE_NONE: Used by the drivers that do not report hardware > * info > * @IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d iommu info type > + * @IOMMU_HW_INFO_TYPE_ARM_SMMUV3: ARM SMMUv3 iommu info type > */ > enum iommu_hw_info_type { > IOMMU_HW_INFO_TYPE_NONE = 0, > IOMMU_HW_INFO_TYPE_INTEL_VTD = 1, > + IOMMU_HW_INFO_TYPE_ARM_SMMUV3 = 2, > }; > > /** > -- > 2.46.0 >