[PATCH] irqchip/riscv-aplic: Fix crash when MSI domain is missing
Samuel Holland
samuel.holland at sifive.com
Fri Nov 15 07:57:00 PST 2024
Hi Anup,
On 2024-11-15 9:42 AM, Anup Patel wrote:
> On Fri, Nov 15, 2024 at 1:31 AM Samuel Holland
> <samuel.holland at sifive.com> wrote:
>>
>> If the APLIC driver is probed before the IMSIC driver, the parent MSI
>> domain will be missing, which causes a NULL pointer dereference in
>> msi_create_device_irq_domain(). Avoid this by deferring probe until the
>> parent MSI domain is available. Use dev_err_probe() to avoid printing an
>> error message when returning -EPROBE_DEFER.
>
> The -EPROBE_DEFER is not needed because we expect that platforms to
> use "msi-parent" DT property in APLIC DT node which in-turn allows Linux
> DD framework to re-order probing based on fw_devlink dependencies. The
> APLIC DT bindings mandates that any of "interrupt-extended" or "msi-parent"
> DT properties MUST be present.
>
> Can you elaborate a bit more on how you are hitting this issue ?
I agree that fw_devlink should help avoid the situation where we need to return
-EPROBE_DEFER, but the kernel must still not crash even if fw_devlink is
disabled (which is a perfectly valid thing to do: "fw_devlink=off" on the kernel
command line) or if fw_devlink fails to come up with the ideal probe order.
fw_devlink is an optimization. It should not be relied on for correctness. In my
specific case, fw_devlink got the order wrong due to some false dependency
cycles, which I sent a patch for separately[1].
Regards,
Samuel
[1]:
https://lore.kernel.org/lkml/20241114195652.3068725-1-samuel.holland@sifive.com/
>> Fixes: ca8df97fe679 ("irqchip/riscv-aplic: Add support for MSI-mode")
>> Signed-off-by: Samuel Holland <samuel.holland at sifive.com>
>> ---
>>
>> drivers/irqchip/irq-riscv-aplic-main.c | 3 ++-
>> drivers/irqchip/irq-riscv-aplic-msi.c | 3 +++
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
>> index 900e72541db9..93e7c51f944a 100644
>> --- a/drivers/irqchip/irq-riscv-aplic-main.c
>> +++ b/drivers/irqchip/irq-riscv-aplic-main.c
>> @@ -207,7 +207,8 @@ static int aplic_probe(struct platform_device *pdev)
>> else
>> rc = aplic_direct_setup(dev, regs);
>> if (rc)
>> - dev_err(dev, "failed to setup APLIC in %s mode\n", msi_mode ? "MSI" : "direct");
>> + dev_err_probe(dev, rc, "failed to setup APLIC in %s mode\n",
>> + msi_mode ? "MSI" : "direct");
>>
>> #ifdef CONFIG_ACPI
>> if (!acpi_disabled)
>> diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
>> index 945bff28265c..fb8d1838609f 100644
>> --- a/drivers/irqchip/irq-riscv-aplic-msi.c
>> +++ b/drivers/irqchip/irq-riscv-aplic-msi.c
>> @@ -266,6 +266,9 @@ int aplic_msi_setup(struct device *dev, void __iomem *regs)
>> if (msi_domain)
>> dev_set_msi_domain(dev, msi_domain);
>> }
>> +
>> + if (!dev_get_msi_domain(dev))
>> + return -EPROBE_DEFER;
>> }
>>
>> if (!msi_create_device_irq_domain(dev, MSI_DEFAULT_DOMAIN, &aplic_msi_template,
>> --
>> 2.45.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
More information about the linux-riscv
mailing list