[PATCH 0/2] riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup

[PATCH 0/2] riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The early version of T-Head C9xx cores has a store merge buffer
delay problem. The store merge buffer could improve the store queue
performance by merging multi-store requests, but when there are not
continued store requests, the prior single store request would be
waiting in the store queue for a long time. That would cause
significant problems for communication between multi-cores. This
problem was found on sg2042 & th1520 platforms with the qspinlock
lock torture test.

The orignal patch is from:
https://lore.kernel.org/linux-riscv/20231225125847.2778638-5-guoren@kernel.org/

Guo Ren (2):
  riscv: Move vendor errata definitions into vendorid_list.h
  riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup

 arch/riscv/Kconfig.errata              | 19 +++++++++++++++
 arch/riscv/errata/thead/errata.c       | 20 ++++++++++++++++
 arch/riscv/include/asm/errata_list.h   | 17 -------------
 arch/riscv/include/asm/rwonce.h        | 33 ++++++++++++++++++++++++++
 arch/riscv/include/asm/vendorid_list.h | 18 ++++++++++++++
 include/asm-generic/rwonce.h           |  2 ++
 6 files changed, 92 insertions(+), 17 deletions(-)
 create mode 100644 arch/riscv/include/asm/rwonce.h

-- 
2.40.1

[PATCH 1/2] riscv: Move vendor errata definitions into vendorid_list.h

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

Move vendor errata definitions into vendorid_list and make it re-useable
for other header files.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
 arch/riscv/include/asm/errata_list.h   | 17 -----------------
 arch/riscv/include/asm/vendorid_list.h | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 7c8a71a526a3..589a3ebe2ae2 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -11,23 +11,6 @@
 #include <asm/hwcap.h>
 #include <asm/vendorid_list.h>
 
-#ifdef CONFIG_ERRATA_ANDES
-#define ERRATA_ANDES_NO_IOCP 0
-#define ERRATA_ANDES_NUMBER 1
-#endif
-
-#ifdef CONFIG_ERRATA_SIFIVE
-#define	ERRATA_SIFIVE_CIP_453 0
-#define	ERRATA_SIFIVE_CIP_1200 1
-#define	ERRATA_SIFIVE_NUMBER 2
-#endif
-
-#ifdef CONFIG_ERRATA_THEAD
-#define	ERRATA_THEAD_MAE 0
-#define	ERRATA_THEAD_PMU 1
-#define	ERRATA_THEAD_NUMBER 2
-#endif
-
 #ifdef __ASSEMBLY__
 
 #define ALT_INSN_FAULT(x)						\
diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
index 2f2bb0c84f9a..7a387368633a 100644
--- a/arch/riscv/include/asm/vendorid_list.h
+++ b/arch/riscv/include/asm/vendorid_list.h
@@ -9,4 +9,21 @@
 #define SIFIVE_VENDOR_ID	0x489
 #define THEAD_VENDOR_ID		0x5b7
 
+#ifdef CONFIG_ERRATA_ANDES
+#define ERRATA_ANDES_NO_IOCP 0
+#define ERRATA_ANDES_NUMBER 1
+#endif
+
+#ifdef CONFIG_ERRATA_SIFIVE
+#define	ERRATA_SIFIVE_CIP_453 0
+#define	ERRATA_SIFIVE_CIP_1200 1
+#define	ERRATA_SIFIVE_NUMBER 2
+#endif
+
+#ifdef CONFIG_ERRATA_THEAD
+#define	ERRATA_THEAD_MAE 0
+#define	ERRATA_THEAD_PMU 1
+#define	ERRATA_THEAD_NUMBER 2
+#endif
+
 #endif
-- 
2.40.1

[PATCH 2/2] riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup

[guoren]

From: Guo Ren <guoren@linux.alibaba.com>

The early version of T-Head C9xx cores has a store merge buffer
delay problem. The store merge buffer could improve the store queue
performance by merging multi-store requests, but when there are not
continued store requests, the prior single store request would be
waiting in the store queue for a long time. That would cause
significant problems for communication between multi-cores. This
problem was found on sg2042 & th1520 platforms with the qspinlock
lock torture test.

So appending a fence w.o could immediately flush the store merge
buffer and let other cores see the write result.

This will apply the WRITE_ONCE errata to handle the non-standard
behavior via appending a fence w.o instruction for WRITE_ONCE().

This problem is only observed on the sg2042 hardware platform by
running the lock_torture test program for half an hour. The problem
was not found in the user space application, because interrupt can
break the livelock.

Reviewed-by: Leonardo Bras <leobras@redhat.com>
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
 arch/riscv/Kconfig.errata              | 19 +++++++++++++++
 arch/riscv/errata/thead/errata.c       | 20 ++++++++++++++++
 arch/riscv/include/asm/rwonce.h        | 33 ++++++++++++++++++++++++++
 arch/riscv/include/asm/vendorid_list.h |  3 ++-
 include/asm-generic/rwonce.h           |  2 ++
 5 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/rwonce.h

diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
index 2acc7d876e1f..fd6ba85b7a72 100644
--- a/arch/riscv/Kconfig.errata
+++ b/arch/riscv/Kconfig.errata
@@ -119,4 +119,23 @@ config ERRATA_THEAD_PMU
 
 	  If you don't know what to do here, say "Y".
 
+config ERRATA_THEAD_WRITE_ONCE
+	bool "Apply T-Head WRITE_ONCE errata"
+	depends on ERRATA_THEAD
+	default y
+	help
+	  The early version of T-Head C9xx cores of sg2042 & th1520 have a store
+	  merge buffer delay problem. The store merge buffer could improve the
+	  store queue performance by merging multi-store requests, but when there
+	  are no continued store requests, the prior single store request would be
+	  waiting in the store queue for a long time. That would cause signifi-
+	  cant problems for communication between multi-cores. Appending a
+	  fence w.o could immediately flush the store merge buffer and let other
+	  cores see the write result.
+
+	  This will apply the WRITE_ONCE errata to handle the non-standard beh-
+	  avior via appending a fence w.o instruction for WRITE_ONCE().
+
+	  If you don't know what to do here, say "Y".
+
 endmenu # "CPU errata selection"
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
index e24770a77932..dd41f0221fe9 100644
--- a/arch/riscv/errata/thead/errata.c
+++ b/arch/riscv/errata/thead/errata.c
@@ -142,6 +142,23 @@ static bool errata_probe_pmu(unsigned int stage,
 	return true;
 }
 
+static bool errata_probe_write_once(unsigned int stage,
+				    unsigned long arch_id, unsigned long impid)
+{
+	if (!IS_ENABLED(CONFIG_ERRATA_THEAD_WRITE_ONCE))
+		return false;
+
+	/* target-c9xx cores report arch_id and impid as 0 */
+	if (arch_id != 0 || impid != 0)
+		return false;
+
+	if (stage == RISCV_ALTERNATIVES_BOOT ||
+	    stage == RISCV_ALTERNATIVES_MODULE)
+		return true;
+
+	return false;
+}
+
 static u32 thead_errata_probe(unsigned int stage,
 			      unsigned long archid, unsigned long impid)
 {
@@ -155,6 +172,9 @@ static u32 thead_errata_probe(unsigned int stage,
 	if (errata_probe_pmu(stage, archid, impid))
 		cpu_req_errata |= BIT(ERRATA_THEAD_PMU);
 
+	if (errata_probe_write_once(stage, archid, impid))
+		cpu_req_errata |= BIT(ERRATA_THEAD_WRITE_ONCE);
+
 	return cpu_req_errata;
 }
 
diff --git a/arch/riscv/include/asm/rwonce.h b/arch/riscv/include/asm/rwonce.h
new file mode 100644
index 000000000000..756586746a4f
--- /dev/null
+++ b/arch/riscv/include/asm/rwonce.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ASM_RWONCE_H
+#define __ASM_RWONCE_H
+
+#include <linux/compiler_types.h>
+#include <asm/alternative-macros.h>
+#include <asm/vendorid_list.h>
+
+#if defined(CONFIG_ERRATA_THEAD_WRITE_ONCE) && !defined(NO_ALTERNATIVE)
+
+#define write_once_fence()				\
+do {							\
+	asm volatile(ALTERNATIVE(			\
+		"nop",					\
+		"fence w, o",				\
+		THEAD_VENDOR_ID,			\
+		ERRATA_THEAD_WRITE_ONCE,		\
+		CONFIG_ERRATA_THEAD_WRITE_ONCE)		\
+		: : : "memory");			\
+} while (0)
+
+#define __WRITE_ONCE(x, val)				\
+do {							\
+	*(volatile typeof(x) *)&(x) = (val);		\
+	write_once_fence();				\
+} while (0)
+
+#endif /* defined(CONFIG_ERRATA_THEAD_WRITE_ONCE) && !defined(NO_ALTERNATIVE) */
+
+#include <asm-generic/rwonce.h>
+
+#endif	/* __ASM_RWONCE_H */
diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
index 7a387368633a..d5a6e3963feb 100644
--- a/arch/riscv/include/asm/vendorid_list.h
+++ b/arch/riscv/include/asm/vendorid_list.h
@@ -23,7 +23,8 @@
 #ifdef CONFIG_ERRATA_THEAD
 #define	ERRATA_THEAD_MAE 0
 #define	ERRATA_THEAD_PMU 1
-#define	ERRATA_THEAD_NUMBER 2
+#define	ERRATA_THEAD_WRITE_ONCE 2
+#define	ERRATA_THEAD_NUMBER 3
 #endif
 
 #endif
diff --git a/include/asm-generic/rwonce.h b/include/asm-generic/rwonce.h
index 8d0a6280e982..fb07fe8c6e45 100644
--- a/include/asm-generic/rwonce.h
+++ b/include/asm-generic/rwonce.h
@@ -50,10 +50,12 @@
 	__READ_ONCE(x);							\
 })
 
+#ifndef __WRITE_ONCE
 #define __WRITE_ONCE(x, val)						\
 do {									\
 	*(volatile typeof(x) *)&(x) = (val);				\
 } while (0)
+#endif
 
 #define WRITE_ONCE(x, val)						\
 do {									\
-- 
2.40.1

Re: [PATCH 0/2] riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup

[Inochi Amaoto]

On Sat, Dec 14, 2024 at 09:30:37AM -0500, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> The early version of T-Head C9xx cores has a store merge buffer
> delay problem. The store merge buffer could improve the store queue
> performance by merging multi-store requests, but when there are not
> continued store requests, the prior single store request would be
> waiting in the store queue for a long time. That would cause
> significant problems for communication between multi-cores. This
> problem was found on sg2042 & th1520 platforms with the qspinlock
> lock torture test.
> 
> The orignal patch is from:
> https://lore.kernel.org/linux-riscv/20231225125847.2778638-5-guoren@kernel.org/
> 
> Guo Ren (2):
>   riscv: Move vendor errata definitions into vendorid_list.h
>   riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup
> 
>  arch/riscv/Kconfig.errata              | 19 +++++++++++++++
>  arch/riscv/errata/thead/errata.c       | 20 ++++++++++++++++
>  arch/riscv/include/asm/errata_list.h   | 17 -------------
>  arch/riscv/include/asm/rwonce.h        | 33 ++++++++++++++++++++++++++
>  arch/riscv/include/asm/vendorid_list.h | 18 ++++++++++++++
>  include/asm-generic/rwonce.h           |  2 ++
>  6 files changed, 92 insertions(+), 17 deletions(-)
>  create mode 100644 arch/riscv/include/asm/rwonce.h
> 
> -- 
> 2.40.1
> 

It works on SG2042.

Tested-by: Inochi Amaoto <inochiama@gmail.com>

Re: [PATCH 1/2] riscv: Move vendor errata definitions into vendorid_list.h

[Charlie Jenkins]

On Sat, Dec 14, 2024 at 09:30:38AM -0500, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> Move vendor errata definitions into vendorid_list and make it re-useable
> for other header files.

Why can't errata_list.h be included wherever the errata definitions are
needed?

- Charlie

Re: [PATCH 1/2] riscv: Move vendor errata definitions into vendorid_list.h

[Guo Ren]

On Sat, Jan 18, 2025 at 4:27 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> On Sat, Dec 14, 2024 at 09:30:38AM -0500, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Move vendor errata definitions into vendorid_list and make it re-useable
> > for other header files.
>
> Why can't errata_list.h be included wherever the errata definitions are
> needed?
errata_list.h can't be included in rwonce.h.

>
> - Charlie
>


-- 
Best Regards
 Guo Ren

Re: [PATCH 1/2] riscv: Move vendor errata definitions into vendorid_list.h

[Alexandre Ghiti]

Hi Guo,

On 18/01/2025 04:46, Guo Ren wrote:
> On Sat, Jan 18, 2025 at 4:27 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
>> On Sat, Dec 14, 2024 at 09:30:38AM -0500, guoren@kernel.org wrote:
>>> From: Guo Ren <guoren@linux.alibaba.com>
>>>
>>> Move vendor errata definitions into vendorid_list and make it re-useable
>>> for other header files.
>> Why can't errata_list.h be included wherever the errata definitions are
>> needed?
> errata_list.h can't be included in rwonce.h.


vendorid_list.h does not seem appropriate, I think the best solution 
would be to fix the header issue and if not simply possible, introduce a 
new header.

Thanks,

Alex


>
>> - Charlie
>>
>