Re: [PATCH v3 2/2] powerpc/64s: Make POWER10 and later use pause_short in cpu_relax loops

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Wed, Jul 20, 2022 at 11:21:32PM +1000, Nicholas Piggin wrote:
> We want to move away from using SMT prioroty updates for cpu_relax, and

(typo, "priority")

> +#define spin_begin()							\
> +do {									\
> +	asm volatile(ASM_FTR_IFCLR(					\
> +		"or 1,1,1", /* HMT_LOW */				\
> +		"nop",/* POWER10 onward uses pause_short (wait 2,0) */	\
> +		%0) :: "i" (CPU_FTR_ARCH_31) : "memory");		\
> +} while (0)

Is that nop patched later?  Or should you change the comment, maybe?

> +#define spin_cpu_relax()						\
> +do {									\
> +	asm volatile(ASM_FTR_IFCLR(					\
> +		/* Pre-POWER10 uses low / medium priority nops */	\
> +		"nop",							\

"nop" aka "or 0,0,0" does not change program priority?  Medium low would
be "or 6,6,6", not sure if that is the ppr you wanted here?

> +		/* POWER10 onward uses pause_short (wait 2,0) */	\
> +		PPC_WAIT(2, 0),						\
> +		%0) :: "i" (CPU_FTR_ARCH_31) : "memory");		\
> +} while (0)
> +
> +#define spin_end()							\
> +do {									\
> +	asm volatile(ASM_FTR_IFCLR(					\
> +		"or 2,2,2", /* HMT_MEDIUM */				\
> +		"nop",/* POWER10 onward uses pause_short (wait 2,0) */	\
> +		%0) :: "i" (CPU_FTR_ARCH_31) : "memory");		\
> +} while (0)

Same comment as for spin_begin.

Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>


Segher