Re: [RFC PATCH] riscv: Fix PCI warning by enabling PCI_MSI_ARCH_FALLBACKS

From: Alexandre Ghiti
Date: Fri Dec 13 2024 - 08:56:25 EST


Hi Thomas,

On Fri, Dec 13, 2024 at 2:12 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> On Fri, Dec 13 2024 at 12:57, Alexandre Ghiti wrote:
> > When the interrupt controller is not using the IMSIC and ACPI is enabled,
> > the following warning appears:
> >
> > [ 0.866401] WARNING: CPU: 1 PID: 1 at drivers/pci/msi/msi.h:121 pci_msi_setup_msi_irqs+0x2c/0x32
> > [ 0.867071] Modules linked in:
> > [ 0.867389] CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-rc2-00001-g795582ce7e24-dirty #44
> > [ 0.867538] Hardware name: QEMU QEMU Virtual Machine, BIOS
> > [ 0.867672] epc : pci_msi_setup_msi_irqs+0x2c/0x32
> > [ 0.867738] ra : __pci_enable_msix_range+0x30c/0x596
>
> Removing a ton of badly formatted stack trace:
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages

Thanks for the pointer.

>
> >
> > So enable PCI_MSI_ARCH_FALLBACKS to get rid of this.
>
> No. PCI_MSI_ARCH_FALLBACKS is really only meant for architectures which
> implement the legacy fallbacks and not to paper over the underlying
> logic bug in the pci/msi code. Of course the loongson folks ran into the
> same problem two years ago and went for the sloppy fix without talking
> to anyone...
>
> Thanks for bringing it up instead of silently slapping it into the RISCV
> tree !
>
> The uncompiled patch below should fix this for real.

It does, when applied the warning disappears (on riscv at least). You can add:

Tested-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> # riscv

Thanks for your quick answer!

Alex

>
> Thanks,
>
> tglx
> ---
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -185,7 +185,6 @@ config LOONGARCH
> select PCI_DOMAINS_GENERIC
> select PCI_ECAM if ACPI
> select PCI_LOONGSON
> - select PCI_MSI_ARCH_FALLBACKS
> select PCI_QUIRKS
> select PERF_USE_VMALLOC
> select RTC_LIB
> --- a/drivers/pci/msi/irqdomain.c
> +++ b/drivers/pci/msi/irqdomain.c
> @@ -350,8 +350,11 @@ bool pci_msi_domain_supports(struct pci_
>
> domain = dev_get_msi_domain(&pdev->dev);
>
> - if (!domain || !irq_domain_is_hierarchy(domain))
> - return mode == ALLOW_LEGACY;
> + if (!domain || !irq_domain_is_hierarchy(domain)) {
> + if (IS_ENABLED(CONFIG_PCI_MSI_ARCH_FALLBACKS))
> + return mode == ALLOW_LEGACY;
> + return false;
> + }
>
> if (!irq_domain_is_msi_parent(domain)) {
> /*
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -442,6 +442,10 @@ int __pci_enable_msi_range(struct pci_de
> if (nvec > maxvec)
> nvec = maxvec;
>
> + /* Test for the availability of MSI support */
> + if (!pci_msi_domain_supports(dev, 0, ALLOW_LEGACY))
> + return -ENOTSUPP;
> +
> rc = pci_setup_msi_context(dev);
> if (rc)
> return rc;