* [PATCH 0/2] amd/iommu: Preserve domain ids inside the kdump kernel
@ 2025-11-14 10:44 Sairaj Kodilkar
2025-11-14 10:44 ` [PATCH 1/2] " Sairaj Kodilkar
2025-11-14 10:44 ` [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static Sairaj Kodilkar
0 siblings, 2 replies; 13+ messages in thread
From: Sairaj Kodilkar @ 2025-11-14 10:44 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: joro, suravee.suthikulpanit, vasant.hegde, ashish.kalra,
robin.murphy, will, 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
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 | 26 +++++++++++++++++++++++---
drivers/iommu/amd/iommu.c | 27 ++++++++++++++++++---------
3 files changed, 46 insertions(+), 12 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-14 10:44 [PATCH 0/2] amd/iommu: Preserve domain ids inside the kdump kernel Sairaj Kodilkar @ 2025-11-14 10:44 ` Sairaj Kodilkar 2025-11-17 5:21 ` Ankit Soni 2025-11-19 10:50 ` Vasant Hegde 2025-11-14 10:44 ` [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static Sairaj Kodilkar 1 sibling, 2 replies; 13+ messages in thread From: Sairaj Kodilkar @ 2025-11-14 10:44 UTC (permalink / raw) To: iommu, linux-kernel Cc: joro, suravee.suthikulpanit, vasant.hegde, ashish.kalra, robin.murphy, will, 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> --- drivers/iommu/amd/init.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c index f2991c11867c..9375fba1071c 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,25 @@ 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 = old_dev_tbl_entry->data[0] & DTE_FLAG_V; + dom_id = old_dev_tbl_entry->data[1] & DEV_DOMID_MASK; + + if (!dte_v || !dom_id) + continue; + /* + * ID reseveration 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_ATOMIC); + if (ret == -ENOMEM) { + pr_err("Failed to reserve domain ID 0x%x\n", dom_id); + return false; + } + } + return true; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-14 10:44 ` [PATCH 1/2] " Sairaj Kodilkar @ 2025-11-17 5:21 ` Ankit Soni 2025-11-19 10:50 ` Vasant Hegde 1 sibling, 0 replies; 13+ messages in thread From: Ankit Soni @ 2025-11-17 5:21 UTC (permalink / raw) To: Sairaj Kodilkar Cc: iommu, linux-kernel, joro, suravee.suthikulpanit, vasant.hegde, ashish.kalra, robin.murphy, will Hi, On Fri, Nov 14, 2025 at 04:14:41PM +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> > --- > drivers/iommu/amd/init.c | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c > index f2991c11867c..9375fba1071c 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,25 @@ 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 = old_dev_tbl_entry->data[0] & DTE_FLAG_V; > + dom_id = old_dev_tbl_entry->data[1] & DEV_DOMID_MASK; Consider using FIELD_GET(). > + > + if (!dte_v || !dom_id) > + continue; > + /* > + * ID reseveration can fail with -ENOSPC when there s/reseveration/reservation > + * are multiple devices present in the same domain, > + * hence check only for -ENOMEM. > + */ > + ret = ida_alloc_range(&pdom_ids, dom_id, dom_id, GFP_ATOMIC); > + if (ret == -ENOMEM) { > + pr_err("Failed to reserve domain ID 0x%x\n", dom_id); Can we use ratelimit, as in kdump kernel pr_err can be bit noisy. -Ankit > + return false; > + } > + } > + > return true; > } > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-14 10:44 ` [PATCH 1/2] " Sairaj Kodilkar 2025-11-17 5:21 ` Ankit Soni @ 2025-11-19 10:50 ` Vasant Hegde 2025-11-19 13:43 ` Jason Gunthorpe 1 sibling, 1 reply; 13+ messages in thread From: Vasant Hegde @ 2025-11-19 10:50 UTC (permalink / raw) To: Sairaj Kodilkar, iommu, linux-kernel Cc: joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will, Jason Gunthorpe On 11/14/2025 4:14 PM, 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> I missed to inform you. This was reported by Jason while reviewing nested series. So Reported-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com> -Vasant > --- > drivers/iommu/amd/init.c | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c > index f2991c11867c..9375fba1071c 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,25 @@ 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 = old_dev_tbl_entry->data[0] & DTE_FLAG_V; > + dom_id = old_dev_tbl_entry->data[1] & DEV_DOMID_MASK; > + > + if (!dte_v || !dom_id) > + continue; > + /* > + * ID reseveration 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_ATOMIC); > + if (ret == -ENOMEM) { > + pr_err("Failed to reserve domain ID 0x%x\n", dom_id); > + return false; > + } > + } > + > return true; > } > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-19 10:50 ` Vasant Hegde @ 2025-11-19 13:43 ` Jason Gunthorpe 2025-11-20 5:02 ` Sairaj Kodilkar ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Jason Gunthorpe @ 2025-11-19 13:43 UTC (permalink / raw) To: Vasant Hegde Cc: Sairaj Kodilkar, iommu, linux-kernel, joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will On Wed, Nov 19, 2025 at 04:20:28PM +0530, Vasant Hegde wrote: > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c > > index f2991c11867c..9375fba1071c 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,25 @@ 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 = old_dev_tbl_entry->data[0] & DTE_FLAG_V; > > + dom_id = old_dev_tbl_entry->data[1] & DEV_DOMID_MASK; > > + > > + if (!dte_v || !dom_id) > > + continue; > > + /* > > + * ID reseveration 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_ATOMIC); Is it really an atomic context? Why? > > + if (ret == -ENOMEM) { > > + pr_err("Failed to reserve domain ID 0x%x\n", dom_id); > > + return false; Please don't print on ENOMEM, there is already a print. I think you should also keep iterating as other dom_ids may still be fit in already allocated bitmaps. Though the system is probably toast if this happens anyhow. Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-19 13:43 ` Jason Gunthorpe @ 2025-11-20 5:02 ` Sairaj Kodilkar 2025-11-20 12:56 ` Jason Gunthorpe 2025-11-20 6:22 ` Vasant Hegde 2025-11-20 6:56 ` Vasant Hegde 2 siblings, 1 reply; 13+ messages in thread From: Sairaj Kodilkar @ 2025-11-20 5:02 UTC (permalink / raw) To: Jason Gunthorpe, Vasant Hegde Cc: iommu, linux-kernel, joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will On 11/19/2025 7:13 PM, Jason Gunthorpe wrote: > On Wed, Nov 19, 2025 at 04:20:28PM +0530, Vasant Hegde wrote: >>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c >>> index f2991c11867c..9375fba1071c 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,25 @@ 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 = old_dev_tbl_entry->data[0] & DTE_FLAG_V; >>> + dom_id = old_dev_tbl_entry->data[1] & DEV_DOMID_MASK; >>> + >>> + if (!dte_v || !dom_id) >>> + continue; >>> + /* >>> + * ID reseveration 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_ATOMIC); > Is it really an atomic context? Why? No, its not atomic context. My intension was to ensure that the memory allocation succeeds. But I am not sure if it's the most appropriate choice for this scenario Could you please suggest which GFP flag would be best suited for this use ? > >>> + if (ret == -ENOMEM) { >>> + pr_err("Failed to reserve domain ID 0x%x\n", dom_id); >>> + return false; > Please don't print on ENOMEM, there is already a print. > > I think you should also keep iterating as other dom_ids may still be > fit in already allocated bitmaps. Though the system is probably toast > if this happens anyhow. Sure Thanks Sairaj > Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-20 5:02 ` Sairaj Kodilkar @ 2025-11-20 12:56 ` Jason Gunthorpe 2025-11-21 5:21 ` Sairaj Kodilkar 0 siblings, 1 reply; 13+ messages in thread From: Jason Gunthorpe @ 2025-11-20 12:56 UTC (permalink / raw) To: Sairaj Kodilkar Cc: Vasant Hegde, iommu, linux-kernel, joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will On Thu, Nov 20, 2025 at 10:32:32AM +0530, Sairaj Kodilkar wrote: > No, its not atomic context. My intension was to ensure that the > memory allocation succeeds. But I am not sure if it's the most > appropriate choice for this scenario This is early boot, it won't fail either way. Use GFP_KERNEL because it is clearer Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-20 12:56 ` Jason Gunthorpe @ 2025-11-21 5:21 ` Sairaj Kodilkar 0 siblings, 0 replies; 13+ messages in thread From: Sairaj Kodilkar @ 2025-11-21 5:21 UTC (permalink / raw) To: Jason Gunthorpe Cc: Vasant Hegde, iommu, linux-kernel, joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will On 11/20/2025 6:26 PM, Jason Gunthorpe wrote: > On Thu, Nov 20, 2025 at 10:32:32AM +0530, Sairaj Kodilkar wrote: > >> No, its not atomic context. My intension was to ensure that the >> memory allocation succeeds. But I am not sure if it's the most >> appropriate choice for this scenario > This is early boot, it won't fail either way. Use GFP_KERNEL because > it is clearer Sure, Thanks for suggestion -Sairaj ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-19 13:43 ` Jason Gunthorpe 2025-11-20 5:02 ` Sairaj Kodilkar @ 2025-11-20 6:22 ` Vasant Hegde 2025-11-20 6:56 ` Vasant Hegde 2 siblings, 0 replies; 13+ messages in thread From: Vasant Hegde @ 2025-11-20 6:22 UTC (permalink / raw) To: Jason Gunthorpe Cc: Sairaj Kodilkar, iommu, linux-kernel, joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will On 11/19/2025 7:13 PM, Jason Gunthorpe wrote: > On Wed, Nov 19, 2025 at 04:20:28PM +0530, Vasant Hegde wrote: >>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c >>> index f2991c11867c..9375fba1071c 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,25 @@ 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 = old_dev_tbl_entry->data[0] & DTE_FLAG_V; >>> + dom_id = old_dev_tbl_entry->data[1] & DEV_DOMID_MASK; >>> + >>> + if (!dte_v || !dom_id) >>> + continue; >>> + /* >>> + * ID reseveration 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_ATOMIC); > > Is it really an atomic context? Why? > >>> + if (ret == -ENOMEM) { >>> + pr_err("Failed to reserve domain ID 0x%x\n", dom_id); >>> + return false; > > Please don't print on ENOMEM, there is already a print. > > I think you should also keep iterating as other dom_ids may still be > fit in already allocated bitmaps. Though the system is probably toast > if this happens anyhow. Right. No point in continuing if it fails to reserve the domain ID. Things can go wrong. So it returns error. -Vasant ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] amd/iommu: Preserve domain ids inside the kdump kernel 2025-11-19 13:43 ` Jason Gunthorpe 2025-11-20 5:02 ` Sairaj Kodilkar 2025-11-20 6:22 ` Vasant Hegde @ 2025-11-20 6:56 ` Vasant Hegde 2 siblings, 0 replies; 13+ messages in thread From: Vasant Hegde @ 2025-11-20 6:56 UTC (permalink / raw) To: Jason Gunthorpe Cc: Sairaj Kodilkar, iommu, linux-kernel, joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will On 11/19/2025 7:13 PM, Jason Gunthorpe wrote: > On Wed, Nov 19, 2025 at 04:20:28PM +0530, Vasant Hegde wrote: >>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c >>> index f2991c11867c..9375fba1071c 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,25 @@ 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 = old_dev_tbl_entry->data[0] & DTE_FLAG_V; >>> + dom_id = old_dev_tbl_entry->data[1] & DEV_DOMID_MASK; >>> + >>> + if (!dte_v || !dom_id) >>> + continue; >>> + /* >>> + * ID reseveration 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_ATOMIC); > > Is it really an atomic context? Why? I had to dig git history to figureout why I used GFP_ATOMIC for ida allocator. When we moved to ida allocator we used GPF_ATOMIC as there were paths were code was holding spinlock before calling ida allocator. Now that those paths are moved to mutex lock and remvoed redundant locks, I think its fine to change it to GFP_KERNEL. -Vasant ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static 2025-11-14 10:44 [PATCH 0/2] amd/iommu: Preserve domain ids inside the kdump kernel Sairaj Kodilkar 2025-11-14 10:44 ` [PATCH 1/2] " Sairaj Kodilkar @ 2025-11-14 10:44 ` Sairaj Kodilkar 2025-11-19 10:47 ` Vasant Hegde 2025-11-19 13:47 ` Jason Gunthorpe 1 sibling, 2 replies; 13+ messages in thread From: Sairaj Kodilkar @ 2025-11-14 10:44 UTC (permalink / raw) To: iommu, linux-kernel Cc: joro, suravee.suthikulpanit, vasant.hegde, ashish.kalra, robin.murphy, will, 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> --- 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..6335b577f80d 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(int id); +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 9375fba1071c..c78fa67f34c6 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_ATOMIC); - if (ret == -ENOMEM) { + if (amd_iommu_pdom_id_reserve(dom_id) == -ENOMEM) { pr_err("Failed to reserve domain ID 0x%x\n", dom_id); return false; } @@ -3134,8 +3132,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..4d57e820b824 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(int id) +{ + return ida_alloc_range(&pdom_ids, id, id, GFP_ATOMIC); +} + +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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static 2025-11-14 10:44 ` [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static Sairaj Kodilkar @ 2025-11-19 10:47 ` Vasant Hegde 2025-11-19 13:47 ` Jason Gunthorpe 1 sibling, 0 replies; 13+ messages in thread From: Vasant Hegde @ 2025-11-19 10:47 UTC (permalink / raw) To: Sairaj Kodilkar, iommu, linux-kernel Cc: joro, suravee.suthikulpanit, ashish.kalra, robin.murphy, will On 11/14/2025 4:14 PM, Sairaj Kodilkar wrote: > 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> -Vasant ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static 2025-11-14 10:44 ` [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static Sairaj Kodilkar 2025-11-19 10:47 ` Vasant Hegde @ 2025-11-19 13:47 ` Jason Gunthorpe 1 sibling, 0 replies; 13+ messages in thread From: Jason Gunthorpe @ 2025-11-19 13:47 UTC (permalink / raw) To: Sairaj Kodilkar Cc: iommu, linux-kernel, joro, suravee.suthikulpanit, vasant.hegde, ashish.kalra, robin.murphy, will On Fri, Nov 14, 2025 at 04:14:42PM +0530, Sairaj Kodilkar wrote: > 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> > --- > 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..6335b577f80d 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(int id); > +void amd_iommu_pdom_id_free(int id); > +void amd_iommu_pdom_id_destroy(void); Domain IDs are not signed integers. Use u32. > diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c > index 48bca4dc8eb6..4d57e820b824 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(int id) > +{ > + return ida_alloc_range(&pdom_ids, id, id, GFP_ATOMIC); > +} The GFP should probably be passed in as a function argument Jason ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-11-21 5:22 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-14 10:44 [PATCH 0/2] amd/iommu: Preserve domain ids inside the kdump kernel Sairaj Kodilkar 2025-11-14 10:44 ` [PATCH 1/2] " Sairaj Kodilkar 2025-11-17 5:21 ` Ankit Soni 2025-11-19 10:50 ` Vasant Hegde 2025-11-19 13:43 ` Jason Gunthorpe 2025-11-20 5:02 ` Sairaj Kodilkar 2025-11-20 12:56 ` Jason Gunthorpe 2025-11-21 5:21 ` Sairaj Kodilkar 2025-11-20 6:22 ` Vasant Hegde 2025-11-20 6:56 ` Vasant Hegde 2025-11-14 10:44 ` [PATCH 2/2] amd/iommu: Make protection domain ID functions non-static Sairaj Kodilkar 2025-11-19 10:47 ` Vasant Hegde 2025-11-19 13:47 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox