Re: [PATCH] powerpc/xive: Change IRQ domain to a tree domain

From: Marc Zyngier <maz@kernel.org>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: <linuxppc-dev@lists.ozlabs.org>,
	Michael Ellerman <mpe@ellerman.id.au>, Greg Kurz <groug@kaod.org>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH] powerpc/xive: Change IRQ domain to a tree domain
Date: Tue, 16 Nov 2021 16:58:00 +0000	[thread overview]
Message-ID: <874k8c82cn.wl-maz@kernel.org> (raw)
In-Reply-To: <20211116134022.420412-1-clg@kaod.org>

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.

-- 
Without deviation from the norm, progress is not possible.