| Message ID | 20211116134022.420412-1-clg@kaod.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/xive: Change IRQ domain to a tree domain | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
On Tue, 16 Nov 2021 14:40:22 +0100 Cédric Le Goater <clg@kaod.org> wrote: > Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains > exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the > 'nomap' domains still in use under the powerpc arch. With this new > flag, the revmap_tree of the IRQ domain is not used anymore. This > change broke the support of shared LSIs [1] in the XIVE driver because > it was relying on a lookup in the revmap_tree to query previously > mapped interrupts. Linux now creates two distinct IRQ mappings on the > same HW IRQ which can lead to unexpected behavior in the drivers. > > The XIVE IRQ domain is not a direct mapping domain and its HW IRQ > interrupt number space is rather large : 1M/socket on POWER9 and > POWER10, change the XIVE driver to use a 'tree' domain type instead. > > [1] For instance, a linux KVM guest with virtio-rng and virtio-balloon > devices. > > Cc: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # v5.14+ > Fixes: 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive") > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > Tested-by: Greg Kurz <groug@kaod.org> with a KVM guest + virtio-rng + virtio-balloon on a POWER9 host. > Marc, > > The Fixes tag is there because the patch in question revealed that > something was broken in XIVE. genirq is not in cause. However, I > don't know for PS3 and Cell. May be less critical for now. > > arch/powerpc/sysdev/xive/common.c | 3 +-- > arch/powerpc/sysdev/xive/Kconfig | 1 - > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c > index fed6fd16c8f4..9d0f0fe25598 100644 > --- a/arch/powerpc/sysdev/xive/common.c > +++ b/arch/powerpc/sysdev/xive/common.c > @@ -1536,8 +1536,7 @@ static const struct irq_domain_ops xive_irq_domain_ops = { > > static void __init xive_init_host(struct device_node *np) > { > - xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, > - &xive_irq_domain_ops, NULL); > + xive_irq_domain = irq_domain_add_tree(np, &xive_irq_domain_ops, NULL); > if (WARN_ON(xive_irq_domain == NULL)) > return; > irq_set_default_host(xive_irq_domain); > diff --git a/arch/powerpc/sysdev/xive/Kconfig b/arch/powerpc/sysdev/xive/Kconfig > index 97796c6b63f0..785c292d104b 100644 > --- a/arch/powerpc/sysdev/xive/Kconfig > +++ b/arch/powerpc/sysdev/xive/Kconfig > @@ -3,7 +3,6 @@ config PPC_XIVE > bool > select PPC_SMP_MUXED_IPI > select HARDIRQS_SW_RESEND > - select IRQ_DOMAIN_NOMAP > > config PPC_XIVE_NATIVE > bool
On 11/16/21 15:23, Greg Kurz wrote: > On Tue, 16 Nov 2021 14:40:22 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains >> exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the >> 'nomap' domains still in use under the powerpc arch. With this new >> flag, the revmap_tree of the IRQ domain is not used anymore. This >> change broke the support of shared LSIs [1] in the XIVE driver because >> it was relying on a lookup in the revmap_tree to query previously >> mapped interrupts. Linux now creates two distinct IRQ mappings on the >> same HW IRQ which can lead to unexpected behavior in the drivers. >> >> The XIVE IRQ domain is not a direct mapping domain and its HW IRQ >> interrupt number space is rather large : 1M/socket on POWER9 and >> POWER10, change the XIVE driver to use a 'tree' domain type instead. >> >> [1] For instance, a linux KVM guest with virtio-rng and virtio-balloon >> devices. >> >> Cc: Marc Zyngier <maz@kernel.org> >> Cc: stable@vger.kernel.org # v5.14+ >> Fixes: 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive") >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> > > Tested-by: Greg Kurz <groug@kaod.org> > > with a KVM guest + virtio-rng + virtio-balloon on a POWER9 host. Did you test on a 5.14 backport or mainline ? I am asking because a large change adding support for MSI domains to XIVE was merged in 5.15. Thanks, C. > >> Marc, >> >> The Fixes tag is there because the patch in question revealed that >> something was broken in XIVE. genirq is not in cause. However, I >> don't know for PS3 and Cell. May be less critical for now. >> >> arch/powerpc/sysdev/xive/common.c | 3 +-- >> arch/powerpc/sysdev/xive/Kconfig | 1 - >> 2 files changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c >> index fed6fd16c8f4..9d0f0fe25598 100644 >> --- a/arch/powerpc/sysdev/xive/common.c >> +++ b/arch/powerpc/sysdev/xive/common.c >> @@ -1536,8 +1536,7 @@ static const struct irq_domain_ops xive_irq_domain_ops = { >> >> static void __init xive_init_host(struct device_node *np) >> { >> - xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, >> - &xive_irq_domain_ops, NULL); >> + xive_irq_domain = irq_domain_add_tree(np, &xive_irq_domain_ops, NULL); >> if (WARN_ON(xive_irq_domain == NULL)) >> return; >> irq_set_default_host(xive_irq_domain); >> diff --git a/arch/powerpc/sysdev/xive/Kconfig b/arch/powerpc/sysdev/xive/Kconfig >> index 97796c6b63f0..785c292d104b 100644 >> --- a/arch/powerpc/sysdev/xive/Kconfig >> +++ b/arch/powerpc/sysdev/xive/Kconfig >> @@ -3,7 +3,6 @@ config PPC_XIVE >> bool >> select PPC_SMP_MUXED_IPI >> select HARDIRQS_SW_RESEND >> - select IRQ_DOMAIN_NOMAP >> >> config PPC_XIVE_NATIVE >> bool >
On Tue, 16 Nov 2021 15:49:13 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 11/16/21 15:23, Greg Kurz wrote: > > On Tue, 16 Nov 2021 14:40:22 +0100 > > Cédric Le Goater <clg@kaod.org> wrote: > > > >> Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains > >> exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the > >> 'nomap' domains still in use under the powerpc arch. With this new > >> flag, the revmap_tree of the IRQ domain is not used anymore. This > >> change broke the support of shared LSIs [1] in the XIVE driver because > >> it was relying on a lookup in the revmap_tree to query previously > >> mapped interrupts. Linux now creates two distinct IRQ mappings on the > >> same HW IRQ which can lead to unexpected behavior in the drivers. > >> > >> The XIVE IRQ domain is not a direct mapping domain and its HW IRQ > >> interrupt number space is rather large : 1M/socket on POWER9 and > >> POWER10, change the XIVE driver to use a 'tree' domain type instead. > >> > >> [1] For instance, a linux KVM guest with virtio-rng and virtio-balloon > >> devices. > >> > >> Cc: Marc Zyngier <maz@kernel.org> > >> Cc: stable@vger.kernel.org # v5.14+ > >> Fixes: 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive") > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >> --- > >> > > > > Tested-by: Greg Kurz <groug@kaod.org> > > > > with a KVM guest + virtio-rng + virtio-balloon on a POWER9 host. > > Did you test on a 5.14 backport or mainline ? > I've tested on a 5.14 backport only. > I am asking because a large change adding support for MSI domains > to XIVE was merged in 5.15. > > Thanks, > > C. > > > > > >> Marc, > >> > >> The Fixes tag is there because the patch in question revealed that > >> something was broken in XIVE. genirq is not in cause. However, I > >> don't know for PS3 and Cell. May be less critical for now. > >> > >> arch/powerpc/sysdev/xive/common.c | 3 +-- > >> arch/powerpc/sysdev/xive/Kconfig | 1 - > >> 2 files changed, 1 insertion(+), 3 deletions(-) > >> > >> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c > >> index fed6fd16c8f4..9d0f0fe25598 100644 > >> --- a/arch/powerpc/sysdev/xive/common.c > >> +++ b/arch/powerpc/sysdev/xive/common.c > >> @@ -1536,8 +1536,7 @@ static const struct irq_domain_ops xive_irq_domain_ops = { > >> > >> static void __init xive_init_host(struct device_node *np) > >> { > >> - xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, > >> - &xive_irq_domain_ops, NULL); > >> + xive_irq_domain = irq_domain_add_tree(np, &xive_irq_domain_ops, NULL); > >> if (WARN_ON(xive_irq_domain == NULL)) > >> return; > >> irq_set_default_host(xive_irq_domain); > >> diff --git a/arch/powerpc/sysdev/xive/Kconfig b/arch/powerpc/sysdev/xive/Kconfig > >> index 97796c6b63f0..785c292d104b 100644 > >> --- a/arch/powerpc/sysdev/xive/Kconfig > >> +++ b/arch/powerpc/sysdev/xive/Kconfig > >> @@ -3,7 +3,6 @@ config PPC_XIVE > >> bool > >> select PPC_SMP_MUXED_IPI > >> select HARDIRQS_SW_RESEND > >> - select IRQ_DOMAIN_NOMAP > >> > >> config PPC_XIVE_NATIVE > >> bool > > >
On Tue, 16 Nov 2021 13:40:22 +0000, Cédric Le Goater <clg@kaod.org> wrote: > > Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains > exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the > 'nomap' domains still in use under the powerpc arch. With this new > flag, the revmap_tree of the IRQ domain is not used anymore. This > change broke the support of shared LSIs [1] in the XIVE driver because > it was relying on a lookup in the revmap_tree to query previously > mapped interrupts. Just a lookup? Surely there is more to it, no? > Linux now creates two distinct IRQ mappings on the > same HW IRQ which can lead to unexpected behavior in the drivers. > > The XIVE IRQ domain is not a direct mapping domain and its HW IRQ > interrupt number space is rather large : 1M/socket on POWER9 and > POWER10, change the XIVE driver to use a 'tree' domain type instead. > > [1] For instance, a linux KVM guest with virtio-rng and virtio-balloon > devices. > > Cc: Marc Zyngier <maz@kernel.org> > Cc: stable@vger.kernel.org # v5.14+ > Fixes: 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive") > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > > Marc, > > The Fixes tag is there because the patch in question revealed that > something was broken in XIVE. genirq is not in cause. However, I > don't know for PS3 and Cell. May be less critical for now. Depends if they expect something that a no-map domain cannot provide. > > arch/powerpc/sysdev/xive/common.c | 3 +-- > arch/powerpc/sysdev/xive/Kconfig | 1 - > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c > index fed6fd16c8f4..9d0f0fe25598 100644 > --- a/arch/powerpc/sysdev/xive/common.c > +++ b/arch/powerpc/sysdev/xive/common.c > @@ -1536,8 +1536,7 @@ static const struct irq_domain_ops xive_irq_domain_ops = { > > static void __init xive_init_host(struct device_node *np) > { > - xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, > - &xive_irq_domain_ops, NULL); > + xive_irq_domain = irq_domain_add_tree(np, &xive_irq_domain_ops, NULL); > if (WARN_ON(xive_irq_domain == NULL)) > return; > irq_set_default_host(xive_irq_domain); > diff --git a/arch/powerpc/sysdev/xive/Kconfig b/arch/powerpc/sysdev/xive/Kconfig > index 97796c6b63f0..785c292d104b 100644 > --- a/arch/powerpc/sysdev/xive/Kconfig > +++ b/arch/powerpc/sysdev/xive/Kconfig > @@ -3,7 +3,6 @@ config PPC_XIVE > bool > select PPC_SMP_MUXED_IPI > select HARDIRQS_SW_RESEND > - select IRQ_DOMAIN_NOMAP > > config PPC_XIVE_NATIVE > bool As long as this works, I'm happy with one less no-map user. Acked-by: Marc Zyngier <maz@kernel.org> M.
On 11/16/21 17:58, Marc Zyngier wrote: > On Tue, 16 Nov 2021 13:40:22 +0000, > Cédric Le Goater <clg@kaod.org> wrote: >> >> Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains >> exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the >> 'nomap' domains still in use under the powerpc arch. With this new >> flag, the revmap_tree of the IRQ domain is not used anymore. This >> change broke the support of shared LSIs [1] in the XIVE driver because >> it was relying on a lookup in the revmap_tree to query previously >> mapped interrupts. > > Just a lookup? Surely there is more to it, no? nope. The HW IRQ for the INTx is defined in the DT. It is caught by of_irq_parse_and_map_pci() which simply adds an extra mapping on the same INTx since the previous one is not found. Using an INTx is quite rare now days and a shared one is even more uncommon I guess, I could only reproduced on the baremetal platform with the QEMU PowerNV machine using the same virtio devices. Thanks, C. > >> Linux now creates two distinct IRQ mappings on the >> same HW IRQ which can lead to unexpected behavior in the drivers. >> >> The XIVE IRQ domain is not a direct mapping domain and its HW IRQ >> interrupt number space is rather large : 1M/socket on POWER9 and >> POWER10, change the XIVE driver to use a 'tree' domain type instead. >> >> [1] For instance, a linux KVM guest with virtio-rng and virtio-balloon >> devices. >> >> Cc: Marc Zyngier <maz@kernel.org> >> Cc: stable@vger.kernel.org # v5.14+ >> Fixes: 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive") >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> >> Marc, >> >> The Fixes tag is there because the patch in question revealed that >> something was broken in XIVE. genirq is not in cause. However, I >> don't know for PS3 and Cell. May be less critical for now. > > Depends if they expect something that a no-map domain cannot provide.> >> >> arch/powerpc/sysdev/xive/common.c | 3 +-- >> arch/powerpc/sysdev/xive/Kconfig | 1 - >> 2 files changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c >> index fed6fd16c8f4..9d0f0fe25598 100644 >> --- a/arch/powerpc/sysdev/xive/common.c >> +++ b/arch/powerpc/sysdev/xive/common.c >> @@ -1536,8 +1536,7 @@ static const struct irq_domain_ops xive_irq_domain_ops = { >> >> static void __init xive_init_host(struct device_node *np) >> { >> - xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, >> - &xive_irq_domain_ops, NULL); >> + xive_irq_domain = irq_domain_add_tree(np, &xive_irq_domain_ops, NULL); >> if (WARN_ON(xive_irq_domain == NULL)) >> return; >> irq_set_default_host(xive_irq_domain); >> diff --git a/arch/powerpc/sysdev/xive/Kconfig b/arch/powerpc/sysdev/xive/Kconfig >> index 97796c6b63f0..785c292d104b 100644 >> --- a/arch/powerpc/sysdev/xive/Kconfig >> +++ b/arch/powerpc/sysdev/xive/Kconfig >> @@ -3,7 +3,6 @@ config PPC_XIVE >> bool >> select PPC_SMP_MUXED_IPI >> select HARDIRQS_SW_RESEND >> - select IRQ_DOMAIN_NOMAP >> >> config PPC_XIVE_NATIVE >> bool > > As long as this works, I'm happy with one less no-map user. > > Acked-by: Marc Zyngier <maz@kernel.org> > > M. >
On Tue, 16 Nov 2021 14:40:22 +0100, Cédric Le Goater wrote: > Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains > exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the > 'nomap' domains still in use under the powerpc arch. With this new > flag, the revmap_tree of the IRQ domain is not used anymore. This > change broke the support of shared LSIs [1] in the XIVE driver because > it was relying on a lookup in the revmap_tree to query previously > mapped interrupts. Linux now creates two distinct IRQ mappings on the > same HW IRQ which can lead to unexpected behavior in the drivers. > > [...] Applied to powerpc/fixes. [1/1] powerpc/xive: Change IRQ domain to a tree domain https://git.kernel.org/powerpc/c/8e80a73fa9a7747e3e8255cb149c543aabf65a24 cheers
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c index fed6fd16c8f4..9d0f0fe25598 100644 --- a/arch/powerpc/sysdev/xive/common.c +++ b/arch/powerpc/sysdev/xive/common.c @@ -1536,8 +1536,7 @@ static const struct irq_domain_ops xive_irq_domain_ops = { static void __init xive_init_host(struct device_node *np) { - xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, - &xive_irq_domain_ops, NULL); + xive_irq_domain = irq_domain_add_tree(np, &xive_irq_domain_ops, NULL); if (WARN_ON(xive_irq_domain == NULL)) return; irq_set_default_host(xive_irq_domain); diff --git a/arch/powerpc/sysdev/xive/Kconfig b/arch/powerpc/sysdev/xive/Kconfig index 97796c6b63f0..785c292d104b 100644 --- a/arch/powerpc/sysdev/xive/Kconfig +++ b/arch/powerpc/sysdev/xive/Kconfig @@ -3,7 +3,6 @@ config PPC_XIVE bool select PPC_SMP_MUXED_IPI select HARDIRQS_SW_RESEND - select IRQ_DOMAIN_NOMAP config PPC_XIVE_NATIVE bool
Commit 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive") introduced an IRQ_DOMAIN_FLAG_NO_MAP flag to isolate the 'nomap' domains still in use under the powerpc arch. With this new flag, the revmap_tree of the IRQ domain is not used anymore. This change broke the support of shared LSIs [1] in the XIVE driver because it was relying on a lookup in the revmap_tree to query previously mapped interrupts. Linux now creates two distinct IRQ mappings on the same HW IRQ which can lead to unexpected behavior in the drivers. The XIVE IRQ domain is not a direct mapping domain and its HW IRQ interrupt number space is rather large : 1M/socket on POWER9 and POWER10, change the XIVE driver to use a 'tree' domain type instead. [1] For instance, a linux KVM guest with virtio-rng and virtio-balloon devices. Cc: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org # v5.14+ Fixes: 4f86a06e2d6e ("irqdomain: Make normal and nomap irqdomains exclusive") Signed-off-by: Cédric Le Goater <clg@kaod.org> --- Marc, The Fixes tag is there because the patch in question revealed that something was broken in XIVE. genirq is not in cause. However, I don't know for PS3 and Cell. May be less critical for now. arch/powerpc/sysdev/xive/common.c | 3 +-- arch/powerpc/sysdev/xive/Kconfig | 1 - 2 files changed, 1 insertion(+), 3 deletions(-)