[PATCH 0/2] iommu/amd: Support for HATdis and HATS features

[PATCH 0/2] iommu/amd: Support for HATdis and HATS features

[Ankit Soni]

This series comprises two features:

HATDis: When host address translation is not supported by the IOMMU,
the IVHD->attr[HATDis] bit is set. For instance, QEMU emulated vIOMMU
does not support dma tranlation for VFIO passthrough device.
Support has been added to check this bit and switch to guest page table 
mode, if supported. This feature is useful for cases where only interrupt
remapping is required.

EFR[HATS]: These bits indicate the maximum level supported for host page table
by the IOMMU. Modifications have been made to adjust the maximum host page table
level, starting with level 4. If these bits are set to 11b, the driver will
attempt to use the guest page table.

If guest page table mode is not supported, the driver will fail to probe
devices for DMA translations.
The interrupt remapping will continue to be supported.

Ankit Soni (2):
  iommu/amd: Add HATDis feature support
  iommu/amd: Add efr[HATS] max v1 page table level

 drivers/iommu/amd/amd_iommu.h       |  2 ++
 drivers/iommu/amd/amd_iommu_types.h |  7 +++++-
 drivers/iommu/amd/init.c            | 39 +++++++++++++++++++++++++++--
 drivers/iommu/amd/io_pgtable.c      |  4 +--
 drivers/iommu/amd/iommu.c           | 15 ++++++++++-
 5 files changed, 61 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH 1/2] iommu/amd: Add HATDis feature support

[Ankit Soni]

Current AMD IOMMU assumes Host Address Translation (HAT) is always
supported, and Linux kernel enables this capability by default. However,
in case of emulated and virtualized IOMMU, this might not be the case.
For example,current QEMU-emulated AMD vIOMMU does not support host
translation for VFIO pass-through device, but the interrupt remapping
support is required for x2APIC (i.e. kvm-msi-ext-dest-id is also not
supported by the guest OS). This would require the guest kernel to boot
with guest kernel option iommu=pt to by-pass the initialization of
host (v1) table.

The AMD I/O Virtualization Technology (IOMMU) Specification Rev 3.10 [1]
introduces a new flag 'HATDis' in the IVHD 11h IOMMU attributes to indicate
that HAT is not supported on a particular IOMMU instance.

Therefore, modifies the AMD IOMMU driver to detect the new HATDis
attributes, and disable host translation and switch to use guest
translation if it is available. Otherwise, the driver will disable DMA
translation.

[1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>
---
 drivers/iommu/amd/amd_iommu.h       |  1 +
 drivers/iommu/amd/amd_iommu_types.h |  6 +++++-
 drivers/iommu/amd/init.c            | 23 +++++++++++++++++++++--
 drivers/iommu/amd/iommu.c           | 13 +++++++++++++
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 220c598b7e14..bb14c4800dd0 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -43,6 +43,7 @@ extern int amd_iommu_guest_ir;
 extern enum protection_domain_mode amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 extern unsigned long amd_iommu_pgsize_bitmap;
+extern bool amd_iommu_hatdis;
 
 /* Protection domain ops */
 void amd_iommu_init_identity_domain(void);
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 5089b58e528a..284ff4309660 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -460,6 +460,9 @@
 /* IOMMU Feature Reporting Field (for IVHD type 10h */
 #define IOMMU_FEAT_GASUP_SHIFT	6
 
+/* IOMMU HATDIS for IVHD type 11h and 40h */
+#define IOMMU_IVHD_ATTR_HATDIS_SHIFT	0
+
 /* IOMMU Extended Feature Register (EFR) */
 #define IOMMU_EFR_XTSUP_SHIFT	2
 #define IOMMU_EFR_GASUP_SHIFT	7
@@ -558,7 +561,8 @@ struct amd_io_pgtable {
 };
 
 enum protection_domain_mode {
-	PD_MODE_V1 = 1,
+	PD_MODE_NONE,
+	PD_MODE_V1,
 	PD_MODE_V2,
 };
 
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index dd9e26b7b718..f71b236c2af2 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -151,7 +151,7 @@ struct ivmd_header {
 bool amd_iommu_dump;
 bool amd_iommu_irq_remap __read_mostly;
 
-enum protection_domain_mode amd_iommu_pgtable = PD_MODE_V1;
+enum protection_domain_mode amd_iommu_pgtable = PD_MODE_NONE;
 /* Guest page table level */
 int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
 
@@ -168,6 +168,9 @@ static int amd_iommu_target_ivhd_type;
 u64 amd_iommu_efr;
 u64 amd_iommu_efr2;
 
+/* dma translation not supported*/
+bool amd_iommu_hatdis;
+
 /* SNP is enabled on the system? */
 bool amd_iommu_snp_en;
 EXPORT_SYMBOL(amd_iommu_snp_en);
@@ -1798,6 +1801,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h,
 		if (h->efr_reg & BIT(IOMMU_EFR_XTSUP_SHIFT))
 			amd_iommu_xt_mode = IRQ_REMAP_X2APIC_MODE;
 
+		if (h->efr_attr & BIT(IOMMU_IVHD_ATTR_HATDIS_SHIFT)) {
+			pr_warn_once("Host Address Translation is not supported.\n");
+			amd_iommu_hatdis = true;
+		}
+
 		early_iommu_features_init(iommu, h);
 
 		break;
@@ -2582,7 +2590,7 @@ static void init_device_table_dma(struct amd_iommu_pci_seg *pci_seg)
 	u32 devid;
 	struct dev_table_entry *dev_table = pci_seg->dev_table;
 
-	if (dev_table == NULL)
+	if (!dev_table || amd_iommu_pgtable == PD_MODE_NONE)
 		return;
 
 	for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
@@ -3095,6 +3103,17 @@ static int __init early_amd_iommu_init(void)
 		}
 	}
 
+	if (amd_iommu_hatdis) {
+		if (amd_iommu_v2_pgtbl_supported())
+			amd_iommu_pgtable = PD_MODE_V2;
+	} else if (amd_iommu_pgtable == PD_MODE_NONE)
+		/*
+		 * If v1 page table is supported (i.e., amd_iommu_hatdis == 0)
+		 * and page table type is not specified in command line, then
+		 * use v1 page table.
+		 */
+		amd_iommu_pgtable = PD_MODE_V1;
+
 	/* Disable any previously enabled IOMMUs */
 	if (!is_kdump_kernel() || amd_iommu_disabled)
 		disable_iommus();
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index be8761bbef0f..0ebc264726da 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2393,6 +2393,13 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 					     pci_max_pasids(to_pci_dev(dev)));
 	}
 
+	if (amd_iommu_pgtable == PD_MODE_NONE) {
+		pr_warn_once("%s: DMA translation not supported by iommu.\n",
+			     __func__);
+		iommu_dev = ERR_PTR(-ENODEV);
+		goto out_err;
+	}
+
 out_err:
 
 	iommu_completion_wait(iommu);
@@ -2480,6 +2487,9 @@ static int pdom_setup_pgtable(struct protection_domain *domain,
 	case PD_MODE_V2:
 		fmt = AMD_IOMMU_V2;
 		break;
+	case PD_MODE_NONE:
+		WARN_ON_ONCE(1);
+		return -EPERM;
 	}
 
 	domain->iop.pgtbl.cfg.amd.nid = dev_to_node(dev);
@@ -2501,6 +2511,9 @@ static inline u64 dma_max_address(enum protection_domain_mode pgtable)
 
 static bool amd_iommu_hd_support(struct amd_iommu *iommu)
 {
+	if (amd_iommu_hatdis)
+		return false;
+
 	return iommu && (iommu->features & FEATURE_HDSUP);
 }
 
-- 
2.43.0

[PATCH 2/2] iommu/amd: Add efr[HATS] max v1 page table level

[Ankit Soni]

The EFR[HATS] bits indicate maximum host translation level supported by
IOMMU. Adding support to set the maximum host page table level as indicated
by EFR[HATS]. If the HATS=11b (reserved), the driver will attempt to use
guest page table for DMA API.

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>
---
 drivers/iommu/amd/amd_iommu.h       |  1 +
 drivers/iommu/amd/amd_iommu_types.h |  1 +
 drivers/iommu/amd/init.c            | 16 ++++++++++++++++
 drivers/iommu/amd/io_pgtable.c      |  4 ++--
 drivers/iommu/amd/iommu.c           |  2 +-
 5 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index bb14c4800dd0..0286120ad4a5 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -42,6 +42,7 @@ int amd_iommu_enable_faulting(unsigned int cpu);
 extern int amd_iommu_guest_ir;
 extern enum protection_domain_mode amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
+extern u8 amd_iommu_hpt_level;
 extern unsigned long amd_iommu_pgsize_bitmap;
 extern bool amd_iommu_hatdis;
 
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 284ff4309660..6bf81197c2c8 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -96,6 +96,7 @@
 #define FEATURE_GA		BIT_ULL(7)
 #define FEATURE_HE		BIT_ULL(8)
 #define FEATURE_PC		BIT_ULL(9)
+#define FEATURE_HATS		GENMASK_ULL(11, 10)
 #define FEATURE_GATS		GENMASK_ULL(13, 12)
 #define FEATURE_GLX		GENMASK_ULL(15, 14)
 #define FEATURE_GAM_VAPIC	BIT_ULL(21)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index f71b236c2af2..c713756fa44e 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -152,6 +152,8 @@ bool amd_iommu_dump;
 bool amd_iommu_irq_remap __read_mostly;
 
 enum protection_domain_mode amd_iommu_pgtable = PD_MODE_NONE;
+/* Host page table level */
+u8 amd_iommu_hpt_level;
 /* Guest page table level */
 int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
 
@@ -3052,6 +3054,7 @@ static int __init early_amd_iommu_init(void)
 	struct acpi_table_header *ivrs_base;
 	int ret;
 	acpi_status status;
+	u8 efr_hats;
 
 	if (!amd_iommu_detected)
 		return -ENODEV;
@@ -3096,6 +3099,19 @@ static int __init early_amd_iommu_init(void)
 	    FIELD_GET(FEATURE_GATS, amd_iommu_efr) == GUEST_PGTABLE_5_LEVEL)
 		amd_iommu_gpt_level = PAGE_MODE_5_LEVEL;
 
+	efr_hats = FIELD_GET(FEATURE_HATS, amd_iommu_efr);
+	if (efr_hats != 0x3) {
+		/*
+		 * efr[HATS] bits specify the maximum host translation level
+		 * supported, with LEVEL 4 being initial max level.
+		 */
+		amd_iommu_hpt_level = efr_hats + PAGE_MODE_4_LEVEL;
+	} else {
+		pr_warn_once("Disable host address translation due to invalid max level (%#x).\n",
+			     efr_hats);
+		amd_iommu_hatdis = true;
+	}
+
 	if (amd_iommu_pgtable == PD_MODE_V2) {
 		if (!amd_iommu_v2_pgtbl_supported()) {
 			pr_warn("Cannot enable v2 page table for DMA-API. Fallback to v1.\n");
diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
index 26cf562dde11..0d8bc06f63d7 100644
--- a/drivers/iommu/amd/io_pgtable.c
+++ b/drivers/iommu/amd/io_pgtable.c
@@ -132,7 +132,7 @@ static bool increase_address_space(struct amd_io_pgtable *pgtable,
 		goto out;
 
 	ret = false;
-	if (WARN_ON_ONCE(pgtable->mode == PAGE_MODE_6_LEVEL))
+	if (WARN_ON_ONCE(pgtable->mode == amd_iommu_hpt_level))
 		goto out;
 
 	*pte = PM_LEVEL_PDE(pgtable->mode, iommu_virt_to_phys(pgtable->root));
@@ -531,7 +531,7 @@ static void v1_free_pgtable(struct io_pgtable *iop)
 
 	/* Page-table is not visible to IOMMU anymore, so free it */
 	BUG_ON(pgtable->mode < PAGE_MODE_NONE ||
-	       pgtable->mode > PAGE_MODE_6_LEVEL);
+	       pgtable->mode > amd_iommu_hpt_level);
 
 	free_sub_pt(pgtable->root, pgtable->mode, &freelist);
 	iommu_put_pages_list(&freelist);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 0ebc264726da..a2f957ee9110 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2503,7 +2503,7 @@ static int pdom_setup_pgtable(struct protection_domain *domain,
 static inline u64 dma_max_address(enum protection_domain_mode pgtable)
 {
 	if (pgtable == PD_MODE_V1)
-		return ~0ULL;
+		return PM_LEVEL_SIZE(amd_iommu_hpt_level);
 
 	/* V2 with 4/5 level page table */
 	return ((1ULL << PM_LEVEL_SHIFT(amd_iommu_gpt_level)) - 1);
-- 
2.43.0

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Joao Martins]

On 23/04/2025 07:50, Ankit Soni wrote:
> Current AMD IOMMU assumes Host Address Translation (HAT) is always
> supported, and Linux kernel enables this capability by default. However,
> in case of emulated and virtualized IOMMU, this might not be the case.

+Alejandro as he is filling that gap

> For example,current QEMU-emulated AMD vIOMMU does not support host
> translation for VFIO pass-through device, but the interrupt remapping
> support is required for x2APIC (i.e. kvm-msi-ext-dest-id is also not
> supported by the guest OS). This would require the guest kernel to boot
> with guest kernel option iommu=pt to by-pass the initialization of
> host (v1) table.
> 
> The AMD I/O Virtualization Technology (IOMMU) Specification Rev 3.10 [1]>
introduces a new flag 'HATDis' in the IVHD 11h IOMMU attributes to indicate
> that HAT is not supported on a particular IOMMU instance.
> 
> Therefore, modifies the AMD IOMMU driver to detect the new HATDis
> attributes, and disable host translation and switch to use guest
> translation if it is available. Otherwise, the driver will disable DMA
> translation.
> 
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
> 
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu.h       |  1 +
>  drivers/iommu/amd/amd_iommu_types.h |  6 +++++-
>  drivers/iommu/amd/init.c            | 23 +++++++++++++++++++++--
>  drivers/iommu/amd/iommu.c           | 13 +++++++++++++
>  4 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 220c598b7e14..bb14c4800dd0 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -43,6 +43,7 @@ extern int amd_iommu_guest_ir;
>  extern enum protection_domain_mode amd_iommu_pgtable;
>  extern int amd_iommu_gpt_level;
>  extern unsigned long amd_iommu_pgsize_bitmap;
> +extern bool amd_iommu_hatdis;
>  
>  /* Protection domain ops */
>  void amd_iommu_init_identity_domain(void);
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 5089b58e528a..284ff4309660 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -460,6 +460,9 @@
>  /* IOMMU Feature Reporting Field (for IVHD type 10h */
>  #define IOMMU_FEAT_GASUP_SHIFT	6
>  
> +/* IOMMU HATDIS for IVHD type 11h and 40h */
> +#define IOMMU_IVHD_ATTR_HATDIS_SHIFT	0
> +
>  /* IOMMU Extended Feature Register (EFR) */
>  #define IOMMU_EFR_XTSUP_SHIFT	2
>  #define IOMMU_EFR_GASUP_SHIFT	7
> @@ -558,7 +561,8 @@ struct amd_io_pgtable {
>  };
>  
>  enum protection_domain_mode {
> -	PD_MODE_V1 = 1,
> +	PD_MODE_NONE,
> +	PD_MODE_V1,
>  	PD_MODE_V2,
>  };
>  
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index dd9e26b7b718..f71b236c2af2 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -151,7 +151,7 @@ struct ivmd_header {
>  bool amd_iommu_dump;
>  bool amd_iommu_irq_remap __read_mostly;
>  
> -enum protection_domain_mode amd_iommu_pgtable = PD_MODE_V1;
> +enum protection_domain_mode amd_iommu_pgtable = PD_MODE_NONE;
>  /* Guest page table level */
>  int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
>  
> @@ -168,6 +168,9 @@ static int amd_iommu_target_ivhd_type;
>  u64 amd_iommu_efr;
>  u64 amd_iommu_efr2;
>  
> +/* dma translation not supported*/
> +bool amd_iommu_hatdis;
> +
>  /* SNP is enabled on the system? */
>  bool amd_iommu_snp_en;
>  EXPORT_SYMBOL(amd_iommu_snp_en);
> @@ -1798,6 +1801,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h,
>  		if (h->efr_reg & BIT(IOMMU_EFR_XTSUP_SHIFT))
>  			amd_iommu_xt_mode = IRQ_REMAP_X2APIC_MODE;
>  
> +		if (h->efr_attr & BIT(IOMMU_IVHD_ATTR_HATDIS_SHIFT)) {
> +			pr_warn_once("Host Address Translation is not supported.\n");
> +			amd_iommu_hatdis = true;
> +		}
> +
>  		early_iommu_features_init(iommu, h);
>  
>  		break;
> @@ -2582,7 +2590,7 @@ static void init_device_table_dma(struct amd_iommu_pci_seg *pci_seg)
>  	u32 devid;
>  	struct dev_table_entry *dev_table = pci_seg->dev_table;
>  
> -	if (dev_table == NULL)
> +	if (!dev_table || amd_iommu_pgtable == PD_MODE_NONE)
>  		return;
>  
>  	for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
> @@ -3095,6 +3103,17 @@ static int __init early_amd_iommu_init(void)
>  		}
>  	}
>  
> +	if (amd_iommu_hatdis) {
> +		if (amd_iommu_v2_pgtbl_supported())
> +			amd_iommu_pgtable = PD_MODE_V2;
> +	} else if (amd_iommu_pgtable == PD_MODE_NONE)
> +		/*
> +		 * If v1 page table is supported (i.e., amd_iommu_hatdis == 0)
> +		 * and page table type is not specified in command line, then
> +		 * use v1 page table.
> +		 */
> +		amd_iommu_pgtable = PD_MODE_V1;
> +
>  	/* Disable any previously enabled IOMMUs */
>  	if (!is_kdump_kernel() || amd_iommu_disabled)
>  		disable_iommus();
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index be8761bbef0f..0ebc264726da 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2393,6 +2393,13 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
>  					     pci_max_pasids(to_pci_dev(dev)));
>  	}
>  
> +	if (amd_iommu_pgtable == PD_MODE_NONE) {
> +		pr_warn_once("%s: DMA translation not supported by iommu.\n",
> +			     __func__);
> +		iommu_dev = ERR_PTR(-ENODEV);
> +		goto out_err;
> +	}
> +
>  out_err:
>  
>  	iommu_completion_wait(iommu);
> @@ -2480,6 +2487,9 @@ static int pdom_setup_pgtable(struct protection_domain *domain,
>  	case PD_MODE_V2:
>  		fmt = AMD_IOMMU_V2;
>  		break;
> +	case PD_MODE_NONE:
> +		WARN_ON_ONCE(1);
> +		return -EPERM;
>  	}
>  
>  	domain->iop.pgtbl.cfg.amd.nid = dev_to_node(dev);
> @@ -2501,6 +2511,9 @@ static inline u64 dma_max_address(enum protection_domain_mode pgtable)
>  
>  static bool amd_iommu_hd_support(struct amd_iommu *iommu)
>  {
> +	if (amd_iommu_hatdis)
> +		return false;
> +
>  	return iommu && (iommu->features & FEATURE_HDSUP);
>  }
>  

It's strange we seem to somehow have host translation disabled, while it
advertises other translation-related features like the normal case.

In any case we should probably follow Intel's example (which does similar thing
to HATSDis) where we only call invoke IOMMU groups
iommu_device_register()/iommu_device_sysfs_add() with DMA translation enabled?
That should simplify most of the patch as those codepaths are not reachable via
kernel/userspace? Unless I am missing something ofc

See also commit c40aaaac10 ("iommu/vt-d: Gracefully handle DMAR units with no
supported address widths"). I am not sure what else is the closest example here
besides intel-iommu equivalent.

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Ankit Soni]

Hi Joao,
Thank you for review this patch.

On Thu, Apr 24, 2025 at 01:19:43PM +0100, Joao Martins wrote:
> >  {
> > +	if (amd_iommu_hatdis)
> > +		return false;
> > +
> >  	return iommu && (iommu->features & FEATURE_HDSUP);
> >  }
> >  
> 
> It's strange we seem to somehow have host translation disabled, while it
> advertises other translation-related features like the normal case.

If HATDis bit is enabled driver will ignore host(v1) page table related field,
this includes nested page table.

> 
> In any case we should probably follow Intel's example (which does similar thing
> to HATSDis) where we only call invoke IOMMU groups
> iommu_device_register()/iommu_device_sysfs_add() with DMA translation enabled?
> That should simplify most of the patch as those codepaths are not reachable via
> kernel/userspace? Unless I am missing something ofc
> 
> See also commit c40aaaac10 ("iommu/vt-d: Gracefully handle DMAR units with no
> supported address widths"). I am not sure what else is the closest example here
> besides intel-iommu equivalent.

With intel patch you mentioned above, it seems that it is mostly handling 
"second stage translation support" disable, which will eventually disable dma 
translation. And in AMD case, HATDis bit indicates host(v1) translation is not
available, then attempt to use guest(v2) translation, and if both page
table modes are not available then disable dma tranlation.

Thanks,
-Ankit

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Vasant Hegde]

Hi,

On 4/23/2025 12:20 PM, Ankit Soni wrote:
> Current AMD IOMMU assumes Host Address Translation (HAT) is always
> supported, and Linux kernel enables this capability by default. However,
> in case of emulated and virtualized IOMMU, this might not be the case.
> For example,current QEMU-emulated AMD vIOMMU does not support host
> translation for VFIO pass-through device, but the interrupt remapping
> support is required for x2APIC (i.e. kvm-msi-ext-dest-id is also not
> supported by the guest OS). This would require the guest kernel to boot
> with guest kernel option iommu=pt to by-pass the initialization of> host (v1)
table.
> 
> The AMD I/O Virtualization Technology (IOMMU) Specification Rev 3.10 [1]
> introduces a new flag 'HATDis' in the IVHD 11h IOMMU attributes to indicate
> that HAT is not supported on a particular IOMMU instance.
> 
> Therefore, modifies the AMD IOMMU driver to detect the new HATDis
> attributes, and disable host translation and switch to use guest
> translation if it is available. Otherwise, the driver will disable DMA
> translation.
> 
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
> 
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu.h       |  1 +
>  drivers/iommu/amd/amd_iommu_types.h |  6 +++++-
>  drivers/iommu/amd/init.c            | 23 +++++++++++++++++++++--
>  drivers/iommu/amd/iommu.c           | 13 +++++++++++++
>  4 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 220c598b7e14..bb14c4800dd0 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -43,6 +43,7 @@ extern int amd_iommu_guest_ir;
>  extern enum protection_domain_mode amd_iommu_pgtable;
>  extern int amd_iommu_gpt_level;
>  extern unsigned long amd_iommu_pgsize_bitmap;
> +extern bool amd_iommu_hatdis;
>  
>  /* Protection domain ops */
>  void amd_iommu_init_identity_domain(void);
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 5089b58e528a..284ff4309660 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -460,6 +460,9 @@
>  /* IOMMU Feature Reporting Field (for IVHD type 10h */
>  #define IOMMU_FEAT_GASUP_SHIFT	6
>  
> +/* IOMMU HATDIS for IVHD type 11h and 40h */
> +#define IOMMU_IVHD_ATTR_HATDIS_SHIFT	0
> +
>  /* IOMMU Extended Feature Register (EFR) */
>  #define IOMMU_EFR_XTSUP_SHIFT	2
>  #define IOMMU_EFR_GASUP_SHIFT	7
> @@ -558,7 +561,8 @@ struct amd_io_pgtable {
>  };
>  
>  enum protection_domain_mode {
> -	PD_MODE_V1 = 1,
> +	PD_MODE_NONE,
> +	PD_MODE_V1,
>  	PD_MODE_V2,
>  };
>  
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index dd9e26b7b718..f71b236c2af2 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -151,7 +151,7 @@ struct ivmd_header {
>  bool amd_iommu_dump;
>  bool amd_iommu_irq_remap __read_mostly;
>  
> -enum protection_domain_mode amd_iommu_pgtable = PD_MODE_V1;
> +enum protection_domain_mode amd_iommu_pgtable = PD_MODE_NONE;


Why not keep it as `PD_MODE_V1` (as its our default page table) so that we can
remove explict `PD_MODE_V1` assignment in below hunk?

>  /* Guest page table level */
>  int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
>  
> @@ -168,6 +168,9 @@ static int amd_iommu_target_ivhd_type;
>  u64 amd_iommu_efr;
>  u64 amd_iommu_efr2;
>  
> +/* dma translation not supported*/

Host (v2) page table is not supported.

> +bool amd_iommu_hatdis;
> +
>  /* SNP is enabled on the system? */
>  bool amd_iommu_snp_en;
>  EXPORT_SYMBOL(amd_iommu_snp_en);
> @@ -1798,6 +1801,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h,
>  		if (h->efr_reg & BIT(IOMMU_EFR_XTSUP_SHIFT))
>  			amd_iommu_xt_mode = IRQ_REMAP_X2APIC_MODE;
>  
> +		if (h->efr_attr & BIT(IOMMU_IVHD_ATTR_HATDIS_SHIFT)) {
> +			pr_warn_once("Host Address Translation is not supported.\n");
> +			amd_iommu_hatdis = true;
> +		}
> +
>  		early_iommu_features_init(iommu, h);
>  
>  		break;
> @@ -2582,7 +2590,7 @@ static void init_device_table_dma(struct amd_iommu_pci_seg *pci_seg)
>  	u32 devid;
>  	struct dev_table_entry *dev_table = pci_seg->dev_table;
>  
> -	if (dev_table == NULL)
> +	if (!dev_table || amd_iommu_pgtable == PD_MODE_NONE)
>  		return;
>  
>  	for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
> @@ -3095,6 +3103,17 @@ static int __init early_amd_iommu_init(void)
>  		}
>  	}
>  
> +	if (amd_iommu_hatdis) {

I missed it in the internal review. This introduces ordering dependency (v2 page
table check should be done before this). IMO its worth to add an explicit
comment so that its clear.

-Vasant

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Joao Martins]

On 25/04/2025 17:25, Ankit Soni wrote:
> Hi Joao,
> Thank you for review this patch.
> 
> On Thu, Apr 24, 2025 at 01:19:43PM +0100, Joao Martins wrote:
>>>  {
>>> +	if (amd_iommu_hatdis)
>>> +		return false;
>>> +
>>>  	return iommu && (iommu->features & FEATURE_HDSUP);
>>>  }
>>>  
>>
>> It's strange we seem to somehow have host translation disabled, while it
>> advertises other translation-related features like the normal case.
> 
> If HATDis bit is enabled driver will ignore host(v1) page table related field,
> this includes nested page table.
> 

OK, I thought it was both actually

>>
>> In any case we should probably follow Intel's example (which does similar thing
>> to HATSDis) where we only call invoke IOMMU groups
>> iommu_device_register()/iommu_device_sysfs_add() with DMA translation enabled?
>> That should simplify most of the patch as those codepaths are not reachable via
>> kernel/userspace? Unless I am missing something ofc
>>
>> See also commit c40aaaac10 ("iommu/vt-d: Gracefully handle DMAR units with no
>> supported address widths"). I am not sure what else is the closest example here
>> besides intel-iommu equivalent.
> 
> With intel patch you mentioned above, it seems that it is mostly handling 
> "second stage translation support" disable, which will eventually disable dma 
> translation. And in AMD case, HATDis bit indicates host(v1) translation is not
> available, then attempt to use guest(v2) translation, and if both page
> table modes are not available then disable dma tranlation.

OK, I guess it makes sense if HATDis is v1 only.

My other call out was that when we disable dma-translation all together (aka
both modes), then we shouldn't advertise the IOMMU groups (internally and to
userspace) by not calling iommu_device_register()/iommu_device_sysfs_add().

	Joao

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Vasant Hegde]

Hi Joao,

[+Baolu to get clarification on intel driver behaviour.]

On 4/24/2025 5:49 PM, Joao Martins wrote:
> On 23/04/2025 07:50, Ankit Soni wrote:
>> Current AMD IOMMU assumes Host Address Translation (HAT) is always
>> supported, and Linux kernel enables this capability by default. However,
>> in case of emulated and virtualized IOMMU, this might not be the case.
> 
> +Alejandro as he is filling that gap
> 
>> For example,current QEMU-emulated AMD vIOMMU does not support host
>> translation for VFIO pass-through device, but the interrupt remapping
>> support is required for x2APIC (i.e. kvm-msi-ext-dest-id is also not
>> supported by the guest OS). This would require the guest kernel to boot
>> with guest kernel option iommu=pt to by-pass the initialization of
>> host (v1) table.
>>
>> The AMD I/O Virtualization Technology (IOMMU) Specification Rev 3.10 [1]>
> introduces a new flag 'HATDis' in the IVHD 11h IOMMU attributes to indicate
>> that HAT is not supported on a particular IOMMU instance.
>>
>> Therefore, modifies the AMD IOMMU driver to detect the new HATDis
>> attributes, and disable host translation and switch to use guest
>> translation if it is available. Otherwise, the driver will disable DMA
>> translation.
>>
>> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
>>
>> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>
>> ---
>>  drivers/iommu/amd/amd_iommu.h       |  1 +
>>  drivers/iommu/amd/amd_iommu_types.h |  6 +++++-
>>  drivers/iommu/amd/init.c            | 23 +++++++++++++++++++++--
>>  drivers/iommu/amd/iommu.c           | 13 +++++++++++++
>>  4 files changed, 40 insertions(+), 3 deletions(-)
>>


.../...

>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index be8761bbef0f..0ebc264726da 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -2393,6 +2393,13 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
>>  					     pci_max_pasids(to_pci_dev(dev)));
>>  	}
>>  
>> +	if (amd_iommu_pgtable == PD_MODE_NONE) {
>> +		pr_warn_once("%s: DMA translation not supported by iommu.\n",
>> +			     __func__);
>> +		iommu_dev = ERR_PTR(-ENODEV);
>> +		goto out_err;
>> +	}
>> +
>>  out_err:
>>  
>>  	iommu_completion_wait(iommu);
>> @@ -2480,6 +2487,9 @@ static int pdom_setup_pgtable(struct protection_domain *domain,
>>  	case PD_MODE_V2:
>>  		fmt = AMD_IOMMU_V2;
>>  		break;
>> +	case PD_MODE_NONE:
>> +		WARN_ON_ONCE(1);
>> +		return -EPERM;
>>  	}
>>  
>>  	domain->iop.pgtbl.cfg.amd.nid = dev_to_node(dev);
>> @@ -2501,6 +2511,9 @@ static inline u64 dma_max_address(enum protection_domain_mode pgtable)
>>  
>>  static bool amd_iommu_hd_support(struct amd_iommu *iommu)
>>  {
>> +	if (amd_iommu_hatdis)
>> +		return false;
>> +
>>  	return iommu && (iommu->features & FEATURE_HDSUP);
>>  }
>>  
> 
> It's strange we seem to somehow have host translation disabled, while it
> advertises other translation-related features like the normal case.

I think even if qemu advertises those features we shouldn't hit this code path
as probe() will fail and core shouldn't try to allocate domain, etc.

I was extra cautious and suggested to add this check!

> 
> In any case we should probably follow Intel's example (which does similar thing
> to HATSDis) where we only call invoke IOMMU groups
> iommu_device_register()/iommu_device_sysfs_add() with DMA translation enabled?
> That should simplify most of the patch as those codepaths are not reachable via
> kernel/userspace? Unless I am missing something ofc
> 
> See also commit c40aaaac10 ("iommu/vt-d: Gracefully handle DMAR units with no
> supported address widths"). I am not sure what else is the closest example here
> besides intel-iommu equivalent.

@Baolu,
  Looking into intel driver, my understanding is if DMA is not supported, intel
driver will set `drhd->ignored` to 1 (alloc_iommu()). So iommu_is_dummy() check
will return `true` and hence intel_iommu_probe_device() will fail. But it will
continue to support interrupt remapping. Is that the correct understanding?

-Vasant

Re: [PATCH 2/2] iommu/amd: Add efr[HATS] max v1 page table level

[Vasant Hegde]

On 4/23/2025 12:20 PM, Ankit Soni wrote:
> The EFR[HATS] bits indicate maximum host translation level supported by
> IOMMU. Adding support to set the maximum host page table level as indicated
> by EFR[HATS]. If the HATS=11b (reserved), the driver will attempt to use
> guest page table for DMA API.
> 
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Ankit Soni <Ankit.Soni@amd.com>


Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

Minor nit.

> ---
>  drivers/iommu/amd/amd_iommu.h       |  1 +
>  drivers/iommu/amd/amd_iommu_types.h |  1 +
>  drivers/iommu/amd/init.c            | 16 ++++++++++++++++
>  drivers/iommu/amd/io_pgtable.c      |  4 ++--
>  drivers/iommu/amd/iommu.c           |  2 +-
>  5 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index bb14c4800dd0..0286120ad4a5 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -42,6 +42,7 @@ int amd_iommu_enable_faulting(unsigned int cpu);
>  extern int amd_iommu_guest_ir;
>  extern enum protection_domain_mode amd_iommu_pgtable;
>  extern int amd_iommu_gpt_level;
> +extern u8 amd_iommu_hpt_level;
>  extern unsigned long amd_iommu_pgsize_bitmap;
>  extern bool amd_iommu_hatdis;
>  
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 284ff4309660..6bf81197c2c8 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -96,6 +96,7 @@
>  #define FEATURE_GA		BIT_ULL(7)
>  #define FEATURE_HE		BIT_ULL(8)
>  #define FEATURE_PC		BIT_ULL(9)
> +#define FEATURE_HATS		GENMASK_ULL(11, 10)
>  #define FEATURE_GATS		GENMASK_ULL(13, 12)
>  #define FEATURE_GLX		GENMASK_ULL(15, 14)
>  #define FEATURE_GAM_VAPIC	BIT_ULL(21)
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index f71b236c2af2..c713756fa44e 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -152,6 +152,8 @@ bool amd_iommu_dump;
>  bool amd_iommu_irq_remap __read_mostly;
>  
>  enum protection_domain_mode amd_iommu_pgtable = PD_MODE_NONE;
> +/* Host page table level */
> +u8 amd_iommu_hpt_level;
>  /* Guest page table level */
>  int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
>  
> @@ -3052,6 +3054,7 @@ static int __init early_amd_iommu_init(void)
>  	struct acpi_table_header *ivrs_base;
>  	int ret;
>  	acpi_status status;
> +	u8 efr_hats;
>  
>  	if (!amd_iommu_detected)
>  		return -ENODEV;
> @@ -3096,6 +3099,19 @@ static int __init early_amd_iommu_init(void)
>  	    FIELD_GET(FEATURE_GATS, amd_iommu_efr) == GUEST_PGTABLE_5_LEVEL)
>  		amd_iommu_gpt_level = PAGE_MODE_5_LEVEL;
>  
> +	efr_hats = FIELD_GET(FEATURE_HATS, amd_iommu_efr);
> +	if (efr_hats != 0x3) {
> +		/*
> +		 * efr[HATS] bits specify the maximum host translation level
> +		 * supported, with LEVEL 4 being initial max level.
> +		 */
> +		amd_iommu_hpt_level = efr_hats + PAGE_MODE_4_LEVEL;
> +	} else {
> +		pr_warn_once("Disable host address translation due to invalid max level (%#x).\n",

s/invalid max level/invalid translation level/

-Vasant

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Ankit Soni]

Hi,

On Wed, Apr 30, 2025 at 05:05:02PM +0530, Vasant Hegde wrote:
> Hi,
> 
> On 4/23/2025 12:20 PM, Ankit Soni wrote:
> > --- a/drivers/iommu/amd/init.c
> > +++ b/drivers/iommu/amd/init.c
> > @@ -151,7 +151,7 @@ struct ivmd_header {
> >  bool amd_iommu_dump;
> >  bool amd_iommu_irq_remap __read_mostly;
> >  
> > -enum protection_domain_mode amd_iommu_pgtable = PD_MODE_V1;
> > +enum protection_domain_mode amd_iommu_pgtable = PD_MODE_NONE;
> 
> 
> Why not keep it as `PD_MODE_V1` (as its our default page table) so that we can
> remove explict `PD_MODE_V1` assignment in below hunk?
> 

Yes it will remove `PD_MODE_V1` assignment hunk. But, Intension to keep
PD_MODE_NONE is more clean code, where by default page table mode is NONE,
and with proper checks and default/command line input, driver changes it.

> >  /* Guest page table level */
> >  int amd_iommu_gpt_level = PAGE_MODE_4_LEVEL;
> >  
> > @@ -168,6 +168,9 @@ static int amd_iommu_target_ivhd_type;
> >  u64 amd_iommu_efr;
> >  u64 amd_iommu_efr2;
> >  
> > +/* dma translation not supported*/
> 
> Host (v2) page table is not supported.
> 

I'll rephrase this.

> > +bool amd_iommu_hatdis;
> > +
> >  /* SNP is enabled on the system? */
> >  bool amd_iommu_snp_en;
> >  EXPORT_SYMBOL(amd_iommu_snp_en);
> > @@ -1798,6 +1801,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h,
> >  		if (h->efr_reg & BIT(IOMMU_EFR_XTSUP_SHIFT))
> >  			amd_iommu_xt_mode = IRQ_REMAP_X2APIC_MODE;
> >  
> > +		if (h->efr_attr & BIT(IOMMU_IVHD_ATTR_HATDIS_SHIFT)) {
> > +			pr_warn_once("Host Address Translation is not supported.\n");
> > +			amd_iommu_hatdis = true;
> > +		}
> > +
> >  		early_iommu_features_init(iommu, h);
> >  
> >  		break;
> > @@ -2582,7 +2590,7 @@ static void init_device_table_dma(struct amd_iommu_pci_seg *pci_seg)
> >  	u32 devid;
> >  	struct dev_table_entry *dev_table = pci_seg->dev_table;
> >  
> > -	if (dev_table == NULL)
> > +	if (!dev_table || amd_iommu_pgtable == PD_MODE_NONE)
> >  		return;
> >  
> >  	for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
> > @@ -3095,6 +3103,17 @@ static int __init early_amd_iommu_init(void)
> >  		}
> >  	}
> >  
> > +	if (amd_iommu_hatdis) {
> 
> I missed it in the internal review. This introduces ordering dependency (v2 page
> table check should be done before this). IMO its worth to add an explicit
> comment so that its clear.
> 
> -Vasant
> 

Sure, i'll move this below v2 page table check.

-Ankit

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Ankit Soni]

Hi,

On Wed, Apr 30, 2025 at 12:41:04PM +0100, Joao Martins wrote:
> > With intel patch you mentioned above, it seems that it is mostly handling 
> > "second stage translation support" disable, which will eventually disable dma 
> > translation. And in AMD case, HATDis bit indicates host(v1) translation is not
> > available, then attempt to use guest(v2) translation, and if both page
> > table modes are not available then disable dma tranlation.
> 
> OK, I guess it makes sense if HATDis is v1 only.
> 
> My other call out was that when we disable dma-translation all together (aka
> both modes), then we shouldn't advertise the IOMMU groups (internally and to
> userspace) by not calling iommu_device_register()/iommu_device_sysfs_add().
> 
> 	Joao

Sorry for the late reply. I had cross-checked it; if the probe fails,
then IOMMU groups will not be populated, and eventually, it will not
have significance for calling iommu_device_register()/iommu_device_sysfs_add().

-Ankit

Re: [PATCH 2/2] iommu/amd: Add efr[HATS] max v1 page table level

[Jason Gunthorpe]

On Wed, Apr 30, 2025 at 05:27:10PM +0530, Vasant Hegde wrote:
> > +	efr_hats = FIELD_GET(FEATURE_HATS, amd_iommu_efr);
> > +	if (efr_hats != 0x3) {
> > +		/*
> > +		 * efr[HATS] bits specify the maximum host translation level
> > +		 * supported, with LEVEL 4 being initial max level.
> > +		 */
> > +		amd_iommu_hpt_level = efr_hats + PAGE_MODE_4_LEVEL;
> > +	} else {
> > +		pr_warn_once("Disable host address translation due to invalid max level (%#x).\n",
> 
> s/invalid max level/invalid translation level/

This should have a FW_BUG annotation too?

Jason

Re: [PATCH 2/2] iommu/amd: Add efr[HATS] max v1 page table level

[Vasant Hegde]

On 5/6/2025 8:57 PM, Jason Gunthorpe wrote:
> On Wed, Apr 30, 2025 at 05:27:10PM +0530, Vasant Hegde wrote:
>>> +	efr_hats = FIELD_GET(FEATURE_HATS, amd_iommu_efr);
>>> +	if (efr_hats != 0x3) {
>>> +		/*
>>> +		 * efr[HATS] bits specify the maximum host translation level
>>> +		 * supported, with LEVEL 4 being initial max level.
>>> +		 */
>>> +		amd_iommu_hpt_level = efr_hats + PAGE_MODE_4_LEVEL;
>>> +	} else {
>>> +		pr_warn_once("Disable host address translation due to invalid max level (%#x).\n",
>>
>> s/invalid max level/invalid translation level/
> 
> This should have a FW_BUG annotation too?

Ack. We can add FW_BUG.

-Vasant

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Joao Martins]

On 06/05/2025 06:12, Ankit Soni wrote:
> On Wed, Apr 30, 2025 at 12:41:04PM +0100, Joao Martins wrote:
>>> With intel patch you mentioned above, it seems that it is mostly handling 
>>> "second stage translation support" disable, which will eventually disable dma 
>>> translation. And in AMD case, HATDis bit indicates host(v1) translation is not
>>> available, then attempt to use guest(v2) translation, and if both page
>>> table modes are not available then disable dma tranlation.
>>
>> OK, I guess it makes sense if HATDis is v1 only.
>>
>> My other call out was that when we disable dma-translation all together (aka
>> both modes), then we shouldn't advertise the IOMMU groups (internally and to
>> userspace) by not calling iommu_device_register()/iommu_device_sysfs_add().
>>
> 
> Sorry for the late reply. I had cross-checked it; if the probe fails,
> then IOMMU groups will not be populated, and eventually, it will not
> have significance for calling iommu_device_register()/iommu_device_sysfs_add().
> 

It would nonetheless populate a ivhd entry in sysfs needlessly but with an empty
devices list (qemu diff at the tail end for how I checked it; it's only missing
the ILLEGAL_DEVICE_TABLE_ENTRY event being generated, but enough to check the
first patch with sw iommu) e.g. as far as I checked:

$ find /sys | grep ivhd
/sys/class/iommu/ivhd0
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/uevent
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/cap
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/features
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/devices
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/device
/sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/subsystem

-->8--

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 59593582be23..78801672ee3d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2326,7 +2326,11 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker,
const char *oem_id,
     /* IOMMU info */
     build_append_int_noprefix(table_data, 0, 2);
     /* IOMMU Attributes */
-    build_append_int_noprefix(table_data, 0, 4);
+    if (!s->dma_translation) {
+        build_append_int_noprefix(table_data, (1UL << 0) /* HATDis */, 4);
+    } else {
+        build_append_int_noprefix(table_data, 0, 4);
+    }
     /* EFR Register Image */
     build_append_int_noprefix(table_data,
                               amdvi_extended_feature_register(s),
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 34abb61e092e..d033d309a210 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -83,6 +83,9 @@ uint64_t amdvi_extended_feature_register(AMDVIState *s)
     if (s->xtsup) {
         feature |= AMDVI_FEATURE_XT;
     }
+    if (!s->dma_translation) {
+        feature |= AMDVI_HATS_MODE_RESERVED;
+    }

     return feature;
 }
@@ -1665,6 +1668,7 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error
**errp)
 }

 static Property amdvi_properties[] = {
+    DEFINE_PROP_BOOL("dma-translation", AMDVIState, dma_translation, true),
     DEFINE_PROP_BOOL("xtsup", AMDVIState, xtsup, false),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index e0dac4d9a96c..b5358dc30533 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -169,6 +169,7 @@
 /* AMDVI paging mode */
 #define AMDVI_GATS_MODE                 (2ULL <<  12)
 #define AMDVI_HATS_MODE                 (2ULL <<  10)
+#define AMDVI_HATS_MODE_RESERVED        (2ULL <<  11)

 /* IOTLB */
 #define AMDVI_IOTLB_MAX_SIZE 1024
@@ -371,6 +372,7 @@ struct AMDVIState {
     /* Interrupt remapping */
     bool ga_enabled;
     bool xtsup;
+    bool dma_translation;
 };

 uint64_t amdvi_extended_feature_register(AMDVIState *s);

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Ankit Soni]

Hi,

On Thu, May 08, 2025 at 06:03:44PM +0100, Joao Martins wrote:
> On 06/05/2025 06:12, Ankit Soni wrote:
> > On Wed, Apr 30, 2025 at 12:41:04PM +0100, Joao Martins wrote:
> >>> With intel patch you mentioned above, it seems that it is mostly handling 
> >>> "second stage translation support" disable, which will eventually disable dma 
> >>> translation. And in AMD case, HATDis bit indicates host(v1) translation is not
> >>> available, then attempt to use guest(v2) translation, and if both page
> >>> table modes are not available then disable dma tranlation.
> >>
> >> OK, I guess it makes sense if HATDis is v1 only.
> >>
> >> My other call out was that when we disable dma-translation all together (aka
> >> both modes), then we shouldn't advertise the IOMMU groups (internally and to
> >> userspace) by not calling iommu_device_register()/iommu_device_sysfs_add().
> >>
> > 
> > Sorry for the late reply. I had cross-checked it; if the probe fails,
> > then IOMMU groups will not be populated, and eventually, it will not
> > have significance for calling iommu_device_register()/iommu_device_sysfs_add().
> > 
> 
> It would nonetheless populate a ivhd entry in sysfs needlessly but with an empty
> devices list (qemu diff at the tail end for how I checked it; it's only missing
> the ILLEGAL_DEVICE_TABLE_ENTRY event being generated, but enough to check the
> first patch with sw iommu) e.g. as far as I checked:
> 
> $ find /sys | grep ivhd
> /sys/class/iommu/ivhd0
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/uevent
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/cap
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/features
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/devices
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/device
> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/subsystem
> 

I was assuming, since iommu is still active for interrupt remapping,
user may need info for cap and feature using /sys fs.
@vasant: can you please suggest on this?

> -->8--
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 59593582be23..78801672ee3d 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2326,7 +2326,11 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker,
> const char *oem_id,
>      /* IOMMU info */
>      build_append_int_noprefix(table_data, 0, 2);
>      /* IOMMU Attributes */
> -    build_append_int_noprefix(table_data, 0, 4);
> +    if (!s->dma_translation) {
> +        build_append_int_noprefix(table_data, (1UL << 0) /* HATDis */, 4);
> +    } else {
> +        build_append_int_noprefix(table_data, 0, 4);
> +    }
>      /* EFR Register Image */
>      build_append_int_noprefix(table_data,
>                                amdvi_extended_feature_register(s),
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 34abb61e092e..d033d309a210 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -83,6 +83,9 @@ uint64_t amdvi_extended_feature_register(AMDVIState *s)
>      if (s->xtsup) {
>          feature |= AMDVI_FEATURE_XT;
>      }
> +    if (!s->dma_translation) {
> +        feature |= AMDVI_HATS_MODE_RESERVED;
> +    }
> 
>      return feature;
>  }
> @@ -1665,6 +1668,7 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error
> **errp)
>  }
> 
>  static Property amdvi_properties[] = {
> +    DEFINE_PROP_BOOL("dma-translation", AMDVIState, dma_translation, true),
>      DEFINE_PROP_BOOL("xtsup", AMDVIState, xtsup, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index e0dac4d9a96c..b5358dc30533 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -169,6 +169,7 @@
>  /* AMDVI paging mode */
>  #define AMDVI_GATS_MODE                 (2ULL <<  12)
>  #define AMDVI_HATS_MODE                 (2ULL <<  10)
> +#define AMDVI_HATS_MODE_RESERVED        (2ULL <<  11)
> 
>  /* IOTLB */
>  #define AMDVI_IOTLB_MAX_SIZE 1024
> @@ -371,6 +372,7 @@ struct AMDVIState {
>      /* Interrupt remapping */
>      bool ga_enabled;
>      bool xtsup;
> +    bool dma_translation;
>  };
> 
>  uint64_t amdvi_extended_feature_register(AMDVIState *s);

Thank you for QEMU patch, i also had similar patch ready, i was waiting
for kernel patch to be submitted.

- Ankit

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Vasant Hegde]

Hi Ankit,


On 5/12/2025 12:00 PM, Ankit Soni wrote:
> Hi,
> 
> On Thu, May 08, 2025 at 06:03:44PM +0100, Joao Martins wrote:
>> On 06/05/2025 06:12, Ankit Soni wrote:
>>> On Wed, Apr 30, 2025 at 12:41:04PM +0100, Joao Martins wrote:
>>>>> With intel patch you mentioned above, it seems that it is mostly handling 
>>>>> "second stage translation support" disable, which will eventually disable dma 
>>>>> translation. And in AMD case, HATDis bit indicates host(v1) translation is not
>>>>> available, then attempt to use guest(v2) translation, and if both page
>>>>> table modes are not available then disable dma tranlation.
>>>>
>>>> OK, I guess it makes sense if HATDis is v1 only.
>>>>
>>>> My other call out was that when we disable dma-translation all together (aka
>>>> both modes), then we shouldn't advertise the IOMMU groups (internally and to
>>>> userspace) by not calling iommu_device_register()/iommu_device_sysfs_add().
>>>>
>>>
>>> Sorry for the late reply. I had cross-checked it; if the probe fails,
>>> then IOMMU groups will not be populated, and eventually, it will not
>>> have significance for calling iommu_device_register()/iommu_device_sysfs_add().
>>>
>>
>> It would nonetheless populate a ivhd entry in sysfs needlessly but with an empty
>> devices list (qemu diff at the tail end for how I checked it; it's only missing
>> the ILLEGAL_DEVICE_TABLE_ENTRY event being generated, but enough to check the
>> first patch with sw iommu) e.g. as far as I checked:
>>
>> $ find /sys | grep ivhd
>> /sys/class/iommu/ivhd0
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/uevent
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/cap
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/features
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/devices
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/device
>> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/subsystem
>>
> 
> I was assuming, since iommu is still active for interrupt remapping,
> user may need info for cap and feature using /sys fs.
> @vasant: can you please suggest on this?

I don't have strong preference. But it looks like intel skips populating sysfs
properties. May be we can match the behaviour and skip populating ivdh entries.

-Vasant

Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

[Ankit Soni]

On Wed, May 28, 2025 at 05:49:35PM +0530, Vasant Hegde wrote:
> Hi Ankit,
> 
> 
> On 5/12/2025 12:00 PM, Ankit Soni wrote:
> > Hi,
> > 
> > On Thu, May 08, 2025 at 06:03:44PM +0100, Joao Martins wrote:
> >> On 06/05/2025 06:12, Ankit Soni wrote:
> >>> On Wed, Apr 30, 2025 at 12:41:04PM +0100, Joao Martins wrote:
> >>>>> With intel patch you mentioned above, it seems that it is mostly handling 
> >>>>> "second stage translation support" disable, which will eventually disable dma 
> >>>>> translation. And in AMD case, HATDis bit indicates host(v1) translation is not
> >>>>> available, then attempt to use guest(v2) translation, and if both page
> >>>>> table modes are not available then disable dma tranlation.
> >>>>
> >>>> OK, I guess it makes sense if HATDis is v1 only.
> >>>>
> >>>> My other call out was that when we disable dma-translation all together (aka
> >>>> both modes), then we shouldn't advertise the IOMMU groups (internally and to
> >>>> userspace) by not calling iommu_device_register()/iommu_device_sysfs_add().
> >>>>
> >>>
> >>> Sorry for the late reply. I had cross-checked it; if the probe fails,
> >>> then IOMMU groups will not be populated, and eventually, it will not
> >>> have significance for calling iommu_device_register()/iommu_device_sysfs_add().
> >>>
> >>
> >> It would nonetheless populate a ivhd entry in sysfs needlessly but with an empty
> >> devices list (qemu diff at the tail end for how I checked it; it's only missing
> >> the ILLEGAL_DEVICE_TABLE_ENTRY event being generated, but enough to check the
> >> first patch with sw iommu) e.g. as far as I checked:
> >>
> >> $ find /sys | grep ivhd
> >> /sys/class/iommu/ivhd0
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/uevent
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/cap
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/amd-iommu/features
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/devices
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/device
> >> /sys/devices/pci0000:00/0000:00:05.0/iommu/ivhd0/subsystem
> >>
> > 
> > I was assuming, since iommu is still active for interrupt remapping,
> > user may need info for cap and feature using /sys fs.
> > @vasant: can you please suggest on this?
> 
> I don't have strong preference. But it looks like intel skips populating sysfs
> properties. May be we can match the behaviour and skip populating ivdh entries.
> 
> -Vasant
> 
> 

Sure vasant, thanks for confirming. I'll post v3 soon with this change.
Thanks,
-Ankit