Re: [RFC PATCH 2/6] iommu/amd: Add support for hw_info for iommu capability query

Jason Gunthorpe
2023-12-13T13:27:35Z

On Tue, Dec 12, 2023 at 10:01:35AM -0600, Suravee Suthikulpanit wrote:
> AMD IOMMU Extended Feature (EFR) and Extended Feature 2 (EFR2) registers
> specify features supported by each IOMMU hardware instance.
> The IOMMU driver checks each feature-specific bits before enabling
> each feature at run time.
>
> For IOMMUFD, the hypervisor determines which IOMMU features to support
> in the guest, and communicates this information to user-space (e.g. QEMU)
> via iommufd IOMMU_DEVICE_GET_HW_INFO ioctl.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> drivers/iommu/amd/amd_iommu.h | 2 ++
> drivers/iommu/amd/amd_iommu_types.h | 3 +++
> drivers/iommu/amd/iommu.c | 38 +++++++++++++++++++++++++++++
> include/uapi/linux/iommufd.h | 13 ++++++++++
> 4 files changed, 56 insertions(+)
>
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 108253edbeb0..4118129f4a24 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -72,6 +72,8 @@ void amd_iommu_dev_flush_pasid_pages(struct iommu_dev_data *dev_data,
> void amd_iommu_dev_flush_pasid_all(struct iommu_dev_data *dev_data,
> ioasid_t pasid);
>
> +void amd_iommu_build_efr(u64 *efr, u64 *efr2);
> +
> #ifdef CONFIG_IRQ_REMAP
> int amd_iommu_create_irq_domain(struct amd_iommu *iommu);
> #else
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 14f67a8cf755..956fd4658a4a 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -100,12 +100,15 @@
> #define FEATURE_HDSUP BIT_ULL(52)
> #define FEATURE_SNP BIT_ULL(63)
>
> +#define FEATURE_GATS_5LEVEL 1ULL
> #define FEATURE_GATS_SHIFT 12
> #define FEATURE_GATS_MASK (0x03ULL << FEATURE_GATS_SHIFT)
>
> +#define FEATURE_GLX_3LEVEL 0ULL
> #define FEATURE_GLX_SHIFT 14
> #define FEATURE_GLX_MASK (0x03ULL << FEATURE_GLX_SHIFT)
>
> +#define FEATURE_PASMAX_16 0xFULL
> #define FEATURE_PASMAX_SHIFT 32
> #define FEATURE_PASMAX_MASK (0x1FULL << FEATURE_PASMAX_SHIFT)
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4e4ff1550cf3..c41932e9f16a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2822,8 +2822,46 @@ static const struct iommu_dirty_ops amd_dirty_ops = {
> .read_and_clear_dirty = amd_iommu_read_and_clear_dirty,
> };
>
> +void amd_iommu_build_efr(u64 *efr, u64 *efr2)
> +{
> + if (efr) {
> + *efr = (FEATURE_GT | FEATURE_GIOSUP | FEATURE_PPR);
> +
> + /* 5-level v2 page table support */
> + *efr |= ((FEATURE_GATS_5LEVEL << FEATURE_GATS_SHIFT) &
> + FEATURE_GATS_MASK);
> +
> + /* 3-level GCR3 table support */
> + *efr |= ((FEATURE_GLX_3LEVEL << FEATURE_GLX_SHIFT) &
> + FEATURE_GLX_MASK);
> +
> + /* 16-bit PASMAX support */
> + *efr |= ((FEATURE_PASMAX_16 << FEATURE_PASMAX_SHIFT) &
> + FEATURE_PASMAX_MASK);
> + }
> +
> + if (efr2)
> + *efr2 = 0;

Why are you checking for null here? It is never called with null

> +/**
> + * struct iommu_hw_info_amd - AMD IOMMU device info
> + *
> + * @efr : Value of AMD IOMMU Extended Feature Register (EFR)
> + * @efr2: Value of AMD IOMMU Extended Feature 2 Register (EFR2)

Please reference a section in the spec for what these are just for
clarity

> + */
> +struct iommu_hw_info_amd {
> + __u64 efr;
> + __u64 efr2;
> +};

__aligned_u64

Jason