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
>
