Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described
From: Robin Murphy
Date: Wed May 20 2026 - 08:29:43 EST
On 2026-05-20 10:54 am, fangyu.yu@xxxxxxxxxxxxxxxxx wrote:
Won't it change the DT behavior as it doesn't check msi-parent
From: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
The RISC-V IOMMU spec defines IGS=BOTH as supporting both MSI and
WSI, with software selecting the path. The DT path already behaves
as expected by selecting WSI when wired IRQ resources are described.
The ACPI path, however, currently falls back to MSI even when
firmware describes wired IRQ resources.
Use firmware-described wired IRQ resources as the trigger to select
WSI for IGS=BOTH:
- DT: "interrupts" present, no "msi-parent"
- ACPI: DSDT _CRS Interrupt() descriptors
(mainline does not yet parse the RIMT Interrupt Wire Array)
When triggered, rewrite igs to IGS_WSI and reuse the existing WSI
handling. Keep the existing behaviour otherwise.
Fixes: d5f88acdd6ff ("iommu/riscv: Add support for platform msi")
Signed-off-by: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
---
drivers/iommu/riscv/iommu-platform.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/iommu/riscv/iommu-platform.c b/drivers/iommu/riscv/iommu-platform.c
index 399ba8fe1b3e..bd7712231140 100644
--- a/drivers/iommu/riscv/iommu-platform.c
+++ b/drivers/iommu/riscv/iommu-platform.c
@@ -71,6 +71,21 @@ static int riscv_iommu_platform_probe(struct platform_device *pdev)
iommu->irqs_count = RISCV_IOMMU_INTR_COUNT;
igs = FIELD_GET(RISCV_IOMMU_CAPABILITIES_IGS, iommu->caps);
+
+ /*
+ * IGS=BOTH means the IOMMU supports either MSI or WSI;
+ * the spec leaves the choice to software. Use the firmware-described
+ * wired interrupt resources as the trigger:
+ * - DT : "interrupts" property present, no "msi-parent" -> WSI
+ * - ACPI: DSDT _CRS Interrupt() present -> WSI
+ * Otherwise default to the MSI path.
+ */
+ if (igs == RISCV_IOMMU_CAPABILITIES_IGS_BOTH &&
+ platform_irq_count(pdev) > 0) {
+ dev_info(dev, "firmware describes wired IRQs; preferring WSI on IGS=BOTH\n");
+ igs = RISCV_IOMMU_CAPABILITIES_IGS_WSI;
+ }
+
anymore? IOW, should this be made specific to ACPI
by checking whether the device node is acpi node?
Thanks for the review, you're right that the current condition would
affect DT as well.
My assumption was that DT would describe either "interrupts" for WSI
or "msi-parent" for MSI, but not both, so platform_irq_count() > 0
would effectively imply "no msi-parent" in practice.
That said, I agree that this should be limited to the ACPI path. That
matches the actual bug scope — only ACPI was falling back to MSI when
wired IRQs were described — and leaves DT unchanged.
I'll respin a v2 to scope the fix to ACPI only, and tighten the commit
message accordingly.
Why would this be specific to ACPI? AFAICS if DT specifies an "msi-parent" property such that an MSI domain exists, then MSIs will be preferred as well, since the entire function is structured to prefer MSIs if available, and fall back to wired if not. If the system does support both options then DT should describe that, same as ACPI; what Linux chooses to use is entirely Linux's own policy.
But if you do want to change the Linux driver's policy for some reason that the commit message isn't really explaining, then rearrange the whole switch statement; don't just add a weird hack to try to defeat the existing logic _in the same function_...
In general though, I would have thought preferring MSIs makes the most sense, since MSI vectors are generally cheaper than wires, so there's a better chance of being able to have unique IRQs per interrupt source, whereas with wires you may be stuck with a combined IRQ.
(As a side note, is there a reason this is calling of_msi_configure() on a platform device when of_platform_device_create_pdata() will have done that already?)
Thanks,
Robin.