arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+)
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
[ 0.867783] epc : ffffffff8050af80 ra : ffffffff8050a66e sp : ff20000000023750
[ 0.867809] gp : ffffffff815153b0 tp : ff60000080108000 t0 : ff60000081109600
[ 0.867833] t1 : 0000000000000228 t2 : 0000000000000004 s0 : ff20000000023860
[ 0.867857] s1 : ff60000080de1000 a0 : ff60000080de1000 a1 : 0000000000000005
[ 0.867880] a2 : 0000000000000011 a3 : 0000000000000000 a4 : 0000000000000000
[ 0.867902] a5 : 0000000000000000 a6 : ff600000806368f0 a7 : fffffffffffffff0
[ 0.867925] s2 : 0000000000000005 s3 : ffffffffffffffff s4 : 0000000000000000
[ 0.867948] s5 : ff60000080de10c0 s6 : 0000000000000005 s7 : 0000000000000005
[ 0.867970] s8 : ff20000000023a08 s9 : ff600000811093c0 s10: 000000000000002c
[ 0.867993] s11: ff60000081109410 t3 : 0000000000000001 t4 : ff600000803a2878
[ 0.868014] t5 : 0000000000000004 t6 : ff60000080357450
[ 0.868036] status: 0000000200000120 badaddr: ffffffff8050af80 cause: 0000000000000003
[ 0.868186] [<ffffffff8050af80>] pci_msi_setup_msi_irqs+0x2c/0x32
[ 0.868339] [<ffffffff80509172>] pci_alloc_irq_vectors_affinity+0xb8/0xe2
[ 0.868362] [<ffffffff8059d62c>] vp_find_vqs_msix+0x12a/0x370
[ 0.868385] [<ffffffff8059d8a0>] vp_find_vqs+0x2e/0x1de
[ 0.868402] [<ffffffff8059bd80>] vp_modern_find_vqs+0x12/0x4e
[ 0.868425] [<ffffffff80624a50>] init_vq+0x2b4/0x336
[ 0.868448] [<ffffffff80624c36>] virtblk_probe+0xd4/0x90e
[ 0.868469] [<ffffffff80594e02>] virtio_dev_probe+0x14a/0x1e6
[ 0.868488] [<ffffffff805fe04c>] really_probe+0x86/0x234
[ 0.868509] [<ffffffff805fe256>] __driver_probe_device+0x5c/0xda
[ 0.868529] [<ffffffff805fe392>] driver_probe_device+0x2c/0xb2
[ 0.868549] [<ffffffff805fe512>] __driver_attach+0x6c/0x11a
[ 0.868569] [<ffffffff805fc17e>] bus_for_each_dev+0x60/0xae
[ 0.868588] [<ffffffff805fda7c>] driver_attach+0x1a/0x22
[ 0.868607] [<ffffffff805fd398>] bus_add_driver+0xce/0x1d6
[ 0.868627] [<ffffffff805ff0b2>] driver_register+0x3e/0xd8
[ 0.868647] [<ffffffff80594614>] __register_virtio_driver+0x1e/0x2c
[ 0.868694] [<ffffffff80a31b82>] virtio_blk_init+0x6a/0x9e
[ 0.868733] [<ffffffff8000f128>] do_one_initcall+0x58/0x194
[ 0.868755] [<ffffffff80a011b0>] kernel_init_freeable+0x224/0x28e
[ 0.868775] [<ffffffff809e4e48>] kernel_init+0x1e/0x13a
[ 0.868795] [<ffffffff809ed952>] ret_from_fork+0xe/0x18
So enable PCI_MSI_ARCH_FALLBACKS to get rid of this.
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
This is an RFC as I'm really not sure this is the right fix,
Anup/Sunil/Thomas if you have any idea, please step in! Thanks
arch/riscv/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d4a7ca0388c0..40d51feac2bb 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -199,6 +199,7 @@ config RISCV
select PCI_DOMAINS_GENERIC if PCI
select PCI_ECAM if (ACPI && PCI)
select PCI_MSI if PCI
+ select PCI_MSI_ARCH_FALLBACKS if PCI
select RISCV_ALTERNATIVE if !XIP_KERNEL
select RISCV_APLIC
select RISCV_IMSIC
--
2.39.2On 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
>
> 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.
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;
Hi Thomas,
On Fri, Dec 13, 2024 at 2:12 PM Thomas Gleixner <tglx@linutronix.de> 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@rivosinc.com> # 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;
Alexandre observed a warning emitted from pci_msi_setup_msi_irqs() on a
RISCV platform which does not provide PCI/MSI support:
WARNING: CPU: 1 PID: 1 at drivers/pci/msi/msi.h:121 pci_msi_setup_msi_irqs+0x2c/0x32
__pci_enable_msix_range+0x30c/0x596
pci_msi_setup_msi_irqs+0x2c/0x32
pci_alloc_irq_vectors_affinity+0xb8/0xe2
RISCV uses hierarchical interrupt domains and correctly does not implement
the legacy fallback. The warning triggers from the legacy fallback stub.
That warning is bogus as the PCI/MSI layer knows whether a PCI/MSI parent
domain is associated with the device or not. There is a check for MSI-X,
which has a legacy assumption. But that legacy fallback assumption is only
valid when legacy support is enabled, but otherwise the check should simply
return -ENOTSUPP.
Loongarch tripped over the same problem and blindly enabled legacy support
without implementing the legacy fallbacks. There are weak implementations
which return an error, so the problem was papered over.
Correct pci_msi_domain_supports() to evaluate the legacy mode and add
the missing supported check into the MSI enable path to complete it.
Fixes: d2a463b29741 ("PCI/MSI: Reject multi-MSI early")
Reported-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Cc: stable@vger.kernel.org
---
drivers/pci/msi/irqdomain.c | 7 +++++--
drivers/pci/msi/msi.c | 4 ++++
2 files changed, 9 insertions(+), 2 deletions(-)
--- 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
@@ -433,6 +433,10 @@ int __pci_enable_msi_range(struct pci_de
if (WARN_ON_ONCE(dev->msi_enabled))
return -EINVAL;
+ /* Test for the availability of MSI support */
+ if (!pci_msi_domain_supports(dev, 0, ALLOW_LEGACY))
+ return -ENOTSUPP;
+
nvec = pci_msi_vec_count(dev);
if (nvec < 0)
return nvec;The following commit has been merged into the irq/urgent branch of tip:
Commit-ID: a60b990798eb17433d0283788280422b1bd94b18
Gitweb: https://git.kernel.org/tip/a60b990798eb17433d0283788280422b1bd94b18
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Sat, 14 Dec 2024 12:50:18 +01:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 16 Dec 2024 10:59:47 +01:00
PCI/MSI: Handle lack of irqdomain gracefully
Alexandre observed a warning emitted from pci_msi_setup_msi_irqs() on a
RISCV platform which does not provide PCI/MSI support:
WARNING: CPU: 1 PID: 1 at drivers/pci/msi/msi.h:121 pci_msi_setup_msi_irqs+0x2c/0x32
__pci_enable_msix_range+0x30c/0x596
pci_msi_setup_msi_irqs+0x2c/0x32
pci_alloc_irq_vectors_affinity+0xb8/0xe2
RISCV uses hierarchical interrupt domains and correctly does not implement
the legacy fallback. The warning triggers from the legacy fallback stub.
That warning is bogus as the PCI/MSI layer knows whether a PCI/MSI parent
domain is associated with the device or not. There is a check for MSI-X,
which has a legacy assumption. But that legacy fallback assumption is only
valid when legacy support is enabled, but otherwise the check should simply
return -ENOTSUPP.
Loongarch tripped over the same problem and blindly enabled legacy support
without implementing the legacy fallbacks. There are weak implementations
which return an error, so the problem was papered over.
Correct pci_msi_domain_supports() to evaluate the legacy mode and add
the missing supported check into the MSI enable path to complete it.
Fixes: d2a463b29741 ("PCI/MSI: Reject multi-MSI early")
Reported-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/87ed2a8ow5.ffs@tglx
---
drivers/pci/msi/irqdomain.c | 7 +++++--
drivers/pci/msi/msi.c | 4 ++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c
index 5691257..d7ba879 100644
--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -350,8 +350,11 @@ bool pci_msi_domain_supports(struct pci_dev *pdev, unsigned int feature_mask,
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)) {
/*
diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
index 3a45879..2f647ca 100644
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -433,6 +433,10 @@ int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
if (WARN_ON_ONCE(dev->msi_enabled))
return -EINVAL;
+ /* Test for the availability of MSI support */
+ if (!pci_msi_domain_supports(dev, 0, ALLOW_LEGACY))
+ return -ENOTSUPP;
+
nvec = pci_msi_vec_count(dev);
if (nvec < 0)
return nvec;© 2016 - 2025 Red Hat, Inc.