From: Thomas Gleixner <[email protected]>
The arch_.*_msi_irq[s] fallbacks are compiled in whether an architecture
requires them or not. Architectures which are fully utilizing hierarchical
irq domains should never call into that code.
It's not only architectures which depend on that by implementing one or
more of the weak functions, there is also a bunch of drivers which relies
on the weak functions which invoke msi_controller::setup_irq[s] and
msi_controller::teardown_irq.
Make the architectures and drivers which rely on them select them in Kconfig
and if not selected replace them by stub functions which emit a warning and
fail the PCI/MSI interrupt allocation.
Signed-off-by: Thomas Gleixner <[email protected]>
---
V2: Make the architectures (and drivers) which need the fallbacks select them
and not the other way round (Bjorn).
---
arch/ia64/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/x86/Kconfig | 1 +
drivers/pci/Kconfig | 3 +++
drivers/pci/controller/Kconfig | 3 +++
drivers/pci/msi.c | 3 ++-
include/linux/msi.h | 31 ++++++++++++++++++++++++++-----
10 files changed, 40 insertions(+), 6 deletions(-)
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -56,6 +56,7 @@ config IA64
select NEED_DMA_MAP_STATE
select NEED_SG_DMA_LENGTH
select NUMA if !FLATMEM
+ select PCI_MSI_ARCH_FALLBACKS
default y
help
The Itanium Processor Family is Intel's 64-bit successor to
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -86,6 +86,7 @@ config MIPS
select MODULES_USE_ELF_REL if MODULES
select MODULES_USE_ELF_RELA if MODULES && 64BIT
select PERF_USE_VMALLOC
+ select PCI_MSI_ARCH_FALLBACKS
select RTC_LIB
select SYSCTL_EXCEPTION_TRACE
select VIRT_TO_BUS
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -246,6 +246,7 @@ config PPC
select OLD_SIGACTION if PPC32
select OLD_SIGSUSPEND
select PCI_DOMAINS if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select PCI_SYSCALL if PCI
select PPC_DAWR if PPC64
select RTC_LIB
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -185,6 +185,7 @@ config S390
select OLD_SIGSUSPEND3
select PCI_DOMAINS if PCI
select PCI_MSI if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select SPARSE_IRQ
select SYSCTL_EXCEPTION_TRACE
select THREAD_INFO_IN_TASK
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -43,6 +43,7 @@ config SPARC
select GENERIC_STRNLEN_USER
select MODULES_USE_ELF_RELA
select PCI_SYSCALL if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select ODD_RT_SIGACTION
select OLD_SIGSUSPEND
select CPU_NO_EFFICIENT_FFS
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -225,6 +225,7 @@ config X86
select NEED_SG_DMA_LENGTH
select PCI_DOMAINS if PCI
select PCI_LOCKLESS_CONFIG if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select PERF_EVENTS
select RTC_LIB
select RTC_MC146818_LIB
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -56,6 +56,9 @@ config PCI_MSI_IRQ_DOMAIN
depends on PCI_MSI
select GENERIC_MSI_IRQ_DOMAIN
+config PCI_MSI_ARCH_FALLBACKS
+ bool
+
config PCI_QUIRKS
default y
bool "Enable PCI quirk workarounds" if EXPERT
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -41,6 +41,7 @@ config PCI_TEGRA
bool "NVIDIA Tegra PCIe controller"
depends on ARCH_TEGRA || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
+ select PCI_MSI_ARCH_FALLBACKS
help
Say Y here if you want support for the PCIe host controller found
on NVIDIA Tegra SoCs.
@@ -67,6 +68,7 @@ config PCIE_RCAR_HOST
bool "Renesas R-Car PCIe host controller"
depends on ARCH_RENESAS || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
+ select PCI_MSI_ARCH_FALLBACKS
help
Say Y here if you want PCIe controller support on R-Car SoCs in host
mode.
@@ -103,6 +105,7 @@ config PCIE_XILINX_CPM
bool "Xilinx Versal CPM host bridge support"
depends on ARCH_ZYNQMP || COMPILE_TEST
select PCI_HOST_COMMON
+ select PCI_MSI_ARCH_FALLBACKS
help
Say 'Y' here if you want kernel support for the
Xilinx Versal CPM host bridge.
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -58,8 +58,8 @@ static void pci_msi_teardown_msi_irqs(st
#define pci_msi_teardown_msi_irqs arch_teardown_msi_irqs
#endif
+#ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS
/* Arch hooks */
-
int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
{
struct msi_controller *chip = dev->bus->msi;
@@ -132,6 +132,7 @@ void __weak arch_teardown_msi_irqs(struc
{
return default_teardown_msi_irqs(dev);
}
+#endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */
static void default_restore_msi_irq(struct pci_dev *dev, int irq)
{
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -193,17 +193,38 @@ void pci_msi_mask_irq(struct irq_data *d
void pci_msi_unmask_irq(struct irq_data *data);
/*
- * The arch hooks to setup up msi irqs. Those functions are
- * implemented as weak symbols so that they /can/ be overriden by
- * architecture specific code if needed.
+ * The arch hooks to setup up msi irqs. Default functions are implemented
+ * as weak symbols so that they /can/ be overriden by architecture specific
+ * code if needed. These hooks must be enabled by the architecture or by
+ * drivers which depend on them via msi_controller based MSI handling.
+ *
+ * If CONFIG_PCI_MSI_ARCH_FALLBACKS is not selected they are replaced by
+ * stubs with warnings.
*/
+#ifdef CONFIG_PCI_MSI_DISABLE_ARCH_FALLBACKS
int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
void arch_teardown_msi_irq(unsigned int irq);
int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
void arch_teardown_msi_irqs(struct pci_dev *dev);
-void arch_restore_msi_irqs(struct pci_dev *dev);
-
void default_teardown_msi_irqs(struct pci_dev *dev);
+#else
+static inline int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+ WARN_ON_ONCE(1);
+ return -ENODEV;
+}
+
+static inline void arch_teardown_msi_irqs(struct pci_dev *dev)
+{
+ WARN_ON_ONCE(1);
+}
+#endif
+
+/*
+ * The restore hooks are still available as they are useful even
+ * for fully irq domain based setups. Courtesy to XEN/X86.
+ */
+void arch_restore_msi_irqs(struct pci_dev *dev);
void default_restore_msi_irqs(struct pci_dev *dev);
struct msi_controller {
On Wed, Aug 26 2020 at 13:17, Thomas Gleixner wrote:
> + * If CONFIG_PCI_MSI_ARCH_FALLBACKS is not selected they are replaced by
> + * stubs with warnings.
> */
> +#ifdef CONFIG_PCI_MSI_DISABLE_ARCH_FALLBACKS
Groan, I obviously failed to pull that back from the test box where I
fixed it. That wants to be:
+#ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS
Doing five things at once does not work well
On Wed, 26 Aug 2020 12:17:02 +0100,
Thomas Gleixner <[email protected]> wrote:
>
> From: Thomas Gleixner <[email protected]>
>
> The arch_.*_msi_irq[s] fallbacks are compiled in whether an architecture
> requires them or not. Architectures which are fully utilizing hierarchical
> irq domains should never call into that code.
>
> It's not only architectures which depend on that by implementing one or
> more of the weak functions, there is also a bunch of drivers which relies
> on the weak functions which invoke msi_controller::setup_irq[s] and
> msi_controller::teardown_irq.
>
> Make the architectures and drivers which rely on them select them in Kconfig
> and if not selected replace them by stub functions which emit a warning and
> fail the PCI/MSI interrupt allocation.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> V2: Make the architectures (and drivers) which need the fallbacks select them
> and not the other way round (Bjorn).
> ---
> arch/ia64/Kconfig | 1 +
> arch/mips/Kconfig | 1 +
> arch/powerpc/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/sparc/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> drivers/pci/Kconfig | 3 +++
> drivers/pci/controller/Kconfig | 3 +++
> drivers/pci/msi.c | 3 ++-
> include/linux/msi.h | 31 ++++++++++++++++++++++++++-----
> 10 files changed, 40 insertions(+), 6 deletions(-)
>
[...]
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -41,6 +41,7 @@ config PCI_TEGRA
> bool "NVIDIA Tegra PCIe controller"
> depends on ARCH_TEGRA || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> + select PCI_MSI_ARCH_FALLBACKS
> help
> Say Y here if you want support for the PCIe host controller found
> on NVIDIA Tegra SoCs.
> @@ -67,6 +68,7 @@ config PCIE_RCAR_HOST
> bool "Renesas R-Car PCIe host controller"
> depends on ARCH_RENESAS || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> + select PCI_MSI_ARCH_FALLBACKS
> help
> Say Y here if you want PCIe controller support on R-Car SoCs in host
> mode.
> @@ -103,6 +105,7 @@ config PCIE_XILINX_CPM
> bool "Xilinx Versal CPM host bridge support"
> depends on ARCH_ZYNQMP || COMPILE_TEST
> select PCI_HOST_COMMON
> + select PCI_MSI_ARCH_FALLBACKS
This guy actually doesn't implement MSIs at all (it seems to delegate
them to an ITS present in the system, if I read the DT binding
correctly). However its older brother from the same silicon dealer
seems to need it. The patchlet below should fix it.
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 9ad13919bcaa..f56ff049d469 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -96,6 +96,7 @@ config PCI_HOST_GENERIC
config PCIE_XILINX
bool "Xilinx AXI PCIe host bridge support"
+ select PCI_MSI_ARCH_FALLBACKS
depends on OF || COMPILE_TEST
help
Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
@@ -105,7 +106,6 @@ config PCIE_XILINX_CPM
bool "Xilinx Versal CPM host bridge support"
depends on ARCH_ZYNQMP || COMPILE_TEST
select PCI_HOST_COMMON
- select PCI_MSI_ARCH_FALLBACKS
help
Say 'Y' here if you want kernel support for the
Xilinx Versal CPM host bridge.
With that fixed,
Acked-by: Marc Zyngier <[email protected]>
M.
--
Without deviation from the norm, progress is not possible.
On Wed, Aug 26 2020 at 22:14, Marc Zyngier wrote:
> On Wed, 26 Aug 2020 12:17:02 +0100,
> Thomas Gleixner <[email protected]> wrote:
>> @@ -103,6 +105,7 @@ config PCIE_XILINX_CPM
>> bool "Xilinx Versal CPM host bridge support"
>> depends on ARCH_ZYNQMP || COMPILE_TEST
>> select PCI_HOST_COMMON
>> + select PCI_MSI_ARCH_FALLBACKS
>
> This guy actually doesn't implement MSIs at all (it seems to delegate
> them to an ITS present in the system, if I read the DT binding
> correctly). However its older brother from the same silicon dealer
> seems to need it. The patchlet below should fix it.
Gah, at some point my eyes went squared and I lost track..
[+cc Rob,
cover https://lore.kernel.org/r/[email protected]/
this https://lore.kernel.org/r/[email protected]/]
On Wed, Aug 26, 2020 at 01:17:02PM +0200, Thomas Gleixner wrote:
> From: Thomas Gleixner <[email protected]>
>
> The arch_.*_msi_irq[s] fallbacks are compiled in whether an architecture
> requires them or not. Architectures which are fully utilizing hierarchical
> irq domains should never call into that code.
>
> It's not only architectures which depend on that by implementing one or
> more of the weak functions, there is also a bunch of drivers which relies
> on the weak functions which invoke msi_controller::setup_irq[s] and
> msi_controller::teardown_irq.
>
> Make the architectures and drivers which rely on them select them in Kconfig
> and if not selected replace them by stub functions which emit a warning and
> fail the PCI/MSI interrupt allocation.
Sorry, I really don't understand this, so these are probably stupid
questions.
If CONFIG_PCI_MSI_ARCH_FALLBACKS is defined, we will supply
implementations of:
arch_setup_msi_irq
arch_teardown_msi_irq
arch_setup_msi_irqs
arch_teardown_msi_irqs
default_teardown_msi_irqs # non-weak
You select CONFIG_PCI_MSI_ARCH_FALLBACKS for ia64, mips, powerpc,
s390, sparc, and x86. I see that all of those arches implement at
least one of the functions above. But x86 doesn't and I can't figure
out why it needs to select CONFIG_PCI_MSI_ARCH_FALLBACKS.
I assume there's a way to convert these arches to hierarchical irq
domains so they wouldn't need this at all? Is there a sample
conversion to look at?
And I can't figure out what's special about tegra, rcar, and xilinx
that makes them need it as well. Is there something I could grep for
to identify them? Is there a way to convert them so they don't need
it?
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -193,17 +193,38 @@ void pci_msi_mask_irq(struct irq_data *d
> void pci_msi_unmask_irq(struct irq_data *data);
>
> /*
> - * The arch hooks to setup up msi irqs. Those functions are
> - * implemented as weak symbols so that they /can/ be overriden by
> - * architecture specific code if needed.
> + * The arch hooks to setup up msi irqs. Default functions are implemented
s/msi/MSI/ to match the one below.
> + * as weak symbols so that they /can/ be overriden by architecture specific
> + * code if needed. These hooks must be enabled by the architecture or by
> + * drivers which depend on them via msi_controller based MSI handling.
On Thu, Aug 27, 2020 at 01:20:40PM -0500, Bjorn Helgaas wrote:
[...]
> And I can't figure out what's special about tegra, rcar, and xilinx
> that makes them need it as well. Is there something I could grep for
> to identify them? Is there a way to convert them so they don't need
> it?
I think DT binding and related firmware support are needed to setup the
MSI IRQ domains correctly, there is nothing special about tegra, rcar
and xilinx AFAIK (well, all native host controllers MSI handling is
*special* just to be polite but let's gloss over this for the time
being).
struct msi_controller, to answer the first question.
I have doubts about pci_mvebu too, they do allocate an msi_controller
but without methods so it looks pretty much useless.
Hyper-V code too seems questionable, maybe there is room for more
clean-ups.
Lorenzo
> > --- a/include/linux/msi.h
> > +++ b/include/linux/msi.h
> > @@ -193,17 +193,38 @@ void pci_msi_mask_irq(struct irq_data *d
> > void pci_msi_unmask_irq(struct irq_data *data);
> >
> > /*
> > - * The arch hooks to setup up msi irqs. Those functions are
> > - * implemented as weak symbols so that they /can/ be overriden by
> > - * architecture specific code if needed.
> > + * The arch hooks to setup up msi irqs. Default functions are implemented
>
> s/msi/MSI/ to match the one below.
>
> > + * as weak symbols so that they /can/ be overriden by architecture specific
> > + * code if needed. These hooks must be enabled by the architecture or by
> > + * drivers which depend on them via msi_controller based MSI handling.
On Fri, Aug 28, 2020 at 12:21:42PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Aug 27, 2020 at 01:20:40PM -0500, Bjorn Helgaas wrote:
>
> [...]
>
> > And I can't figure out what's special about tegra, rcar, and xilinx
> > that makes them need it as well. Is there something I could grep for
> > to identify them? Is there a way to convert them so they don't need
> > it?
>
> I think DT binding and related firmware support are needed to setup the
> MSI IRQ domains correctly, there is nothing special about tegra, rcar
> and xilinx AFAIK (well, all native host controllers MSI handling is
> *special* just to be polite but let's gloss over this for the time
> being).
>
> struct msi_controller, to answer the first question.
>
> I have doubts about pci_mvebu too, they do allocate an msi_controller
> but without methods so it looks pretty much useless.
Oh, I did once know things about mvebu..
I suspect the msi controller pointer assignment is dead code at this
point. The only implementation of MSI with that PCI root port is
drivers/irqchip/irq-armada-370-xp.c which looks like it uses
irq_domain.
Actually looks like things are very close to eliminating
msi_controller.
This is dead code, can't find a setter for hw_pci->msi_ctrl:
arch/arm/include/asm/mach/pci.h: struct msi_controller *msi_ctrl;
arch/arm/kernel/bios32.c: bridge->msi = hw->msi_ctrl;
This is probably just copying NULL from one place to another:
drivers/pci/controller/pci-mvebu.c: struct msi_controller *msi;
These need conversion to irq_domain (right?):
drivers/pci/controller/pci-hyperv.c: struct msi_controller msi_chip;
drivers/pci/controller/pci-tegra.c: struct msi_controller chip;
drivers/pci/controller/pcie-rcar-host.c: struct msi_controller chip;
drivers/pci/controller/pcie-xilinx.c:static struct msi_controller xilinx_pcie_msi_chip = {
Then the stuff in drivers/pci/msi.c can go away.
So the arch_setup_msi_irq/etc is not really an arch hook, but some
infrastructure to support those 4 PCI root port drivers.
Jason
Hi Jason,
On 2020-08-28 13:19, Jason Gunthorpe wrote:
> On Fri, Aug 28, 2020 at 12:21:42PM +0100, Lorenzo Pieralisi wrote:
>> On Thu, Aug 27, 2020 at 01:20:40PM -0500, Bjorn Helgaas wrote:
>>
>> [...]
>>
>> > And I can't figure out what's special about tegra, rcar, and xilinx
>> > that makes them need it as well. Is there something I could grep for
>> > to identify them? Is there a way to convert them so they don't need
>> > it?
>>
>> I think DT binding and related firmware support are needed to setup
>> the
>> MSI IRQ domains correctly, there is nothing special about tegra, rcar
>> and xilinx AFAIK (well, all native host controllers MSI handling is
>> *special* just to be polite but let's gloss over this for the time
>> being).
>>
>> struct msi_controller, to answer the first question.
>>
>> I have doubts about pci_mvebu too, they do allocate an msi_controller
>> but without methods so it looks pretty much useless.
>
> Oh, I did once know things about mvebu..
>
> I suspect the msi controller pointer assignment is dead code at this
> point. The only implementation of MSI with that PCI root port is
> drivers/irqchip/irq-armada-370-xp.c which looks like it uses
> irq_domain.
>
> Actually looks like things are very close to eliminating
> msi_controller.
>
> This is dead code, can't find a setter for hw_pci->msi_ctrl:
>
> arch/arm/include/asm/mach/pci.h: struct msi_controller
> *msi_ctrl;
> arch/arm/kernel/bios32.c: bridge->msi =
> hw->msi_ctrl;
>
> This is probably just copying NULL from one place to another:
>
> drivers/pci/controller/pci-mvebu.c: struct msi_controller *msi;
>
> These need conversion to irq_domain (right?):
>
> drivers/pci/controller/pci-hyperv.c: struct msi_controller msi_chip;
> drivers/pci/controller/pci-tegra.c: struct msi_controller chip;
> drivers/pci/controller/pcie-rcar-host.c: struct msi_controller
> chip;
> drivers/pci/controller/pcie-xilinx.c:static struct msi_controller
> xilinx_pcie_msi_chip = {
>
> Then the stuff in drivers/pci/msi.c can go away.
>
> So the arch_setup_msi_irq/etc is not really an arch hook, but some
> infrastructure to support those 4 PCI root port drivers.
I happen to have a *really old* patch addressing Tegra [1], which
I was never able to test (no HW). Rebasing it shouldn't be too hard,
and maybe you can find someone internally willing to give it a spin?
That'd be a good start towards the removal of this cruft.
Thanks,
M.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/kill-msi-controller&id=83b3602fcee7972b9d549ed729b56ec28de16081
--
Jazz is not dead. It just smells funny...
On Fri, Aug 28, 2020 at 01:47:59PM +0100, Marc Zyngier wrote:
> > So the arch_setup_msi_irq/etc is not really an arch hook, but some
> > infrastructure to support those 4 PCI root port drivers.
>
> I happen to have a *really old* patch addressing Tegra [1], which
> I was never able to test (no HW). Rebasing it shouldn't be too hard,
> and maybe you can find someone internally willing to give it a spin?
Sure, that helps a bunch, I will ask internally if someone in that BU
can take a look.
Thanks,
Jason
On 2020-08-28 13:54, Jason Gunthorpe wrote:
> On Fri, Aug 28, 2020 at 01:47:59PM +0100, Marc Zyngier wrote:
>
>> > So the arch_setup_msi_irq/etc is not really an arch hook, but some
>> > infrastructure to support those 4 PCI root port drivers.
>>
>> I happen to have a *really old* patch addressing Tegra [1], which
>> I was never able to test (no HW). Rebasing it shouldn't be too hard,
>> and maybe you can find someone internally willing to give it a spin?
>
> Sure, that helps a bunch, I will ask internally if someone in that BU
> can take a look.
I just rebased[1] the patch onto -rc2, and provided a TODO list in the
commit message.
Thanks,
M.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/tegra-msi
--
Jazz is not dead. It just smells funny...
On Thu, Aug 27 2020 at 13:20, Bjorn Helgaas wrote:
> On Wed, Aug 26, 2020 at 01:17:02PM +0200, Thomas Gleixner wrote:
>> Make the architectures and drivers which rely on them select them in Kconfig
>> and if not selected replace them by stub functions which emit a warning and
>> fail the PCI/MSI interrupt allocation.
>
> Sorry, I really don't understand this, so these are probably stupid
> questions.
>
> If CONFIG_PCI_MSI_ARCH_FALLBACKS is defined, we will supply
> implementations of:
>
> arch_setup_msi_irq
> arch_teardown_msi_irq
> arch_setup_msi_irqs
> arch_teardown_msi_irqs
> default_teardown_msi_irqs # non-weak
>
> You select CONFIG_PCI_MSI_ARCH_FALLBACKS for ia64, mips, powerpc,
> s390, sparc, and x86. I see that all of those arches implement at
> least one of the functions above. But x86 doesn't and I can't figure
> out why it needs to select CONFIG_PCI_MSI_ARCH_FALLBACKS.
X86 still has them at that point in the series and the next patch
removes them. I wanted to have the warnings in place before doing so.
> I assume there's a way to convert these arches to hierarchical irq
> domains so they wouldn't need this at all? Is there a sample
> conversion to look at?
For a quick and dirty step it's pretty much the wrapper I used for XEN
and then make sure that the msi_domain pointer is populated is
pci_device::device.
> And I can't figure out what's special about tegra, rcar, and xilinx
> that makes them need it as well.
Those are old drivers from the time where ARM did not use hierarchical
irq domains and nobody cared to fix them up.
> Is there something I could grep for
> to identify them?
git grep arch_setup_msi_irq
git grep arch_teardown_msi_irq
> Is there a way to convert them so they don't need it?
Sure, it just needs some work and probably hardware to test.
Thanks,
tglx
The following commit has been merged into the x86/irq branch of tip:
Commit-ID: 077ee78e392869e46ae6bdc6ba2a3c4249d0b5e1
Gitweb: https://git.kernel.org/tip/077ee78e392869e46ae6bdc6ba2a3c4249d0b5e1
Author: Thomas Gleixner <[email protected]>
AuthorDate: Wed, 26 Aug 2020 13:17:02 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 16 Sep 2020 16:52:37 +02:00
PCI/MSI: Make arch_.*_msi_irq[s] fallbacks selectable
The arch_.*_msi_irq[s] fallbacks are compiled in whether an architecture
requires them or not. Architectures which are fully utilizing hierarchical
irq domains should never call into that code.
It's not only architectures which depend on that by implementing one or
more of the weak functions, there is also a bunch of drivers which relies
on the weak functions which invoke msi_controller::setup_irq[s] and
msi_controller::teardown_irq.
Make the architectures and drivers which rely on them select them in Kconfig
and if not selected replace them by stub functions which emit a warning and
fail the PCI/MSI interrupt allocation.
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/ia64/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/x86/Kconfig | 1 +
drivers/pci/Kconfig | 3 +++
drivers/pci/controller/Kconfig | 3 +++
drivers/pci/msi.c | 3 ++-
include/linux/msi.h | 31 ++++++++++++++++++++++++++-----
10 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 5b4ec80..7ff5b3b 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -56,6 +56,7 @@ config IA64
select NEED_DMA_MAP_STATE
select NEED_SG_DMA_LENGTH
select NUMA if !FLATMEM
+ select PCI_MSI_ARCH_FALLBACKS
default y
help
The Itanium Processor Family is Intel's 64-bit successor to
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index c95fa3a..3690582 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -86,6 +86,7 @@ config MIPS
select MODULES_USE_ELF_REL if MODULES
select MODULES_USE_ELF_RELA if MODULES && 64BIT
select PERF_USE_VMALLOC
+ select PCI_MSI_ARCH_FALLBACKS
select RTC_LIB
select SYSCTL_EXCEPTION_TRACE
select VIRT_TO_BUS
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 65bed1f..9e66ca1 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -246,6 +246,7 @@ config PPC
select OLD_SIGACTION if PPC32
select OLD_SIGSUSPEND
select PCI_DOMAINS if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select PCI_SYSCALL if PCI
select PPC_DAWR if PPC64
select RTC_LIB
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b29fcc6..63dd5a0 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -185,6 +185,7 @@ config S390
select OLD_SIGSUSPEND3
select PCI_DOMAINS if PCI
select PCI_MSI if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select SPARSE_IRQ
select SYSCTL_EXCEPTION_TRACE
select THREAD_INFO_IN_TASK
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index efeff2c..21a3239 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -43,6 +43,7 @@ config SPARC
select GENERIC_STRNLEN_USER
select MODULES_USE_ELF_RELA
select PCI_SYSCALL if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select ODD_RT_SIGACTION
select OLD_SIGSUSPEND
select CPU_NO_EFFICIENT_FFS
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7101ac6..196e068 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -225,6 +225,7 @@ config X86
select NEED_SG_DMA_LENGTH
select PCI_DOMAINS if PCI
select PCI_LOCKLESS_CONFIG if PCI
+ select PCI_MSI_ARCH_FALLBACKS
select PERF_EVENTS
select RTC_LIB
select RTC_MC146818_LIB
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 4bef5c2..438a792 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -56,6 +56,9 @@ config PCI_MSI_IRQ_DOMAIN
depends on PCI_MSI
select GENERIC_MSI_IRQ_DOMAIN
+config PCI_MSI_ARCH_FALLBACKS
+ bool
+
config PCI_QUIRKS
default y
bool "Enable PCI quirk workarounds" if EXPERT
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index f18c372..4a7afbe 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -41,6 +41,7 @@ config PCI_TEGRA
bool "NVIDIA Tegra PCIe controller"
depends on ARCH_TEGRA || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
+ select PCI_MSI_ARCH_FALLBACKS
help
Say Y here if you want support for the PCIe host controller found
on NVIDIA Tegra SoCs.
@@ -67,6 +68,7 @@ config PCIE_RCAR_HOST
bool "Renesas R-Car PCIe host controller"
depends on ARCH_RENESAS || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
+ select PCI_MSI_ARCH_FALLBACKS
help
Say Y here if you want PCIe controller support on R-Car SoCs in host
mode.
@@ -95,6 +97,7 @@ config PCI_HOST_GENERIC
config PCIE_XILINX
bool "Xilinx AXI PCIe host bridge support"
depends on OF || COMPILE_TEST
+ select PCI_MSI_ARCH_FALLBACKS
help
Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
Host Bridge driver.
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index a2f00d1..d52d118 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -58,8 +58,8 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)
#define pci_msi_teardown_msi_irqs arch_teardown_msi_irqs
#endif
+#ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS
/* Arch hooks */
-
int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
{
struct msi_controller *chip = dev->bus->msi;
@@ -132,6 +132,7 @@ void __weak arch_teardown_msi_irqs(struct pci_dev *dev)
{
return default_teardown_msi_irqs(dev);
}
+#endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */
static void default_restore_msi_irq(struct pci_dev *dev, int irq)
{
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 0180534..6b584cc 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -193,17 +193,38 @@ void pci_msi_mask_irq(struct irq_data *data);
void pci_msi_unmask_irq(struct irq_data *data);
/*
- * The arch hooks to setup up msi irqs. Those functions are
- * implemented as weak symbols so that they /can/ be overriden by
- * architecture specific code if needed.
+ * The arch hooks to setup up msi irqs. Default functions are implemented
+ * as weak symbols so that they /can/ be overriden by architecture specific
+ * code if needed. These hooks must be enabled by the architecture or by
+ * drivers which depend on them via msi_controller based MSI handling.
+ *
+ * If CONFIG_PCI_MSI_ARCH_FALLBACKS is not selected they are replaced by
+ * stubs with warnings.
*/
+#ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS
int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
void arch_teardown_msi_irq(unsigned int irq);
int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
void arch_teardown_msi_irqs(struct pci_dev *dev);
-void arch_restore_msi_irqs(struct pci_dev *dev);
-
void default_teardown_msi_irqs(struct pci_dev *dev);
+#else
+static inline int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+ WARN_ON_ONCE(1);
+ return -ENODEV;
+}
+
+static inline void arch_teardown_msi_irqs(struct pci_dev *dev)
+{
+ WARN_ON_ONCE(1);
+}
+#endif
+
+/*
+ * The restore hooks are still available as they are useful even
+ * for fully irq domain based setups. Courtesy to XEN/X86.
+ */
+void arch_restore_msi_irqs(struct pci_dev *dev);
void default_restore_msi_irqs(struct pci_dev *dev);
struct msi_controller {
On Wed, 2020-08-26 at 13:17 +0200, Thomas Gleixner wrote:
> From: Thomas Gleixner <[email protected]>
>
> The arch_.*_msi_irq[s] fallbacks are compiled in whether an architecture
> requires them or not. Architectures which are fully utilizing hierarchical
> irq domains should never call into that code.
>
> It's not only architectures which depend on that by implementing one or
> more of the weak functions, there is also a bunch of drivers which relies
> on the weak functions which invoke msi_controller::setup_irq[s] and
> msi_controller::teardown_irq.
>
> Make the architectures and drivers which rely on them select them in Kconfig
> and if not selected replace them by stub functions which emit a warning and
> fail the PCI/MSI interrupt allocation.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
Today's linux-next will have some warnings on s390x:
.config: https://gitlab.com/cailca/linux-mm/-/blob/master/s390.config
WARNING: unmet direct dependencies detected for PCI_MSI_ARCH_FALLBACKS
Depends on [n]: PCI [=n]
Selected by [y]:
- S390 [=y]
WARNING: unmet direct dependencies detected for PCI_MSI_ARCH_FALLBACKS
Depends on [n]: PCI [=n]
Selected by [y]:
- S390 [=y]
> ---
> V2: Make the architectures (and drivers) which need the fallbacks select them
> and not the other way round (Bjorn).
> ---
> arch/ia64/Kconfig | 1 +
> arch/mips/Kconfig | 1 +
> arch/powerpc/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/sparc/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> drivers/pci/Kconfig | 3 +++
> drivers/pci/controller/Kconfig | 3 +++
> drivers/pci/msi.c | 3 ++-
> include/linux/msi.h | 31 ++++++++++++++++++++++++++-----
> 10 files changed, 40 insertions(+), 6 deletions(-)
>
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -56,6 +56,7 @@ config IA64
> select NEED_DMA_MAP_STATE
> select NEED_SG_DMA_LENGTH
> select NUMA if !FLATMEM
> + select PCI_MSI_ARCH_FALLBACKS
> default y
> help
> The Itanium Processor Family is Intel's 64-bit successor to
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -86,6 +86,7 @@ config MIPS
> select MODULES_USE_ELF_REL if MODULES
> select MODULES_USE_ELF_RELA if MODULES && 64BIT
> select PERF_USE_VMALLOC
> + select PCI_MSI_ARCH_FALLBACKS
> select RTC_LIB
> select SYSCTL_EXCEPTION_TRACE
> select VIRT_TO_BUS
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -246,6 +246,7 @@ config PPC
> select OLD_SIGACTION if PPC32
> select OLD_SIGSUSPEND
> select PCI_DOMAINS if PCI
> + select PCI_MSI_ARCH_FALLBACKS
> select PCI_SYSCALL if PCI
> select PPC_DAWR if PPC64
> select RTC_LIB
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -185,6 +185,7 @@ config S390
> select OLD_SIGSUSPEND3
> select PCI_DOMAINS if PCI
> select PCI_MSI if PCI
> + select PCI_MSI_ARCH_FALLBACKS
> select SPARSE_IRQ
> select SYSCTL_EXCEPTION_TRACE
> select THREAD_INFO_IN_TASK
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -43,6 +43,7 @@ config SPARC
> select GENERIC_STRNLEN_USER
> select MODULES_USE_ELF_RELA
> select PCI_SYSCALL if PCI
> + select PCI_MSI_ARCH_FALLBACKS
> select ODD_RT_SIGACTION
> select OLD_SIGSUSPEND
> select CPU_NO_EFFICIENT_FFS
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -225,6 +225,7 @@ config X86
> select NEED_SG_DMA_LENGTH
> select PCI_DOMAINS if PCI
> select PCI_LOCKLESS_CONFIG if PCI
> + select PCI_MSI_ARCH_FALLBACKS
> select PERF_EVENTS
> select RTC_LIB
> select RTC_MC146818_LIB
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -56,6 +56,9 @@ config PCI_MSI_IRQ_DOMAIN
> depends on PCI_MSI
> select GENERIC_MSI_IRQ_DOMAIN
>
> +config PCI_MSI_ARCH_FALLBACKS
> + bool
> +
> config PCI_QUIRKS
> default y
> bool "Enable PCI quirk workarounds" if EXPERT
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -41,6 +41,7 @@ config PCI_TEGRA
> bool "NVIDIA Tegra PCIe controller"
> depends on ARCH_TEGRA || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> + select PCI_MSI_ARCH_FALLBACKS
> help
> Say Y here if you want support for the PCIe host controller found
> on NVIDIA Tegra SoCs.
> @@ -67,6 +68,7 @@ config PCIE_RCAR_HOST
> bool "Renesas R-Car PCIe host controller"
> depends on ARCH_RENESAS || COMPILE_TEST
> depends on PCI_MSI_IRQ_DOMAIN
> + select PCI_MSI_ARCH_FALLBACKS
> help
> Say Y here if you want PCIe controller support on R-Car SoCs in host
> mode.
> @@ -103,6 +105,7 @@ config PCIE_XILINX_CPM
> bool "Xilinx Versal CPM host bridge support"
> depends on ARCH_ZYNQMP || COMPILE_TEST
> select PCI_HOST_COMMON
> + select PCI_MSI_ARCH_FALLBACKS
> help
> Say 'Y' here if you want kernel support for the
> Xilinx Versal CPM host bridge.
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -58,8 +58,8 @@ static void pci_msi_teardown_msi_irqs(st
> #define pci_msi_teardown_msi_irqs arch_teardown_msi_irqs
> #endif
>
> +#ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS
> /* Arch hooks */
> -
> int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
> {
> struct msi_controller *chip = dev->bus->msi;
> @@ -132,6 +132,7 @@ void __weak arch_teardown_msi_irqs(struc
> {
> return default_teardown_msi_irqs(dev);
> }
> +#endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */
>
> static void default_restore_msi_irq(struct pci_dev *dev, int irq)
> {
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -193,17 +193,38 @@ void pci_msi_mask_irq(struct irq_data *d
> void pci_msi_unmask_irq(struct irq_data *data);
>
> /*
> - * The arch hooks to setup up msi irqs. Those functions are
> - * implemented as weak symbols so that they /can/ be overriden by
> - * architecture specific code if needed.
> + * The arch hooks to setup up msi irqs. Default functions are implemented
> + * as weak symbols so that they /can/ be overriden by architecture specific
> + * code if needed. These hooks must be enabled by the architecture or by
> + * drivers which depend on them via msi_controller based MSI handling.
> + *
> + * If CONFIG_PCI_MSI_ARCH_FALLBACKS is not selected they are replaced by
> + * stubs with warnings.
> */
> +#ifdef CONFIG_PCI_MSI_DISABLE_ARCH_FALLBACKS
> int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
> void arch_teardown_msi_irq(unsigned int irq);
> int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
> void arch_teardown_msi_irqs(struct pci_dev *dev);
> -void arch_restore_msi_irqs(struct pci_dev *dev);
> -
> void default_teardown_msi_irqs(struct pci_dev *dev);
> +#else
> +static inline int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int
> type)
> +{
> + WARN_ON_ONCE(1);
> + return -ENODEV;
> +}
> +
> +static inline void arch_teardown_msi_irqs(struct pci_dev *dev)
> +{
> + WARN_ON_ONCE(1);
> +}
> +#endif
> +
> +/*
> + * The restore hooks are still available as they are useful even
> + * for fully irq domain based setups. Courtesy to XEN/X86.
> + */
> +void arch_restore_msi_irqs(struct pci_dev *dev);
> void default_restore_msi_irqs(struct pci_dev *dev);
>
> struct msi_controller {
>
On Fri, Sep 25, 2020 at 09:54:52AM -0400, Qian Cai wrote:
> On Wed, 2020-08-26 at 13:17 +0200, Thomas Gleixner wrote:
> > From: Thomas Gleixner <[email protected]>
> >
> > The arch_.*_msi_irq[s] fallbacks are compiled in whether an architecture
> > requires them or not. Architectures which are fully utilizing hierarchical
> > irq domains should never call into that code.
> >
> > It's not only architectures which depend on that by implementing one or
> > more of the weak functions, there is also a bunch of drivers which relies
> > on the weak functions which invoke msi_controller::setup_irq[s] and
> > msi_controller::teardown_irq.
> >
> > Make the architectures and drivers which rely on them select them in Kconfig
> > and if not selected replace them by stub functions which emit a warning and
> > fail the PCI/MSI interrupt allocation.
> >
> > Signed-off-by: Thomas Gleixner <[email protected]>
>
> Today's linux-next will have some warnings on s390x:
>
> .config: https://gitlab.com/cailca/linux-mm/-/blob/master/s390.config
>
> WARNING: unmet direct dependencies detected for PCI_MSI_ARCH_FALLBACKS
> Depends on [n]: PCI [=n]
> Selected by [y]:
> - S390 [=y]
>
> WARNING: unmet direct dependencies detected for PCI_MSI_ARCH_FALLBACKS
> Depends on [n]: PCI [=n]
> Selected by [y]:
> - S390 [=y]
>
Yes, as well as on mips and sparc which also don't FORCE_PCI.
This seems to work for s390:
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b0b7acf07eb8..41136fbe909b 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -192,3 +192,3 @@ config S390
select PCI_MSI if PCI
- select PCI_MSI_ARCH_FALLBACKS
+ select PCI_MSI_ARCH_FALLBACKS if PCI
select SET_FS
On Sat, Sep 26 2020 at 14:38, Vasily Gorbik wrote:
> On Fri, Sep 25, 2020 at 09:54:52AM -0400, Qian Cai wrote:
> Yes, as well as on mips and sparc which also don't FORCE_PCI.
> This seems to work for s390:
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0b7acf07eb8..41136fbe909b 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -192,3 +192,3 @@ config S390
> select PCI_MSI if PCI
> - select PCI_MSI_ARCH_FALLBACKS
> + select PCI_MSI_ARCH_FALLBACKS if PCI
> select SET_FS
lemme fix that for all of them ...