[PATCH 1/4] powerpc: Make mmiowb a wmb

[PATCH 1/4] powerpc: Make mmiowb a wmb

[Nicholas Piggin]

mmiowb must ensure MMIO stores inside a spin lock critical section on
one CPU will not be seen by the device after another CPU takes the
same lock and performs MMIOs.

This just requires cache inhibited stores to be ordered with the store
to unlock the spinlock, so wmb() can be used.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/mmiowb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/mmiowb.h b/arch/powerpc/include/asm/mmiowb.h
index 74a00127eb20..cd071fb97eba 100644
--- a/arch/powerpc/include/asm/mmiowb.h
+++ b/arch/powerpc/include/asm/mmiowb.h
@@ -9,7 +9,7 @@
 #include <asm/paca.h>
 
 #define arch_mmiowb_state()	(&local_paca->mmiowb_state)
-#define mmiowb()		mb()
+#define mmiowb()		wmb()
 
 #endif /* CONFIG_MMIOWB */
 
-- 
2.40.1

[PATCH 2/4] powerpc/64s: Add POWER10 store sync mnemonics

[Nicholas Piggin]

ISA v3.1 introduces new sync types for store ordering.

  stncisync
  stcisync
  stsync

Add ppc-opcode defines for these. This changes PPC_RAW_SYNC to take
L,SC parameters and adds a PPC_RAW_HWSYNC for callers that want the
plain old sync (aka hwsync).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/ppc-opcode.h | 19 ++++++++++++++-----
 arch/powerpc/kernel/traps.c           |  2 +-
 arch/powerpc/lib/feature-fixups.c     |  6 +++---
 arch/powerpc/net/bpf_jit_comp64.c     |  2 +-
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index ca5a0da7df4e..7bc8bbcd4adb 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -326,6 +326,8 @@
 #define ___PPC_R(r)	(((r) & 0x1) << 16)
 #define ___PPC_PRS(prs)	(((prs) & 0x1) << 17)
 #define ___PPC_RIC(ric)	(((ric) & 0x3) << 18)
+#define ___PPC_L(l)	(((l) & 0x7) << 21)
+#define ___PPC_SC(sc)	(((sc) & 0x3) << 16)
 #define __PPC_RA(a)	___PPC_RA(__REG_##a)
 #define __PPC_RA0(a)	___PPC_RA(__REGA0_##a)
 #define __PPC_RB(b)	___PPC_RB(__REG_##b)
@@ -378,8 +380,6 @@
 #define PPC_RAW_LQARX(t, a, b, eh)	(0x7c000228 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
 #define PPC_RAW_LDARX(t, a, b, eh)	(0x7c0000a8 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
 #define PPC_RAW_LWARX(t, a, b, eh)	(0x7c000028 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
-#define PPC_RAW_PHWSYNC			(0x7c8004ac)
-#define PPC_RAW_PLWSYNC			(0x7ca004ac)
 #define PPC_RAW_STQCX(t, a, b)		(0x7c00016d | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b))
 #define PPC_RAW_MADDHD(t, a, b, c)	(0x10000030 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | ___PPC_RC(c))
 #define PPC_RAW_MADDHDU(t, a, b, c)	(0x10000031 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | ___PPC_RC(c))
@@ -396,6 +396,13 @@
 #define PPC_RAW_RFCI			(0x4c000066)
 #define PPC_RAW_RFDI			(0x4c00004e)
 #define PPC_RAW_RFMCI			(0x4c00004c)
+#define PPC_RAW_SYNC(l, sc)		(0x7c0004ac | ___PPC_L(l) | ___PPC_SC(sc))
+#define PPC_RAW_HWSYNC()		PPC_RAW_SYNC(0, 0)
+#define PPC_RAW_STNCISYNC()		PPC_RAW_SYNC(1, 1)
+#define PPC_RAW_STCISYNC()		PPC_RAW_SYNC(0, 2)
+#define PPC_RAW_STSYNC()		PPC_RAW_SYNC(0, 3)
+#define PPC_RAW_PHWSYNC()		PPC_RAW_SYNC(4, 0)
+#define PPC_RAW_PLWSYNC()		PPC_RAW_SYNC(5, 0)
 #define PPC_RAW_TLBILX(t, a, b)		(0x7c000024 | __PPC_T_TLB(t) | 	__PPC_RA0(a) | __PPC_RB(b))
 #define PPC_RAW_WAIT_v203		(0x7c00007c)
 #define PPC_RAW_WAIT(w, p)		(0x7c00003c | __PPC_WC(w) | __PPC_PL(p))
@@ -421,7 +428,6 @@
 #define PPC_RAW_DCBFPS(a, b)		(0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (4 << 21))
 #define PPC_RAW_DCBSTPS(a, b)		(0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (6 << 21))
 #define PPC_RAW_SC()			(0x44000002)
-#define PPC_RAW_SYNC()			(0x7c0004ac)
 #define PPC_RAW_ISYNC()			(0x4c00012c)
 
 /*
@@ -641,8 +647,11 @@
 #define STBCIX(s, a, b)		stringify_in_c(.long PPC_RAW_STBCIX(s, a, b))
 #define PPC_DCBFPS(a, b)	stringify_in_c(.long PPC_RAW_DCBFPS(a, b))
 #define PPC_DCBSTPS(a, b)	stringify_in_c(.long PPC_RAW_DCBSTPS(a, b))
-#define PPC_PHWSYNC		stringify_in_c(.long PPC_RAW_PHWSYNC)
-#define PPC_PLWSYNC		stringify_in_c(.long PPC_RAW_PLWSYNC)
+#define PPC_STNCISYNC		stringify_in_c(.long PPC_RAW_STNCISYNC())
+#define PPC_STCISYNC		stringify_in_c(.long PPC_RAW_STCISYNC())
+#define PPC_STSYNC		stringify_in_c(.long PPC_RAW_STSYNC())
+#define PPC_PHWSYNC		stringify_in_c(.long PPC_RAW_PHWSYNC())
+#define PPC_PLWSYNC		stringify_in_c(.long PPC_RAW_PLWSYNC())
 #define STXVD2X(s, a, b)	stringify_in_c(.long PPC_RAW_STXVD2X(s, a, b))
 #define LXVD2X(s, a, b)		stringify_in_c(.long PPC_RAW_LXVD2X(s, a, b))
 #define MFVRD(a, t)		stringify_in_c(.long PPC_RAW_MFVRD(a, t))
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 9bdd79aa51cf..4b216c208f41 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -550,7 +550,7 @@ static inline int check_io_access(struct pt_regs *regs)
 			nip -= 2;
 		else if (*nip == PPC_RAW_ISYNC())
 			--nip;
-		if (*nip == PPC_RAW_SYNC() || get_op(*nip) == OP_TRAP) {
+		if (*nip == PPC_RAW_HWSYNC() || get_op(*nip) == OP_TRAP) {
 			unsigned int rb;
 
 			--nip;
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 80def1c2afcb..4c6e7111354f 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -193,7 +193,7 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
 	} else if (types & STF_BARRIER_EIEIO) {
 		instrs[i++] = PPC_RAW_EIEIO() | 0x02000000; /* eieio + bit 6 hint */
 	} else if (types & STF_BARRIER_SYNC_ORI) {
-		instrs[i++] = PPC_RAW_SYNC();
+		instrs[i++] = PPC_RAW_HWSYNC();
 		instrs[i++] = PPC_RAW_LD(_R10, _R13, 0);
 		instrs[i++] = PPC_RAW_ORI(_R31, _R31, 0); /* speculation barrier */
 	}
@@ -234,7 +234,7 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
 			instrs[i++] = PPC_RAW_MTSPR(SPRN_SPRG2, _R13);
 			instrs[i++] = PPC_RAW_MFSPR(_R13, SPRN_SPRG1);
 	        }
-		instrs[i++] = PPC_RAW_SYNC();
+		instrs[i++] = PPC_RAW_HWSYNC();
 		instrs[i++] = PPC_RAW_LD(_R13, _R13, 0);
 		instrs[i++] = PPC_RAW_ORI(_R31, _R31, 0); /* speculation barrier */
 		if (cpu_has_feature(CPU_FTR_HVMODE))
@@ -543,7 +543,7 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
 	if (enable) {
 		pr_info("barrier-nospec: using isync; sync as speculation barrier\n");
 		instr[0] = PPC_RAW_ISYNC();
-		instr[1] = PPC_RAW_SYNC();
+		instr[1] = PPC_RAW_HWSYNC();
 	}
 
 	i = do_patch_fixups(start, end, instr, ARRAY_SIZE(instr));
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 0f8048f6dad6..e8257bfc1cb4 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -737,7 +737,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
 				EMIT(PPC_RAW_EIEIO() | 0x02000000);
 				break;
 			case STF_BARRIER_SYNC_ORI:
-				EMIT(PPC_RAW_SYNC());
+				EMIT(PPC_RAW_HWSYNC());
 				EMIT(PPC_RAW_LD(tmp1_reg, _R13, 0));
 				EMIT(PPC_RAW_ORI(_R31, _R31, 0));
 				break;
-- 
2.40.1

Re: [PATCH 2/4] powerpc/64s: Add POWER10 store sync mnemonics

[Joel Stanley]

On Fri, 9 Jun 2023 at 10:01, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> ISA v3.1 introduces new sync types for store ordering.
>
>   stncisync
>   stcisync
>   stsync
>
> Add ppc-opcode defines for these. This changes PPC_RAW_SYNC to take
> L,SC parameters and adds a PPC_RAW_HWSYNC for callers that want the
> plain old sync (aka hwsync).

I checked these against the ISA and they seem correct.

Did you consider changing LWSYNC to be defined in terms of your new
PPC_RAW_SYNC?

Reviewed-by: Joel Stanley <joel@jms.id.au>.

>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/ppc-opcode.h | 19 ++++++++++++++-----
>  arch/powerpc/kernel/traps.c           |  2 +-
>  arch/powerpc/lib/feature-fixups.c     |  6 +++---
>  arch/powerpc/net/bpf_jit_comp64.c     |  2 +-
>  4 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index ca5a0da7df4e..7bc8bbcd4adb 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -326,6 +326,8 @@
>  #define ___PPC_R(r)    (((r) & 0x1) << 16)
>  #define ___PPC_PRS(prs)        (((prs) & 0x1) << 17)
>  #define ___PPC_RIC(ric)        (((ric) & 0x3) << 18)
> +#define ___PPC_L(l)    (((l) & 0x7) << 21)
> +#define ___PPC_SC(sc)  (((sc) & 0x3) << 16)
>  #define __PPC_RA(a)    ___PPC_RA(__REG_##a)
>  #define __PPC_RA0(a)   ___PPC_RA(__REGA0_##a)
>  #define __PPC_RB(b)    ___PPC_RB(__REG_##b)
> @@ -378,8 +380,6 @@
>  #define PPC_RAW_LQARX(t, a, b, eh)     (0x7c000228 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
>  #define PPC_RAW_LDARX(t, a, b, eh)     (0x7c0000a8 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
>  #define PPC_RAW_LWARX(t, a, b, eh)     (0x7c000028 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))
> -#define PPC_RAW_PHWSYNC                        (0x7c8004ac)
> -#define PPC_RAW_PLWSYNC                        (0x7ca004ac)
>  #define PPC_RAW_STQCX(t, a, b)         (0x7c00016d | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b))
>  #define PPC_RAW_MADDHD(t, a, b, c)     (0x10000030 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | ___PPC_RC(c))
>  #define PPC_RAW_MADDHDU(t, a, b, c)    (0x10000031 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | ___PPC_RC(c))
> @@ -396,6 +396,13 @@
>  #define PPC_RAW_RFCI                   (0x4c000066)
>  #define PPC_RAW_RFDI                   (0x4c00004e)
>  #define PPC_RAW_RFMCI                  (0x4c00004c)
> +#define PPC_RAW_SYNC(l, sc)            (0x7c0004ac | ___PPC_L(l) | ___PPC_SC(sc))
> +#define PPC_RAW_HWSYNC()               PPC_RAW_SYNC(0, 0)
> +#define PPC_RAW_STNCISYNC()            PPC_RAW_SYNC(1, 1)
> +#define PPC_RAW_STCISYNC()             PPC_RAW_SYNC(0, 2)
> +#define PPC_RAW_STSYNC()               PPC_RAW_SYNC(0, 3)
> +#define PPC_RAW_PHWSYNC()              PPC_RAW_SYNC(4, 0)
> +#define PPC_RAW_PLWSYNC()              PPC_RAW_SYNC(5, 0)
>  #define PPC_RAW_TLBILX(t, a, b)                (0x7c000024 | __PPC_T_TLB(t) |  __PPC_RA0(a) | __PPC_RB(b))
>  #define PPC_RAW_WAIT_v203              (0x7c00007c)
>  #define PPC_RAW_WAIT(w, p)             (0x7c00003c | __PPC_WC(w) | __PPC_PL(p))
> @@ -421,7 +428,6 @@
>  #define PPC_RAW_DCBFPS(a, b)           (0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (4 << 21))
>  #define PPC_RAW_DCBSTPS(a, b)          (0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (6 << 21))
>  #define PPC_RAW_SC()                   (0x44000002)
> -#define PPC_RAW_SYNC()                 (0x7c0004ac)
>  #define PPC_RAW_ISYNC()                        (0x4c00012c)
>
>  /*
> @@ -641,8 +647,11 @@
>  #define STBCIX(s, a, b)                stringify_in_c(.long PPC_RAW_STBCIX(s, a, b))
>  #define PPC_DCBFPS(a, b)       stringify_in_c(.long PPC_RAW_DCBFPS(a, b))
>  #define PPC_DCBSTPS(a, b)      stringify_in_c(.long PPC_RAW_DCBSTPS(a, b))
> -#define PPC_PHWSYNC            stringify_in_c(.long PPC_RAW_PHWSYNC)
> -#define PPC_PLWSYNC            stringify_in_c(.long PPC_RAW_PLWSYNC)
> +#define PPC_STNCISYNC          stringify_in_c(.long PPC_RAW_STNCISYNC())
> +#define PPC_STCISYNC           stringify_in_c(.long PPC_RAW_STCISYNC())
> +#define PPC_STSYNC             stringify_in_c(.long PPC_RAW_STSYNC())
> +#define PPC_PHWSYNC            stringify_in_c(.long PPC_RAW_PHWSYNC())
> +#define PPC_PLWSYNC            stringify_in_c(.long PPC_RAW_PLWSYNC())
>  #define STXVD2X(s, a, b)       stringify_in_c(.long PPC_RAW_STXVD2X(s, a, b))
>  #define LXVD2X(s, a, b)                stringify_in_c(.long PPC_RAW_LXVD2X(s, a, b))
>  #define MFVRD(a, t)            stringify_in_c(.long PPC_RAW_MFVRD(a, t))
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 9bdd79aa51cf..4b216c208f41 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -550,7 +550,7 @@ static inline int check_io_access(struct pt_regs *regs)
>                         nip -= 2;
>                 else if (*nip == PPC_RAW_ISYNC())
>                         --nip;
> -               if (*nip == PPC_RAW_SYNC() || get_op(*nip) == OP_TRAP) {
> +               if (*nip == PPC_RAW_HWSYNC() || get_op(*nip) == OP_TRAP) {
>                         unsigned int rb;
>
>                         --nip;
> diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
> index 80def1c2afcb..4c6e7111354f 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -193,7 +193,7 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
>         } else if (types & STF_BARRIER_EIEIO) {
>                 instrs[i++] = PPC_RAW_EIEIO() | 0x02000000; /* eieio + bit 6 hint */
>         } else if (types & STF_BARRIER_SYNC_ORI) {
> -               instrs[i++] = PPC_RAW_SYNC();
> +               instrs[i++] = PPC_RAW_HWSYNC();
>                 instrs[i++] = PPC_RAW_LD(_R10, _R13, 0);
>                 instrs[i++] = PPC_RAW_ORI(_R31, _R31, 0); /* speculation barrier */
>         }
> @@ -234,7 +234,7 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
>                         instrs[i++] = PPC_RAW_MTSPR(SPRN_SPRG2, _R13);
>                         instrs[i++] = PPC_RAW_MFSPR(_R13, SPRN_SPRG1);
>                 }
> -               instrs[i++] = PPC_RAW_SYNC();
> +               instrs[i++] = PPC_RAW_HWSYNC();
>                 instrs[i++] = PPC_RAW_LD(_R13, _R13, 0);
>                 instrs[i++] = PPC_RAW_ORI(_R31, _R31, 0); /* speculation barrier */
>                 if (cpu_has_feature(CPU_FTR_HVMODE))
> @@ -543,7 +543,7 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
>         if (enable) {
>                 pr_info("barrier-nospec: using isync; sync as speculation barrier\n");
>                 instr[0] = PPC_RAW_ISYNC();
> -               instr[1] = PPC_RAW_SYNC();
> +               instr[1] = PPC_RAW_HWSYNC();
>         }
>
>         i = do_patch_fixups(start, end, instr, ARRAY_SIZE(instr));
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 0f8048f6dad6..e8257bfc1cb4 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -737,7 +737,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
>                                 EMIT(PPC_RAW_EIEIO() | 0x02000000);
>                                 break;
>                         case STF_BARRIER_SYNC_ORI:
> -                               EMIT(PPC_RAW_SYNC());
> +                               EMIT(PPC_RAW_HWSYNC());
>                                 EMIT(PPC_RAW_LD(tmp1_reg, _R13, 0));
>                                 EMIT(PPC_RAW_ORI(_R31, _R31, 0));
>                                 break;
> --
> 2.40.1
>

Re: [PATCH 2/4] powerpc/64s: Add POWER10 store sync mnemonics

[Nicholas Piggin]

On Tue Jun 13, 2023 at 3:31 PM AEST, Joel Stanley wrote:
> On Fri, 9 Jun 2023 at 10:01, Nicholas Piggin <npiggin@gmail.com> wrote:
> >
> > ISA v3.1 introduces new sync types for store ordering.
> >
> >   stncisync
> >   stcisync
> >   stsync
> >
> > Add ppc-opcode defines for these. This changes PPC_RAW_SYNC to take
> > L,SC parameters and adds a PPC_RAW_HWSYNC for callers that want the
> > plain old sync (aka hwsync).
>
> I checked these against the ISA and they seem correct.
>
> Did you consider changing LWSYNC to be defined in terms of your new
> PPC_RAW_SYNC?

Oh I haven't but it would be consistent to change that wouldn't it?

>
> Reviewed-by: Joel Stanley <joel@jms.id.au>.

Thanks,
Nick

[PATCH 3/4] powerpc/64s: Use stncisync instruction for smp_wmb() when available

[Nicholas Piggin]

stncisync orders less than lwsync (only cacheable store-store, not
load-load or load-store) so it should be as cheap or cheaper.

Microbenchmarks with no actual loads to order shows that the basic
execution cost is the same on POWER10.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/barrier.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index b95b666f0374..f0ff5737b0d8 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -6,6 +6,8 @@
 #define _ASM_POWERPC_BARRIER_H
 
 #include <asm/asm-const.h>
+#include <asm/cputable.h>
+#include <asm/feature-fixups.h>
 
 #ifndef __ASSEMBLY__
 #include <asm/ppc-opcode.h>
@@ -41,7 +43,12 @@
 
 /* The sub-arch has lwsync */
 #if defined(CONFIG_PPC64) || defined(CONFIG_PPC_E500MC)
-#    define SMPWMB      LWSYNC
+#    define SMPWMB					\
+	BEGIN_FTR_SECTION;				\
+	LWSYNC;						\
+	FTR_SECTION_ELSE;				\
+	.long PPC_RAW_STNCISYNC();			\
+	ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_31)
 #elif defined(CONFIG_BOOKE)
 #    define SMPWMB      mbar
 #else
-- 
2.40.1

[PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Nicholas Piggin]

The most expensive ordering for hwsync to provide is the store-load
barrier, because all prior stores have to be drained to the caches
before subsequent instructions can complete.

stsync just orders stores which means it can just be a barrer that
goes down the store queue and orders draining, and does not prevent
completion of subsequent instructions. So it should be faster than
hwsync.

Use stsync for wmb(). Older processors that don't recognise the SC
field should treat this as hwsync.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/barrier.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index f0ff5737b0d8..95e637c1a3b6 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -39,7 +39,7 @@
  */
 #define __mb()   __asm__ __volatile__ ("sync" : : : "memory")
 #define __rmb()  __asm__ __volatile__ ("sync" : : : "memory")
-#define __wmb()  __asm__ __volatile__ ("sync" : : : "memory")
+#define __wmb()  __asm__ __volatile__ (PPC_STSYNC : : : "memory")
 
 /* The sub-arch has lwsync */
 #if defined(CONFIG_PPC64) || defined(CONFIG_PPC_E500MC)
-- 
2.40.1

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Michael Ellerman]

Nicholas Piggin <npiggin@gmail.com> writes:
> The most expensive ordering for hwsync to provide is the store-load
> barrier, because all prior stores have to be drained to the caches
> before subsequent instructions can complete.
>
> stsync just orders stores which means it can just be a barrer that
> goes down the store queue and orders draining, and does not prevent
> completion of subsequent instructions. So it should be faster than
> hwsync.
>
> Use stsync for wmb(). Older processors that don't recognise the SC
> field should treat this as hwsync.

qemu (7.1) emulating ppc64e does not :/

  mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
  mpic: ISU size: 256, shift: 8, mask: ff
  mpic: Initializing for 256 sources
  Oops: Exception in kernel mode, sig: 4 [#1]

No more output.

(qemu) info registers                                                                                                │
NIP c000000000df4264   LR c0000000000ce49c CTR 0000000000000000 XER 0000000020000000 CPU#0                           │
MSR 0000000080001000 HID0 0000000000000000  HF 24020006 iidx 1 didx 1                                                │
...
 SRR0 c0000000000ce7c4  SRR1 0000000080081000    PVR 0000000080240020 VRSAVE 0000000000000000

$ objdump -d vmlinux | grep c0000000000ce7c4
c0000000000ce7c4:       7c 03 04 ac     stsync


That's qemu -M ppce500 -cpu e5500 or e6500.

I guess just put it behind an #ifdef 64S.

cheers

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Michael Ellerman]

Michael Ellerman <mpe@ellerman.id.au> writes:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> The most expensive ordering for hwsync to provide is the store-load
>> barrier, because all prior stores have to be drained to the caches
>> before subsequent instructions can complete.
>>
>> stsync just orders stores which means it can just be a barrer that
>> goes down the store queue and orders draining, and does not prevent
>> completion of subsequent instructions. So it should be faster than
>> hwsync.
>>
>> Use stsync for wmb(). Older processors that don't recognise the SC
>> field should treat this as hwsync.
>
> qemu (7.1) emulating ppc64e does not :/
>
>   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
>   mpic: ISU size: 256, shift: 8, mask: ff
>   mpic: Initializing for 256 sources
>   Oops: Exception in kernel mode, sig: 4 [#1]
..
>
> I guess just put it behind an #ifdef 64S.

That doesn't work because qemu emulating a G5 also doesn't accept it.

So either we need to get qemu updated and wait a while for that to
percolate, or do some runtime patching of wmbs in the kernel >_<

cheers

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Nicholas Piggin]

On Wed Jun 14, 2023 at 3:56 PM AEST, Michael Ellerman wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
> > Nicholas Piggin <npiggin@gmail.com> writes:
> >> The most expensive ordering for hwsync to provide is the store-load
> >> barrier, because all prior stores have to be drained to the caches
> >> before subsequent instructions can complete.
> >>
> >> stsync just orders stores which means it can just be a barrer that
> >> goes down the store queue and orders draining, and does not prevent
> >> completion of subsequent instructions. So it should be faster than
> >> hwsync.
> >>
> >> Use stsync for wmb(). Older processors that don't recognise the SC
> >> field should treat this as hwsync.
> >
> > qemu (7.1) emulating ppc64e does not :/
> >
> >   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
> >   mpic: ISU size: 256, shift: 8, mask: ff
> >   mpic: Initializing for 256 sources
> >   Oops: Exception in kernel mode, sig: 4 [#1]
> ..
> >
> > I guess just put it behind an #ifdef 64S.
>
> That doesn't work because qemu emulating a G5 also doesn't accept it.
>
> So either we need to get qemu updated and wait a while for that to
> percolate, or do some runtime patching of wmbs in the kernel >_<

Gah, sorry. QEMU really should be ignoring reserved fields in
instructions :(

I guess leave it out for now. Should fix QEMU but we probably also need
to do patching so as not to break older QEMUs.

Thanks,
Nick

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Michael Ellerman]

"Nicholas Piggin" <npiggin@gmail.com> writes:
> On Wed Jun 14, 2023 at 3:56 PM AEST, Michael Ellerman wrote:
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>> > Nicholas Piggin <npiggin@gmail.com> writes:
>> >> The most expensive ordering for hwsync to provide is the store-load
>> >> barrier, because all prior stores have to be drained to the caches
>> >> before subsequent instructions can complete.
>> >>
>> >> stsync just orders stores which means it can just be a barrer that
>> >> goes down the store queue and orders draining, and does not prevent
>> >> completion of subsequent instructions. So it should be faster than
>> >> hwsync.
>> >>
>> >> Use stsync for wmb(). Older processors that don't recognise the SC
>> >> field should treat this as hwsync.
>> >
>> > qemu (7.1) emulating ppc64e does not :/
>> >
>> >   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
>> >   mpic: ISU size: 256, shift: 8, mask: ff
>> >   mpic: Initializing for 256 sources
>> >   Oops: Exception in kernel mode, sig: 4 [#1]
>> ..
>> >
>> > I guess just put it behind an #ifdef 64S.
>>
>> That doesn't work because qemu emulating a G5 also doesn't accept it.
>>
>> So either we need to get qemu updated and wait a while for that to
>> percolate, or do some runtime patching of wmbs in the kernel >_<
>
> Gah, sorry. QEMU really should be ignoring reserved fields in
> instructions :(

Yeah, it's an annoying discrepancy vs real hardware and the ISA.

> I guess leave it out for now. Should fix QEMU but we probably also need
> to do patching so as not to break older QEMUs.

I'll plan to take the first 3 patches, they seem OK as-is.

cheers

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Michael Ellerman]

Michael Ellerman <mpe@ellerman.id.au> writes:
> "Nicholas Piggin" <npiggin@gmail.com> writes:
>> On Wed Jun 14, 2023 at 3:56 PM AEST, Michael Ellerman wrote:
>>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>> > Nicholas Piggin <npiggin@gmail.com> writes:
>>> >> The most expensive ordering for hwsync to provide is the store-load
>>> >> barrier, because all prior stores have to be drained to the caches
>>> >> before subsequent instructions can complete.
>>> >>
>>> >> stsync just orders stores which means it can just be a barrer that
>>> >> goes down the store queue and orders draining, and does not prevent
>>> >> completion of subsequent instructions. So it should be faster than
>>> >> hwsync.
>>> >>
>>> >> Use stsync for wmb(). Older processors that don't recognise the SC
>>> >> field should treat this as hwsync.
>>> >
>>> > qemu (7.1) emulating ppc64e does not :/
>>> >
>>> >   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
>>> >   mpic: ISU size: 256, shift: 8, mask: ff
>>> >   mpic: Initializing for 256 sources
>>> >   Oops: Exception in kernel mode, sig: 4 [#1]
>>> ..
>>> >
>>> > I guess just put it behind an #ifdef 64S.
>>>
>>> That doesn't work because qemu emulating a G5 also doesn't accept it.
>>>
>>> So either we need to get qemu updated and wait a while for that to
>>> percolate, or do some runtime patching of wmbs in the kernel >_<
>>
>> Gah, sorry. QEMU really should be ignoring reserved fields in
>> instructions :(
>
> Yeah, it's an annoying discrepancy vs real hardware and the ISA.
>
>> I guess leave it out for now. Should fix QEMU but we probably also need
>> to do patching so as not to break older QEMUs.
>
> I'll plan to take the first 3 patches, they seem OK as-is.

I didn't do that in the end, because patch 2 suffers from the same
problem of not working on QEMU.

cheers

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Michael Ellerman]

Michael Ellerman <mpe@ellerman.id.au> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> "Nicholas Piggin" <npiggin@gmail.com> writes:
>>> On Wed Jun 14, 2023 at 3:56 PM AEST, Michael Ellerman wrote:
>>>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>>> > Nicholas Piggin <npiggin@gmail.com> writes:
>>>> >> The most expensive ordering for hwsync to provide is the store-load
>>>> >> barrier, because all prior stores have to be drained to the caches
>>>> >> before subsequent instructions can complete.
>>>> >>
>>>> >> stsync just orders stores which means it can just be a barrer that
>>>> >> goes down the store queue and orders draining, and does not prevent
>>>> >> completion of subsequent instructions. So it should be faster than
>>>> >> hwsync.
>>>> >>
>>>> >> Use stsync for wmb(). Older processors that don't recognise the SC
>>>> >> field should treat this as hwsync.
>>>> >
>>>> > qemu (7.1) emulating ppc64e does not :/
>>>> >
>>>> >   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
>>>> >   mpic: ISU size: 256, shift: 8, mask: ff
>>>> >   mpic: Initializing for 256 sources
>>>> >   Oops: Exception in kernel mode, sig: 4 [#1]
>>>> ..
>>>> >
>>>> > I guess just put it behind an #ifdef 64S.
>>>>
>>>> That doesn't work because qemu emulating a G5 also doesn't accept it.
>>>>
>>>> So either we need to get qemu updated and wait a while for that to
>>>> percolate, or do some runtime patching of wmbs in the kernel >_<
>>>
>>> Gah, sorry. QEMU really should be ignoring reserved fields in
>>> instructions :(
>>
>> Yeah, it's an annoying discrepancy vs real hardware and the ISA.
>>
>>> I guess leave it out for now. Should fix QEMU but we probably also need
>>> to do patching so as not to break older QEMUs.
>>
>> I'll plan to take the first 3 patches, they seem OK as-is.
>
> I didn't do that in the end, because patch 2 suffers from the same
                                             ^
                                             3
> problem of not working on QEMU.
>
> cheers

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Joel Stanley]

On Thu, 24 Aug 2023 at 12:12, Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Michael Ellerman <mpe@ellerman.id.au> writes:
> > Michael Ellerman <mpe@ellerman.id.au> writes:
> >> "Nicholas Piggin" <npiggin@gmail.com> writes:
> >>> On Wed Jun 14, 2023 at 3:56 PM AEST, Michael Ellerman wrote:
> >>>> Michael Ellerman <mpe@ellerman.id.au> writes:
> >>>> > Nicholas Piggin <npiggin@gmail.com> writes:
> >>>> >> The most expensive ordering for hwsync to provide is the store-load
> >>>> >> barrier, because all prior stores have to be drained to the caches
> >>>> >> before subsequent instructions can complete.
> >>>> >>
> >>>> >> stsync just orders stores which means it can just be a barrer that
> >>>> >> goes down the store queue and orders draining, and does not prevent
> >>>> >> completion of subsequent instructions. So it should be faster than
> >>>> >> hwsync.
> >>>> >>
> >>>> >> Use stsync for wmb(). Older processors that don't recognise the SC
> >>>> >> field should treat this as hwsync.
> >>>> >
> >>>> > qemu (7.1) emulating ppc64e does not :/
> >>>> >
> >>>> >   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
> >>>> >   mpic: ISU size: 256, shift: 8, mask: ff
> >>>> >   mpic: Initializing for 256 sources
> >>>> >   Oops: Exception in kernel mode, sig: 4 [#1]
> >>>> ..
> >>>> >
> >>>> > I guess just put it behind an #ifdef 64S.
> >>>>
> >>>> That doesn't work because qemu emulating a G5 also doesn't accept it.
> >>>>
> >>>> So either we need to get qemu updated and wait a while for that to
> >>>> percolate, or do some runtime patching of wmbs in the kernel >_<
> >>>
> >>> Gah, sorry. QEMU really should be ignoring reserved fields in
> >>> instructions :(
> >>
> >> Yeah, it's an annoying discrepancy vs real hardware and the ISA.
> >>
> >>> I guess leave it out for now. Should fix QEMU but we probably also need
> >>> to do patching so as not to break older QEMUs.
> >>
> >> I'll plan to take the first 3 patches, they seem OK as-is.
> >
> > I didn't do that in the end, because patch 2 suffers from the same
>                                              ^
>                                              3
> > problem of not working on QEMU.

Did we get a patch to fix this in to Qemu?

Qemu has recently developed a stable tree process, so if we had a
backportable fix we could get it in there too.

Cheers,

Joel

Re: [PATCH 4/4] powerpc/64s: Use POWER10 stsync barrier for wmb()

[Michael Ellerman]

Joel Stanley <joel@jms.id.au> writes:
> On Thu, 24 Aug 2023 at 12:12, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>> > Michael Ellerman <mpe@ellerman.id.au> writes:
>> >> "Nicholas Piggin" <npiggin@gmail.com> writes:
>> >>> On Wed Jun 14, 2023 at 3:56 PM AEST, Michael Ellerman wrote:
>> >>>> Michael Ellerman <mpe@ellerman.id.au> writes:
>> >>>> > Nicholas Piggin <npiggin@gmail.com> writes:
>> >>>> >> The most expensive ordering for hwsync to provide is the store-load
>> >>>> >> barrier, because all prior stores have to be drained to the caches
>> >>>> >> before subsequent instructions can complete.
>> >>>> >>
>> >>>> >> stsync just orders stores which means it can just be a barrer that
>> >>>> >> goes down the store queue and orders draining, and does not prevent
>> >>>> >> completion of subsequent instructions. So it should be faster than
>> >>>> >> hwsync.
>> >>>> >>
>> >>>> >> Use stsync for wmb(). Older processors that don't recognise the SC
>> >>>> >> field should treat this as hwsync.
>> >>>> >
>> >>>> > qemu (7.1) emulating ppc64e does not :/
>> >>>> >
>> >>>> >   mpic: Setting up MPIC " OpenPIC  " version 1.2 at fe0040000, max 1 CPUs
>> >>>> >   mpic: ISU size: 256, shift: 8, mask: ff
>> >>>> >   mpic: Initializing for 256 sources
>> >>>> >   Oops: Exception in kernel mode, sig: 4 [#1]
>> >>>> ..
>> >>>> >
>> >>>> > I guess just put it behind an #ifdef 64S.
>> >>>>
>> >>>> That doesn't work because qemu emulating a G5 also doesn't accept it.
>> >>>>
>> >>>> So either we need to get qemu updated and wait a while for that to
>> >>>> percolate, or do some runtime patching of wmbs in the kernel >_<
>> >>>
>> >>> Gah, sorry. QEMU really should be ignoring reserved fields in
>> >>> instructions :(
>> >>
>> >> Yeah, it's an annoying discrepancy vs real hardware and the ISA.
>> >>
>> >>> I guess leave it out for now. Should fix QEMU but we probably also need
>> >>> to do patching so as not to break older QEMUs.
>> >>
>> >> I'll plan to take the first 3 patches, they seem OK as-is.
>> >
>> > I didn't do that in the end, because patch 2 suffers from the same
>>                                              ^
>>                                              3
>> > problem of not working on QEMU.
>
> Did we get a patch to fix this in to Qemu?

No. Nick might have looked at it but he hasn't posted anything AFAIK.

cheers