[PATCH v2 0/2] amd/iommu: Preserve domain ids inside the kdump kernel

[PATCH v2 0/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Sairaj Kodilkar]

Patch 1: Reserves the Domain IDs allocated by the host kernel while reusing its
	 device table inside kdump kernel
Patch 2: Cleanup around pdom allocation

-------------------------------------------------------------------------------

Changes since v1:
https://lore.kernel.org/all/20251114104442.23296-1-sarunkod@amd.com/
P1:
  - Use FIELD_GET to read the DTE fields [Ankit]
  - Use GFP_KERNEL instead of GFP_ATOMIC [Jason]
  - Remove print on ENOMEM [Jason]

P2:
  - Use u32 instead of int [Jason]
  - pass gfp flags as argument to 'amd_iommu_pdom_id_reserve' [Jason]

-------------------------------------------------------------------------------

Base Commit: 91920a9d87f5192c56ba5bf3e133aeb3576b705d (iommu-next)

-------------------------------------------------------------------------------

Fixes: 38e5f33ee359 ("iommu/amd: Reuse device table for kdump")
Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>

Sairaj Kodilkar (2):
  amd/iommu: Preserve domain ids inside the kdump kernel
  amd/iommu: Make protection domain ID functions non-static

 drivers/iommu/amd/amd_iommu.h |  5 +++++
 drivers/iommu/amd/init.c      | 24 +++++++++++++++++++++---
 drivers/iommu/amd/iommu.c     | 27 ++++++++++++++++++---------
 3 files changed, 44 insertions(+), 12 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Sairaj Kodilkar]

Currently AMD IOMMU driver does not reserve domain ids programmed in the
DTE while reusing the device table inside kdump kernel. This can cause
reallocation of these domain ids for newer domains that are created by
the kdump kernel, which can lead to potential IO_PAGE_FAULTs

Hence reserve these ids inside pdom_ids.

Fixes: 38e5f33ee359 ("iommu/amd: Reuse device table for kdump")
Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reported-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/init.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index f2991c11867c..14eb9de33ccb 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1136,9 +1136,13 @@ static void set_dte_bit(struct dev_table_entry *dte, u8 bit)
 static bool __reuse_device_table(struct amd_iommu *iommu)
 {
 	struct amd_iommu_pci_seg *pci_seg = iommu->pci_seg;
-	u32 lo, hi, old_devtb_size;
+	struct dev_table_entry *old_dev_tbl_entry;
+	u32 lo, hi, old_devtb_size, devid;
 	phys_addr_t old_devtb_phys;
+	u16 dom_id;
+	bool dte_v;
 	u64 entry;
+	int ret;
 
 	/* Each IOMMU use separate device table with the same size */
 	lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
@@ -1173,6 +1177,23 @@ static bool __reuse_device_table(struct amd_iommu *iommu)
 		return false;
 	}
 
+	for (devid = 0; devid <= pci_seg->last_bdf; devid++) {
+		old_dev_tbl_entry = &pci_seg->old_dev_tbl_cpy[devid];
+		dte_v = FIELD_GET(DTE_FLAG_V, old_dev_tbl_entry->data[0]);
+		dom_id = FIELD_GET(DEV_DOMID_MASK, old_dev_tbl_entry->data[1]);
+
+		if (!dte_v || !dom_id)
+			continue;
+		/*
+		 * ID reservation can fail with -ENOSPC when there
+		 * are multiple devices present in the same domain,
+		 * hence check only for -ENOMEM.
+		 */
+		ret = ida_alloc_range(&pdom_ids, dom_id, dom_id, GFP_KERNEL);
+		if (ret == -ENOMEM)
+			return false;
+	}
+
 	return true;
 }
 
-- 
2.34.1

[PATCH v2 2/2] amd/iommu: Make protection domain ID functions non-static

[Sairaj Kodilkar]

So that both iommu.c and init.c can utilize them. Also define a new
function 'pdom_id_destroy()' to destroy 'pdom_ids' instead of directly
calling ida functions.

Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h |  5 +++++
 drivers/iommu/amd/init.c      |  7 ++-----
 drivers/iommu/amd/iommu.c     | 27 ++++++++++++++++++---------
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 25044d28f28a..b742ef1adb35 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -173,6 +173,11 @@ static inline struct protection_domain *to_pdomain(struct iommu_domain *dom)
 bool translation_pre_enabled(struct amd_iommu *iommu);
 int __init add_special_device(u8 type, u8 id, u32 *devid, bool cmd_line);
 
+int amd_iommu_pdom_id_alloc(void);
+int amd_iommu_pdom_id_reserve(u16 id, gfp_t gfp);
+void amd_iommu_pdom_id_free(int id);
+void amd_iommu_pdom_id_destroy(void);
+
 #ifdef CONFIG_DMI
 void amd_iommu_apply_ivrs_quirks(void);
 #else
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 14eb9de33ccb..fe0851ebf397 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1142,7 +1142,6 @@ static bool __reuse_device_table(struct amd_iommu *iommu)
 	u16 dom_id;
 	bool dte_v;
 	u64 entry;
-	int ret;
 
 	/* Each IOMMU use separate device table with the same size */
 	lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
@@ -1189,8 +1188,7 @@ static bool __reuse_device_table(struct amd_iommu *iommu)
 		 * are multiple devices present in the same domain,
 		 * hence check only for -ENOMEM.
 		 */
-		ret = ida_alloc_range(&pdom_ids, dom_id, dom_id, GFP_KERNEL);
-		if (ret == -ENOMEM)
+		if (amd_iommu_pdom_id_reserve(dom_id, GFP_KERNEL) == -ENOMEM)
 			return false;
 	}
 
@@ -3132,8 +3130,7 @@ static bool __init check_ioapic_information(void)
 
 static void __init free_dma_resources(void)
 {
-	ida_destroy(&pdom_ids);
-
+	amd_iommu_pdom_id_destroy();
 	free_unity_maps();
 }
 
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 48bca4dc8eb6..af517e7cfffd 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1785,17 +1785,26 @@ int amd_iommu_complete_ppr(struct device *dev, u32 pasid, int status, int tag)
  * contain.
  *
  ****************************************************************************/
-
-static int pdom_id_alloc(void)
+int amd_iommu_pdom_id_alloc(void)
 {
 	return ida_alloc_range(&pdom_ids, 1, MAX_DOMAIN_ID - 1, GFP_ATOMIC);
 }
 
-static void pdom_id_free(int id)
+int amd_iommu_pdom_id_reserve(u16 id, gfp_t gfp)
+{
+	return ida_alloc_range(&pdom_ids, id, id, gfp);
+}
+
+void amd_iommu_pdom_id_free(int id)
 {
 	ida_free(&pdom_ids, id);
 }
 
+void amd_iommu_pdom_id_destroy(void)
+{
+	ida_destroy(&pdom_ids);
+}
+
 static void free_gcr3_tbl_level1(u64 *tbl)
 {
 	u64 *ptr;
@@ -1838,7 +1847,7 @@ static void free_gcr3_table(struct gcr3_tbl_info *gcr3_info)
 	gcr3_info->glx = 0;
 
 	/* Free per device domain ID */
-	pdom_id_free(gcr3_info->domid);
+	amd_iommu_pdom_id_free(gcr3_info->domid);
 
 	iommu_free_pages(gcr3_info->gcr3_tbl);
 	gcr3_info->gcr3_tbl = NULL;
@@ -1874,14 +1883,14 @@ static int setup_gcr3_table(struct gcr3_tbl_info *gcr3_info,
 		return -EBUSY;
 
 	/* Allocate per device domain ID */
-	domid = pdom_id_alloc();
+	domid = amd_iommu_pdom_id_alloc();
 	if (domid <= 0)
 		return -ENOSPC;
 	gcr3_info->domid = domid;
 
 	gcr3_info->gcr3_tbl = iommu_alloc_pages_node_sz(nid, GFP_ATOMIC, SZ_4K);
 	if (gcr3_info->gcr3_tbl == NULL) {
-		pdom_id_free(domid);
+		amd_iommu_pdom_id_free(domid);
 		return -ENOMEM;
 	}
 
@@ -2477,7 +2486,7 @@ struct protection_domain *protection_domain_alloc(void)
 	if (!domain)
 		return NULL;
 
-	domid = pdom_id_alloc();
+	domid = amd_iommu_pdom_id_alloc();
 	if (domid <= 0) {
 		kfree(domain);
 		return NULL;
@@ -2773,7 +2782,7 @@ void amd_iommu_domain_free(struct iommu_domain *dom)
 
 	WARN_ON(!list_empty(&domain->dev_list));
 	pt_iommu_deinit(&domain->iommu);
-	pdom_id_free(domain->id);
+	amd_iommu_pdom_id_free(domain->id);
 	kfree(domain);
 }
 
@@ -2824,7 +2833,7 @@ void amd_iommu_init_identity_domain(void)
 	domain->ops = &identity_domain_ops;
 	domain->owner = &amd_iommu_ops;
 
-	identity_domain.id = pdom_id_alloc();
+	identity_domain.id = amd_iommu_pdom_id_alloc();
 
 	protection_domain_init(&identity_domain);
 }
-- 
2.34.1

Re: [PATCH v2 1/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Jason Gunthorpe]

On Fri, Nov 21, 2025 at 02:41:15PM +0530, Sairaj Kodilkar wrote:
> Currently AMD IOMMU driver does not reserve domain ids programmed in the
> DTE while reusing the device table inside kdump kernel. This can cause
> reallocation of these domain ids for newer domains that are created by
> the kdump kernel, which can lead to potential IO_PAGE_FAULTs
> 
> Hence reserve these ids inside pdom_ids.
> 
> Fixes: 38e5f33ee359 ("iommu/amd: Reuse device table for kdump")
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> Reported-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/init.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)

This seems OK

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

But the a point of this work was to remove this code:

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 48bca4dc8eb61f..1cd799913cbcd6 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2024,7 +2024,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
                          phys_addr_t top_paddr, unsigned int top_level)
 {
        u16 domid;
-       u32 old_domid;
        struct dev_table_entry *initial_dte;
        struct dev_table_entry new = {};
        struct protection_domain *domain = dev_data->domain;
@@ -2080,7 +2079,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
        if (dev_data->ats_enabled)
                new.data[1] |= DTE_FLAG_IOTLB;
 
-       old_domid = READ_ONCE(dte->data[1]) & DEV_DOMID_MASK;
        new.data[1] |= domid;
 
        /*
@@ -2096,15 +2094,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
        set_dte_gcr3_table(iommu, dev_data, &new);
 
        update_dte256(iommu, dev_data, &new);
-
-       /*
-        * A kdump kernel might be replacing a domain ID that was copied from
-        * the previous kernel--if so, it needs to flush the translation cache
-        * entries for the old domain ID that is being overwritten
-        */
-       if (old_domid) {
-               amd_iommu_flush_tlb_domid(iommu, old_domid);
-       }
 }
 
 /*

Under the reasoning that:
 - domids in use by the prior kernel are reserved in the IDA and are
   never used by this kernel
 - domids in the IDA must be clean
 - There is no reason to flush a domid until it is returned to the IDA
 - detach_device() calls amd_iommu_domain_flush_all() before the
   domain can be freed and the domid returned the IDA which clears the
   IOTLB

Please add a patch?

Jason

Re: [PATCH v2 1/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Sairaj Kodilkar]

On 11/24/2025 8:14 PM, Jason Gunthorpe wrote:
> On Fri, Nov 21, 2025 at 02:41:15PM +0530, Sairaj Kodilkar wrote:
>> Currently AMD IOMMU driver does not reserve domain ids programmed in the
>> DTE while reusing the device table inside kdump kernel. This can cause
>> reallocation of these domain ids for newer domains that are created by
>> the kdump kernel, which can lead to potential IO_PAGE_FAULTs
>>
>> Hence reserve these ids inside pdom_ids.
>>
>> Fixes: 38e5f33ee359 ("iommu/amd: Reuse device table for kdump")
>> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
>> Reported-by: Jason Gunthorpe <jgg@nvidia.com>
>> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>   drivers/iommu/amd/init.c | 23 ++++++++++++++++++++++-
>>   1 file changed, 22 insertions(+), 1 deletion(-)
> This seems OK
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>
> But the a point of this work was to remove this code:
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 48bca4dc8eb61f..1cd799913cbcd6 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2024,7 +2024,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
>                            phys_addr_t top_paddr, unsigned int top_level)
>   {
>          u16 domid;
> -       u32 old_domid;
>          struct dev_table_entry *initial_dte;
>          struct dev_table_entry new = {};
>          struct protection_domain *domain = dev_data->domain;
> @@ -2080,7 +2079,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
>          if (dev_data->ats_enabled)
>                  new.data[1] |= DTE_FLAG_IOTLB;
>   
> -       old_domid = READ_ONCE(dte->data[1]) & DEV_DOMID_MASK;
>          new.data[1] |= domid;
>   
>          /*
> @@ -2096,15 +2094,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
>          set_dte_gcr3_table(iommu, dev_data, &new);
>   
>          update_dte256(iommu, dev_data, &new);
> -
> -       /*
> -        * A kdump kernel might be replacing a domain ID that was copied from
> -        * the previous kernel--if so, it needs to flush the translation cache
> -        * entries for the old domain ID that is being overwritten
> -        */
> -       if (old_domid) {
> -               amd_iommu_flush_tlb_domid(iommu, old_domid);
> -       }
>   }
>   
>   /*
>
> Under the reasoning that:
>   - domids in use by the prior kernel are reserved in the IDA and are
>     never used by this kernel
>   - domids in the IDA must be clean
>   - There is no reason to flush a domid until it is returned to the IDA
>   - detach_device() calls amd_iommu_domain_flush_all() before the
>     domain can be freed and the domid returned the IDA which clears the
>     IOTLB
>
> Please add a patch?

Thanks for the explaination
Will add the patch for it

Thanks
Sairaj

Re: [PATCH v2 1/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Vasant Hegde]

On 11/24/2025 8:14 PM, Jason Gunthorpe wrote:
> On Fri, Nov 21, 2025 at 02:41:15PM +0530, Sairaj Kodilkar wrote:
>> Currently AMD IOMMU driver does not reserve domain ids programmed in the
>> DTE while reusing the device table inside kdump kernel. This can cause
>> reallocation of these domain ids for newer domains that are created by
>> the kdump kernel, which can lead to potential IO_PAGE_FAULTs
>>
>> Hence reserve these ids inside pdom_ids.
>>
>> Fixes: 38e5f33ee359 ("iommu/amd: Reuse device table for kdump")
>> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
>> Reported-by: Jason Gunthorpe <jgg@nvidia.com>
>> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>  drivers/iommu/amd/init.c | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> This seems OK
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> But the a point of this work was to remove this code:
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 48bca4dc8eb61f..1cd799913cbcd6 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2024,7 +2024,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
>                           phys_addr_t top_paddr, unsigned int top_level)
>  {
>         u16 domid;
> -       u32 old_domid;
>         struct dev_table_entry *initial_dte;
>         struct dev_table_entry new = {};
>         struct protection_domain *domain = dev_data->domain;
> @@ -2080,7 +2079,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
>         if (dev_data->ats_enabled)
>                 new.data[1] |= DTE_FLAG_IOTLB;
>  
> -       old_domid = READ_ONCE(dte->data[1]) & DEV_DOMID_MASK;
>         new.data[1] |= domid;
>  
>         /*
> @@ -2096,15 +2094,6 @@ static void set_dte_entry(struct amd_iommu *iommu,
>         set_dte_gcr3_table(iommu, dev_data, &new);
>  
>         update_dte256(iommu, dev_data, &new);
> -
> -       /*
> -        * A kdump kernel might be replacing a domain ID that was copied from
> -        * the previous kernel--if so, it needs to flush the translation cache
> -        * entries for the old domain ID that is being overwritten
> -        */
> -       if (old_domid) {
> -               amd_iommu_flush_tlb_domid(iommu, old_domid);
> -       }
>  }
>  
>  /*
> 
> Under the reasoning that:
>  - domids in use by the prior kernel are reserved in the IDA and are
>    never used by this kernel

It looks like on non-SNP system in kdump path, if it fails to reserve old DTE
table, it tries to allocate new one! Looking into code again, it may be OK to
fail in that path as well.. so that we will have common flow.

That needs to be fixed before removing old_domid check.


-Vasant

>  - domids in the IDA must be clean
>  - There is no reason to flush a domid until it is returned to the IDA
>  - detach_device() calls amd_iommu_domain_flush_all() before the
>    domain can be freed and the domid returned the IDA which clears the
>    IOTLB
> 
> Please add a patch?
> 
> Jason

Re: [PATCH v2 1/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Jason Gunthorpe]

On Wed, Nov 26, 2025 at 01:51:27PM +0530, Vasant Hegde wrote:
> It looks like on non-SNP system in kdump path, if it fails to reserve old DTE
> table, it tries to allocate new one! Looking into code again, it may be OK to
> fail in that path as well.. so that we will have common flow.

If it allocates a new (empty) one and does a full flush of the iommu
that would be OK?

Jason

Re: [PATCH v2 1/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Vasant Hegde]

Jason,


On 11/26/2025 8:50 PM, Jason Gunthorpe wrote:
> On Wed, Nov 26, 2025 at 01:51:27PM +0530, Vasant Hegde wrote:
>> It looks like on non-SNP system in kdump path, if it fails to reserve old DTE
>> table, it tries to allocate new one! Looking into code again, it may be OK to
>> fail in that path as well.. so that we will have common flow.
> 
> If it allocates a new (empty) one and does a full flush of the iommu
> that would be OK?

I did look into the init sequence. We have kdump check everywhere which will
create issues. We will try to cleanup that path along with your suggestion to
remove flush from ste_dte_entry() code path. I will have separate series for that.


-Vasant

Re: [PATCH v2 0/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Vasant Hegde]

Hi Joerg,


On 11/21/2025 2:41 PM, Sairaj Kodilkar wrote:
> Patch 1: Reserves the Domain IDs allocated by the host kernel while reusing its
> 	 device table inside kdump kernel
> Patch 2: Cleanup around pdom allocation

Can you please pick this series as it fixes regression (may be for rc)?

Jason's suggestion are valid. We will address them along with some cleanup in
kdump path as separate series.


-Vasant

Re: [PATCH v2 0/2] amd/iommu: Preserve domain ids inside the kdump kernel

[Joerg Roedel]

On Tue, Dec 02, 2025 at 10:53:45AM +0530, Vasant Hegde wrote:
> Hi Joerg,
> 
> 
> On 11/21/2025 2:41 PM, Sairaj Kodilkar wrote:
> > Patch 1: Reserves the Domain IDs allocated by the host kernel while reusing its
> > 	 device table inside kdump kernel
> > Patch 2: Cleanup around pdom allocation
> 
> Can you please pick this series as it fixes regression (may be for rc)?
> 
> Jason's suggestion are valid. We will address them along with some cleanup in
> kdump path as separate series.

Now applied, thanks.