diff mbox series

[v3,13/14] PCI/MSI: Document the various ways of ending up with NO_MSI

Message ID 20210330151145.997953-14-maz@kernel.org
State New
Headers show
Series PCI/MSI: Getting rid of msi_controller, and other cleanups | expand

Commit Message

Marc Zyngier March 30, 2021, 3:11 p.m. UTC
We have now three ways of ending up with NO_MSI being set.
Document them.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/msi.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Jeremy Linton May 18, 2021, 4:28 a.m. UTC | #1
Hi,


On 3/30/21 10:11 AM, Marc Zyngier wrote:
> We have now three ways of ending up with NO_MSI being set.
> Document them.
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>   drivers/pci/msi.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index d9c73c173c14..217dc9f0231f 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -871,8 +871,15 @@ static int pci_msi_supported(struct pci_dev *dev, int nvec)
>   	 * Any bridge which does NOT route MSI transactions from its
>   	 * secondary bus to its primary bus must set NO_MSI flag on
>   	 * the secondary pci_bus.
> -	 * We expect only arch-specific PCI host bus controller driver
> -	 * or quirks for specific PCI bridges to be setting NO_MSI.
> +	 *
> +	 * The NO_MSI flag can either be set directly by:
> +	 * - arch-specific PCI host bus controller drivers (deprecated)
> +	 * - quirks for specific PCI bridges
> +	 *
> +	 * or indirectly by platform-specific PCI host bridge drivers by
> +	 * advertising the 'msi_domain' property, which results in
> +	 * the NO_MSI flag when no MSI domain is found for this bridge
> +	 * at probe time.

I have an ACPI machine with a gicv2 (no m), and a MSI region that isn't 
described by ACPI because its non-standard. In the past this tended to 
work because PCIe device drivers would fall back to legacy pci intx 
silently. But, with 5.13, it seems this series now triggers the 
WARN_ON_ONCE() in arch_setup_msi_irq, because duh, no MSI support.

Everything of course continues to work, it just gets this ugly splat on 
bootup telling me basically the machine doesn't support MSIs. So, I 
considered a few patches, including just basically setting nomsi if 
gicv2 && acpi, or eek a host bridge quirk.

None of these seem great, so how can this be fixed?

Thanks,




>   	 */
>   	for (bus = dev->bus; bus; bus = bus->parent)
>   		if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
>
Marc Zyngier May 18, 2021, 8:59 a.m. UTC | #2
Hi Jeremy,

On Tue, 18 May 2021 05:28:56 +0100,
Jeremy Linton <jeremy.linton@arm.com> wrote:
> 
> Hi,
> 
> 
> On 3/30/21 10:11 AM, Marc Zyngier wrote:
> > We have now three ways of ending up with NO_MSI being set.
> > Document them.
> > 
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >   drivers/pci/msi.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index d9c73c173c14..217dc9f0231f 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -871,8 +871,15 @@ static int pci_msi_supported(struct pci_dev *dev, int nvec)
> >   	 * Any bridge which does NOT route MSI transactions from its
> >   	 * secondary bus to its primary bus must set NO_MSI flag on
> >   	 * the secondary pci_bus.
> > -	 * We expect only arch-specific PCI host bus controller driver
> > -	 * or quirks for specific PCI bridges to be setting NO_MSI.
> > +	 *
> > +	 * The NO_MSI flag can either be set directly by:
> > +	 * - arch-specific PCI host bus controller drivers (deprecated)
> > +	 * - quirks for specific PCI bridges
> > +	 *
> > +	 * or indirectly by platform-specific PCI host bridge drivers by
> > +	 * advertising the 'msi_domain' property, which results in
> > +	 * the NO_MSI flag when no MSI domain is found for this bridge
> > +	 * at probe time.
> 
> I have an ACPI machine with a gicv2 (no m), and a MSI region that
> isn't described by ACPI because its non-standard. In the past this
> tended to work because PCIe device drivers would fall back to legacy
> pci intx silently. But, with 5.13, it seems this series now triggers
> the WARN_ON_ONCE() in arch_setup_msi_irq, because duh, no MSI support.

This is nothing new, and you could get the exact same warning if you
didn't have legacy drivers compiled in (any of the 3 drivers that were
fixed in this series).

This series now makes sure you definitely know about this issue. And
look, it worked! :-)

> Everything of course continues to work, it just gets this ugly splat
> on bootup telling me basically the machine doesn't support MSIs. So, I
> considered a few patches, including just basically setting nomsi if
> gicv2 && acpi, or eek a host bridge quirk.
> 
> None of these seem great, so how can this be fixed?

The host bridge quirk seems the most likely route to address this, but
you could just as well advertise msi_domain in the ACPI PCI path,
*and* check for IORT mappings in pci_register_host_bridge(), similarly
to what Jean-Philippe has proposed for DT in [1].

Thanks,

	M.

[1] https://lore.kernel.org/r/20210510173129.750496-1-jean-philippe@linaro.org
diff mbox series

Patch

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d9c73c173c14..217dc9f0231f 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -871,8 +871,15 @@  static int pci_msi_supported(struct pci_dev *dev, int nvec)
 	 * Any bridge which does NOT route MSI transactions from its
 	 * secondary bus to its primary bus must set NO_MSI flag on
 	 * the secondary pci_bus.
-	 * We expect only arch-specific PCI host bus controller driver
-	 * or quirks for specific PCI bridges to be setting NO_MSI.
+	 *
+	 * The NO_MSI flag can either be set directly by:
+	 * - arch-specific PCI host bus controller drivers (deprecated)
+	 * - quirks for specific PCI bridges
+	 *
+	 * or indirectly by platform-specific PCI host bridge drivers by
+	 * advertising the 'msi_domain' property, which results in
+	 * the NO_MSI flag when no MSI domain is found for this bridge
+	 * at probe time.
 	 */
 	for (bus = dev->bus; bus; bus = bus->parent)
 		if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)