Message ID | 20240415170113.662318-4-sunilvl@ventanamicro.com |
---|---|
State | New |
Headers | show |
Series | RISC-V: ACPI: Add external interrupt controller support | expand |
On Mon, Apr 15, 2024 at 10:30:56PM +0530, Sunil V L wrote: > Similar to commit 9ec37efb8783 ("PCI/MSI: Make > pci_host_common_probe() declare its reliance on MSI domains"), declare > this dependency for PCI probe in ACPI based flow. > > This is required especially for RISC-V platforms where MSI controller > can be absent. However, setting this for all architectures seem to cause > issues on non RISC-V architectures [1]. Hence, enabled this only for > RISC-V. > > [1] - https://lore.kernel.org/oe-lkp/202403041047.791cb18e-oliver.sang@intel.com > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > --- > drivers/pci/probe.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 1325fbae2f28..e09915bee2ee 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -3048,6 +3048,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > bridge->sysdata = sysdata; > bridge->busnr = bus; > bridge->ops = ops; > +#ifdef CONFIG_RISCV > + bridge->msi_domain = true; > +#endif Ugh. I looked at [1], but that's not a very good justification for this #ifdef. The fault mentioned in [1] would need to be fixed, but not this way. > error = pci_register_host_bridge(bridge); > if (error < 0) > -- > 2.40.1 >
Hi Bjorn, On Mon, Apr 15, 2024 at 06:15:23PM -0500, Bjorn Helgaas wrote: > On Mon, Apr 15, 2024 at 10:30:56PM +0530, Sunil V L wrote: > > Similar to commit 9ec37efb8783 ("PCI/MSI: Make > > pci_host_common_probe() declare its reliance on MSI domains"), declare > > this dependency for PCI probe in ACPI based flow. > > > > This is required especially for RISC-V platforms where MSI controller > > can be absent. However, setting this for all architectures seem to cause > > issues on non RISC-V architectures [1]. Hence, enabled this only for > > RISC-V. > > > > [1] - https://lore.kernel.org/oe-lkp/202403041047.791cb18e-oliver.sang@intel.com > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > --- > > drivers/pci/probe.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 1325fbae2f28..e09915bee2ee 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -3048,6 +3048,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, > > bridge->sysdata = sysdata; > > bridge->busnr = bus; > > bridge->ops = ops; > > +#ifdef CONFIG_RISCV > > + bridge->msi_domain = true; > > +#endif > > Ugh. I looked at [1], but that's not a very good justification for > this #ifdef. The fault mentioned in [1] would need to be fixed, but > not this way. > Thank you again for the feedback! I agree. This is due to my limitation with knowledge and resources to debug the issue happening on non-UEFI x86 system with some particular PCIe RC. Also, I was worried that we get into a rat hole of assumptions/quirks with various architecture/PCIe RC combinations. For ex: I think the issue is, somehow MSI domain is not set at the time of PCI host bridge registration in pci_register_host_bridge() causing PCI_BUS_FLAGS_NO_MSI to be set. This causes pci_alloc_irq_vectors() to fail. In portdrv.c, pcie_init_service_irqs() doesn't switch to INTx handling if MSI can not be used. It switches only if pcie_pme_no_msi() returns true. I couldn't find who actually sets up MSI domain bit late on this platform so that it somehow worked when we didn't set this flag. Unfortunately, I don't have system to root cause and fix this issue with confidence. Also, I don't know if any other architectures have similar issues which are not caught yet. Hence, I thought it may be better just restrict the change to RISC-V. Let me know your thoughts. If there are better ways, I will be happy to update. Thanks, Sunil
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 1325fbae2f28..e09915bee2ee 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -3048,6 +3048,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, bridge->sysdata = sysdata; bridge->busnr = bus; bridge->ops = ops; +#ifdef CONFIG_RISCV + bridge->msi_domain = true; +#endif error = pci_register_host_bridge(bridge); if (error < 0)
Similar to commit 9ec37efb8783 ("PCI/MSI: Make pci_host_common_probe() declare its reliance on MSI domains"), declare this dependency for PCI probe in ACPI based flow. This is required especially for RISC-V platforms where MSI controller can be absent. However, setting this for all architectures seem to cause issues on non RISC-V architectures [1]. Hence, enabled this only for RISC-V. [1] - https://lore.kernel.org/oe-lkp/202403041047.791cb18e-oliver.sang@intel.com Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> --- drivers/pci/probe.c | 3 +++ 1 file changed, 3 insertions(+)