[RFC,2/6] iommu/amd: Add support for hw_info for iommu capability query
Commit Message
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(+)
Comments
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
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Sent: Wednesday, December 13, 2023 12:02 AM
>
> +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;
> +}
Don't this need to check the support in hw?
Hi Jason
On 12/13/2023 8:27 PM, Jason Gunthorpe wrote:
> On Tue, Dec 12, 2023 at 10:01:35AM -0600, Suravee Suthikulpanit wrote:
>> AMD IOMMU Extended Feature (EFR) and Extended Feature 2 (EFR2) registers
>>
>> ...
>>
>> 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
In subsequent patches of the part 2, this helper function will be used
to help populate the EFR and/or EFR2 in different call paths, which can
pass only efr or efr2 parameter individually.
>> +/**
>> + * 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
Sure
>> + */
>> +struct iommu_hw_info_amd {
>> + __u64 efr;
>> + __u64 efr2;
>> +};
>
> __aligned_u64
Okey.
Thanks,
Suravee
Hi Kevin,
On 12/15/2023 2:32 PM, Tian, Kevin wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Sent: Wednesday, December 13, 2023 12:02 AM
>>
>> +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;
>> +}
>
> Don't this need to check the support in hw?
Ah.. Good point. Let me add check for the support for these features in
the actual hardware.
Thanks,
Suravee
@@ -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
@@ -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)
@@ -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;
+}
+
+static void *amd_iommu_hw_info(struct device *dev, u32 *length, u32 *type)
+{
+ struct iommu_hw_info_amd *hwinfo;
+
+ hwinfo = kzalloc(sizeof(*hwinfo), GFP_KERNEL);
+ if (!hwinfo)
+ return ERR_PTR(-ENOMEM);
+
+ *length = sizeof(*hwinfo);
+ *type = IOMMU_HW_INFO_TYPE_AMD;
+
+ amd_iommu_build_efr(&hwinfo->efr, &hwinfo->efr2);
+ return hwinfo;
+}
+
const struct iommu_ops amd_iommu_ops = {
.capable = amd_iommu_capable,
+ .hw_info = amd_iommu_hw_info,
.domain_alloc = amd_iommu_domain_alloc,
.domain_alloc_user = amd_iommu_domain_alloc_user,
.probe_device = amd_iommu_probe_device,
@@ -474,15 +474,28 @@ struct iommu_hw_info_vtd {
__aligned_u64 ecap_reg;
};
+/**
+ * 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)
+ */
+struct iommu_hw_info_amd {
+ __u64 efr;
+ __u64 efr2;
+};
+
/**
* 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_AMD: AMD IOMMU info type
*/
enum iommu_hw_info_type {
IOMMU_HW_INFO_TYPE_NONE,
IOMMU_HW_INFO_TYPE_INTEL_VTD,
+ IOMMU_HW_INFO_TYPE_AMD,
};
/**