Re: [PATCH 03/13] powerpc/xive: Introduce XIVE_IPI_HW_IRQ

[Greg Kurz <groug@kaod.org>]

On Tue, 8 Dec 2020 16:11:14 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> The XIVE driver deals with CPU IPIs in a peculiar way. Each CPU has
> its own XIVE IPI interrupt allocated at the HW level, for PowerNV, or
> at the hypervisor level for pSeries. In practice, these interrupts are
> not always used. pSeries/PowerVM prefers local doorbells for local
> threads since they are faster. On PowerNV, global doorbells are also
> preferred for the same reason.
> 
> The mapping in the Linux is reduced to a single interrupt using HW
> interrupt number 0 and a custom irq_chip to handle EOI. This can cause
> performance issues in some benchmark (ipistorm) on multichip systems.
> 
> Clarify the use of the 0 value, it will help in improving multichip
> support.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  arch/powerpc/sysdev/xive/xive-internal.h |  2 ++
>  arch/powerpc/sysdev/xive/common.c        | 10 +++++-----
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
> index b7b901da2168..d701af7fb48c 100644
> --- a/arch/powerpc/sysdev/xive/xive-internal.h
> +++ b/arch/powerpc/sysdev/xive/xive-internal.h
> @@ -5,6 +5,8 @@
>  #ifndef __XIVE_INTERNAL_H
>  #define __XIVE_INTERNAL_H
>  
> +#define XIVE_IPI_HW_IRQ		0 /* interrupt source # for IPIs */
> +
>  /*
>   * A "disabled" interrupt should never fire, to catch problems
>   * we set its logical number to this
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 65af34ac1fa2..ee375daf8114 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1142,7 +1142,7 @@ static void __init xive_request_ipi(void)
>  		return;
>  
>  	/* Initialize it */
> -	virq = irq_create_mapping(xive_irq_domain, 0);
> +	virq = irq_create_mapping(xive_irq_domain, XIVE_IPI_HW_IRQ);
>  	xive_ipi_irq = virq;
>  
>  	WARN_ON(request_irq(virq, xive_muxed_ipi_action,
> @@ -1242,7 +1242,7 @@ static int xive_irq_domain_map(struct irq_domain *h, unsigned int virq,
>  
>  #ifdef CONFIG_SMP
>  	/* IPIs are special and come up with HW number 0 */
> -	if (hw == 0) {
> +	if (hw == XIVE_IPI_HW_IRQ) {
>  		/*
>  		 * IPIs are marked per-cpu. We use separate HW interrupts under
>  		 * the hood but associated with the same "linux" interrupt
> @@ -1271,7 +1271,7 @@ static void xive_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
>  	if (!data)
>  		return;
>  	hw_irq = (unsigned int)irqd_to_hwirq(data);
> -	if (hw_irq)
> +	if (hw_irq != XIVE_IPI_HW_IRQ)
>  		xive_irq_free_data(virq);
>  }
>  
> @@ -1421,7 +1421,7 @@ static void xive_flush_cpu_queue(unsigned int cpu, struct xive_cpu *xc)
>  		 * Ignore anything that isn't a XIVE irq and ignore
>  		 * IPIs, so can just be dropped.
>  		 */
> -		if (d->domain != xive_irq_domain || hw_irq == 0)
> +		if (d->domain != xive_irq_domain || hw_irq == XIVE_IPI_HW_IRQ)
>  			continue;
>  
>  		/*
> @@ -1655,7 +1655,7 @@ static int xive_core_debug_show(struct seq_file *m, void *private)
>  		hw_irq = (unsigned int)irqd_to_hwirq(d);
>  
>  		/* IPIs are special (HW number 0) */
> -		if (hw_irq)
> +		if (hw_irq != XIVE_IPI_HW_IRQ)
>  			xive_debug_show_irq(m, hw_irq, d);
>  	}
>  	return 0;