[PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[fangyu.yu]

From: Fangyu Yu <fangyu.yu@linux.alibaba.com>

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@linux.alibaba.com>
---
 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;
+	}
+
 	switch (igs) {
 	case RISCV_IOMMU_CAPABILITIES_IGS_BOTH:
 	case RISCV_IOMMU_CAPABILITIES_IGS_MSI:
-- 
2.50.1

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[Sunil V L]

On Tue, May 19, 2026 at 6:28 PM <fangyu.yu@linux.alibaba.com> wrote:
>
> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>
> 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@linux.alibaba.com>
> ---
>  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;
> +       }
> +
Won't it change the DT behavior as it doesn't check msi-parent
anymore?  IOW, should this be made specific to ACPI
by checking whether the device node is acpi node?

Thanks,
Sunil

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[fangyu.yu]

>>
>> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>>
>> 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@linux.alibaba.com>
>> ---
>>  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;
>> +       }
>> +
>Won't it change the DT behavior as it doesn't check msi-parent
>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.

Thanks,
Fangyu

>Thanks,
>Sunil
>

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[Robin Murphy]

On 2026-05-20 10:54 am, fangyu.yu@linux.alibaba.com wrote:
>>>
>>> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>>>
>>> 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@linux.alibaba.com>
>>> ---
>>>   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;
>>> +       }
>>> +
>> Won't it change the DT behavior as it doesn't check msi-parent
>> 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.

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[fangyu.yu]

>>> Won't it change the DT behavior as it doesn't check msi-parent
>>> 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.

Thanks, Robin.

To clarify, the goal here is not to change the general MSI-vs-WSI
preference policy. The bug is that, when the hardware advertises
IGS=BOTH, DT already provides a way to select WSI by describing wired
IRQs without an msi-parent, but the ACPI path does not currently have
an equivalent way to steer the driver to WSI.

    - DT: if "msi-parent" is omitted, of_msi_get_domain() returns NULL,
      IGS=BOTH then falls through to the WSI path, so DT has a real knob
      ("don't describe MSI") to select WSI.
    - ACPI: there is no per-device equivalent. On systems with IMSIC, the
      IGS=BOTH IOMMU always enters the MSI branch and resolves a non-NULL
      msi_domain, regardless of what its own _CRS describes. The MSI fallback
      only triggers when the system has no IMSIC at all, which is a
      firmware-wide decision rather than a per-device one.

As a result, with ACPI and IGS=BOTH, the driver always uses MSI even when
firmware describes wired IRQs, so the platform cannot express the same
intent as it can with DT.

So this patch is meant to fix that ACPI gap for IGS=BOTH, not to make
WSI generally preferred over MSI. I agree the commit message should
make that bug scope explicit.

>
>(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?)

Good catch, agreed.

Thanks,
Fangyu

>
>Thanks,
>Robin.

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[Andrew Jones]

On Wed, May 20, 2026 at 09:44:16PM +0800, fangyu.yu@linux.alibaba.com wrote:
...
> >(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?)
> 
> Good catch, agreed.
>

This is here for a device creation order issue. We have similar patterns
elsewhere, e.g. drivers/irqchip/irq-riscv-rpmi-sysmsi.c:268. When the
device is created the MSI controller may not yet have been probed, so
msi_domain will be NULL. The of_msi_configure() here ensures it gets a
second chance. Anup says he has a device driver framework solution to
avoid this pattern that he intends to post soon. I'll clean this up
after that.

Thanks,
drew

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[Robin Murphy]

On 2026-05-20 5:03 pm, Andrew Jones wrote:
> On Wed, May 20, 2026 at 09:44:16PM +0800, fangyu.yu@linux.alibaba.com wrote:
> ...
>>> (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?)
>>
>> Good catch, agreed.
>>
> 
> This is here for a device creation order issue. We have similar patterns
> elsewhere, e.g. drivers/irqchip/irq-riscv-rpmi-sysmsi.c:268. When the
> device is created the MSI controller may not yet have been probed, so
> msi_domain will be NULL. The of_msi_configure() here ensures it gets a
> second chance. Anup says he has a device driver framework solution to
> avoid this pattern that he intends to post soon. I'll clean this up
> after that.

Ah, that sounds like something somewhere between of_msi_configure() and 
platform_device_msi_init_and_alloc_irqs() needs to be able to return an 
EPROBE_DEFER if a parent is specified but not ready yet, similar to 
regular of_irq_get(). I guess this hasn't been noticed much yet since 
most MSI controllers run off early initcalls rather than being regular 
driver model drivers - OK, fair enough for now :)

Cheers,
Robin.

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[Robin Murphy]

On 2026-05-20 2:44 pm, fangyu.yu@linux.alibaba.com wrote:
>>>> Won't it change the DT behavior as it doesn't check msi-parent
>>>> 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.
> 
> Thanks, Robin.
> 
> To clarify, the goal here is not to change the general MSI-vs-WSI
> preference policy. The bug is that, when the hardware advertises
> IGS=BOTH, DT already provides a way to select WSI by describing wired
> IRQs without an msi-parent,

And my point is that that sounds wrong - if the system can support both 
then DT should have both properties, because DT describes the system, 
not Linux policy. Of course it is presumably possible to have a reusable 
IOMMU IP that itself supports IGS=BOTH, integrated into systems which 
either don't have an MSI controller, or haven't connected the wires, in 
which case only one or other property is valid to describe anyway, but 
that's fine - IGS describes what the IOMMU is capable of sending, the 
firmware properties describe what the rest of the system is capable of 
receiving, and if there's any choice left in the intersection of the two 
then that's up to the OS.

> but the ACPI path does not currently have
> an equivalent way to steer the driver to WSI.
> 
>      - DT: if "msi-parent" is omitted, of_msi_get_domain() returns NULL,
>        IGS=BOTH then falls through to the WSI path, so DT has a real knob
>        ("don't describe MSI") to select WSI.
>      - ACPI: there is no per-device equivalent. On systems with IMSIC, the
>        IGS=BOTH IOMMU always enters the MSI branch and resolves a non-NULL
>        msi_domain, regardless of what its own _CRS describes. The MSI fallback
>        only triggers when the system has no IMSIC at all, which is a
>        firmware-wide decision rather than a per-device one.
> 
> As a result, with ACPI and IGS=BOTH, the driver always uses MSI even when
> firmware describes wired IRQs, so the platform cannot express the same
> intent as it can with DT.

What do you mean by "intent"? Again, people should not be hacking 
firmware just to influence OS driver policy.

If you mean you have a case where a system does have an MSI controller, 
and a device which is capable of emitting MSIs, but the device's MSI 
writes cannot reach that MSI controller, and ACPI is incapable of 
describing that so Linux ends up incorrectly assuming that MSIs should 
work, then you have a spec-level ACPI problem and you need to fix RIMT 
and/or any other relevant tables/properties to be able to indicate that 
properly; bodging the Linux IOMMU driver alone does not help other OSes, 
nor even other MSI-capable devices under Linux.

Amusingly, that would then be the opposite problem from what we had with 
Arm SMMU, where initially we couldn't use MSIs with ACPI at all since 
early IORT version had no way to describe the necessary GIC DeviceID :)

> So this patch is meant to fix that ACPI gap for IGS=BOTH, not to make
> WSI generally preferred over MSI. I agree the commit message should
> make that bug scope explicit.

But even then it does change the policy in general for all ACPI systems 
that genuinely do have both and could happily still use MSIs.

Thanks,
Robin.

Re: [PATCH] iommu/riscv: prefer WSI on IGS=BOTH when wired IRQs are described

[fangyu.yu]

>>>>> Won't it change the DT behavior as it doesn't check msi-parent
>>>>> 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.
>> 
>> Thanks, Robin.
>> 
>> To clarify, the goal here is not to change the general MSI-vs-WSI
>> preference policy. The bug is that, when the hardware advertises
>> IGS=BOTH, DT already provides a way to select WSI by describing wired
>> IRQs without an msi-parent,
>
>And my point is that that sounds wrong - if the system can support both 
>then DT should have both properties, because DT describes the system, 
>not Linux policy. Of course it is presumably possible to have a reusable 
>IOMMU IP that itself supports IGS=BOTH, integrated into systems which 
>either don't have an MSI controller, or haven't connected the wires, in 
>which case only one or other property is valid to describe anyway, but 
>that's fine - IGS describes what the IOMMU is capable of sending, the 
>firmware properties describe what the rest of the system is capable of 
>receiving, and if there's any choice left in the intersection of the two 
>then that's up to the OS.

Agreed, Firmware describes hardware capability; when both options are
properly described, prefer-MSI is the right default.

>> but the ACPI path does not currently have
>> an equivalent way to steer the driver to WSI.
>> 
>>      - DT: if "msi-parent" is omitted, of_msi_get_domain() returns NULL,
>>        IGS=BOTH then falls through to the WSI path, so DT has a real knob
>>        ("don't describe MSI") to select WSI.
>>      - ACPI: there is no per-device equivalent. On systems with IMSIC, the
>>        IGS=BOTH IOMMU always enters the MSI branch and resolves a non-NULL
>>        msi_domain, regardless of what its own _CRS describes. The MSI fallback
>>        only triggers when the system has no IMSIC at all, which is a
>>        firmware-wide decision rather than a per-device one.
>> 
>> As a result, with ACPI and IGS=BOTH, the driver always uses MSI even when
>> firmware describes wired IRQs, so the platform cannot express the same
>> intent as it can with DT.
>
>What do you mean by "intent"? Again, people should not be hacking 
>firmware just to influence OS driver policy.
>
>If you mean you have a case where a system does have an MSI controller, 
>and a device which is capable of emitting MSIs, but the device's MSI 
>writes cannot reach that MSI controller, and ACPI is incapable of 
>describing that so Linux ends up incorrectly assuming that MSIs should 
>work, then you have a spec-level ACPI problem and you need to fix RIMT 
>and/or any other relevant tables/properties to be able to indicate that 
>properly; bodging the Linux IOMMU driver alone does not help other OSes, 
>nor even other MSI-capable devices under Linux.

You're right - But RIMT v1.0 lacks a per-IOMMU equivalent of IORT SMMUv3
Flags bit 4 (DeviceID mapping index valid).

I was wondering whether the old ARM IORT compatibility pattern could be
considered here: if a given RIMT IOMMU node fully populates its Interrupt
Wire Array (civ, fiv, piv, pmiv), could that be treated as a temporary
signal for the wired path until RIMT grows an explicit flag?

If that is still too policy-like for ACPI, is there any comparable
transitional pattern that would be more acceptable until RIMT gains the
missing explicit signal?

Thanks,
Fangyu

>Amusingly, that would then be the opposite problem from what we had with 
>Arm SMMU, where initially we couldn't use MSIs with ACPI at all since 
>early IORT version had no way to describe the necessary GIC DeviceID :)
>
>> So this patch is meant to fix that ACPI gap for IGS=BOTH, not to make
>> WSI generally preferred over MSI. I agree the commit message should
>> make that bug scope explicit.
>
>But even then it does change the policy in general for all ACPI systems 
>that genuinely do have both and could happily still use MSIs.
>
>Thanks,
>Robin.