[PATCH AUTOSEL 5.4 1/2] Revert "iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices"
From: Lukas Wunner lukas@wunner.de
[ Upstream commit 3be5fa236649da6404f1bca1491bf02d4b0d5cce ]
Commit 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and pcibios_free_irq()") changed IRQ handling on PCI driver probing. It inadvertently broke resume from system sleep on AMD platforms:
https://lore.kernel.org/r/20150926164651.GA3640@pd.tnic/
This was fixed by two independent commits:
* 8affb487d4a4 ("x86/PCI: Don't alloc pcibios-irq when MSI is enabled") * cbbc00be2ce3 ("iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices")
The breaking change and one of these two fixes were subsequently reverted:
* fe25d078874f ("Revert "x86/PCI: Don't alloc pcibios-irq when MSI is enabled"") * 6c777e8799a9 ("Revert "PCI, x86: Implement pcibios_alloc_irq() and pcibios_free_irq()"")
This rendered the second fix unnecessary, so revert it as well. It used the match_driver flag in struct pci_dev, which is internal to the PCI core and not supposed to be touched by arbitrary drivers.
Signed-off-by: Lukas Wunner lukas@wunner.de Signed-off-by: Bjorn Helgaas bhelgaas@google.com Signed-off-by: Krzysztof Wilczyński kwilczynski@kernel.org Acked-by: Joerg Roedel jroedel@suse.de Link: https://patch.msgid.link/9a3ddff5cc49512044f963ba0904347bd404094d.1745572340... Signed-off-by: Sasha Levin sashal@kernel.org ---
**YES**
This commit should be backported to stable kernel trees.
**Extensive Analysis:**
**1. Context and Purpose:** This commit is part of a coordinated series of reverts addressing regressions introduced by commit 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and pcibios_free_irq()") which broke system suspend/resume on AMD platforms in v4.3. The original issue was fixed by two independent commits, one of which (cbbc00be2ce3) added the line `iommu->dev->match_driver = false;` to prevent other PCI drivers from binding to IOMMU devices.
**2. Code Change Analysis:** The commit removes exactly one line from `drivers/iommu/amd/init.c:2034`: ```c /* Prevent binding other PCI device drivers to IOMMU devices */ iommu->dev->match_driver = false; ```
This line was setting the `match_driver` field of `struct pci_dev`, which the commit message correctly identifies as "internal to the PCI core and not supposed to be touched by arbitrary drivers."
**3. Why This Should Be Backported:**
**a) Part of Coordinated Fix Series:** This revert is the logical completion of a series of reverts that fixed major regressions. Looking at the similar commits provided as examples: - Similar Commit #1 (YES): Revert of x86/PCI pcibios-irq allocation - Similar Commit #2 (YES): Revert of PCI IRQ management helpers - Similar Commit #5 (YES): Revert of the original problematic pcibios_alloc_irq implementation
All these related reverts were marked for backporting, making this commit part of the same logical fix series.
**b) Removes Inappropriate Code:** The commit eliminates code that violates kernel design principles by accessing internal PCI core structures from a driver. The `match_driver` field is not meant to be manipulated by individual drivers.
**c) Cleanup After Main Fix:** Once the root cause (commit 991de2e59090) was reverted by commit 6c777e8799a9, the workaround became unnecessary. Keeping unnecessary workaround code, especially code that inappropriately accesses internal structures, is problematic.
**d) Minimal Risk:** The change is extremely small and low-risk - it simply removes one line that was setting an internal field inappropriately.
**e) Consistency and Completeness:** Since the other reverts in this series addressing the 991de2e59090 regressions were backported, this should be included for consistency and to ensure the cleanup is complete.
**4. Stable Tree Criteria Met:** - ✅ Fixes inappropriate driver behavior (accessing internal PCI structures) - ✅ Small and contained change - ✅ Part of a series addressing known regressions - ✅ Minimal risk of introducing new issues - ✅ Consistent with backporting decisions for related commits
The commit represents necessary cleanup after a coordinated regression fix and should be backported to maintain consistency with the related reverts and to remove code that inappropriately accesses internal kernel structures.
drivers/iommu/amd_iommu_init.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c index de29512c75ccc..cd17deeedf349 100644 --- a/drivers/iommu/amd_iommu_init.c +++ b/drivers/iommu/amd_iommu_init.c @@ -1751,9 +1751,6 @@ static int __init iommu_init_pci(struct amd_iommu *iommu) if (!iommu->dev) return -ENODEV;
- /* Prevent binding other PCI device drivers to IOMMU devices */ - iommu->dev->match_driver = false; - /* ACPI _PRT won't have an IRQ for IOMMU */ iommu->dev->irq_managed = 1;
From: Thomas Gessler thomas.gessler@brueckmann-gmbh.de
[ Upstream commit 7e01511443c30a55a5ae78d3debd46d4d872517e ]
Coalesce the direction bits from the enabled TX and/or RX channels into the directions bit mask of dma_device. Without this mask set, dma_get_slave_caps() in the DMAEngine fails, which prevents the driver from being used with an IIO DMAEngine buffer.
Signed-off-by: Thomas Gessler thomas.gessler@brueckmann-gmbh.de Reviewed-by: Suraj Gupta suraj.gupta2@amd.com Tested-by: Folker Schwesinger dev@folker-schwesinger.de Link: https://lore.kernel.org/r/20250507182101.909010-1-thomas.gessler@brueckmann-... Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
**YES**
## Analysis
This commit should be backported to stable kernel trees because it fixes a critical functionality bug that prevents the Xilinx DMA driver from working with important subsystems.
### The Problem The commit addresses a fundamental issue where `dma_get_slave_caps()` fails when called on Xilinx DMA channels. The code change shows that the driver was not setting the `directions` field in the `dma_device` structure:
```c + xdev->common.directions |= chan->direction; ```
This single line fix accumulates the direction capabilities from individual channels (either `DMA_MEM_TO_DEV` or `DMA_DEV_TO_MEM`) into the device-level `directions` bitmask.
### Why This Qualifies for Backporting
1. **Fixes Important User-Facing Bug**: The commit message explicitly states this prevents the driver from being used with IIO DMAEngine buffers, which is a significant functional regression affecting real users.
2. **Small and Contained Fix**: The change is minimal - just one line of code that sets a required field during channel probe. This has extremely low risk of introducing regressions.
3. **Critical Subsystem Integration**: Without this fix, `dma_get_slave_caps()` calls fail with `-ENXIO`, breaking integration with any subsystem that queries DMA capabilities (like IIO).
4. **Clear Root Cause**: The fix directly addresses the root cause - the missing `directions` field that the DMAEngine core requires to be set.
5. **No Architectural Changes**: This doesn't introduce new features or change driver architecture; it simply provides required capability information that was missing.
### Comparison to Reference Commits This closely matches **Similar Commit #1** (marked YES) which also fixed a missing capability flag (`DMA_CYCLIC cap_mask bit`) that prevented proper DMA channel allocation. Both commits: - Fix missing capability declarations - Are small, single-line changes - Address integration failures with other subsystems - Have minimal regression risk
The commit also mirrors **Similar Commit #2** (marked YES) which fixed incorrect struct usage in the same driver - both address functional correctness issues in the Xilinx DMA driver.
### Risk Assessment The risk is minimal because: - The change only affects the capability reporting mechanism - It doesn't modify any data paths or transfer logic - The direction values being OR'd together are already correctly set per-channel - Failure mode is obvious (capability queries will work instead of failing)
This is a textbook example of a stable tree candidate: it fixes an important bug affecting real users with a minimal, low-risk change that doesn't introduce new functionality.
drivers/dma/xilinx/xilinx_dma.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c index be44c86a1e037..5b5cbea997ff7 100644 --- a/drivers/dma/xilinx/xilinx_dma.c +++ b/drivers/dma/xilinx/xilinx_dma.c @@ -2490,6 +2490,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev, return -EINVAL; }
+ xdev->common.directions |= chan->direction; + /* Request the interrupt */ chan->irq = irq_of_parse_and_map(node, 0); err = request_irq(chan->irq, xilinx_dma_irq_handler, IRQF_SHARED,
linux-stable-mirror@lists.linaro.org
-
Sasha Levin