* [PATCH v3 0/3] iommu/amd: Invalidate IRT cache for DMA aliases
@ 2026-02-25 20:23 Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables Magnus Kalland
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Magnus Kalland @ 2026-02-25 20:23 UTC (permalink / raw)
To: vasant.hegde, suravee.suthikulpanit, joro, iommu, linux-kernel
Cc: dhsrivas, Magnus Kalland, Lars B . Kristiansen, Jonas Markussen,
Tore H . Larsen
DMA aliasing causes interrupt remapping table entries (IRTEs) to be shared
between multiple device IDs. See commit 3c124435e8dd
("iommu/amd: Support multiple PCI DMA aliases in IRQ Remapping") for more
information on this. However, the AMD IOMMU driver currently invalidates
IRTE cache entries on a per-device basis whenever an IRTE is updated, not
for each alias.
This approach leaves stale IRTE cache entries when an IRTE is cached under
one DMA alias but later updated and invalidated through a different alias.
In such cases, the original device ID is never invalidated, since it is
programmed via aliasing.
This incoherency bug has been observed when IRTEs are cached for one
Non-Transparent Bridge (NTB) DMA alias, later updated via another.
Fix this by invalidating the interrupt remapping table cache for all DMA
aliases when updating an IRTE.
Changes since v2:
- Look for aliases with pci_seg->alias_table instead of
pci_for_each_dma_alias since we can't get the pdev (lockdep).
Track the aliases in set_remap_table_entry_alias. Invalidate IRT cache
for each BDF sharing alias with the given devid in
iommu_flush_irt_and_complete.
- Make iommu_table_lock a raw spinlock to use it when invalidating
IRT caches.
- Rebased and applied cleanly on the IOMMU development tree
Co-developed-by: Lars B. Kristiansen <larsk@dolphinics.com>
Signed-off-by: Lars B. Kristiansen <larsk@dolphinics.com>
Co-developed-by: Jonas Markussen <jonas@dolphinics.com>
Signed-off-by: Jonas Markussen <jonas@dolphinics.com>
Co-developed-by: Tore H. Larsen <torel@simula.no>
Signed-off-by: Tore H. Larsen <torel@simula.no>
Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
Link: https://lore.kernel.org/linux-iommu/26cfa307-6c33-41f9-a7a0-fbf202b38a00@amd.com/
Magnus Kalland (3):
iommu/amd: Use raw spinlock for interrupt remapping tables
iommu/amd: Track PCIe DMA aliases in set_remap_table_entry_alias
iommu/amd: Invalidate IRT cache for DMA aliases
drivers/iommu/amd/iommu.c | 52 +++++++++++++++++++++++++++++++++------
1 file changed, 45 insertions(+), 7 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables
2026-02-25 20:23 [PATCH v3 0/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
@ 2026-02-25 20:23 ` Magnus Kalland
2026-03-30 15:23 ` Vasant Hegde
2026-02-25 20:23 ` [PATCH v3 2/3] iommu/amd: Track PCIe DMA aliases in set_remap_table_entry_alias Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
2 siblings, 1 reply; 13+ messages in thread
From: Magnus Kalland @ 2026-02-25 20:23 UTC (permalink / raw)
To: vasant.hegde, suravee.suthikulpanit, joro, iommu, linux-kernel
Cc: dhsrivas, Magnus Kalland
Use raw spinlock for interrupt remapping tables since
iommu_flush_irt_and_complete is called under a raw spinlock.
Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
---
drivers/iommu/amd/iommu.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 81c4d7733872..f3193c6428c9 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3164,7 +3164,8 @@ const struct iommu_ops amd_iommu_ops = {
*****************************************************************************/
static struct irq_chip amd_ir_chip;
-static DEFINE_SPINLOCK(iommu_table_lock);
+static DEFINE_RAW_SPINLOCK(iommu_table_lock);
+
static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
{
@@ -3310,7 +3311,7 @@ static struct irq_remap_table *alloc_irq_table(struct amd_iommu *iommu,
int nid = iommu->dev ? dev_to_node(&iommu->dev->dev) : NUMA_NO_NODE;
u16 alias;
- spin_lock_irqsave(&iommu_table_lock, flags);
+ raw_spin_lock_irqsave(&iommu_table_lock, flags);
pci_seg = iommu->pci_seg;
table = pci_seg->irq_lookup_table[devid];
@@ -3323,14 +3324,14 @@ static struct irq_remap_table *alloc_irq_table(struct amd_iommu *iommu,
set_remap_table_entry(iommu, devid, table);
goto out_wait;
}
- spin_unlock_irqrestore(&iommu_table_lock, flags);
+ raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
/* Nothing there yet, allocate new irq remapping table */
new_table = __alloc_irq_table(nid, get_irq_table_size(max_irqs));
if (!new_table)
return NULL;
- spin_lock_irqsave(&iommu_table_lock, flags);
+ raw_spin_lock_irqsave(&iommu_table_lock, flags);
table = pci_seg->irq_lookup_table[devid];
if (table)
@@ -3358,7 +3359,7 @@ static struct irq_remap_table *alloc_irq_table(struct amd_iommu *iommu,
iommu_completion_wait(iommu);
out_unlock:
- spin_unlock_irqrestore(&iommu_table_lock, flags);
+ raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
if (new_table) {
iommu_free_pages(new_table->table);
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/3] iommu/amd: Track PCIe DMA aliases in set_remap_table_entry_alias
2026-02-25 20:23 [PATCH v3 0/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables Magnus Kalland
@ 2026-02-25 20:23 ` Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
2 siblings, 0 replies; 13+ messages in thread
From: Magnus Kalland @ 2026-02-25 20:23 UTC (permalink / raw)
To: vasant.hegde, suravee.suthikulpanit, joro, iommu, linux-kernel
Cc: dhsrivas, Magnus Kalland
Track PCIe DMA aliases in set_remap_table_entry_alias so that we can
iterate over shared IRTs by alias in iommu_flush_irt_for_aliases.
Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
---
drivers/iommu/amd/iommu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index f3193c6428c9..5dec3502c8b3 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3280,12 +3280,14 @@ static int set_remap_table_entry_alias(struct pci_dev *pdev, u16 alias,
struct irq_remap_table *table = data;
struct amd_iommu_pci_seg *pci_seg;
struct amd_iommu *iommu = rlookup_amd_iommu(&pdev->dev);
+ u16 devid = pci_dev_id(pdev);
if (!iommu)
return -EINVAL;
pci_seg = iommu->pci_seg;
pci_seg->irq_lookup_table[alias] = table;
+ pci_seg->alias_table[alias] = devid;
set_dte_irq_entry(iommu, alias, table);
iommu_flush_dte(pci_seg->rlookup_table[alias], alias);
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-02-25 20:23 [PATCH v3 0/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 2/3] iommu/amd: Track PCIe DMA aliases in set_remap_table_entry_alias Magnus Kalland
@ 2026-02-25 20:23 ` Magnus Kalland
2026-03-30 11:18 ` Vasant Hegde
2 siblings, 1 reply; 13+ messages in thread
From: Magnus Kalland @ 2026-02-25 20:23 UTC (permalink / raw)
To: vasant.hegde, suravee.suthikulpanit, joro, iommu, linux-kernel
Cc: dhsrivas, Magnus Kalland, Lars B . Kristiansen, Jonas Markussen,
Tore H . Larsen
IRTEs may be shared between multiple device IDs when PCIe DMA
aliasing is in use. The AMD IOMMU driver currently invalidates
the interrupt remapping table cache only for the device ID used
to update the IRTE.
If the same IRTE is cached under a different DMA alias, this
leaves stale cache entries that are never invalidated.
Iterate over all device IDs sharing the same DMA alias and
invalidate the IRT cache for each of them when an IRTE is updated.
Co-developed-by: Lars B. Kristiansen <larsk@dolphinics.com>
Signed-off-by: Lars B. Kristiansen <larsk@dolphinics.com>
Co-developed-by: Jonas Markussen <jonas@dolphinics.com>
Signed-off-by: Jonas Markussen <jonas@dolphinics.com>
Co-developed-by: Tore H. Larsen <torel@simula.no>
Signed-off-by: Tore H. Larsen <torel@simula.no>
Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
Link: https://lore.kernel.org/linux-iommu/26cfa307-6c33-41f9-a7a0-fbf202b38a00@amd.com/
---
drivers/iommu/amd/iommu.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 5dec3502c8b3..d9a91d1a083e 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3166,6 +3166,31 @@ const struct iommu_ops amd_iommu_ops = {
static struct irq_chip amd_ir_chip;
static DEFINE_RAW_SPINLOCK(iommu_table_lock);
+static int iommu_flush_irt_for_aliases(struct amd_iommu *iommu,
+ u16 alias)
+{
+ struct amd_iommu_pci_seg *pci_seg = iommu->pci_seg;
+ struct iommu_cmd cmd;
+ unsigned long flags;
+ u32 devid;
+ int ret = 0;
+
+ raw_spin_lock_irqsave(&iommu_table_lock, flags);
+
+ for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
+ if (pci_seg->alias_table[devid] != alias)
+ continue;
+
+ build_inv_irt(&cmd, devid);
+ ret = __iommu_queue_command_sync(iommu, &cmd, true);
+ if (ret)
+ goto out;
+ }
+
+out:
+ raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
+ return ret;
+}
static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
{
@@ -3173,19 +3198,29 @@ static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
u64 data;
unsigned long flags;
struct iommu_cmd cmd, cmd2;
+ u16 alias;
if (iommu->irtcachedis_enabled)
return;
- build_inv_irt(&cmd, devid);
+ raw_spin_lock_irqsave(&iommu_table_lock, flags);
+ alias = iommu->pci_seg->alias_table[devid];
+ raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
raw_spin_lock_irqsave(&iommu->lock, flags);
data = get_cmdsem_val(iommu);
build_completion_wait(&cmd2, iommu, data);
- ret = __iommu_queue_command_sync(iommu, &cmd, true);
+ if (alias == devid) {
+ build_inv_irt(&cmd, devid);
+ ret = __iommu_queue_command_sync(iommu, &cmd, true);
+ } else {
+ ret = iommu_flush_irt_for_aliases(iommu, alias);
+ }
+
if (ret)
goto out_err;
+
ret = __iommu_queue_command_sync(iommu, &cmd2, false);
if (ret)
goto out_err;
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-02-25 20:23 ` [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
@ 2026-03-30 11:18 ` Vasant Hegde
2026-03-31 12:48 ` Magnus Kalland
0 siblings, 1 reply; 13+ messages in thread
From: Vasant Hegde @ 2026-03-30 11:18 UTC (permalink / raw)
To: Magnus Kalland, suravee.suthikulpanit, joro, iommu, linux-kernel
Cc: dhsrivas, Lars B . Kristiansen, Jonas Markussen, Tore H . Larsen
Hi Magnus,
On 2/26/2026 1:53 AM, Magnus Kalland wrote:
> IRTEs may be shared between multiple device IDs when PCIe DMA
> aliasing is in use. The AMD IOMMU driver currently invalidates
> the interrupt remapping table cache only for the device ID used
> to update the IRTE.
>
> If the same IRTE is cached under a different DMA alias, this
> leaves stale cache entries that are never invalidated.
>
> Iterate over all device IDs sharing the same DMA alias and
> invalidate the IRT cache for each of them when an IRTE is updated.
>
> Co-developed-by: Lars B. Kristiansen <larsk@dolphinics.com>
> Signed-off-by: Lars B. Kristiansen <larsk@dolphinics.com>
> Co-developed-by: Jonas Markussen <jonas@dolphinics.com>
> Signed-off-by: Jonas Markussen <jonas@dolphinics.com>
> Co-developed-by: Tore H. Larsen <torel@simula.no>
> Signed-off-by: Tore H. Larsen <torel@simula.no>
> Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
> Link: https://lore.kernel.org/linux-iommu/26cfa307-6c33-41f9-a7a0-fbf202b38a00@amd.com/
>
> ---
> drivers/iommu/amd/iommu.c | 39 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 5dec3502c8b3..d9a91d1a083e 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3166,6 +3166,31 @@ const struct iommu_ops amd_iommu_ops = {
> static struct irq_chip amd_ir_chip;
> static DEFINE_RAW_SPINLOCK(iommu_table_lock);
>
> +static int iommu_flush_irt_for_aliases(struct amd_iommu *iommu,
> + u16 alias)
> +{
> + struct amd_iommu_pci_seg *pci_seg = iommu->pci_seg;
> + struct iommu_cmd cmd;
> + unsigned long flags;
> + u32 devid;
> + int ret = 0;
> +
> + raw_spin_lock_irqsave(&iommu_table_lock, flags);
There is a possible deadlock. Actually we can remove lock here?
path 1) alloc_irq_table() -> holds iommu_table_lock [A] -> iommu->lock [B]
path 2) iommu_flush_irt_and_complete -> holds iommu->lock [B] ->
iommu_table_lock [A]
> +
> + for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
> + if (pci_seg->alias_table[devid] != alias)
This is heavy hammer. Why not use pci_for_each_dma_alias() like we do in DTE
flush path?
-Vasant
> + continue;
> +
> + build_inv_irt(&cmd, devid);
> + ret = __iommu_queue_command_sync(iommu, &cmd, true);
> + if (ret)
> + goto out;
> + }
> +
> +out:
> + raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
> + return ret;
> +}
>
> static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
> {
> @@ -3173,19 +3198,29 @@ static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
> u64 data;
> unsigned long flags;
> struct iommu_cmd cmd, cmd2;
> + u16 alias;
>
> if (iommu->irtcachedis_enabled)
> return;
>
> - build_inv_irt(&cmd, devid);
> + raw_spin_lock_irqsave(&iommu_table_lock, flags);
> + alias = iommu->pci_seg->alias_table[devid];
> + raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
>
> raw_spin_lock_irqsave(&iommu->lock, flags);
> data = get_cmdsem_val(iommu);
> build_completion_wait(&cmd2, iommu, data);
>
> - ret = __iommu_queue_command_sync(iommu, &cmd, true);
> + if (alias == devid) {
> + build_inv_irt(&cmd, devid);
> + ret = __iommu_queue_command_sync(iommu, &cmd, true);
> + } else {
> + ret = iommu_flush_irt_for_aliases(iommu, alias);
> + }
> +
> if (ret)
> goto out_err;
> +
> ret = __iommu_queue_command_sync(iommu, &cmd2, false);
> if (ret)
> goto out_err;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables
2026-02-25 20:23 ` [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables Magnus Kalland
@ 2026-03-30 15:23 ` Vasant Hegde
2026-03-31 13:10 ` Magnus Kalland
0 siblings, 1 reply; 13+ messages in thread
From: Vasant Hegde @ 2026-03-30 15:23 UTC (permalink / raw)
To: Magnus Kalland, suravee.suthikulpanit, joro, iommu, linux-kernel; +Cc: dhsrivas
Magnus,
On 2/26/2026 1:53 AM, Magnus Kalland wrote:
> Use raw spinlock for interrupt remapping tables since
> iommu_flush_irt_and_complete is called under a raw spinlock.
>
I spent sometime going through entire flow. I think we can re-arrange code so
that we can avoid this lock change.
#1 ) alloc_irq_table()
Remove iommu_flush_dte() from set_remap_table_entry /
set_remap_table_entry_alias
Once table is updated, at the end of this function we can have a logic to
flush/completion wait.
#2) iommu_flush_irt_and_complete()
This needs to be reworked to match device_flush_dte()
Makes sense? Did I miss anything?
-Vasant
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-03-30 11:18 ` Vasant Hegde
@ 2026-03-31 12:48 ` Magnus Kalland
2026-04-01 7:44 ` Vasant Hegde
0 siblings, 1 reply; 13+ messages in thread
From: Magnus Kalland @ 2026-03-31 12:48 UTC (permalink / raw)
To: vasant.hegde
Cc: dhsrivas, iommu, jonas, joro, larsk, linux-kernel, magnus,
suravee.suthikulpanit, torel
Hi Vasant, and thank you for the review.
On Mon, Mar 30, 2026 at 04:48:23PM +0530, Vasant Hegde wrote:
> There is a possible deadlock. Actually we can remove lock here?
> path 1) alloc_irq_table() -> holds iommu_table_lock [A] -> iommu->lock [B]
> path 2) iommu_flush_irt_and_complete -> holds iommu->lock [B] ->
> iommu_table_lock [A]
You are correct, there is a possible deadlock. We can avoid it by grabbing
iommu_table_lock, creating a local copy of the aliases, releasing it, then
flushing the aliases. This way, there is no nested locking in
iommu_flush_irt_and_complete.
We notice that the alias table is read in other paths without holding
iommu_table_lock (amd_iommu_change_top, setup_aliases). If we can do the same,
then that is of course another way to avoid the deadlock. What do you think?
> > +
> > + for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
> > + if (pci_seg->alias_table[devid] != alias)
> This is heavy hammer. Why not use pci_for_each_dma_alias() like we do in DTE
> flush path?
Agree this is heavy.
We cannot use pci_for_each_dma_alias. See the reply to our v1:
https://lore.kernel.org/linux-iommu/26cfa307-6c33-41f9-a7a0-fbf202b38a00@amd.com/
However, we have a v4 ready doing 256 loop iterations instead, since aliases
are always on the same bus. I think that's a better approach.
What do you think?
Thanks,
Magnus
Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables
2026-03-30 15:23 ` Vasant Hegde
@ 2026-03-31 13:10 ` Magnus Kalland
0 siblings, 0 replies; 13+ messages in thread
From: Magnus Kalland @ 2026-03-31 13:10 UTC (permalink / raw)
To: vasant.hegde
Cc: dhsrivas, iommu, jonas, joro, larsk, linux-kernel, magnus,
suravee.suthikulpanit, torel
Hi again Vasant.
Not sure if I understand correctly, but I'll try to answer based on how I
read it.
On Mon, Mar 30, 2026 at 08:53:48PM +0530, Vasant Hegde wrote:
> I spent sometime going through entire flow. I think we can re-arrange code so
> that we can avoid this lock change.
>
> #1 ) alloc_irq_table()
> Remove iommu_flush_dte() from set_remap_table_entry /
> set_remap_table_entry_alias
> Once table is updated, at the end of this function we can have a logic to
> flush/completion wait.
The reason we change iommu_table_lock is not just because of the nested raw
spinlocking (holding iommu_table_lock under iommu_lock): it's also because of
the locking context from __setup_irq. Again, see this:
https://lore.kernel.org/linux-iommu/26cfa307-6c33-41f9-a7a0-fbf202b38a00@amd.com/
> #2) iommu_flush_irt_and_complete()
> This needs to be reworked to match device_flush_dte()
>
> Makes sense? Did I miss anything?
Same here. We cannot rework based on the logic in device_flush_dte using
pci_for_each_dma_alias, so we rely solely on alias_table. But we notice that
device_flush_dte is called without holding iommu_table_lock, so if we can do
the same, then changing the lock type won't be necessary.
Please let me know if there is a misunderstanding somewhere along the way.
Thanks,
Magnus
Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-03-31 12:48 ` Magnus Kalland
@ 2026-04-01 7:44 ` Vasant Hegde
2026-04-01 10:34 ` Magnus Kalland
0 siblings, 1 reply; 13+ messages in thread
From: Vasant Hegde @ 2026-04-01 7:44 UTC (permalink / raw)
To: Magnus Kalland, Srivastava, Dheeraj Kumar
Cc: dhsrivas, iommu, jonas, joro, larsk, linux-kernel,
suravee.suthikulpanit, torel
Hi Magnus,
On 3/31/2026 6:18 PM, Magnus Kalland wrote:
> Hi Vasant, and thank you for the review.
>
> On Mon, Mar 30, 2026 at 04:48:23PM +0530, Vasant Hegde wrote:
>> There is a possible deadlock. Actually we can remove lock here?
>
>
>> path 1) alloc_irq_table() -> holds iommu_table_lock [A] -> iommu->lock [B]
>> path 2) iommu_flush_irt_and_complete -> holds iommu->lock [B] ->
>> iommu_table_lock [A]
>
> You are correct, there is a possible deadlock. We can avoid it by grabbing
> iommu_table_lock, creating a local copy of the aliases, releasing it, then
> flushing the aliases. This way, there is no nested locking in
> iommu_flush_irt_and_complete.
>
> We notice that the alias table is read in other paths without holding
> iommu_table_lock (amd_iommu_change_top, setup_aliases). If we can do the same,
> then that is of course another way to avoid the deadlock. What do you think?
>
>>> +
>>> + for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
>>> + if (pci_seg->alias_table[devid] != alias)
>
>> This is heavy hammer. Why not use pci_for_each_dma_alias() like we do in DTE
>> flush path?
>
> Agree this is heavy.
>
> We cannot use pci_for_each_dma_alias. See the reply to our v1:
> https://lore.kernel.org/linux-iommu/26cfa307-6c33-41f9-a7a0-fbf202b38a00@amd.com/
I went back to those reports and reviewed it again. It looks like
pci_get_domain_bus_and_slot() takes spinlock which is causing the lockdep issue.
I think V2 is good w/ some changes to get the pdev.
Instead of pci_get_domain_bus_and_slot(), we should use dev_data (like we do in
other places).
>
> However, we have a v4 ready doing 256 loop iterations instead, since aliases
> are always on the same bus. I think that's a better approach.
> What do you think?
Its a performance sensitive code path. Also during init path, dma aliases are
set properly. I think we should go w/ pci)fir_each_dma_alias -> flush irte path.
I have added below fix on top of your v2 and did some sanity tests. So far it
looks good. I have requested Dheeraj to rerun the tests.
(https://lore.kernel.org/linux-iommu/20260205140059.11857-2-magnus@dolphinics.com/)
-Vasant
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 050178cf388f..eb23b9e7bf03 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3118,12 +3118,10 @@ static void iommu_flush_irt_and_complete(struct
amd_iommu *iommu, u16 devid)
{
int ret;
u64 data;
- int domain = iommu->pci_seg->id;
- unsigned int bus = PCI_BUS_NUM(devid);
- unsigned int devfn = devid & 0xff;
unsigned long flags;
struct iommu_cmd cmd;
struct pci_dev *pdev = NULL;
+ struct iommu_dev_data *dev_data = search_dev_data(iommu, devid);
if (iommu->irtcachedis_enabled)
return;
@@ -3131,11 +3129,12 @@ static void iommu_flush_irt_and_complete(struct
amd_iommu *iommu, u16 devid)
data = atomic64_inc_return(&iommu->cmd_sem_val);
build_completion_wait(&cmd, iommu, data);
- pdev = pci_get_domain_bus_and_slot(domain, bus, devfn);
+ if (dev_data && dev_data->dev && dev_is_pci(dev_data->dev))
+ pdev = to_pci_dev(dev_data->dev);
+
raw_spin_lock_irqsave(&iommu->lock, flags);
if (pdev) {
ret = pci_for_each_dma_alias(pdev, iommu_flush_dev_irt, iommu);
- pci_dev_put(pdev);
} else {
ret = iommu_flush_dev_irt(NULL, devid, iommu);
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-04-01 7:44 ` Vasant Hegde
@ 2026-04-01 10:34 ` Magnus Kalland
2026-04-01 10:46 ` Vasant Hegde
0 siblings, 1 reply; 13+ messages in thread
From: Magnus Kalland @ 2026-04-01 10:34 UTC (permalink / raw)
To: vasant.hegde
Cc: DheerajKumar.Srivastava, dhsrivas, iommu, jonas, joro, larsk,
linux-kernel, magnus, suravee.suthikulpanit, torel
Hi Vasant,
> I went back to those reports and reviewed it again. It looks like
> pci_get_domain_bus_and_slot() takes spinlock which is causing the lockdep issue.
> I think V2 is good w/ some changes to get the pdev.
> Instead of pci_get_domain_bus_and_slot(), we should use dev_data (like we do in
> other places).
> >
> > However, we have a v4 ready doing 256 loop iterations instead, since aliases
> > are always on the same bus. I think that's a better approach.
> > What do you think?
> Its a performance sensitive code path. Also during init path, dma aliases are
> set properly. I think we should go w/ pci)fir_each_dma_alias -> flush irte path.
> I have added below fix on top of your v2 and did some sanity tests. So far it
> looks good. I have requested Dheeraj to rerun the tests.
> (https://lore.kernel.org/linux-iommu/20260205140059.11857-2-magnus@dolphinics.com/)
> -Vasant
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 050178cf388f..eb23b9e7bf03 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3118,12 +3118,10 @@ static void iommu_flush_irt_and_complete(struct
> amd_iommu *iommu, u16 devid)
>{
> int ret;
> u64 data;
> - int domain = iommu->pci_seg->id;
> - unsigned int bus = PCI_BUS_NUM(devid);
> - unsigned int devfn = devid & 0xff;
> unsigned long flags;
> struct iommu_cmd cmd;
> struct pci_dev *pdev = NULL;
> + struct iommu_dev_data *dev_data = search_dev_data(iommu, devid);
>
> if (iommu->irtcachedis_enabled)
> return;
> @@ -3131,11 +3129,12 @@ static void iommu_flush_irt_and_complete(struct
> amd_iommu *iommu, u16 devid)
> data = atomic64_inc_return(&iommu->cmd_sem_val);
> build_completion_wait(&cmd, iommu, data);
>
> - pdev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> + if (dev_data && dev_data->dev && dev_is_pci(dev_data->dev))
> + pdev = to_pci_dev(dev_data->dev);
> +
> raw_spin_lock_irqsave(&iommu->lock, flags);
> if (pdev) {
> ret = pci_for_each_dma_alias(pdev, iommu_flush_dev_irt, iommu);
> - pci_dev_put(pdev);
> } else {
> ret = iommu_flush_dev_irt(NULL, devid, iommu);
> }
Thanks for the fix. I've tested it and it looks good on our end.
Currently the v2 does not apply to next. Should I send a v4 with
your changes that applies cleanly?
Magnus
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-04-01 10:34 ` Magnus Kalland
@ 2026-04-01 10:46 ` Vasant Hegde
2026-04-01 11:38 ` Magnus Kalland
0 siblings, 1 reply; 13+ messages in thread
From: Vasant Hegde @ 2026-04-01 10:46 UTC (permalink / raw)
To: Magnus Kalland
Cc: DheerajKumar.Srivastava, dhsrivas, iommu, jonas, joro, larsk,
linux-kernel, suravee.suthikulpanit, torel
Hi Magnus,
On 4/1/2026 4:04 PM, Magnus Kalland wrote:
> Hi Vasant,
>
>> I went back to those reports and reviewed it again. It looks like
>> pci_get_domain_bus_and_slot() takes spinlock which is causing the lockdep issue.
>
>> I think V2 is good w/ some changes to get the pdev.
>> Instead of pci_get_domain_bus_and_slot(), we should use dev_data (like we do in
>> other places).
>
.../...
>
>
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index 050178cf388f..eb23b9e7bf03 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -3118,12 +3118,10 @@ static void iommu_flush_irt_and_complete(struct
>> amd_iommu *iommu, u16 devid)
>> {
>> int ret;
>> u64 data;
>> - int domain = iommu->pci_seg->id;
>> - unsigned int bus = PCI_BUS_NUM(devid);
>> - unsigned int devfn = devid & 0xff;
>> unsigned long flags;
>> struct iommu_cmd cmd;
>> struct pci_dev *pdev = NULL;
>> + struct iommu_dev_data *dev_data = search_dev_data(iommu, devid);
>>
>> if (iommu->irtcachedis_enabled)
>> return;
>> @@ -3131,11 +3129,12 @@ static void iommu_flush_irt_and_complete(struct
>> amd_iommu *iommu, u16 devid)
>> data = atomic64_inc_return(&iommu->cmd_sem_val);
>> build_completion_wait(&cmd, iommu, data);
>>
>> - pdev = pci_get_domain_bus_and_slot(domain, bus, devfn);
>> + if (dev_data && dev_data->dev && dev_is_pci(dev_data->dev))
>> + pdev = to_pci_dev(dev_data->dev);
>> +
>> raw_spin_lock_irqsave(&iommu->lock, flags);
>> if (pdev) {
>> ret = pci_for_each_dma_alias(pdev, iommu_flush_dev_irt, iommu);
>> - pci_dev_put(pdev);
>> } else {
>> ret = iommu_flush_dev_irt(NULL, devid, iommu);
>> }
>
> Thanks for the fix. I've tested it and it looks good on our end.
Nice! Please send v4.
Also I have sent one more fix. Can you please test that patch in your setup?
https://lore.kernel.org/linux-iommu/20260401080017.117549-1-vasant.hegde@amd.com/T/#u
-Vasant
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-04-01 10:46 ` Vasant Hegde
@ 2026-04-01 11:38 ` Magnus Kalland
0 siblings, 0 replies; 13+ messages in thread
From: Magnus Kalland @ 2026-04-01 11:38 UTC (permalink / raw)
To: vasant.hegde
Cc: DheerajKumar.Srivastava, dhsrivas, iommu, jonas, joro, larsk,
linux-kernel, magnus, suravee.suthikulpanit, torel
Hi Vasant,
> Nice! Please send v4.
> Also I have sent one more fix. Can you please test that patch in your setup?
> https://lore.kernel.org/linux-iommu/20260401080017.117549-1-vasant.hegde@amd.com/
> -Vasant
Tested with the clone_alias fix.
v4 sent:
https://lore.kernel.org/linux-iommu/20260401112021.76806-1-magnus@dolphinics.com/
Magnus
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases
2026-03-06 9:22 [PATCH v3 0/3] " Magnus Kalland
@ 2026-03-06 9:22 ` Magnus Kalland
0 siblings, 0 replies; 13+ messages in thread
From: Magnus Kalland @ 2026-03-06 9:22 UTC (permalink / raw)
To: vasant.hegde, suravee.suthikulpanit, joro, iommu, linux-kernel
Cc: dhsrivas, Magnus Kalland, Lars B . Kristiansen, Jonas Markussen,
Tore H . Larsen
IRTEs may be shared between multiple device IDs when PCIe DMA
aliasing is in use. The AMD IOMMU driver currently invalidates
the interrupt remapping table cache only for the device ID used
to update the IRTE.
If the same IRTE is cached under a different DMA alias, this
leaves stale cache entries that are never invalidated.
Iterate over all device IDs sharing the same DMA alias and
invalidate the IRT cache for each of them when an IRTE is updated.
Co-developed-by: Lars B. Kristiansen <larsk@dolphinics.com>
Signed-off-by: Lars B. Kristiansen <larsk@dolphinics.com>
Co-developed-by: Jonas Markussen <jonas@dolphinics.com>
Signed-off-by: Jonas Markussen <jonas@dolphinics.com>
Co-developed-by: Tore H. Larsen <torel@simula.no>
Signed-off-by: Tore H. Larsen <torel@simula.no>
Signed-off-by: Magnus Kalland <magnus@dolphinics.com>
Link: https://lore.kernel.org/linux-iommu/26cfa307-6c33-41f9-a7a0-fbf202b38a00@amd.com/
---
drivers/iommu/amd/iommu.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 5dec3502c8b3..d9a91d1a083e 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3166,6 +3166,31 @@ const struct iommu_ops amd_iommu_ops = {
static struct irq_chip amd_ir_chip;
static DEFINE_RAW_SPINLOCK(iommu_table_lock);
+static int iommu_flush_irt_for_aliases(struct amd_iommu *iommu,
+ u16 alias)
+{
+ struct amd_iommu_pci_seg *pci_seg = iommu->pci_seg;
+ struct iommu_cmd cmd;
+ unsigned long flags;
+ u32 devid;
+ int ret = 0;
+
+ raw_spin_lock_irqsave(&iommu_table_lock, flags);
+
+ for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
+ if (pci_seg->alias_table[devid] != alias)
+ continue;
+
+ build_inv_irt(&cmd, devid);
+ ret = __iommu_queue_command_sync(iommu, &cmd, true);
+ if (ret)
+ goto out;
+ }
+
+out:
+ raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
+ return ret;
+}
static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
{
@@ -3173,19 +3198,29 @@ static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
u64 data;
unsigned long flags;
struct iommu_cmd cmd, cmd2;
+ u16 alias;
if (iommu->irtcachedis_enabled)
return;
- build_inv_irt(&cmd, devid);
+ raw_spin_lock_irqsave(&iommu_table_lock, flags);
+ alias = iommu->pci_seg->alias_table[devid];
+ raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
raw_spin_lock_irqsave(&iommu->lock, flags);
data = get_cmdsem_val(iommu);
build_completion_wait(&cmd2, iommu, data);
- ret = __iommu_queue_command_sync(iommu, &cmd, true);
+ if (alias == devid) {
+ build_inv_irt(&cmd, devid);
+ ret = __iommu_queue_command_sync(iommu, &cmd, true);
+ } else {
+ ret = iommu_flush_irt_for_aliases(iommu, alias);
+ }
+
if (ret)
goto out_err;
+
ret = __iommu_queue_command_sync(iommu, &cmd2, false);
if (ret)
goto out_err;
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-04-01 11:38 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-25 20:23 [PATCH v3 0/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 1/3] iommu/amd: Use raw spinlock for interrupt remapping tables Magnus Kalland
2026-03-30 15:23 ` Vasant Hegde
2026-03-31 13:10 ` Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 2/3] iommu/amd: Track PCIe DMA aliases in set_remap_table_entry_alias Magnus Kalland
2026-02-25 20:23 ` [PATCH v3 3/3] iommu/amd: Invalidate IRT cache for DMA aliases Magnus Kalland
2026-03-30 11:18 ` Vasant Hegde
2026-03-31 12:48 ` Magnus Kalland
2026-04-01 7:44 ` Vasant Hegde
2026-04-01 10:34 ` Magnus Kalland
2026-04-01 10:46 ` Vasant Hegde
2026-04-01 11:38 ` Magnus Kalland
2026-03-06 9:22 [PATCH v3 0/3] " Magnus Kalland
2026-03-06 9:22 ` [PATCH v3 3/3] " Magnus Kalland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®