Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter

[Michael Ellerman <mpe@ellerman.id.au>]

Cédric Le Goater <clg@kaod.org> writes:
> On processors with a XIVE interrupt controller (POWER9 and above), the
> kernel can use either doorbells or XIVE to generate CPU IPIs. Sending
> doorbell is generally preferred to using the XIVE IC because it is
> faster. There are cases where we want to avoid doorbells and use XIVE
> only, for debug or performance. Only useful on POWER9 and above.

How much do we want this?

Kernel command line args are a bit of a pain, they tend to be poorly
tested, because someone has to explicitly enable them at boot time, and
then reboot to test the other case.

When would we want to enable this? Can we make the kernel smarter about
when to use doorbells and make it automated?

Could we make it a runtime switch?

cheers

>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  arch/powerpc/include/asm/dbell.h                |  1 +
>  arch/powerpc/kernel/dbell.c                     | 17 +++++++++++++++++
>  arch/powerpc/platforms/powernv/smp.c            |  7 +++++--
>  arch/powerpc/platforms/pseries/smp.c            |  3 +++
>  Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
>  5 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dbell.h b/arch/powerpc/include/asm/dbell.h
> index 3e9da22a2779..07775aa3e81b 100644
> --- a/arch/powerpc/include/asm/dbell.h
> +++ b/arch/powerpc/include/asm/dbell.h
> @@ -90,6 +90,7 @@ static inline void ppc_msgsync(void)
>  #endif /* CONFIG_PPC_BOOK3S */
>  
>  extern void doorbell_exception(struct pt_regs *regs);
> +extern bool doorbell_disabled;
>  
>  static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
>  {
> diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
> index 5545c9cd17c1..681ee4775629 100644
> --- a/arch/powerpc/kernel/dbell.c
> +++ b/arch/powerpc/kernel/dbell.c
> @@ -38,6 +38,23 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
>  
>  	set_irq_regs(old_regs);
>  }
> +
> +bool doorbell_disabled;
> +
> +static int __init doorbell_cmdline(char *arg)
> +{
> +	if (!arg)
> +		return -EINVAL;
> +
> +	if (strncmp(arg, "off", 3) == 0) {
> +		pr_info("Doorbell disabled on kernel command line\n");
> +		doorbell_disabled = true;
> +	}
> +
> +	return 0;
> +}
> +__setup("doorbell=", doorbell_cmdline);
> +
>  #else /* CONFIG_SMP */
>  DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
>  {
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index cbb67813cd5d..1311bda9446a 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -338,10 +338,13 @@ static void __init pnv_smp_probe(void)
>  		ic_cause_ipi = smp_ops->cause_ipi;
>  		WARN_ON(!ic_cause_ipi);
>  
> -		if (cpu_has_feature(CPU_FTR_ARCH_300))
> +		if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> +			if (doorbell_disabled)
> +				return;
>  			smp_ops->cause_ipi = doorbell_global_ipi;
> -		else
> +		} else {
>  			smp_ops->cause_ipi = pnv_cause_ipi;
> +		}
>  	}
>  }
>  
> diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
> index f47429323eee..3bc9e6aaf645 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -229,6 +229,9 @@ static __init void pSeries_smp_probe(void)
>  			return;
>  	}
>  
> +	if (doorbell_disabled)
> +		return;
> +
>  	/*
>  	 * Under PowerVM, FSCR[MSGP] is enabled as guest vCPU siblings are
>  	 * gang scheduled on the same physical core, so doorbells are always
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 10fa093251e8..2e1284febe39 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1041,6 +1041,16 @@
>  			The filter can be disabled or changed to another
>  			driver later using sysfs.
>  
> +	doorbell=off	[PPC]
> +			On processors with a XIVE interrupt controller
> +			(POWER9 and above), the kernel can use either
> +			doorbells or XIVE to generate CPU IPIs.	Sending
> +			doorbell is generally preferred to using the XIVE
> +			IC because it is faster. There are cases where
> +			we want to avoid doorbells and use XIVE only,
> +			for debug or performance. Only useful on
> +			POWER9 and above.
> +
>  	driver_async_probe=  [KNL]
>  			List of driver names to be probed asynchronously.
>  			Format: <driver_name1>,<driver_name2>...
> -- 
> 2.31.1