[PATCH v3 1/2] powerpc: add ISA v3.0 / v3.1 wait opcode macro

[PATCH v3 1/2] powerpc: add ISA v3.0 / v3.1 wait opcode macro

[Nicholas Piggin]

The wait instruction encoding changed between ISA v2.07 and ISA v3.0.
In v3.1 the instruction gained a new field.

Update the PPC_WAIT macro to the current encoding. Rename the older
incompatible one with a _v203 suffix as it was introduced in v2.03
(the WC field was introduced in v2.07 but the kernel only uses WC=0).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
v2: Update naming, patch changelog and title.
v3: v2 sent incorrect patches, sorry. 

 arch/powerpc/include/asm/ppc-opcode.h | 7 +++++--
 arch/powerpc/kernel/idle_book3e.S     | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 89beabf5325c..e6fc3b26c145 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -331,6 +331,7 @@
 #define __PPC_XSP(s)	((((s) & 0x1e) | (((s) >> 5) & 0x1)) << 21)
 #define __PPC_XTP(s)	__PPC_XSP(s)
 #define __PPC_T_TLB(t)	(((t) & 0x3) << 21)
+#define __PPC_PL(p)	(((p) & 0x3) << 16)
 #define __PPC_WC(w)	(((w) & 0x3) << 21)
 #define __PPC_WS(w)	(((w) & 0x1f) << 11)
 #define __PPC_SH(s)	__PPC_WS(s)
@@ -398,7 +399,8 @@
 #define PPC_RAW_RFDI			(0x4c00004e)
 #define PPC_RAW_RFMCI			(0x4c00004c)
 #define PPC_RAW_TLBILX(t, a, b)		(0x7c000024 | __PPC_T_TLB(t) | 	__PPC_RA0(a) | __PPC_RB(b))
-#define PPC_RAW_WAIT(w)			(0x7c00007c | __PPC_WC(w))
+#define PPC_RAW_WAIT_v203		(0x7c00007c)
+#define PPC_RAW_WAIT(w, p)		(0x7c00003c | __PPC_WC(w) | __PPC_PL(p))
 #define PPC_RAW_TLBIE(lp, a)		(0x7c000264 | ___PPC_RB(a) | ___PPC_RS(lp))
 #define PPC_RAW_TLBIE_5(rb, rs, ric, prs, r) \
 	(0x7c000264 | ___PPC_RB(rb) | ___PPC_RS(rs) | ___PPC_RIC(ric) | ___PPC_PRS(prs) | ___PPC_R(r))
@@ -613,7 +615,8 @@
 #define PPC_TLBILX_ALL(a, b)	PPC_TLBILX(0, a, b)
 #define PPC_TLBILX_PID(a, b)	PPC_TLBILX(1, a, b)
 #define PPC_TLBILX_VA(a, b)	PPC_TLBILX(3, a, b)
-#define PPC_WAIT(w)		stringify_in_c(.long PPC_RAW_WAIT(w))
+#define PPC_WAIT_v203		stringify_in_c(.long PPC_RAW_WAIT_v203)
+#define PPC_WAIT(w, p)		stringify_in_c(.long PPC_RAW_WAIT(w, p))
 #define PPC_TLBIE(lp, a) 	stringify_in_c(.long PPC_RAW_TLBIE(lp, a))
 #define	PPC_TLBIE_5(rb, rs, ric, prs, r) \
 				stringify_in_c(.long PPC_RAW_TLBIE_5(rb, rs, ric, prs, r))
diff --git a/arch/powerpc/kernel/idle_book3e.S b/arch/powerpc/kernel/idle_book3e.S
index cc008de58b05..6447de51ea71 100644
--- a/arch/powerpc/kernel/idle_book3e.S
+++ b/arch/powerpc/kernel/idle_book3e.S
@@ -77,7 +77,7 @@ _GLOBAL(\name)
 
 .macro BOOK3E_IDLE_LOOP
 1:
-	PPC_WAIT(0)
+	PPC_WAIT_v203
 	b	1b
 .endm
 
-- 
2.35.1

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

[Nicholas Piggin]

We want to move away from using SMT prioroty updates for cpu_relax, and
use a 'wait' instruction which is similar to x86. As well as being a
much better fit for what everybody else uses and tests with, priority
nops are stateful which is nasty (interrupts have to consider they might
be taken at a different priority), and they're expensive to execute,
similar to a mtSPR which can effect other threads in the pipe.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Unfortunately qemu TCG does not emulate pause_short properly and will
cause hangs. I have a patch for it but not merged yet. But if we tune
qspinlock code it would be best to do it with this patch.

 arch/powerpc/include/asm/processor.h      | 30 +++++++++++++++++++----
 arch/powerpc/include/asm/vdso/processor.h | 10 +++++++-
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index fdfaae194ddd..d926e59f9d1b 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -355,11 +355,31 @@ static inline unsigned long __pack_fe01(unsigned int fpmode)
 
 #ifdef CONFIG_PPC64
 
-#define spin_begin()	HMT_low()
-
-#define spin_cpu_relax()	barrier()
-
-#define spin_end()	HMT_medium()
+#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)
+
+#define spin_cpu_relax()						\
+do {									\
+	asm volatile(ASM_FTR_IFCLR(					\
+		/* Pre-POWER10 uses low / medium priority nops */	\
+		"nop",							\
+		/* 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)
 
 #endif
 
diff --git a/arch/powerpc/include/asm/vdso/processor.h b/arch/powerpc/include/asm/vdso/processor.h
index 8d79f994b4aa..778d2b53041b 100644
--- a/arch/powerpc/include/asm/vdso/processor.h
+++ b/arch/powerpc/include/asm/vdso/processor.h
@@ -22,7 +22,15 @@
 #endif
 
 #ifdef CONFIG_PPC64
-#define cpu_relax()	do { HMT_low(); HMT_medium(); barrier(); } while (0)
+#define cpu_relax()							\
+do {									\
+	asm volatile(ASM_FTR_IFCLR(					\
+		/* Pre-POWER10 uses low ; medium priority nops */	\
+		"or 1,1,1 ; or 2,2,2",					\
+		/* POWER10 onward uses pause_short (wait 2,0) */	\
+		PPC_WAIT(2, 0),						\
+		%0) :: "i" (CPU_FTR_ARCH_31) : "memory");		\
+} while (0)
 #else
 #define cpu_relax()	barrier()
 #endif
-- 
2.35.1

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

[Segher Boessenkool]

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

Re: [PATCH v3 1/2] powerpc: add ISA v3.0 / v3.1 wait opcode macro

[Segher Boessenkool]

On Wed, Jul 20, 2022 at 11:21:31PM +1000, Nicholas Piggin wrote:
> The wait instruction encoding changed between ISA v2.07 and ISA v3.0.
> In v3.1 the instruction gained a new field.
> 
> Update the PPC_WAIT macro to the current encoding. Rename the older
> incompatible one with a _v203 suffix as it was introduced in v2.03
> (the WC field was introduced in v2.07 but the kernel only uses WC=0).

Ah, it wasn't just embedded in 2.03, I see.

Looks good to me, thanks!

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


Segher

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

[Michael Ellerman]

Nicholas Piggin <npiggin@gmail.com> writes:
> We want to move away from using SMT prioroty updates for cpu_relax, and
> use a 'wait' instruction which is similar to x86. As well as being a
> much better fit for what everybody else uses and tests with, priority
> nops are stateful which is nasty (interrupts have to consider they might
> be taken at a different priority), and they're expensive to execute,
> similar to a mtSPR which can effect other threads in the pipe.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Unfortunately qemu TCG does not emulate pause_short properly and will
> cause hangs.

That _really_ sucks for testing, being able to use qemu is a huge
benefit. I can boot test multiple kernels per minute using qemu, vs
multiple minutes per kernel using real hardware.

> I have a patch for it but not merged yet. But if we tune
> qspinlock code it would be best to do it with this patch.

What's the urgency on this patch? Can we wait for the qemu change to
land? I guess Qemu 8 is not due until next year? :/

cheers