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

[fangyu.yu@linux.alibaba.com]

>>
>> 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
>