| Message ID | 20210510173129.750496-1-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/MSI: Fix MSIs for generic hosts that use device-tree's "msi-map" | expand |
Hi Jean-Philippe, On Mon, 10 May 2021 18:31:30 +0100, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > Since commit 9ec37efb8783 ("PCI/MSI: Make pci_host_common_probe() > declare its reliance on MSI domains"), platforms that rely on the > "msi-map" device-tree property don't get MSIs anymore. > > On the Arm Fast Model for example [1], the host bridge doesn't have a > "msi-parent" property since it doesn't itself generate MSIs, and so > doesn't get a MSI domain. It has an "msi-map" property instead to > describe MSI controllers of child devices. As a result, due to the new > msi_domain check in pci_register_host_bridge(), the whole bus gets > PCI_BUS_FLAGS_NO_MSI. Urgh... I knew I was going to break something... :-( Thanks for picking this up. > Check whether the root complex has an "msi-map" property before giving > up on MSIs. > > [1] arch/arm64/boot/dts/arm/fvp-base-revc.dts > > Fixes: 9ec37efb8783 ("PCI/MSI: Make pci_host_common_probe() declare its reliance on MSI domains") > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > include/linux/pci.h | 2 ++ > drivers/pci/of.c | 7 +++++++ > drivers/pci/probe.c | 3 ++- > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index c20211e59a57..24306504226a 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2344,6 +2344,7 @@ int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off, > struct device_node; > struct irq_domain; > struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus); > +bool pci_host_of_has_msi_map(struct device *dev); > > /* Arch may override this (weak) */ > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus); > @@ -2351,6 +2352,7 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus); > #else /* CONFIG_OF */ > static inline struct irq_domain * > pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; } > +static inline bool pci_host_of_has_msi_map(struct device *dev) { return false; } > #endif /* CONFIG_OF */ > > static inline struct device_node * > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index da5b414d585a..85dcb7097da4 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -103,6 +103,13 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus) > #endif > } > > +bool pci_host_of_has_msi_map(struct device *dev) > +{ > + if (dev && dev->of_node) > + return of_get_property(dev->of_node, "msi-map", NULL); > + return false; > +} > + > static inline int __of_pci_pci_compare(struct device_node *node, > unsigned int data) > { > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 3a62d09b8869..275204646c68 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -925,7 +925,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) > device_enable_async_suspend(bus->bridge); > pci_set_bus_of_node(bus); > pci_set_bus_msi_domain(bus); > - if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev)) > + if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) && > + !pci_host_of_has_msi_map(parent)) > bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI; > > if (!parent) Do we need something similar for IORT, which implements a similar functionality to "msi-map"? My ACPI boxes seem to get their MSIs just fine though... Thanks, M.
Hi Marc, On Mon, May 10, 2021 at 07:23:14PM +0100, Marc Zyngier wrote: > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 3a62d09b8869..275204646c68 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -925,7 +925,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) > > device_enable_async_suspend(bus->bridge); > > pci_set_bus_of_node(bus); > > pci_set_bus_msi_domain(bus); > > - if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev)) > > + if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) && > > + !pci_host_of_has_msi_map(parent)) > > bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI; > > > > if (!parent) > > Do we need something similar for IORT, which implements a similar > functionality to "msi-map"? My ACPI boxes seem to get their MSIs just > fine though... I'm not seeing the issue either under ACPI on the fast model, because it doesn't go through pci_host_common_probe(), and bridge->msi_domain is not set: acpi_pci_root_add() pci_acpi_scan_root() acpi_pci_root_create() pci_create_root_bus() pci_register_host_bridge() It doesn't look like any ACPI platform can set bridge->msi_domain at the moment. If they do flip the switch at some point, MSIs won't work and we'll need something for IORT. But I'd leave it out for the moment. Thanks, Jean
On Tue, 11 May 2021 15:12:08 +0100, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > Hi Marc, > > On Mon, May 10, 2021 at 07:23:14PM +0100, Marc Zyngier wrote: > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > > index 3a62d09b8869..275204646c68 100644 > > > --- a/drivers/pci/probe.c > > > +++ b/drivers/pci/probe.c > > > @@ -925,7 +925,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) > > > device_enable_async_suspend(bus->bridge); > > > pci_set_bus_of_node(bus); > > > pci_set_bus_msi_domain(bus); > > > - if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev)) > > > + if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) && > > > + !pci_host_of_has_msi_map(parent)) > > > bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI; > > > > > > if (!parent) > > > > Do we need something similar for IORT, which implements a similar > > functionality to "msi-map"? My ACPI boxes seem to get their MSIs just > > fine though... > > I'm not seeing the issue either under ACPI on the fast model, because it > doesn't go through pci_host_common_probe(), and bridge->msi_domain is not > set: > > acpi_pci_root_add() > pci_acpi_scan_root() > acpi_pci_root_create() > pci_create_root_bus() > pci_register_host_bridge() > > It doesn't look like any ACPI platform can set bridge->msi_domain at the > moment. If they do flip the switch at some point, MSIs won't work and > we'll need something for IORT. But I'd leave it out for the moment. Fair enough, although there seem to be the opposite issue on some platforms that do not have any MSI to offer (see [1]). I guess we'll cross that bridge eventually. For this patch: Acked-by: Marc Zyngier <maz@kernel.org> M. [1] https://lore.kernel.org/r/b5a5a6d8-6ffc-8c5c-c5b1-fb4f5616069f@arm.com
On Mon, May 10, 2021 at 07:31:30PM +0200, Jean-Philippe Brucker wrote: > Since commit 9ec37efb8783 ("PCI/MSI: Make pci_host_common_probe() > declare its reliance on MSI domains"), platforms that rely on the > "msi-map" device-tree property don't get MSIs anymore. > > On the Arm Fast Model for example [1], the host bridge doesn't have a > "msi-parent" property since it doesn't itself generate MSIs, and so > doesn't get a MSI domain. It has an "msi-map" property instead to > describe MSI controllers of child devices. As a result, due to the new > msi_domain check in pci_register_host_bridge(), the whole bus gets > PCI_BUS_FLAGS_NO_MSI. > > Check whether the root complex has an "msi-map" property before giving > up on MSIs. > > [1] arch/arm64/boot/dts/arm/fvp-base-revc.dts > > Fixes: 9ec37efb8783 ("PCI/MSI: Make pci_host_common_probe() declare its reliance on MSI domains") > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Applied to for-linus for v5.13, since we merged 9ec37efb8783 for v5.13-rc1. Thanks! > --- > include/linux/pci.h | 2 ++ > drivers/pci/of.c | 7 +++++++ > drivers/pci/probe.c | 3 ++- > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index c20211e59a57..24306504226a 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2344,6 +2344,7 @@ int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off, > struct device_node; > struct irq_domain; > struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus); > +bool pci_host_of_has_msi_map(struct device *dev); > > /* Arch may override this (weak) */ > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus); > @@ -2351,6 +2352,7 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus); > #else /* CONFIG_OF */ > static inline struct irq_domain * > pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; } > +static inline bool pci_host_of_has_msi_map(struct device *dev) { return false; } > #endif /* CONFIG_OF */ > > static inline struct device_node * > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index da5b414d585a..85dcb7097da4 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -103,6 +103,13 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus) > #endif > } > > +bool pci_host_of_has_msi_map(struct device *dev) > +{ > + if (dev && dev->of_node) > + return of_get_property(dev->of_node, "msi-map", NULL); > + return false; > +} > + > static inline int __of_pci_pci_compare(struct device_node *node, > unsigned int data) > { > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 3a62d09b8869..275204646c68 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -925,7 +925,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) > device_enable_async_suspend(bus->bridge); > pci_set_bus_of_node(bus); > pci_set_bus_msi_domain(bus); > - if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev)) > + if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) && > + !pci_host_of_has_msi_map(parent)) > bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI; > > if (!parent) > -- > 2.31.1 >
diff --git a/include/linux/pci.h b/include/linux/pci.h index c20211e59a57..24306504226a 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2344,6 +2344,7 @@ int pci_vpd_find_info_keyword(const u8 *buf, unsigned int off, struct device_node; struct irq_domain; struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus); +bool pci_host_of_has_msi_map(struct device *dev); /* Arch may override this (weak) */ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus); @@ -2351,6 +2352,7 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus); #else /* CONFIG_OF */ static inline struct irq_domain * pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; } +static inline bool pci_host_of_has_msi_map(struct device *dev) { return false; } #endif /* CONFIG_OF */ static inline struct device_node * diff --git a/drivers/pci/of.c b/drivers/pci/of.c index da5b414d585a..85dcb7097da4 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -103,6 +103,13 @@ struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus) #endif } +bool pci_host_of_has_msi_map(struct device *dev) +{ + if (dev && dev->of_node) + return of_get_property(dev->of_node, "msi-map", NULL); + return false; +} + static inline int __of_pci_pci_compare(struct device_node *node, unsigned int data) { diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 3a62d09b8869..275204646c68 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -925,7 +925,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) device_enable_async_suspend(bus->bridge); pci_set_bus_of_node(bus); pci_set_bus_msi_domain(bus); - if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev)) + if (bridge->msi_domain && !dev_get_msi_domain(&bus->dev) && + !pci_host_of_has_msi_map(parent)) bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI; if (!parent)
Since commit 9ec37efb8783 ("PCI/MSI: Make pci_host_common_probe() declare its reliance on MSI domains"), platforms that rely on the "msi-map" device-tree property don't get MSIs anymore. On the Arm Fast Model for example [1], the host bridge doesn't have a "msi-parent" property since it doesn't itself generate MSIs, and so doesn't get a MSI domain. It has an "msi-map" property instead to describe MSI controllers of child devices. As a result, due to the new msi_domain check in pci_register_host_bridge(), the whole bus gets PCI_BUS_FLAGS_NO_MSI. Check whether the root complex has an "msi-map" property before giving up on MSIs. [1] arch/arm64/boot/dts/arm/fvp-base-revc.dts Fixes: 9ec37efb8783 ("PCI/MSI: Make pci_host_common_probe() declare its reliance on MSI domains") Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- include/linux/pci.h | 2 ++ drivers/pci/of.c | 7 +++++++ drivers/pci/probe.c | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-)