diff mbox series

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

Message ID 20231212160139.174229-3-suravee.suthikulpanit@amd.com
State New
Headers
Series iommu/amd: Introduce hardware info reporting and nested translation support |

Commit Message

Suravee Suthikulpanit Dec. 12, 2023, 4:01 p.m. UTC 
  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

Jason Gunthorpe Dec. 13, 2023, 1:27 p.m. UTC | #1 
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
Tian, Kevin Dec. 15, 2023, 7:32 a.m. UTC | #2 
> 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?
Suravee Suthikulpanit Jan. 5, 2024, 1:39 p.m. UTC | #3 
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
Suravee Suthikulpanit Jan. 5, 2024, 1:40 p.m. UTC | #4 
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
diff mbox series

Patch

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;
+}
+
+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,
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 0b2bc6252e2c..bf4a1f8ab748 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -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,
 };
 
 /**