All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-02 14:10 ` Christoph Muellner
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Muellner @ 2022-06-02 14:10 UTC (permalink / raw)
  To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Heiko Stuebner, Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel

The current RISC-V code uses the generic ticket lock implementation,
that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
Currently, RISC-V uses the generic implementation of these macros.
This patch introduces a RISC-V specific implementation, of these
macros, that peels off the first loop iteration and modifies the waiting
loop such, that it is possible to use the WRS instruction of the Zawrs
ISA extension to stall the CPU.

The resulting implementation of smp_cond_load_*() will only work for
32-bit or 64-bit types for RV64 and 32-bit types for RV32.
This is caused by the restrictions of the LR instruction (RISC-V only
has LR.W and LR.D). Compiler assertions guard this new restriction.

This patch uses the existing RISC-V ISA extension framework
to detect the presents of Zawrs at run-time.
If available a NOP instruction will be replaced by WRS.
A similar patch could add support for the PAUSE instruction of
the Zihintpause ISA extension.

The whole mechanism is gated by Kconfig setting, which defaults to Y.

The Zawrs specification can be found here:
https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc

Note, that the Zawrs extension is not frozen or ratified yet.
Therefore this patch is an RFC and not intended to get merged.

Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
---
 arch/riscv/Kconfig                   | 10 +++
 arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
 arch/riscv/include/asm/errata_list.h | 12 +++-
 arch/riscv/include/asm/hwcap.h       |  3 +-
 arch/riscv/kernel/cpu.c              |  1 +
 arch/riscv/kernel/cpufeature.c       | 13 ++++
 6 files changed, 133 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 905e550e0fd3..054872317d4a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -358,6 +358,16 @@ config RISCV_ISA_C
 
 	   If you don't know what to do here, say Y.
 
+config RISCV_ISA_ZAWRS
+	bool "Zawrs extension support"
+	select RISCV_ALTERNATIVE
+	default y
+	help
+	   Adds support to dynamically detect the presence of the Zawrs extension
+	   (wait for reservation set) and enable its usage.
+
+	   If you don't know what to do here, say Y.
+
 config RISCV_ISA_SVPBMT
 	bool "SVPBMT extension support"
 	depends on 64BIT && MMU
diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index d0e24aaa2aa0..69b8f1f4b80c 100644
--- a/arch/riscv/include/asm/barrier.h
+++ b/arch/riscv/include/asm/barrier.h
@@ -12,6 +12,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <asm/errata_list.h>
+
 #define nop()		__asm__ __volatile__ ("nop")
 
 #define RISCV_FENCE(p, s) \
@@ -42,6 +44,69 @@ do {									\
 	___p1;								\
 })
 
+#if __riscv_xlen == 64
+
+#define __riscv_lrsc_word(t)						\
+	(sizeof(t) == sizeof(int) ||					\
+	 sizeof(t) == sizeof(long))
+
+#define __riscv_lr(ptr)							\
+	sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
+
+#elif __riscv_xlen == 32
+
+#define __riscv_lrsc_word(ptr)						\
+	(sizeof(*ptr) == sizeof(int))
+
+#define __riscv_lr(t) "lr.w"
+
+#else
+#error "Unexpected __riscv_xlen"
+#endif /* __riscv_xlen */
+
+#define compiletime_assert_atomic_lrsc_type(t)				\
+	compiletime_assert(__riscv_lrsc_word(t),			\
+		"Need type compatible with LR/SC instructions.")
+
+#define ___smp_load_reservedN(pfx, ptr)					\
+({									\
+	typeof(*ptr) ___p1;						\
+	__asm__ __volatile__ ("lr." pfx "	%[p], %[c]\n"		\
+			      : [p]"=&r" (___p1), [c]"+A"(*ptr));	\
+	___p1;								\
+})
+
+#define ___smp_load_reserved32(ptr)					\
+	___smp_load_reservedN("w", ptr)
+
+#define ___smp_load_reserved64(ptr)					\
+	___smp_load_reservedN("d", ptr)
+
+#define __smp_load_reserved_relaxed(ptr)				\
+({									\
+	typeof(*ptr) ___p1;						\
+	compiletime_assert_atomic_lrsc_type(*ptr);			\
+	if (sizeof(*ptr) == 32) {					\
+		___p1 = ___smp_load_reserved32(ptr);			\
+	} else {							\
+		___p1 = ___smp_load_reserved64(ptr);			\
+	}								\
+	___p1;								\
+})
+
+#define __smp_load_reserved_acquire(ptr)				\
+({									\
+	typeof(*ptr) ___p1;						\
+	compiletime_assert_atomic_lrsc_type(*ptr);			\
+	if (sizeof(*ptr) == 32) {					\
+		___p1 = ___smp_load_reserved32(ptr);			\
+	} else {							\
+		___p1 = ___smp_load_reserved64(ptr);			\
+	}								\
+	RISCV_FENCE(r,rw);						\
+	___p1;								\
+})
+
 /*
  * This is a very specific barrier: it's currently only used in two places in
  * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
@@ -69,6 +134,38 @@ do {									\
  */
 #define smp_mb__after_spinlock()	RISCV_FENCE(iorw,iorw)
 
+#define smp_cond_load_relaxed(ptr, cond_expr)				\
+({									\
+	typeof(ptr) __PTR = (ptr);					\
+	__unqual_scalar_typeof(*ptr) VAL;				\
+	VAL = READ_ONCE(*__PTR);					\
+	if (!cond_expr) {						\
+		for (;;) {						\
+			VAL = __smp_load_reserved_relaxed(__PTR);	\
+			if (cond_expr)					\
+				break;					\
+			ALT_WRS();					\
+		}							\
+	}								\
+	(typeof(*ptr))VAL;						\
+})
+
+#define smp_cond_load_acquire(ptr, cond_expr)				\
+({									\
+	typeof(ptr) __PTR = (ptr);					\
+	__unqual_scalar_typeof(*ptr) VAL;				\
+	VAL = smp_load_acquire(__PTR);					\
+	if (!cond_expr) {						\
+		for (;;) {						\
+			VAL = __smp_load_reserved_acquire(__PTR);	\
+			if (cond_expr)					\
+				break;					\
+			ALT_WRS();					\
+		}							\
+	}								\
+	(typeof(*ptr))VAL;						\
+})
+
 #include <asm-generic/barrier.h>
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 9e2888dbb5b1..b9aa0b346493 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -19,8 +19,9 @@
 #define	ERRATA_THEAD_NUMBER 1
 #endif
 
-#define	CPUFEATURE_SVPBMT 0
-#define	CPUFEATURE_NUMBER 1
+#define	CPUFEATURE_ZAWRS 0
+#define	CPUFEATURE_SVPBMT 1
+#define	CPUFEATURE_NUMBER 2
 
 #ifdef __ASSEMBLY__
 
@@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,	\
 		ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)	\
 		: : "r" (addr) : "memory")
 
+#define ZAWRS_WRS	".long 0x1000073"
+#define ALT_WRS()							\
+asm volatile(ALTERNATIVE(						\
+	"nop\n\t",							\
+	ZAWRS_WRS "\n\t",						\
+	0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
+
 /*
  * _val is marked as "will be overwritten", so need to set it to 0
  * in the default case.
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 4e2486881840..c7dd8cc38bec 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
  * available logical extension id.
  */
 enum riscv_isa_ext_id {
-	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
+	RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
+	RISCV_ISA_EXT_SSCOFPMF,
 	RISCV_ISA_EXT_SVPBMT,
 	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
 };
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index fba9e9f46a8c..6c3a10ff5358 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
  *    extensions by an underscore.
  */
 static struct riscv_isa_ext_data isa_ext_arr[] = {
+	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
 	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
 	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index dea3ea19deee..fc2c47a1784b 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
 			} else {
 				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
 				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
+				SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
 			}
 #undef SET_ISA_EXT_MAP
 		}
@@ -251,6 +252,14 @@ struct cpufeature_info {
 	bool (*check_func)(unsigned int stage);
 };
 
+static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
+{
+	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
+		return false;
+
+	return riscv_isa_extension_available(NULL, ZAWRS);
+}
+
 static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
 {
 #ifdef CONFIG_RISCV_ISA_SVPBMT
@@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
 
 static const struct cpufeature_info __initdata_or_module
 cpufeature_list[CPUFEATURE_NUMBER] = {
+	{
+		.name = "zawrs",
+		.check_func = cpufeature_zawrs_check_func
+	},
 	{
 		.name = "svpbmt",
 		.check_func = cpufeature_svpbmt_check_func
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-02 14:10 ` Christoph Muellner
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Muellner @ 2022-06-02 14:10 UTC (permalink / raw)
  To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Heiko Stuebner, Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel

The current RISC-V code uses the generic ticket lock implementation,
that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
Currently, RISC-V uses the generic implementation of these macros.
This patch introduces a RISC-V specific implementation, of these
macros, that peels off the first loop iteration and modifies the waiting
loop such, that it is possible to use the WRS instruction of the Zawrs
ISA extension to stall the CPU.

The resulting implementation of smp_cond_load_*() will only work for
32-bit or 64-bit types for RV64 and 32-bit types for RV32.
This is caused by the restrictions of the LR instruction (RISC-V only
has LR.W and LR.D). Compiler assertions guard this new restriction.

This patch uses the existing RISC-V ISA extension framework
to detect the presents of Zawrs at run-time.
If available a NOP instruction will be replaced by WRS.
A similar patch could add support for the PAUSE instruction of
the Zihintpause ISA extension.

The whole mechanism is gated by Kconfig setting, which defaults to Y.

The Zawrs specification can be found here:
https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc

Note, that the Zawrs extension is not frozen or ratified yet.
Therefore this patch is an RFC and not intended to get merged.

Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
---
 arch/riscv/Kconfig                   | 10 +++
 arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
 arch/riscv/include/asm/errata_list.h | 12 +++-
 arch/riscv/include/asm/hwcap.h       |  3 +-
 arch/riscv/kernel/cpu.c              |  1 +
 arch/riscv/kernel/cpufeature.c       | 13 ++++
 6 files changed, 133 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 905e550e0fd3..054872317d4a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -358,6 +358,16 @@ config RISCV_ISA_C
 
 	   If you don't know what to do here, say Y.
 
+config RISCV_ISA_ZAWRS
+	bool "Zawrs extension support"
+	select RISCV_ALTERNATIVE
+	default y
+	help
+	   Adds support to dynamically detect the presence of the Zawrs extension
+	   (wait for reservation set) and enable its usage.
+
+	   If you don't know what to do here, say Y.
+
 config RISCV_ISA_SVPBMT
 	bool "SVPBMT extension support"
 	depends on 64BIT && MMU
diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index d0e24aaa2aa0..69b8f1f4b80c 100644
--- a/arch/riscv/include/asm/barrier.h
+++ b/arch/riscv/include/asm/barrier.h
@@ -12,6 +12,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <asm/errata_list.h>
+
 #define nop()		__asm__ __volatile__ ("nop")
 
 #define RISCV_FENCE(p, s) \
@@ -42,6 +44,69 @@ do {									\
 	___p1;								\
 })
 
+#if __riscv_xlen == 64
+
+#define __riscv_lrsc_word(t)						\
+	(sizeof(t) == sizeof(int) ||					\
+	 sizeof(t) == sizeof(long))
+
+#define __riscv_lr(ptr)							\
+	sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
+
+#elif __riscv_xlen == 32
+
+#define __riscv_lrsc_word(ptr)						\
+	(sizeof(*ptr) == sizeof(int))
+
+#define __riscv_lr(t) "lr.w"
+
+#else
+#error "Unexpected __riscv_xlen"
+#endif /* __riscv_xlen */
+
+#define compiletime_assert_atomic_lrsc_type(t)				\
+	compiletime_assert(__riscv_lrsc_word(t),			\
+		"Need type compatible with LR/SC instructions.")
+
+#define ___smp_load_reservedN(pfx, ptr)					\
+({									\
+	typeof(*ptr) ___p1;						\
+	__asm__ __volatile__ ("lr." pfx "	%[p], %[c]\n"		\
+			      : [p]"=&r" (___p1), [c]"+A"(*ptr));	\
+	___p1;								\
+})
+
+#define ___smp_load_reserved32(ptr)					\
+	___smp_load_reservedN("w", ptr)
+
+#define ___smp_load_reserved64(ptr)					\
+	___smp_load_reservedN("d", ptr)
+
+#define __smp_load_reserved_relaxed(ptr)				\
+({									\
+	typeof(*ptr) ___p1;						\
+	compiletime_assert_atomic_lrsc_type(*ptr);			\
+	if (sizeof(*ptr) == 32) {					\
+		___p1 = ___smp_load_reserved32(ptr);			\
+	} else {							\
+		___p1 = ___smp_load_reserved64(ptr);			\
+	}								\
+	___p1;								\
+})
+
+#define __smp_load_reserved_acquire(ptr)				\
+({									\
+	typeof(*ptr) ___p1;						\
+	compiletime_assert_atomic_lrsc_type(*ptr);			\
+	if (sizeof(*ptr) == 32) {					\
+		___p1 = ___smp_load_reserved32(ptr);			\
+	} else {							\
+		___p1 = ___smp_load_reserved64(ptr);			\
+	}								\
+	RISCV_FENCE(r,rw);						\
+	___p1;								\
+})
+
 /*
  * This is a very specific barrier: it's currently only used in two places in
  * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
@@ -69,6 +134,38 @@ do {									\
  */
 #define smp_mb__after_spinlock()	RISCV_FENCE(iorw,iorw)
 
+#define smp_cond_load_relaxed(ptr, cond_expr)				\
+({									\
+	typeof(ptr) __PTR = (ptr);					\
+	__unqual_scalar_typeof(*ptr) VAL;				\
+	VAL = READ_ONCE(*__PTR);					\
+	if (!cond_expr) {						\
+		for (;;) {						\
+			VAL = __smp_load_reserved_relaxed(__PTR);	\
+			if (cond_expr)					\
+				break;					\
+			ALT_WRS();					\
+		}							\
+	}								\
+	(typeof(*ptr))VAL;						\
+})
+
+#define smp_cond_load_acquire(ptr, cond_expr)				\
+({									\
+	typeof(ptr) __PTR = (ptr);					\
+	__unqual_scalar_typeof(*ptr) VAL;				\
+	VAL = smp_load_acquire(__PTR);					\
+	if (!cond_expr) {						\
+		for (;;) {						\
+			VAL = __smp_load_reserved_acquire(__PTR);	\
+			if (cond_expr)					\
+				break;					\
+			ALT_WRS();					\
+		}							\
+	}								\
+	(typeof(*ptr))VAL;						\
+})
+
 #include <asm-generic/barrier.h>
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 9e2888dbb5b1..b9aa0b346493 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -19,8 +19,9 @@
 #define	ERRATA_THEAD_NUMBER 1
 #endif
 
-#define	CPUFEATURE_SVPBMT 0
-#define	CPUFEATURE_NUMBER 1
+#define	CPUFEATURE_ZAWRS 0
+#define	CPUFEATURE_SVPBMT 1
+#define	CPUFEATURE_NUMBER 2
 
 #ifdef __ASSEMBLY__
 
@@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,	\
 		ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)	\
 		: : "r" (addr) : "memory")
 
+#define ZAWRS_WRS	".long 0x1000073"
+#define ALT_WRS()							\
+asm volatile(ALTERNATIVE(						\
+	"nop\n\t",							\
+	ZAWRS_WRS "\n\t",						\
+	0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
+
 /*
  * _val is marked as "will be overwritten", so need to set it to 0
  * in the default case.
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 4e2486881840..c7dd8cc38bec 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
  * available logical extension id.
  */
 enum riscv_isa_ext_id {
-	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
+	RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
+	RISCV_ISA_EXT_SSCOFPMF,
 	RISCV_ISA_EXT_SVPBMT,
 	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
 };
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index fba9e9f46a8c..6c3a10ff5358 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
  *    extensions by an underscore.
  */
 static struct riscv_isa_ext_data isa_ext_arr[] = {
+	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
 	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
 	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index dea3ea19deee..fc2c47a1784b 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
 			} else {
 				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
 				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
+				SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
 			}
 #undef SET_ISA_EXT_MAP
 		}
@@ -251,6 +252,14 @@ struct cpufeature_info {
 	bool (*check_func)(unsigned int stage);
 };
 
+static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
+{
+	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
+		return false;
+
+	return riscv_isa_extension_available(NULL, ZAWRS);
+}
+
 static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
 {
 #ifdef CONFIG_RISCV_ISA_SVPBMT
@@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
 
 static const struct cpufeature_info __initdata_or_module
 cpufeature_list[CPUFEATURE_NUMBER] = {
+	{
+		.name = "zawrs",
+		.check_func = cpufeature_zawrs_check_func
+	},
 	{
 		.name = "svpbmt",
 		.check_func = cpufeature_svpbmt_check_func
-- 
2.35.3


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
  2022-06-02 14:10 ` Christoph Muellner
@ 2022-06-02 14:21   ` Heiko Stübner
  -1 siblings, 0 replies; 15+ messages in thread
From: Heiko Stübner @ 2022-06-02 14:21 UTC (permalink / raw)
  To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Philipp Tomsich, Aaron Durbin, linux-riscv, linux-kernel,
	Christoph Muellner

Am Donnerstag, 2. Juni 2022, 16:10:32 CEST schrieb Christoph Muellner:
> The current RISC-V code uses the generic ticket lock implementation,
> that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> Currently, RISC-V uses the generic implementation of these macros.
> This patch introduces a RISC-V specific implementation, of these
> macros, that peels off the first loop iteration and modifies the waiting
> loop such, that it is possible to use the WRS instruction of the Zawrs
> ISA extension to stall the CPU.
> 
> The resulting implementation of smp_cond_load_*() will only work for
> 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> This is caused by the restrictions of the LR instruction (RISC-V only
> has LR.W and LR.D). Compiler assertions guard this new restriction.
> 
> This patch uses the existing RISC-V ISA extension framework
> to detect the presents of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.
> A similar patch could add support for the PAUSE instruction of
> the Zihintpause ISA extension.
> 
> The whole mechanism is gated by Kconfig setting, which defaults to Y.
> 
> The Zawrs specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> 
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>

I'm not sure what happened here, but I got the patch 2 times
(16:06 and 16:10 CEST).

response to the other patch:

cool to see the cpufeature patching get a new member :-) .

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

together with the qemu-patch:
Tested-by: Heiko Stuebner <heiko@sntech.de>



> ---
>  arch/riscv/Kconfig                   | 10 +++
>  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/errata_list.h | 12 +++-
>  arch/riscv/include/asm/hwcap.h       |  3 +-
>  arch/riscv/kernel/cpu.c              |  1 +
>  arch/riscv/kernel/cpufeature.c       | 13 ++++
>  6 files changed, 133 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 905e550e0fd3..054872317d4a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -358,6 +358,16 @@ config RISCV_ISA_C
>  
>  	   If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_ZAWRS
> +	bool "Zawrs extension support"
> +	select RISCV_ALTERNATIVE
> +	default y
> +	help
> +	   Adds support to dynamically detect the presence of the Zawrs extension
> +	   (wait for reservation set) and enable its usage.
> +
> +	   If you don't know what to do here, say Y.
> +
>  config RISCV_ISA_SVPBMT
>  	bool "SVPBMT extension support"
>  	depends on 64BIT && MMU
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index d0e24aaa2aa0..69b8f1f4b80c 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -12,6 +12,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#include <asm/errata_list.h>
> +
>  #define nop()		__asm__ __volatile__ ("nop")
>  
>  #define RISCV_FENCE(p, s) \
> @@ -42,6 +44,69 @@ do {									\
>  	___p1;								\
>  })
>  
> +#if __riscv_xlen == 64
> +
> +#define __riscv_lrsc_word(t)						\
> +	(sizeof(t) == sizeof(int) ||					\
> +	 sizeof(t) == sizeof(long))
> +
> +#define __riscv_lr(ptr)							\
> +	sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> +
> +#elif __riscv_xlen == 32
> +
> +#define __riscv_lrsc_word(ptr)						\
> +	(sizeof(*ptr) == sizeof(int))
> +
> +#define __riscv_lr(t) "lr.w"
> +
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif /* __riscv_xlen */
> +
> +#define compiletime_assert_atomic_lrsc_type(t)				\
> +	compiletime_assert(__riscv_lrsc_word(t),			\
> +		"Need type compatible with LR/SC instructions.")
> +
> +#define ___smp_load_reservedN(pfx, ptr)					\
> +({									\
> +	typeof(*ptr) ___p1;						\
> +	__asm__ __volatile__ ("lr." pfx "	%[p], %[c]\n"		\
> +			      : [p]"=&r" (___p1), [c]"+A"(*ptr));	\
> +	___p1;								\
> +})
> +
> +#define ___smp_load_reserved32(ptr)					\
> +	___smp_load_reservedN("w", ptr)
> +
> +#define ___smp_load_reserved64(ptr)					\
> +	___smp_load_reservedN("d", ptr)
> +
> +#define __smp_load_reserved_relaxed(ptr)				\
> +({									\
> +	typeof(*ptr) ___p1;						\
> +	compiletime_assert_atomic_lrsc_type(*ptr);			\
> +	if (sizeof(*ptr) == 32) {					\
> +		___p1 = ___smp_load_reserved32(ptr);			\
> +	} else {							\
> +		___p1 = ___smp_load_reserved64(ptr);			\
> +	}								\
> +	___p1;								\
> +})
> +
> +#define __smp_load_reserved_acquire(ptr)				\
> +({									\
> +	typeof(*ptr) ___p1;						\
> +	compiletime_assert_atomic_lrsc_type(*ptr);			\
> +	if (sizeof(*ptr) == 32) {					\
> +		___p1 = ___smp_load_reserved32(ptr);			\
> +	} else {							\
> +		___p1 = ___smp_load_reserved64(ptr);			\
> +	}								\
> +	RISCV_FENCE(r,rw);						\
> +	___p1;								\
> +})
> +
>  /*
>   * This is a very specific barrier: it's currently only used in two places in
>   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> @@ -69,6 +134,38 @@ do {									\
>   */
>  #define smp_mb__after_spinlock()	RISCV_FENCE(iorw,iorw)
>  
> +#define smp_cond_load_relaxed(ptr, cond_expr)				\
> +({									\
> +	typeof(ptr) __PTR = (ptr);					\
> +	__unqual_scalar_typeof(*ptr) VAL;				\
> +	VAL = READ_ONCE(*__PTR);					\
> +	if (!cond_expr) {						\
> +		for (;;) {						\
> +			VAL = __smp_load_reserved_relaxed(__PTR);	\
> +			if (cond_expr)					\
> +				break;					\
> +			ALT_WRS();					\
> +		}							\
> +	}								\
> +	(typeof(*ptr))VAL;						\
> +})
> +
> +#define smp_cond_load_acquire(ptr, cond_expr)				\
> +({									\
> +	typeof(ptr) __PTR = (ptr);					\
> +	__unqual_scalar_typeof(*ptr) VAL;				\
> +	VAL = smp_load_acquire(__PTR);					\
> +	if (!cond_expr) {						\
> +		for (;;) {						\
> +			VAL = __smp_load_reserved_acquire(__PTR);	\
> +			if (cond_expr)					\
> +				break;					\
> +			ALT_WRS();					\
> +		}							\
> +	}								\
> +	(typeof(*ptr))VAL;						\
> +})
> +
>  #include <asm-generic/barrier.h>
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 9e2888dbb5b1..b9aa0b346493 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -19,8 +19,9 @@
>  #define	ERRATA_THEAD_NUMBER 1
>  #endif
>  
> -#define	CPUFEATURE_SVPBMT 0
> -#define	CPUFEATURE_NUMBER 1
> +#define	CPUFEATURE_ZAWRS 0
> +#define	CPUFEATURE_SVPBMT 1
> +#define	CPUFEATURE_NUMBER 2
>  
>  #ifdef __ASSEMBLY__
>  
> @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,	\
>  		ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)	\
>  		: : "r" (addr) : "memory")
>  
> +#define ZAWRS_WRS	".long 0x1000073"
> +#define ALT_WRS()							\
> +asm volatile(ALTERNATIVE(						\
> +	"nop\n\t",							\
> +	ZAWRS_WRS "\n\t",						\
> +	0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> +
>  /*
>   * _val is marked as "will be overwritten", so need to set it to 0
>   * in the default case.
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 4e2486881840..c7dd8cc38bec 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
>   * available logical extension id.
>   */
>  enum riscv_isa_ext_id {
> -	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> +	RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> +	RISCV_ISA_EXT_SSCOFPMF,
>  	RISCV_ISA_EXT_SVPBMT,
>  	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
>  };
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..6c3a10ff5358 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
>   *    extensions by an underscore.
>   */
>  static struct riscv_isa_ext_data isa_ext_arr[] = {
> +	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
>  	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index dea3ea19deee..fc2c47a1784b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
>  			} else {
>  				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
>  				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> +				SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
>  			}
>  #undef SET_ISA_EXT_MAP
>  		}
> @@ -251,6 +252,14 @@ struct cpufeature_info {
>  	bool (*check_func)(unsigned int stage);
>  };
>  
> +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> +{
> +	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> +		return false;
> +
> +	return riscv_isa_extension_available(NULL, ZAWRS);
> +}
> +
>  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>  {
>  #ifdef CONFIG_RISCV_ISA_SVPBMT
> @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>  
>  static const struct cpufeature_info __initdata_or_module
>  cpufeature_list[CPUFEATURE_NUMBER] = {
> +	{
> +		.name = "zawrs",
> +		.check_func = cpufeature_zawrs_check_func
> +	},
>  	{
>  		.name = "svpbmt",
>  		.check_func = cpufeature_svpbmt_check_func
> 





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-02 14:21   ` Heiko Stübner
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Stübner @ 2022-06-02 14:21 UTC (permalink / raw)
  To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Philipp Tomsich, Aaron Durbin, linux-riscv, linux-kernel,
	Christoph Muellner

Am Donnerstag, 2. Juni 2022, 16:10:32 CEST schrieb Christoph Muellner:
> The current RISC-V code uses the generic ticket lock implementation,
> that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> Currently, RISC-V uses the generic implementation of these macros.
> This patch introduces a RISC-V specific implementation, of these
> macros, that peels off the first loop iteration and modifies the waiting
> loop such, that it is possible to use the WRS instruction of the Zawrs
> ISA extension to stall the CPU.
> 
> The resulting implementation of smp_cond_load_*() will only work for
> 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> This is caused by the restrictions of the LR instruction (RISC-V only
> has LR.W and LR.D). Compiler assertions guard this new restriction.
> 
> This patch uses the existing RISC-V ISA extension framework
> to detect the presents of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.
> A similar patch could add support for the PAUSE instruction of
> the Zihintpause ISA extension.
> 
> The whole mechanism is gated by Kconfig setting, which defaults to Y.
> 
> The Zawrs specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> 
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>

I'm not sure what happened here, but I got the patch 2 times
(16:06 and 16:10 CEST).

response to the other patch:

cool to see the cpufeature patching get a new member :-) .

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

together with the qemu-patch:
Tested-by: Heiko Stuebner <heiko@sntech.de>



> ---
>  arch/riscv/Kconfig                   | 10 +++
>  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/errata_list.h | 12 +++-
>  arch/riscv/include/asm/hwcap.h       |  3 +-
>  arch/riscv/kernel/cpu.c              |  1 +
>  arch/riscv/kernel/cpufeature.c       | 13 ++++
>  6 files changed, 133 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 905e550e0fd3..054872317d4a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -358,6 +358,16 @@ config RISCV_ISA_C
>  
>  	   If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_ZAWRS
> +	bool "Zawrs extension support"
> +	select RISCV_ALTERNATIVE
> +	default y
> +	help
> +	   Adds support to dynamically detect the presence of the Zawrs extension
> +	   (wait for reservation set) and enable its usage.
> +
> +	   If you don't know what to do here, say Y.
> +
>  config RISCV_ISA_SVPBMT
>  	bool "SVPBMT extension support"
>  	depends on 64BIT && MMU
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index d0e24aaa2aa0..69b8f1f4b80c 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -12,6 +12,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#include <asm/errata_list.h>
> +
>  #define nop()		__asm__ __volatile__ ("nop")
>  
>  #define RISCV_FENCE(p, s) \
> @@ -42,6 +44,69 @@ do {									\
>  	___p1;								\
>  })
>  
> +#if __riscv_xlen == 64
> +
> +#define __riscv_lrsc_word(t)						\
> +	(sizeof(t) == sizeof(int) ||					\
> +	 sizeof(t) == sizeof(long))
> +
> +#define __riscv_lr(ptr)							\
> +	sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> +
> +#elif __riscv_xlen == 32
> +
> +#define __riscv_lrsc_word(ptr)						\
> +	(sizeof(*ptr) == sizeof(int))
> +
> +#define __riscv_lr(t) "lr.w"
> +
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif /* __riscv_xlen */
> +
> +#define compiletime_assert_atomic_lrsc_type(t)				\
> +	compiletime_assert(__riscv_lrsc_word(t),			\
> +		"Need type compatible with LR/SC instructions.")
> +
> +#define ___smp_load_reservedN(pfx, ptr)					\
> +({									\
> +	typeof(*ptr) ___p1;						\
> +	__asm__ __volatile__ ("lr." pfx "	%[p], %[c]\n"		\
> +			      : [p]"=&r" (___p1), [c]"+A"(*ptr));	\
> +	___p1;								\
> +})
> +
> +#define ___smp_load_reserved32(ptr)					\
> +	___smp_load_reservedN("w", ptr)
> +
> +#define ___smp_load_reserved64(ptr)					\
> +	___smp_load_reservedN("d", ptr)
> +
> +#define __smp_load_reserved_relaxed(ptr)				\
> +({									\
> +	typeof(*ptr) ___p1;						\
> +	compiletime_assert_atomic_lrsc_type(*ptr);			\
> +	if (sizeof(*ptr) == 32) {					\
> +		___p1 = ___smp_load_reserved32(ptr);			\
> +	} else {							\
> +		___p1 = ___smp_load_reserved64(ptr);			\
> +	}								\
> +	___p1;								\
> +})
> +
> +#define __smp_load_reserved_acquire(ptr)				\
> +({									\
> +	typeof(*ptr) ___p1;						\
> +	compiletime_assert_atomic_lrsc_type(*ptr);			\
> +	if (sizeof(*ptr) == 32) {					\
> +		___p1 = ___smp_load_reserved32(ptr);			\
> +	} else {							\
> +		___p1 = ___smp_load_reserved64(ptr);			\
> +	}								\
> +	RISCV_FENCE(r,rw);						\
> +	___p1;								\
> +})
> +
>  /*
>   * This is a very specific barrier: it's currently only used in two places in
>   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> @@ -69,6 +134,38 @@ do {									\
>   */
>  #define smp_mb__after_spinlock()	RISCV_FENCE(iorw,iorw)
>  
> +#define smp_cond_load_relaxed(ptr, cond_expr)				\
> +({									\
> +	typeof(ptr) __PTR = (ptr);					\
> +	__unqual_scalar_typeof(*ptr) VAL;				\
> +	VAL = READ_ONCE(*__PTR);					\
> +	if (!cond_expr) {						\
> +		for (;;) {						\
> +			VAL = __smp_load_reserved_relaxed(__PTR);	\
> +			if (cond_expr)					\
> +				break;					\
> +			ALT_WRS();					\
> +		}							\
> +	}								\
> +	(typeof(*ptr))VAL;						\
> +})
> +
> +#define smp_cond_load_acquire(ptr, cond_expr)				\
> +({									\
> +	typeof(ptr) __PTR = (ptr);					\
> +	__unqual_scalar_typeof(*ptr) VAL;				\
> +	VAL = smp_load_acquire(__PTR);					\
> +	if (!cond_expr) {						\
> +		for (;;) {						\
> +			VAL = __smp_load_reserved_acquire(__PTR);	\
> +			if (cond_expr)					\
> +				break;					\
> +			ALT_WRS();					\
> +		}							\
> +	}								\
> +	(typeof(*ptr))VAL;						\
> +})
> +
>  #include <asm-generic/barrier.h>
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 9e2888dbb5b1..b9aa0b346493 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -19,8 +19,9 @@
>  #define	ERRATA_THEAD_NUMBER 1
>  #endif
>  
> -#define	CPUFEATURE_SVPBMT 0
> -#define	CPUFEATURE_NUMBER 1
> +#define	CPUFEATURE_ZAWRS 0
> +#define	CPUFEATURE_SVPBMT 1
> +#define	CPUFEATURE_NUMBER 2
>  
>  #ifdef __ASSEMBLY__
>  
> @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,	\
>  		ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)	\
>  		: : "r" (addr) : "memory")
>  
> +#define ZAWRS_WRS	".long 0x1000073"
> +#define ALT_WRS()							\
> +asm volatile(ALTERNATIVE(						\
> +	"nop\n\t",							\
> +	ZAWRS_WRS "\n\t",						\
> +	0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> +
>  /*
>   * _val is marked as "will be overwritten", so need to set it to 0
>   * in the default case.
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 4e2486881840..c7dd8cc38bec 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
>   * available logical extension id.
>   */
>  enum riscv_isa_ext_id {
> -	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> +	RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> +	RISCV_ISA_EXT_SSCOFPMF,
>  	RISCV_ISA_EXT_SVPBMT,
>  	RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
>  };
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..6c3a10ff5358 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
>   *    extensions by an underscore.
>   */
>  static struct riscv_isa_ext_data isa_ext_arr[] = {
> +	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
>  	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>  	__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>  	__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index dea3ea19deee..fc2c47a1784b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
>  			} else {
>  				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
>  				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> +				SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
>  			}
>  #undef SET_ISA_EXT_MAP
>  		}
> @@ -251,6 +252,14 @@ struct cpufeature_info {
>  	bool (*check_func)(unsigned int stage);
>  };
>  
> +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> +{
> +	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> +		return false;
> +
> +	return riscv_isa_extension_available(NULL, ZAWRS);
> +}
> +
>  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>  {
>  #ifdef CONFIG_RISCV_ISA_SVPBMT
> @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>  
>  static const struct cpufeature_info __initdata_or_module
>  cpufeature_list[CPUFEATURE_NUMBER] = {
> +	{
> +		.name = "zawrs",
> +		.check_func = cpufeature_zawrs_check_func
> +	},
>  	{
>  		.name = "svpbmt",
>  		.check_func = cpufeature_svpbmt_check_func
> 





_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
  2022-06-02 14:21   ` Heiko Stübner
@ 2022-06-02 14:24     ` Christoph Müllner
  -1 siblings, 0 replies; 15+ messages in thread
From: Christoph Müllner @ 2022-06-02 14:24 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Philipp Tomsich, Aaron Durbin, linux-riscv, linux-kernel

On Thu, Jun 2, 2022 at 4:21 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Donnerstag, 2. Juni 2022, 16:10:32 CEST schrieb Christoph Muellner:
> > The current RISC-V code uses the generic ticket lock implementation,
> > that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Currently, RISC-V uses the generic implementation of these macros.
> > This patch introduces a RISC-V specific implementation, of these
> > macros, that peels off the first loop iteration and modifies the waiting
> > loop such, that it is possible to use the WRS instruction of the Zawrs
> > ISA extension to stall the CPU.
> >
> > The resulting implementation of smp_cond_load_*() will only work for
> > 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> > This is caused by the restrictions of the LR instruction (RISC-V only
> > has LR.W and LR.D). Compiler assertions guard this new restriction.
> >
> > This patch uses the existing RISC-V ISA extension framework
> > to detect the presents of Zawrs at run-time.
> > If available a NOP instruction will be replaced by WRS.
> > A similar patch could add support for the PAUSE instruction of
> > the Zihintpause ISA extension.
> >
> > The whole mechanism is gated by Kconfig setting, which defaults to Y.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
>
> I'm not sure what happened here, but I got the patch 2 times
> (16:06 and 16:10 CEST).

I forgot to add the list on the first attempt.
Sorry for the confusion.

>
> response to the other patch:
>
> cool to see the cpufeature patching get a new member :-) .
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> together with the qemu-patch:
> Tested-by: Heiko Stuebner <heiko@sntech.de>

Thanks!


>
>
>
> > ---
> >  arch/riscv/Kconfig                   | 10 +++
> >  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/errata_list.h | 12 +++-
> >  arch/riscv/include/asm/hwcap.h       |  3 +-
> >  arch/riscv/kernel/cpu.c              |  1 +
> >  arch/riscv/kernel/cpufeature.c       | 13 ++++
> >  6 files changed, 133 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 905e550e0fd3..054872317d4a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -358,6 +358,16 @@ config RISCV_ISA_C
> >
> >          If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > +     bool "Zawrs extension support"
> > +     select RISCV_ALTERNATIVE
> > +     default y
> > +     help
> > +        Adds support to dynamically detect the presence of the Zawrs extension
> > +        (wait for reservation set) and enable its usage.
> > +
> > +        If you don't know what to do here, say Y.
> > +
> >  config RISCV_ISA_SVPBMT
> >       bool "SVPBMT extension support"
> >       depends on 64BIT && MMU
> > diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> > index d0e24aaa2aa0..69b8f1f4b80c 100644
> > --- a/arch/riscv/include/asm/barrier.h
> > +++ b/arch/riscv/include/asm/barrier.h
> > @@ -12,6 +12,8 @@
> >
> >  #ifndef __ASSEMBLY__
> >
> > +#include <asm/errata_list.h>
> > +
> >  #define nop()                __asm__ __volatile__ ("nop")
> >
> >  #define RISCV_FENCE(p, s) \
> > @@ -42,6 +44,69 @@ do {                                                                       \
> >       ___p1;                                                          \
> >  })
> >
> > +#if __riscv_xlen == 64
> > +
> > +#define __riscv_lrsc_word(t)                                         \
> > +     (sizeof(t) == sizeof(int) ||                                    \
> > +      sizeof(t) == sizeof(long))
> > +
> > +#define __riscv_lr(ptr)                                                      \
> > +     sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> > +
> > +#elif __riscv_xlen == 32
> > +
> > +#define __riscv_lrsc_word(ptr)                                               \
> > +     (sizeof(*ptr) == sizeof(int))
> > +
> > +#define __riscv_lr(t) "lr.w"
> > +
> > +#else
> > +#error "Unexpected __riscv_xlen"
> > +#endif /* __riscv_xlen */
> > +
> > +#define compiletime_assert_atomic_lrsc_type(t)                               \
> > +     compiletime_assert(__riscv_lrsc_word(t),                        \
> > +             "Need type compatible with LR/SC instructions.")
> > +
> > +#define ___smp_load_reservedN(pfx, ptr)                                      \
> > +({                                                                   \
> > +     typeof(*ptr) ___p1;                                             \
> > +     __asm__ __volatile__ ("lr." pfx "       %[p], %[c]\n"           \
> > +                           : [p]"=&r" (___p1), [c]"+A"(*ptr));       \
> > +     ___p1;                                                          \
> > +})
> > +
> > +#define ___smp_load_reserved32(ptr)                                  \
> > +     ___smp_load_reservedN("w", ptr)
> > +
> > +#define ___smp_load_reserved64(ptr)                                  \
> > +     ___smp_load_reservedN("d", ptr)
> > +
> > +#define __smp_load_reserved_relaxed(ptr)                             \
> > +({                                                                   \
> > +     typeof(*ptr) ___p1;                                             \
> > +     compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +     if (sizeof(*ptr) == 32) {                                       \
> > +             ___p1 = ___smp_load_reserved32(ptr);                    \
> > +     } else {                                                        \
> > +             ___p1 = ___smp_load_reserved64(ptr);                    \
> > +     }                                                               \
> > +     ___p1;                                                          \
> > +})
> > +
> > +#define __smp_load_reserved_acquire(ptr)                             \
> > +({                                                                   \
> > +     typeof(*ptr) ___p1;                                             \
> > +     compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +     if (sizeof(*ptr) == 32) {                                       \
> > +             ___p1 = ___smp_load_reserved32(ptr);                    \
> > +     } else {                                                        \
> > +             ___p1 = ___smp_load_reserved64(ptr);                    \
> > +     }                                                               \
> > +     RISCV_FENCE(r,rw);                                              \
> > +     ___p1;                                                          \
> > +})
> > +
> >  /*
> >   * This is a very specific barrier: it's currently only used in two places in
> >   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> > @@ -69,6 +134,38 @@ do {                                                                      \
> >   */
> >  #define smp_mb__after_spinlock()     RISCV_FENCE(iorw,iorw)
> >
> > +#define smp_cond_load_relaxed(ptr, cond_expr)                                \
> > +({                                                                   \
> > +     typeof(ptr) __PTR = (ptr);                                      \
> > +     __unqual_scalar_typeof(*ptr) VAL;                               \
> > +     VAL = READ_ONCE(*__PTR);                                        \
> > +     if (!cond_expr) {                                               \
> > +             for (;;) {                                              \
> > +                     VAL = __smp_load_reserved_relaxed(__PTR);       \
> > +                     if (cond_expr)                                  \
> > +                             break;                                  \
> > +                     ALT_WRS();                                      \
> > +             }                                                       \
> > +     }                                                               \
> > +     (typeof(*ptr))VAL;                                              \
> > +})
> > +
> > +#define smp_cond_load_acquire(ptr, cond_expr)                                \
> > +({                                                                   \
> > +     typeof(ptr) __PTR = (ptr);                                      \
> > +     __unqual_scalar_typeof(*ptr) VAL;                               \
> > +     VAL = smp_load_acquire(__PTR);                                  \
> > +     if (!cond_expr) {                                               \
> > +             for (;;) {                                              \
> > +                     VAL = __smp_load_reserved_acquire(__PTR);       \
> > +                     if (cond_expr)                                  \
> > +                             break;                                  \
> > +                     ALT_WRS();                                      \
> > +             }                                                       \
> > +     }                                                               \
> > +     (typeof(*ptr))VAL;                                              \
> > +})
> > +
> >  #include <asm-generic/barrier.h>
> >
> >  #endif /* __ASSEMBLY__ */
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index 9e2888dbb5b1..b9aa0b346493 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -19,8 +19,9 @@
> >  #define      ERRATA_THEAD_NUMBER 1
> >  #endif
> >
> > -#define      CPUFEATURE_SVPBMT 0
> > -#define      CPUFEATURE_NUMBER 1
> > +#define      CPUFEATURE_ZAWRS 0
> > +#define      CPUFEATURE_SVPBMT 1
> > +#define      CPUFEATURE_NUMBER 2
> >
> >  #ifdef __ASSEMBLY__
> >
> > @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,   \
> >               ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)  \
> >               : : "r" (addr) : "memory")
> >
> > +#define ZAWRS_WRS    ".long 0x1000073"
> > +#define ALT_WRS()                                                    \
> > +asm volatile(ALTERNATIVE(                                            \
> > +     "nop\n\t",                                                      \
> > +     ZAWRS_WRS "\n\t",                                               \
> > +     0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> > +
> >  /*
> >   * _val is marked as "will be overwritten", so need to set it to 0
> >   * in the default case.
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 4e2486881840..c7dd8cc38bec 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
> >   * available logical extension id.
> >   */
> >  enum riscv_isa_ext_id {
> > -     RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> > +     RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> > +     RISCV_ISA_EXT_SSCOFPMF,
> >       RISCV_ISA_EXT_SVPBMT,
> >       RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> >  };
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fba9e9f46a8c..6c3a10ff5358 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
> >   *    extensions by an underscore.
> >   */
> >  static struct riscv_isa_ext_data isa_ext_arr[] = {
> > +     __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> >       __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> >       __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >       __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index dea3ea19deee..fc2c47a1784b 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
> >                       } else {
> >                               SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> >                               SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > +                             SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
> >                       }
> >  #undef SET_ISA_EXT_MAP
> >               }
> > @@ -251,6 +252,14 @@ struct cpufeature_info {
> >       bool (*check_func)(unsigned int stage);
> >  };
> >
> > +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> > +{
> > +     if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > +             return false;
> > +
> > +     return riscv_isa_extension_available(NULL, ZAWRS);
> > +}
> > +
> >  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >  {
> >  #ifdef CONFIG_RISCV_ISA_SVPBMT
> > @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >
> >  static const struct cpufeature_info __initdata_or_module
> >  cpufeature_list[CPUFEATURE_NUMBER] = {
> > +     {
> > +             .name = "zawrs",
> > +             .check_func = cpufeature_zawrs_check_func
> > +     },
> >       {
> >               .name = "svpbmt",
> >               .check_func = cpufeature_svpbmt_check_func
> >
>
>
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-02 14:24     ` Christoph Müllner
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Müllner @ 2022-06-02 14:24 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Philipp Tomsich, Aaron Durbin, linux-riscv, linux-kernel

On Thu, Jun 2, 2022 at 4:21 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Donnerstag, 2. Juni 2022, 16:10:32 CEST schrieb Christoph Muellner:
> > The current RISC-V code uses the generic ticket lock implementation,
> > that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Currently, RISC-V uses the generic implementation of these macros.
> > This patch introduces a RISC-V specific implementation, of these
> > macros, that peels off the first loop iteration and modifies the waiting
> > loop such, that it is possible to use the WRS instruction of the Zawrs
> > ISA extension to stall the CPU.
> >
> > The resulting implementation of smp_cond_load_*() will only work for
> > 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> > This is caused by the restrictions of the LR instruction (RISC-V only
> > has LR.W and LR.D). Compiler assertions guard this new restriction.
> >
> > This patch uses the existing RISC-V ISA extension framework
> > to detect the presents of Zawrs at run-time.
> > If available a NOP instruction will be replaced by WRS.
> > A similar patch could add support for the PAUSE instruction of
> > the Zihintpause ISA extension.
> >
> > The whole mechanism is gated by Kconfig setting, which defaults to Y.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
>
> I'm not sure what happened here, but I got the patch 2 times
> (16:06 and 16:10 CEST).

I forgot to add the list on the first attempt.
Sorry for the confusion.

>
> response to the other patch:
>
> cool to see the cpufeature patching get a new member :-) .
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> together with the qemu-patch:
> Tested-by: Heiko Stuebner <heiko@sntech.de>

Thanks!


>
>
>
> > ---
> >  arch/riscv/Kconfig                   | 10 +++
> >  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/errata_list.h | 12 +++-
> >  arch/riscv/include/asm/hwcap.h       |  3 +-
> >  arch/riscv/kernel/cpu.c              |  1 +
> >  arch/riscv/kernel/cpufeature.c       | 13 ++++
> >  6 files changed, 133 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 905e550e0fd3..054872317d4a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -358,6 +358,16 @@ config RISCV_ISA_C
> >
> >          If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > +     bool "Zawrs extension support"
> > +     select RISCV_ALTERNATIVE
> > +     default y
> > +     help
> > +        Adds support to dynamically detect the presence of the Zawrs extension
> > +        (wait for reservation set) and enable its usage.
> > +
> > +        If you don't know what to do here, say Y.
> > +
> >  config RISCV_ISA_SVPBMT
> >       bool "SVPBMT extension support"
> >       depends on 64BIT && MMU
> > diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> > index d0e24aaa2aa0..69b8f1f4b80c 100644
> > --- a/arch/riscv/include/asm/barrier.h
> > +++ b/arch/riscv/include/asm/barrier.h
> > @@ -12,6 +12,8 @@
> >
> >  #ifndef __ASSEMBLY__
> >
> > +#include <asm/errata_list.h>
> > +
> >  #define nop()                __asm__ __volatile__ ("nop")
> >
> >  #define RISCV_FENCE(p, s) \
> > @@ -42,6 +44,69 @@ do {                                                                       \
> >       ___p1;                                                          \
> >  })
> >
> > +#if __riscv_xlen == 64
> > +
> > +#define __riscv_lrsc_word(t)                                         \
> > +     (sizeof(t) == sizeof(int) ||                                    \
> > +      sizeof(t) == sizeof(long))
> > +
> > +#define __riscv_lr(ptr)                                                      \
> > +     sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> > +
> > +#elif __riscv_xlen == 32
> > +
> > +#define __riscv_lrsc_word(ptr)                                               \
> > +     (sizeof(*ptr) == sizeof(int))
> > +
> > +#define __riscv_lr(t) "lr.w"
> > +
> > +#else
> > +#error "Unexpected __riscv_xlen"
> > +#endif /* __riscv_xlen */
> > +
> > +#define compiletime_assert_atomic_lrsc_type(t)                               \
> > +     compiletime_assert(__riscv_lrsc_word(t),                        \
> > +             "Need type compatible with LR/SC instructions.")
> > +
> > +#define ___smp_load_reservedN(pfx, ptr)                                      \
> > +({                                                                   \
> > +     typeof(*ptr) ___p1;                                             \
> > +     __asm__ __volatile__ ("lr." pfx "       %[p], %[c]\n"           \
> > +                           : [p]"=&r" (___p1), [c]"+A"(*ptr));       \
> > +     ___p1;                                                          \
> > +})
> > +
> > +#define ___smp_load_reserved32(ptr)                                  \
> > +     ___smp_load_reservedN("w", ptr)
> > +
> > +#define ___smp_load_reserved64(ptr)                                  \
> > +     ___smp_load_reservedN("d", ptr)
> > +
> > +#define __smp_load_reserved_relaxed(ptr)                             \
> > +({                                                                   \
> > +     typeof(*ptr) ___p1;                                             \
> > +     compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +     if (sizeof(*ptr) == 32) {                                       \
> > +             ___p1 = ___smp_load_reserved32(ptr);                    \
> > +     } else {                                                        \
> > +             ___p1 = ___smp_load_reserved64(ptr);                    \
> > +     }                                                               \
> > +     ___p1;                                                          \
> > +})
> > +
> > +#define __smp_load_reserved_acquire(ptr)                             \
> > +({                                                                   \
> > +     typeof(*ptr) ___p1;                                             \
> > +     compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +     if (sizeof(*ptr) == 32) {                                       \
> > +             ___p1 = ___smp_load_reserved32(ptr);                    \
> > +     } else {                                                        \
> > +             ___p1 = ___smp_load_reserved64(ptr);                    \
> > +     }                                                               \
> > +     RISCV_FENCE(r,rw);                                              \
> > +     ___p1;                                                          \
> > +})
> > +
> >  /*
> >   * This is a very specific barrier: it's currently only used in two places in
> >   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> > @@ -69,6 +134,38 @@ do {                                                                      \
> >   */
> >  #define smp_mb__after_spinlock()     RISCV_FENCE(iorw,iorw)
> >
> > +#define smp_cond_load_relaxed(ptr, cond_expr)                                \
> > +({                                                                   \
> > +     typeof(ptr) __PTR = (ptr);                                      \
> > +     __unqual_scalar_typeof(*ptr) VAL;                               \
> > +     VAL = READ_ONCE(*__PTR);                                        \
> > +     if (!cond_expr) {                                               \
> > +             for (;;) {                                              \
> > +                     VAL = __smp_load_reserved_relaxed(__PTR);       \
> > +                     if (cond_expr)                                  \
> > +                             break;                                  \
> > +                     ALT_WRS();                                      \
> > +             }                                                       \
> > +     }                                                               \
> > +     (typeof(*ptr))VAL;                                              \
> > +})
> > +
> > +#define smp_cond_load_acquire(ptr, cond_expr)                                \
> > +({                                                                   \
> > +     typeof(ptr) __PTR = (ptr);                                      \
> > +     __unqual_scalar_typeof(*ptr) VAL;                               \
> > +     VAL = smp_load_acquire(__PTR);                                  \
> > +     if (!cond_expr) {                                               \
> > +             for (;;) {                                              \
> > +                     VAL = __smp_load_reserved_acquire(__PTR);       \
> > +                     if (cond_expr)                                  \
> > +                             break;                                  \
> > +                     ALT_WRS();                                      \
> > +             }                                                       \
> > +     }                                                               \
> > +     (typeof(*ptr))VAL;                                              \
> > +})
> > +
> >  #include <asm-generic/barrier.h>
> >
> >  #endif /* __ASSEMBLY__ */
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index 9e2888dbb5b1..b9aa0b346493 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -19,8 +19,9 @@
> >  #define      ERRATA_THEAD_NUMBER 1
> >  #endif
> >
> > -#define      CPUFEATURE_SVPBMT 0
> > -#define      CPUFEATURE_NUMBER 1
> > +#define      CPUFEATURE_ZAWRS 0
> > +#define      CPUFEATURE_SVPBMT 1
> > +#define      CPUFEATURE_NUMBER 2
> >
> >  #ifdef __ASSEMBLY__
> >
> > @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,   \
> >               ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)  \
> >               : : "r" (addr) : "memory")
> >
> > +#define ZAWRS_WRS    ".long 0x1000073"
> > +#define ALT_WRS()                                                    \
> > +asm volatile(ALTERNATIVE(                                            \
> > +     "nop\n\t",                                                      \
> > +     ZAWRS_WRS "\n\t",                                               \
> > +     0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> > +
> >  /*
> >   * _val is marked as "will be overwritten", so need to set it to 0
> >   * in the default case.
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 4e2486881840..c7dd8cc38bec 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
> >   * available logical extension id.
> >   */
> >  enum riscv_isa_ext_id {
> > -     RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> > +     RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> > +     RISCV_ISA_EXT_SSCOFPMF,
> >       RISCV_ISA_EXT_SVPBMT,
> >       RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> >  };
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fba9e9f46a8c..6c3a10ff5358 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
> >   *    extensions by an underscore.
> >   */
> >  static struct riscv_isa_ext_data isa_ext_arr[] = {
> > +     __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> >       __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> >       __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >       __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index dea3ea19deee..fc2c47a1784b 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
> >                       } else {
> >                               SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> >                               SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > +                             SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
> >                       }
> >  #undef SET_ISA_EXT_MAP
> >               }
> > @@ -251,6 +252,14 @@ struct cpufeature_info {
> >       bool (*check_func)(unsigned int stage);
> >  };
> >
> > +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> > +{
> > +     if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > +             return false;
> > +
> > +     return riscv_isa_extension_available(NULL, ZAWRS);
> > +}
> > +
> >  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >  {
> >  #ifdef CONFIG_RISCV_ISA_SVPBMT
> > @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >
> >  static const struct cpufeature_info __initdata_or_module
> >  cpufeature_list[CPUFEATURE_NUMBER] = {
> > +     {
> > +             .name = "zawrs",
> > +             .check_func = cpufeature_zawrs_check_func
> > +     },
> >       {
> >               .name = "svpbmt",
> >               .check_func = cpufeature_svpbmt_check_func
> >
>
>
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
  2022-06-02 14:10 ` Christoph Muellner
@ 2022-06-02 16:24   ` Randy Dunlap
  -1 siblings, 0 replies; 15+ messages in thread
From: Randy Dunlap @ 2022-06-02 16:24 UTC (permalink / raw)
  To: Christoph Muellner, Palmer Dabbelt, Paul Walmsley, Albert Ou,
	Christoph Muellner, Heiko Stuebner, Philipp Tomsich,
	Aaron Durbin, linux-riscv, linux-kernel

Hi--

On 6/2/22 07:10, Christoph Muellner wrote:
> The current RISC-V code uses the generic ticket lock implementation,
> that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> Currently, RISC-V uses the generic implementation of these macros.
> This patch introduces a RISC-V specific implementation, of these
> macros, that peels off the first loop iteration and modifies the waiting
> loop such, that it is possible to use the WRS instruction of the Zawrs
> ISA extension to stall the CPU.
> 
> The resulting implementation of smp_cond_load_*() will only work for
> 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> This is caused by the restrictions of the LR instruction (RISC-V only
> has LR.W and LR.D). Compiler assertions guard this new restriction.
> 
> This patch uses the existing RISC-V ISA extension framework
> to detect the presents of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.
> A similar patch could add support for the PAUSE instruction of
> the Zihintpause ISA extension.
> 
> The whole mechanism is gated by Kconfig setting, which defaults to Y.
> 
> The Zawrs specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> 
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> ---
>  arch/riscv/Kconfig                   | 10 +++
>  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/errata_list.h | 12 +++-
>  arch/riscv/include/asm/hwcap.h       |  3 +-
>  arch/riscv/kernel/cpu.c              |  1 +
>  arch/riscv/kernel/cpufeature.c       | 13 ++++
>  6 files changed, 133 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 905e550e0fd3..054872317d4a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -358,6 +358,16 @@ config RISCV_ISA_C
>  
>  	   If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_ZAWRS
> +	bool "Zawrs extension support"
> +	select RISCV_ALTERNATIVE
> +	default y
> +	help
> +	   Adds support to dynamically detect the presence of the Zawrs extension
> +	   (wait for reservation set) and enable its usage.
> +
> +	   If you don't know what to do here, say Y.
> +

With this patch, it is possible to enable XIP_KERNEL and
RISCV_ISA_ZAWRS at the same time.  That causes a kconfig warning:

WARNING: unmet direct dependencies detected for RISCV_ALTERNATIVE
  Depends on [n]: !XIP_KERNEL [=y]
  Selected by [y]:
  - RISCV_ISA_ZAWRS [=y]
  - RISCV_ISA_SVPBMT [=y] && 64BIT [=y] && MMU [=y]

because RISCV_ALTERNATIVE depends on !XIP_KERNEL:

config RISCV_ALTERNATIVE
	bool
	depends on !XIP_KERNEL


>  config RISCV_ISA_SVPBMT
>  	bool "SVPBMT extension support"
>  	depends on 64BIT && MMU


-- 
~Randy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-02 16:24   ` Randy Dunlap
  0 siblings, 0 replies; 15+ messages in thread
From: Randy Dunlap @ 2022-06-02 16:24 UTC (permalink / raw)
  To: Christoph Muellner, Palmer Dabbelt, Paul Walmsley, Albert Ou,
	Christoph Muellner, Heiko Stuebner, Philipp Tomsich,
	Aaron Durbin, linux-riscv, linux-kernel

Hi--

On 6/2/22 07:10, Christoph Muellner wrote:
> The current RISC-V code uses the generic ticket lock implementation,
> that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> Currently, RISC-V uses the generic implementation of these macros.
> This patch introduces a RISC-V specific implementation, of these
> macros, that peels off the first loop iteration and modifies the waiting
> loop such, that it is possible to use the WRS instruction of the Zawrs
> ISA extension to stall the CPU.
> 
> The resulting implementation of smp_cond_load_*() will only work for
> 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> This is caused by the restrictions of the LR instruction (RISC-V only
> has LR.W and LR.D). Compiler assertions guard this new restriction.
> 
> This patch uses the existing RISC-V ISA extension framework
> to detect the presents of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.
> A similar patch could add support for the PAUSE instruction of
> the Zihintpause ISA extension.
> 
> The whole mechanism is gated by Kconfig setting, which defaults to Y.
> 
> The Zawrs specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> 
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> ---
>  arch/riscv/Kconfig                   | 10 +++
>  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/errata_list.h | 12 +++-
>  arch/riscv/include/asm/hwcap.h       |  3 +-
>  arch/riscv/kernel/cpu.c              |  1 +
>  arch/riscv/kernel/cpufeature.c       | 13 ++++
>  6 files changed, 133 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 905e550e0fd3..054872317d4a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -358,6 +358,16 @@ config RISCV_ISA_C
>  
>  	   If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_ZAWRS
> +	bool "Zawrs extension support"
> +	select RISCV_ALTERNATIVE
> +	default y
> +	help
> +	   Adds support to dynamically detect the presence of the Zawrs extension
> +	   (wait for reservation set) and enable its usage.
> +
> +	   If you don't know what to do here, say Y.
> +

With this patch, it is possible to enable XIP_KERNEL and
RISCV_ISA_ZAWRS at the same time.  That causes a kconfig warning:

WARNING: unmet direct dependencies detected for RISCV_ALTERNATIVE
  Depends on [n]: !XIP_KERNEL [=y]
  Selected by [y]:
  - RISCV_ISA_ZAWRS [=y]
  - RISCV_ISA_SVPBMT [=y] && 64BIT [=y] && MMU [=y]

because RISCV_ALTERNATIVE depends on !XIP_KERNEL:

config RISCV_ALTERNATIVE
	bool
	depends on !XIP_KERNEL


>  config RISCV_ISA_SVPBMT
>  	bool "SVPBMT extension support"
>  	depends on 64BIT && MMU


-- 
~Randy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
  2022-06-02 16:24   ` Randy Dunlap
@ 2022-06-02 16:32     ` Christoph Müllner
  -1 siblings, 0 replies; 15+ messages in thread
From: Christoph Müllner @ 2022-06-02 16:32 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Heiko Stuebner, Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel

On Thu, Jun 2, 2022 at 6:24 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi--
>
> On 6/2/22 07:10, Christoph Muellner wrote:
> > The current RISC-V code uses the generic ticket lock implementation,
> > that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Currently, RISC-V uses the generic implementation of these macros.
> > This patch introduces a RISC-V specific implementation, of these
> > macros, that peels off the first loop iteration and modifies the waiting
> > loop such, that it is possible to use the WRS instruction of the Zawrs
> > ISA extension to stall the CPU.
> >
> > The resulting implementation of smp_cond_load_*() will only work for
> > 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> > This is caused by the restrictions of the LR instruction (RISC-V only
> > has LR.W and LR.D). Compiler assertions guard this new restriction.
> >
> > This patch uses the existing RISC-V ISA extension framework
> > to detect the presents of Zawrs at run-time.
> > If available a NOP instruction will be replaced by WRS.
> > A similar patch could add support for the PAUSE instruction of
> > the Zihintpause ISA extension.
> >
> > The whole mechanism is gated by Kconfig setting, which defaults to Y.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> > ---
> >  arch/riscv/Kconfig                   | 10 +++
> >  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/errata_list.h | 12 +++-
> >  arch/riscv/include/asm/hwcap.h       |  3 +-
> >  arch/riscv/kernel/cpu.c              |  1 +
> >  arch/riscv/kernel/cpufeature.c       | 13 ++++
> >  6 files changed, 133 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 905e550e0fd3..054872317d4a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -358,6 +358,16 @@ config RISCV_ISA_C
> >
> >          If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > +     bool "Zawrs extension support"
> > +     select RISCV_ALTERNATIVE
> > +     default y
> > +     help
> > +        Adds support to dynamically detect the presence of the Zawrs extension
> > +        (wait for reservation set) and enable its usage.
> > +
> > +        If you don't know what to do here, say Y.
> > +
>
> With this patch, it is possible to enable XIP_KERNEL and
> RISCV_ISA_ZAWRS at the same time.  That causes a kconfig warning:
>
> WARNING: unmet direct dependencies detected for RISCV_ALTERNATIVE
>   Depends on [n]: !XIP_KERNEL [=y]
>   Selected by [y]:
>   - RISCV_ISA_ZAWRS [=y]
>   - RISCV_ISA_SVPBMT [=y] && 64BIT [=y] && MMU [=y]
>
> because RISCV_ALTERNATIVE depends on !XIP_KERNEL:
>
> config RISCV_ALTERNATIVE
>         bool
>         depends on !XIP_KERNEL

I will add a "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS.

Thanks!

>
>
> >  config RISCV_ISA_SVPBMT
> >       bool "SVPBMT extension support"
> >       depends on 64BIT && MMU
>
>
> --
> ~Randy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-02 16:32     ` Christoph Müllner
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Müllner @ 2022-06-02 16:32 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Heiko Stuebner, Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel

On Thu, Jun 2, 2022 at 6:24 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi--
>
> On 6/2/22 07:10, Christoph Muellner wrote:
> > The current RISC-V code uses the generic ticket lock implementation,
> > that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Currently, RISC-V uses the generic implementation of these macros.
> > This patch introduces a RISC-V specific implementation, of these
> > macros, that peels off the first loop iteration and modifies the waiting
> > loop such, that it is possible to use the WRS instruction of the Zawrs
> > ISA extension to stall the CPU.
> >
> > The resulting implementation of smp_cond_load_*() will only work for
> > 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> > This is caused by the restrictions of the LR instruction (RISC-V only
> > has LR.W and LR.D). Compiler assertions guard this new restriction.
> >
> > This patch uses the existing RISC-V ISA extension framework
> > to detect the presents of Zawrs at run-time.
> > If available a NOP instruction will be replaced by WRS.
> > A similar patch could add support for the PAUSE instruction of
> > the Zihintpause ISA extension.
> >
> > The whole mechanism is gated by Kconfig setting, which defaults to Y.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> > ---
> >  arch/riscv/Kconfig                   | 10 +++
> >  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/errata_list.h | 12 +++-
> >  arch/riscv/include/asm/hwcap.h       |  3 +-
> >  arch/riscv/kernel/cpu.c              |  1 +
> >  arch/riscv/kernel/cpufeature.c       | 13 ++++
> >  6 files changed, 133 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 905e550e0fd3..054872317d4a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -358,6 +358,16 @@ config RISCV_ISA_C
> >
> >          If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > +     bool "Zawrs extension support"
> > +     select RISCV_ALTERNATIVE
> > +     default y
> > +     help
> > +        Adds support to dynamically detect the presence of the Zawrs extension
> > +        (wait for reservation set) and enable its usage.
> > +
> > +        If you don't know what to do here, say Y.
> > +
>
> With this patch, it is possible to enable XIP_KERNEL and
> RISCV_ISA_ZAWRS at the same time.  That causes a kconfig warning:
>
> WARNING: unmet direct dependencies detected for RISCV_ALTERNATIVE
>   Depends on [n]: !XIP_KERNEL [=y]
>   Selected by [y]:
>   - RISCV_ISA_ZAWRS [=y]
>   - RISCV_ISA_SVPBMT [=y] && 64BIT [=y] && MMU [=y]
>
> because RISCV_ALTERNATIVE depends on !XIP_KERNEL:
>
> config RISCV_ALTERNATIVE
>         bool
>         depends on !XIP_KERNEL

I will add a "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS.

Thanks!

>
>
> >  config RISCV_ISA_SVPBMT
> >       bool "SVPBMT extension support"
> >       depends on 64BIT && MMU
>
>
> --
> ~Randy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
  2022-06-02 14:10 ` Christoph Muellner
                   ` (2 preceding siblings ...)
  (?)
@ 2022-06-04 22:20 ` kernel test robot
  -1 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-06-04 22:20 UTC (permalink / raw)
  To: Christoph Muellner; +Cc: llvm, kbuild-all

Hi Christoph,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linus/master]
[cannot apply to v5.18]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-Muellner/riscv-Add-Zawrs-support-for-spinlocks/20220602-221228
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d1dc87763f406d4e67caf16dbe438a5647692395
config: riscv-randconfig-c006-20220605 (https://download.01.org/0day-ci/archive/20220605/202206050633.lWua2j70-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0a90b72c432d70aae035727ece4ba80ce820f381)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/8bacf50e3f136bfbce3e648e6f53443516125ba2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christoph-Muellner/riscv-Add-Zawrs-support-for-spinlocks/20220602-221228
        git checkout 8bacf50e3f136bfbce3e648e6f53443516125ba2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash kernel/bpf/ kernel/futex/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> kernel/bpf/helpers.c:291:3: error: indirection requires pointer operand ('int' invalid)
                   atomic_cond_read_relaxed(l, !VAL);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/atomic.h:29:40: note: expanded from macro 'atomic_cond_read_relaxed'
   #define atomic_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/barrier.h:144:10: note: expanded from macro 'smp_cond_load_relaxed'
                           VAL = __smp_load_reserved_relaxed(__PTR);       \
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/barrier.h:88:2: note: expanded from macro '__smp_load_reserved_relaxed'
           compiletime_assert_atomic_lrsc_type(*ptr);                      \
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
   1 error generated.
--
>> kernel/futex/requeue.c:182:9: error: indirection requires pointer operand ('int' invalid)
                   (void)atomic_cond_read_relaxed(&q->requeue_state, VAL != Q_REQUEUE_PI_WAIT);
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/atomic.h:29:40: note: expanded from macro 'atomic_cond_read_relaxed'
   #define atomic_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/barrier.h:144:10: note: expanded from macro 'smp_cond_load_relaxed'
                           VAL = __smp_load_reserved_relaxed(__PTR);       \
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/barrier.h:88:2: note: expanded from macro '__smp_load_reserved_relaxed'
           compiletime_assert_atomic_lrsc_type(*ptr);                      \
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
   1 error generated.


vim +291 kernel/bpf/helpers.c

d83525ca62cf8e Alexei Starovoitov 2019-01-31  284  
d83525ca62cf8e Alexei Starovoitov 2019-01-31  285  static inline void __bpf_spin_lock(struct bpf_spin_lock *lock)
d83525ca62cf8e Alexei Starovoitov 2019-01-31  286  {
d83525ca62cf8e Alexei Starovoitov 2019-01-31  287  	atomic_t *l = (void *)lock;
d83525ca62cf8e Alexei Starovoitov 2019-01-31  288  
d83525ca62cf8e Alexei Starovoitov 2019-01-31  289  	BUILD_BUG_ON(sizeof(*l) != sizeof(*lock));
d83525ca62cf8e Alexei Starovoitov 2019-01-31  290  	do {
d83525ca62cf8e Alexei Starovoitov 2019-01-31 @291  		atomic_cond_read_relaxed(l, !VAL);
d83525ca62cf8e Alexei Starovoitov 2019-01-31  292  	} while (atomic_xchg(l, 1));
d83525ca62cf8e Alexei Starovoitov 2019-01-31  293  }
d83525ca62cf8e Alexei Starovoitov 2019-01-31  294  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
  2022-06-02 14:10 ` Christoph Muellner
@ 2022-06-14 15:34   ` Atish Patra
  -1 siblings, 0 replies; 15+ messages in thread
From: Atish Patra @ 2022-06-14 15:34 UTC (permalink / raw)
  To: Christoph Muellner
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Heiko Stuebner, Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel@vger.kernel.org List

On Thu, Jun 2, 2022 at 7:11 AM Christoph Muellner
<christoph.muellner@vrull.eu> wrote:
>
> The current RISC-V code uses the generic ticket lock implementation,
> that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> Currently, RISC-V uses the generic implementation of these macros.
> This patch introduces a RISC-V specific implementation, of these
> macros, that peels off the first loop iteration and modifies the waiting
> loop such, that it is possible to use the WRS instruction of the Zawrs
> ISA extension to stall the CPU.
>
> The resulting implementation of smp_cond_load_*() will only work for
> 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> This is caused by the restrictions of the LR instruction (RISC-V only
> has LR.W and LR.D). Compiler assertions guard this new restriction.
>
> This patch uses the existing RISC-V ISA extension framework
> to detect the presents of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.
> A similar patch could add support for the PAUSE instruction of
> the Zihintpause ISA extension.
>

FYI..The initial Zihintpause support patch
https://www.spinics.net/lists/kernel/msg4380326.html

> The whole mechanism is gated by Kconfig setting, which defaults to Y.
>
> The Zawrs specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
>
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
>
> Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> ---
>  arch/riscv/Kconfig                   | 10 +++
>  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/errata_list.h | 12 +++-
>  arch/riscv/include/asm/hwcap.h       |  3 +-
>  arch/riscv/kernel/cpu.c              |  1 +
>  arch/riscv/kernel/cpufeature.c       | 13 ++++
>  6 files changed, 133 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 905e550e0fd3..054872317d4a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -358,6 +358,16 @@ config RISCV_ISA_C
>
>            If you don't know what to do here, say Y.
>
> +config RISCV_ISA_ZAWRS
> +       bool "Zawrs extension support"
> +       select RISCV_ALTERNATIVE
> +       default y
> +       help
> +          Adds support to dynamically detect the presence of the Zawrs extension
> +          (wait for reservation set) and enable its usage.
> +
> +          If you don't know what to do here, say Y.
> +
>  config RISCV_ISA_SVPBMT
>         bool "SVPBMT extension support"
>         depends on 64BIT && MMU
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index d0e24aaa2aa0..69b8f1f4b80c 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -12,6 +12,8 @@
>
>  #ifndef __ASSEMBLY__
>
> +#include <asm/errata_list.h>
> +
>  #define nop()          __asm__ __volatile__ ("nop")
>
>  #define RISCV_FENCE(p, s) \
> @@ -42,6 +44,69 @@ do {                                                                 \
>         ___p1;                                                          \
>  })
>
> +#if __riscv_xlen == 64
> +
> +#define __riscv_lrsc_word(t)                                           \
> +       (sizeof(t) == sizeof(int) ||                                    \
> +        sizeof(t) == sizeof(long))
> +
> +#define __riscv_lr(ptr)                                                        \
> +       sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> +
> +#elif __riscv_xlen == 32
> +
> +#define __riscv_lrsc_word(ptr)                                         \
> +       (sizeof(*ptr) == sizeof(int))
> +
> +#define __riscv_lr(t) "lr.w"
> +
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif /* __riscv_xlen */
> +
> +#define compiletime_assert_atomic_lrsc_type(t)                         \
> +       compiletime_assert(__riscv_lrsc_word(t),                        \
> +               "Need type compatible with LR/SC instructions.")
> +
> +#define ___smp_load_reservedN(pfx, ptr)                                        \
> +({                                                                     \
> +       typeof(*ptr) ___p1;                                             \
> +       __asm__ __volatile__ ("lr." pfx "       %[p], %[c]\n"           \
> +                             : [p]"=&r" (___p1), [c]"+A"(*ptr));       \
> +       ___p1;                                                          \
> +})
> +
> +#define ___smp_load_reserved32(ptr)                                    \
> +       ___smp_load_reservedN("w", ptr)
> +
> +#define ___smp_load_reserved64(ptr)                                    \
> +       ___smp_load_reservedN("d", ptr)
> +
> +#define __smp_load_reserved_relaxed(ptr)                               \
> +({                                                                     \
> +       typeof(*ptr) ___p1;                                             \
> +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> +       if (sizeof(*ptr) == 32) {                                       \
> +               ___p1 = ___smp_load_reserved32(ptr);                    \
> +       } else {                                                        \
> +               ___p1 = ___smp_load_reserved64(ptr);                    \
> +       }                                                               \
> +       ___p1;                                                          \
> +})
> +
> +#define __smp_load_reserved_acquire(ptr)                               \
> +({                                                                     \
> +       typeof(*ptr) ___p1;                                             \
> +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> +       if (sizeof(*ptr) == 32) {                                       \
> +               ___p1 = ___smp_load_reserved32(ptr);                    \
> +       } else {                                                        \
> +               ___p1 = ___smp_load_reserved64(ptr);                    \
> +       }                                                               \
> +       RISCV_FENCE(r,rw);                                              \
> +       ___p1;                                                          \
> +})
> +
>  /*
>   * This is a very specific barrier: it's currently only used in two places in
>   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> @@ -69,6 +134,38 @@ do {                                                                        \
>   */
>  #define smp_mb__after_spinlock()       RISCV_FENCE(iorw,iorw)
>
> +#define smp_cond_load_relaxed(ptr, cond_expr)                          \
> +({                                                                     \
> +       typeof(ptr) __PTR = (ptr);                                      \
> +       __unqual_scalar_typeof(*ptr) VAL;                               \
> +       VAL = READ_ONCE(*__PTR);                                        \
> +       if (!cond_expr) {                                               \
> +               for (;;) {                                              \
> +                       VAL = __smp_load_reserved_relaxed(__PTR);       \
> +                       if (cond_expr)                                  \
> +                               break;                                  \
> +                       ALT_WRS();                                      \
> +               }                                                       \
> +       }                                                               \
> +       (typeof(*ptr))VAL;                                              \
> +})
> +
> +#define smp_cond_load_acquire(ptr, cond_expr)                          \
> +({                                                                     \
> +       typeof(ptr) __PTR = (ptr);                                      \
> +       __unqual_scalar_typeof(*ptr) VAL;                               \
> +       VAL = smp_load_acquire(__PTR);                                  \
> +       if (!cond_expr) {                                               \
> +               for (;;) {                                              \
> +                       VAL = __smp_load_reserved_acquire(__PTR);       \
> +                       if (cond_expr)                                  \
> +                               break;                                  \
> +                       ALT_WRS();                                      \
> +               }                                                       \
> +       }                                                               \
> +       (typeof(*ptr))VAL;                                              \
> +})
> +
>  #include <asm-generic/barrier.h>
>
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 9e2888dbb5b1..b9aa0b346493 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -19,8 +19,9 @@
>  #define        ERRATA_THEAD_NUMBER 1
>  #endif
>
> -#define        CPUFEATURE_SVPBMT 0
> -#define        CPUFEATURE_NUMBER 1
> +#define        CPUFEATURE_ZAWRS 0
> +#define        CPUFEATURE_SVPBMT 1
> +#define        CPUFEATURE_NUMBER 2
>
>  #ifdef __ASSEMBLY__
>
> @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,     \
>                 ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)  \
>                 : : "r" (addr) : "memory")
>
> +#define ZAWRS_WRS      ".long 0x1000073"
> +#define ALT_WRS()                                                      \
> +asm volatile(ALTERNATIVE(                                              \
> +       "nop\n\t",                                                      \
> +       ZAWRS_WRS "\n\t",                                               \
> +       0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> +
>  /*
>   * _val is marked as "will be overwritten", so need to set it to 0
>   * in the default case.
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 4e2486881840..c7dd8cc38bec 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
>   * available logical extension id.
>   */
>  enum riscv_isa_ext_id {
> -       RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> +       RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> +       RISCV_ISA_EXT_SSCOFPMF,
>         RISCV_ISA_EXT_SVPBMT,
>         RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
>  };
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..6c3a10ff5358 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
>   *    extensions by an underscore.
>   */
>  static struct riscv_isa_ext_data isa_ext_arr[] = {
> +       __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
>         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>         __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>         __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index dea3ea19deee..fc2c47a1784b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
>                         } else {
>                                 SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
>                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> +                               SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
>                         }
>  #undef SET_ISA_EXT_MAP
>                 }
> @@ -251,6 +252,14 @@ struct cpufeature_info {
>         bool (*check_func)(unsigned int stage);
>  };
>
> +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> +{
> +       if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> +               return false;
> +
> +       return riscv_isa_extension_available(NULL, ZAWRS);
> +}
> +
>  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>  {
>  #ifdef CONFIG_RISCV_ISA_SVPBMT
> @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>
>  static const struct cpufeature_info __initdata_or_module
>  cpufeature_list[CPUFEATURE_NUMBER] = {
> +       {
> +               .name = "zawrs",
> +               .check_func = cpufeature_zawrs_check_func
> +       },
>         {
>                 .name = "svpbmt",
>                 .check_func = cpufeature_svpbmt_check_func
> --
> 2.35.3
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-14 15:34   ` Atish Patra
  0 siblings, 0 replies; 15+ messages in thread
From: Atish Patra @ 2022-06-14 15:34 UTC (permalink / raw)
  To: Christoph Muellner
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Heiko Stuebner, Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel@vger.kernel.org List

On Thu, Jun 2, 2022 at 7:11 AM Christoph Muellner
<christoph.muellner@vrull.eu> wrote:
>
> The current RISC-V code uses the generic ticket lock implementation,
> that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> Currently, RISC-V uses the generic implementation of these macros.
> This patch introduces a RISC-V specific implementation, of these
> macros, that peels off the first loop iteration and modifies the waiting
> loop such, that it is possible to use the WRS instruction of the Zawrs
> ISA extension to stall the CPU.
>
> The resulting implementation of smp_cond_load_*() will only work for
> 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> This is caused by the restrictions of the LR instruction (RISC-V only
> has LR.W and LR.D). Compiler assertions guard this new restriction.
>
> This patch uses the existing RISC-V ISA extension framework
> to detect the presents of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.
> A similar patch could add support for the PAUSE instruction of
> the Zihintpause ISA extension.
>

FYI..The initial Zihintpause support patch
https://www.spinics.net/lists/kernel/msg4380326.html

> The whole mechanism is gated by Kconfig setting, which defaults to Y.
>
> The Zawrs specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
>
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
>
> Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> ---
>  arch/riscv/Kconfig                   | 10 +++
>  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/errata_list.h | 12 +++-
>  arch/riscv/include/asm/hwcap.h       |  3 +-
>  arch/riscv/kernel/cpu.c              |  1 +
>  arch/riscv/kernel/cpufeature.c       | 13 ++++
>  6 files changed, 133 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 905e550e0fd3..054872317d4a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -358,6 +358,16 @@ config RISCV_ISA_C
>
>            If you don't know what to do here, say Y.
>
> +config RISCV_ISA_ZAWRS
> +       bool "Zawrs extension support"
> +       select RISCV_ALTERNATIVE
> +       default y
> +       help
> +          Adds support to dynamically detect the presence of the Zawrs extension
> +          (wait for reservation set) and enable its usage.
> +
> +          If you don't know what to do here, say Y.
> +
>  config RISCV_ISA_SVPBMT
>         bool "SVPBMT extension support"
>         depends on 64BIT && MMU
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index d0e24aaa2aa0..69b8f1f4b80c 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -12,6 +12,8 @@
>
>  #ifndef __ASSEMBLY__
>
> +#include <asm/errata_list.h>
> +
>  #define nop()          __asm__ __volatile__ ("nop")
>
>  #define RISCV_FENCE(p, s) \
> @@ -42,6 +44,69 @@ do {                                                                 \
>         ___p1;                                                          \
>  })
>
> +#if __riscv_xlen == 64
> +
> +#define __riscv_lrsc_word(t)                                           \
> +       (sizeof(t) == sizeof(int) ||                                    \
> +        sizeof(t) == sizeof(long))
> +
> +#define __riscv_lr(ptr)                                                        \
> +       sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> +
> +#elif __riscv_xlen == 32
> +
> +#define __riscv_lrsc_word(ptr)                                         \
> +       (sizeof(*ptr) == sizeof(int))
> +
> +#define __riscv_lr(t) "lr.w"
> +
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif /* __riscv_xlen */
> +
> +#define compiletime_assert_atomic_lrsc_type(t)                         \
> +       compiletime_assert(__riscv_lrsc_word(t),                        \
> +               "Need type compatible with LR/SC instructions.")
> +
> +#define ___smp_load_reservedN(pfx, ptr)                                        \
> +({                                                                     \
> +       typeof(*ptr) ___p1;                                             \
> +       __asm__ __volatile__ ("lr." pfx "       %[p], %[c]\n"           \
> +                             : [p]"=&r" (___p1), [c]"+A"(*ptr));       \
> +       ___p1;                                                          \
> +})
> +
> +#define ___smp_load_reserved32(ptr)                                    \
> +       ___smp_load_reservedN("w", ptr)
> +
> +#define ___smp_load_reserved64(ptr)                                    \
> +       ___smp_load_reservedN("d", ptr)
> +
> +#define __smp_load_reserved_relaxed(ptr)                               \
> +({                                                                     \
> +       typeof(*ptr) ___p1;                                             \
> +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> +       if (sizeof(*ptr) == 32) {                                       \
> +               ___p1 = ___smp_load_reserved32(ptr);                    \
> +       } else {                                                        \
> +               ___p1 = ___smp_load_reserved64(ptr);                    \
> +       }                                                               \
> +       ___p1;                                                          \
> +})
> +
> +#define __smp_load_reserved_acquire(ptr)                               \
> +({                                                                     \
> +       typeof(*ptr) ___p1;                                             \
> +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> +       if (sizeof(*ptr) == 32) {                                       \
> +               ___p1 = ___smp_load_reserved32(ptr);                    \
> +       } else {                                                        \
> +               ___p1 = ___smp_load_reserved64(ptr);                    \
> +       }                                                               \
> +       RISCV_FENCE(r,rw);                                              \
> +       ___p1;                                                          \
> +})
> +
>  /*
>   * This is a very specific barrier: it's currently only used in two places in
>   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> @@ -69,6 +134,38 @@ do {                                                                        \
>   */
>  #define smp_mb__after_spinlock()       RISCV_FENCE(iorw,iorw)
>
> +#define smp_cond_load_relaxed(ptr, cond_expr)                          \
> +({                                                                     \
> +       typeof(ptr) __PTR = (ptr);                                      \
> +       __unqual_scalar_typeof(*ptr) VAL;                               \
> +       VAL = READ_ONCE(*__PTR);                                        \
> +       if (!cond_expr) {                                               \
> +               for (;;) {                                              \
> +                       VAL = __smp_load_reserved_relaxed(__PTR);       \
> +                       if (cond_expr)                                  \
> +                               break;                                  \
> +                       ALT_WRS();                                      \
> +               }                                                       \
> +       }                                                               \
> +       (typeof(*ptr))VAL;                                              \
> +})
> +
> +#define smp_cond_load_acquire(ptr, cond_expr)                          \
> +({                                                                     \
> +       typeof(ptr) __PTR = (ptr);                                      \
> +       __unqual_scalar_typeof(*ptr) VAL;                               \
> +       VAL = smp_load_acquire(__PTR);                                  \
> +       if (!cond_expr) {                                               \
> +               for (;;) {                                              \
> +                       VAL = __smp_load_reserved_acquire(__PTR);       \
> +                       if (cond_expr)                                  \
> +                               break;                                  \
> +                       ALT_WRS();                                      \
> +               }                                                       \
> +       }                                                               \
> +       (typeof(*ptr))VAL;                                              \
> +})
> +
>  #include <asm-generic/barrier.h>
>
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 9e2888dbb5b1..b9aa0b346493 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -19,8 +19,9 @@
>  #define        ERRATA_THEAD_NUMBER 1
>  #endif
>
> -#define        CPUFEATURE_SVPBMT 0
> -#define        CPUFEATURE_NUMBER 1
> +#define        CPUFEATURE_ZAWRS 0
> +#define        CPUFEATURE_SVPBMT 1
> +#define        CPUFEATURE_NUMBER 2
>
>  #ifdef __ASSEMBLY__
>
> @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,     \
>                 ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)  \
>                 : : "r" (addr) : "memory")
>
> +#define ZAWRS_WRS      ".long 0x1000073"
> +#define ALT_WRS()                                                      \
> +asm volatile(ALTERNATIVE(                                              \
> +       "nop\n\t",                                                      \
> +       ZAWRS_WRS "\n\t",                                               \
> +       0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> +
>  /*
>   * _val is marked as "will be overwritten", so need to set it to 0
>   * in the default case.
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 4e2486881840..c7dd8cc38bec 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
>   * available logical extension id.
>   */
>  enum riscv_isa_ext_id {
> -       RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> +       RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> +       RISCV_ISA_EXT_SSCOFPMF,
>         RISCV_ISA_EXT_SVPBMT,
>         RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
>  };
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index fba9e9f46a8c..6c3a10ff5358 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
>   *    extensions by an underscore.
>   */
>  static struct riscv_isa_ext_data isa_ext_arr[] = {
> +       __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
>         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>         __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>         __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index dea3ea19deee..fc2c47a1784b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
>                         } else {
>                                 SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
>                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> +                               SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
>                         }
>  #undef SET_ISA_EXT_MAP
>                 }
> @@ -251,6 +252,14 @@ struct cpufeature_info {
>         bool (*check_func)(unsigned int stage);
>  };
>
> +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> +{
> +       if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> +               return false;
> +
> +       return riscv_isa_extension_available(NULL, ZAWRS);
> +}
> +
>  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>  {
>  #ifdef CONFIG_RISCV_ISA_SVPBMT
> @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
>
>  static const struct cpufeature_info __initdata_or_module
>  cpufeature_list[CPUFEATURE_NUMBER] = {
> +       {
> +               .name = "zawrs",
> +               .check_func = cpufeature_zawrs_check_func
> +       },
>         {
>                 .name = "svpbmt",
>                 .check_func = cpufeature_svpbmt_check_func
> --
> 2.35.3
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
  2022-06-14 15:34   ` Atish Patra
@ 2022-06-14 16:29     ` Heiko Stübner
  -1 siblings, 0 replies; 15+ messages in thread
From: Heiko Stübner @ 2022-06-14 16:29 UTC (permalink / raw)
  To: Christoph Muellner, Atish Patra
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel@vger.kernel.org List

Am Dienstag, 14. Juni 2022, 17:34:48 CEST schrieb Atish Patra:
> On Thu, Jun 2, 2022 at 7:11 AM Christoph Muellner
> <christoph.muellner@vrull.eu> wrote:
> >
> > The current RISC-V code uses the generic ticket lock implementation,
> > that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Currently, RISC-V uses the generic implementation of these macros.
> > This patch introduces a RISC-V specific implementation, of these
> > macros, that peels off the first loop iteration and modifies the waiting
> > loop such, that it is possible to use the WRS instruction of the Zawrs
> > ISA extension to stall the CPU.
> >
> > The resulting implementation of smp_cond_load_*() will only work for
> > 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> > This is caused by the restrictions of the LR instruction (RISC-V only
> > has LR.W and LR.D). Compiler assertions guard this new restriction.
> >
> > This patch uses the existing RISC-V ISA extension framework
> > to detect the presents of Zawrs at run-time.
> > If available a NOP instruction will be replaced by WRS.
> > A similar patch could add support for the PAUSE instruction of
> > the Zihintpause ISA extension.
> >
> 
> FYI..The initial Zihintpause support patch
> https://www.spinics.net/lists/kernel/msg4380326.html

Out of curiosity, is this functionally equivalent to WRS,
So can you just replace WRS with Pause?

If so, ALTERNATIVE_2 would make that pretty easy.

Are there performance differences between them?

Heiko

> 
> > The whole mechanism is gated by Kconfig setting, which defaults to Y.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> > ---
> >  arch/riscv/Kconfig                   | 10 +++
> >  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/errata_list.h | 12 +++-
> >  arch/riscv/include/asm/hwcap.h       |  3 +-
> >  arch/riscv/kernel/cpu.c              |  1 +
> >  arch/riscv/kernel/cpufeature.c       | 13 ++++
> >  6 files changed, 133 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 905e550e0fd3..054872317d4a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -358,6 +358,16 @@ config RISCV_ISA_C
> >
> >            If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > +       bool "Zawrs extension support"
> > +       select RISCV_ALTERNATIVE
> > +       default y
> > +       help
> > +          Adds support to dynamically detect the presence of the Zawrs extension
> > +          (wait for reservation set) and enable its usage.
> > +
> > +          If you don't know what to do here, say Y.
> > +
> >  config RISCV_ISA_SVPBMT
> >         bool "SVPBMT extension support"
> >         depends on 64BIT && MMU
> > diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> > index d0e24aaa2aa0..69b8f1f4b80c 100644
> > --- a/arch/riscv/include/asm/barrier.h
> > +++ b/arch/riscv/include/asm/barrier.h
> > @@ -12,6 +12,8 @@
> >
> >  #ifndef __ASSEMBLY__
> >
> > +#include <asm/errata_list.h>
> > +
> >  #define nop()          __asm__ __volatile__ ("nop")
> >
> >  #define RISCV_FENCE(p, s) \
> > @@ -42,6 +44,69 @@ do {                                                                 \
> >         ___p1;                                                          \
> >  })
> >
> > +#if __riscv_xlen == 64
> > +
> > +#define __riscv_lrsc_word(t)                                           \
> > +       (sizeof(t) == sizeof(int) ||                                    \
> > +        sizeof(t) == sizeof(long))
> > +
> > +#define __riscv_lr(ptr)                                                        \
> > +       sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> > +
> > +#elif __riscv_xlen == 32
> > +
> > +#define __riscv_lrsc_word(ptr)                                         \
> > +       (sizeof(*ptr) == sizeof(int))
> > +
> > +#define __riscv_lr(t) "lr.w"
> > +
> > +#else
> > +#error "Unexpected __riscv_xlen"
> > +#endif /* __riscv_xlen */
> > +
> > +#define compiletime_assert_atomic_lrsc_type(t)                         \
> > +       compiletime_assert(__riscv_lrsc_word(t),                        \
> > +               "Need type compatible with LR/SC instructions.")
> > +
> > +#define ___smp_load_reservedN(pfx, ptr)                                        \
> > +({                                                                     \
> > +       typeof(*ptr) ___p1;                                             \
> > +       __asm__ __volatile__ ("lr." pfx "       %[p], %[c]\n"           \
> > +                             : [p]"=&r" (___p1), [c]"+A"(*ptr));       \
> > +       ___p1;                                                          \
> > +})
> > +
> > +#define ___smp_load_reserved32(ptr)                                    \
> > +       ___smp_load_reservedN("w", ptr)
> > +
> > +#define ___smp_load_reserved64(ptr)                                    \
> > +       ___smp_load_reservedN("d", ptr)
> > +
> > +#define __smp_load_reserved_relaxed(ptr)                               \
> > +({                                                                     \
> > +       typeof(*ptr) ___p1;                                             \
> > +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +       if (sizeof(*ptr) == 32) {                                       \
> > +               ___p1 = ___smp_load_reserved32(ptr);                    \
> > +       } else {                                                        \
> > +               ___p1 = ___smp_load_reserved64(ptr);                    \
> > +       }                                                               \
> > +       ___p1;                                                          \
> > +})
> > +
> > +#define __smp_load_reserved_acquire(ptr)                               \
> > +({                                                                     \
> > +       typeof(*ptr) ___p1;                                             \
> > +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +       if (sizeof(*ptr) == 32) {                                       \
> > +               ___p1 = ___smp_load_reserved32(ptr);                    \
> > +       } else {                                                        \
> > +               ___p1 = ___smp_load_reserved64(ptr);                    \
> > +       }                                                               \
> > +       RISCV_FENCE(r,rw);                                              \
> > +       ___p1;                                                          \
> > +})
> > +
> >  /*
> >   * This is a very specific barrier: it's currently only used in two places in
> >   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> > @@ -69,6 +134,38 @@ do {                                                                        \
> >   */
> >  #define smp_mb__after_spinlock()       RISCV_FENCE(iorw,iorw)
> >
> > +#define smp_cond_load_relaxed(ptr, cond_expr)                          \
> > +({                                                                     \
> > +       typeof(ptr) __PTR = (ptr);                                      \
> > +       __unqual_scalar_typeof(*ptr) VAL;                               \
> > +       VAL = READ_ONCE(*__PTR);                                        \
> > +       if (!cond_expr) {                                               \
> > +               for (;;) {                                              \
> > +                       VAL = __smp_load_reserved_relaxed(__PTR);       \
> > +                       if (cond_expr)                                  \
> > +                               break;                                  \
> > +                       ALT_WRS();                                      \
> > +               }                                                       \
> > +       }                                                               \
> > +       (typeof(*ptr))VAL;                                              \
> > +})
> > +
> > +#define smp_cond_load_acquire(ptr, cond_expr)                          \
> > +({                                                                     \
> > +       typeof(ptr) __PTR = (ptr);                                      \
> > +       __unqual_scalar_typeof(*ptr) VAL;                               \
> > +       VAL = smp_load_acquire(__PTR);                                  \
> > +       if (!cond_expr) {                                               \
> > +               for (;;) {                                              \
> > +                       VAL = __smp_load_reserved_acquire(__PTR);       \
> > +                       if (cond_expr)                                  \
> > +                               break;                                  \
> > +                       ALT_WRS();                                      \
> > +               }                                                       \
> > +       }                                                               \
> > +       (typeof(*ptr))VAL;                                              \
> > +})
> > +
> >  #include <asm-generic/barrier.h>
> >
> >  #endif /* __ASSEMBLY__ */
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index 9e2888dbb5b1..b9aa0b346493 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -19,8 +19,9 @@
> >  #define        ERRATA_THEAD_NUMBER 1
> >  #endif
> >
> > -#define        CPUFEATURE_SVPBMT 0
> > -#define        CPUFEATURE_NUMBER 1
> > +#define        CPUFEATURE_ZAWRS 0
> > +#define        CPUFEATURE_SVPBMT 1
> > +#define        CPUFEATURE_NUMBER 2
> >
> >  #ifdef __ASSEMBLY__
> >
> > @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,     \
> >                 ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)  \
> >                 : : "r" (addr) : "memory")
> >
> > +#define ZAWRS_WRS      ".long 0x1000073"
> > +#define ALT_WRS()                                                      \
> > +asm volatile(ALTERNATIVE(                                              \
> > +       "nop\n\t",                                                      \
> > +       ZAWRS_WRS "\n\t",                                               \
> > +       0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> > +
> >  /*
> >   * _val is marked as "will be overwritten", so need to set it to 0
> >   * in the default case.
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 4e2486881840..c7dd8cc38bec 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
> >   * available logical extension id.
> >   */
> >  enum riscv_isa_ext_id {
> > -       RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> > +       RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> > +       RISCV_ISA_EXT_SSCOFPMF,
> >         RISCV_ISA_EXT_SVPBMT,
> >         RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> >  };
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fba9e9f46a8c..6c3a10ff5358 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
> >   *    extensions by an underscore.
> >   */
> >  static struct riscv_isa_ext_data isa_ext_arr[] = {
> > +       __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> >         __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >         __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index dea3ea19deee..fc2c47a1784b 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
> >                         } else {
> >                                 SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > +                               SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
> >                         }
> >  #undef SET_ISA_EXT_MAP
> >                 }
> > @@ -251,6 +252,14 @@ struct cpufeature_info {
> >         bool (*check_func)(unsigned int stage);
> >  };
> >
> > +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> > +{
> > +       if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > +               return false;
> > +
> > +       return riscv_isa_extension_available(NULL, ZAWRS);
> > +}
> > +
> >  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >  {
> >  #ifdef CONFIG_RISCV_ISA_SVPBMT
> > @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >
> >  static const struct cpufeature_info __initdata_or_module
> >  cpufeature_list[CPUFEATURE_NUMBER] = {
> > +       {
> > +               .name = "zawrs",
> > +               .check_func = cpufeature_zawrs_check_func
> > +       },
> >         {
> >                 .name = "svpbmt",
> >                 .check_func = cpufeature_svpbmt_check_func
> > --
> > 2.35.3
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> 
> 
> 





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] riscv: Add Zawrs support for spinlocks
@ 2022-06-14 16:29     ` Heiko Stübner
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Stübner @ 2022-06-14 16:29 UTC (permalink / raw)
  To: Christoph Muellner, Atish Patra
  Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Christoph Muellner,
	Philipp Tomsich, Aaron Durbin, linux-riscv,
	linux-kernel@vger.kernel.org List

Am Dienstag, 14. Juni 2022, 17:34:48 CEST schrieb Atish Patra:
> On Thu, Jun 2, 2022 at 7:11 AM Christoph Muellner
> <christoph.muellner@vrull.eu> wrote:
> >
> > The current RISC-V code uses the generic ticket lock implementation,
> > that calls the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Currently, RISC-V uses the generic implementation of these macros.
> > This patch introduces a RISC-V specific implementation, of these
> > macros, that peels off the first loop iteration and modifies the waiting
> > loop such, that it is possible to use the WRS instruction of the Zawrs
> > ISA extension to stall the CPU.
> >
> > The resulting implementation of smp_cond_load_*() will only work for
> > 32-bit or 64-bit types for RV64 and 32-bit types for RV32.
> > This is caused by the restrictions of the LR instruction (RISC-V only
> > has LR.W and LR.D). Compiler assertions guard this new restriction.
> >
> > This patch uses the existing RISC-V ISA extension framework
> > to detect the presents of Zawrs at run-time.
> > If available a NOP instruction will be replaced by WRS.
> > A similar patch could add support for the PAUSE instruction of
> > the Zihintpause ISA extension.
> >
> 
> FYI..The initial Zihintpause support patch
> https://www.spinics.net/lists/kernel/msg4380326.html

Out of curiosity, is this functionally equivalent to WRS,
So can you just replace WRS with Pause?

If so, ALTERNATIVE_2 would make that pretty easy.

Are there performance differences between them?

Heiko

> 
> > The whole mechanism is gated by Kconfig setting, which defaults to Y.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Signed-off-by: Christoph Muellner <christoph.muellner@vrull.io>
> > ---
> >  arch/riscv/Kconfig                   | 10 +++
> >  arch/riscv/include/asm/barrier.h     | 97 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/errata_list.h | 12 +++-
> >  arch/riscv/include/asm/hwcap.h       |  3 +-
> >  arch/riscv/kernel/cpu.c              |  1 +
> >  arch/riscv/kernel/cpufeature.c       | 13 ++++
> >  6 files changed, 133 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 905e550e0fd3..054872317d4a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -358,6 +358,16 @@ config RISCV_ISA_C
> >
> >            If you don't know what to do here, say Y.
> >
> > +config RISCV_ISA_ZAWRS
> > +       bool "Zawrs extension support"
> > +       select RISCV_ALTERNATIVE
> > +       default y
> > +       help
> > +          Adds support to dynamically detect the presence of the Zawrs extension
> > +          (wait for reservation set) and enable its usage.
> > +
> > +          If you don't know what to do here, say Y.
> > +
> >  config RISCV_ISA_SVPBMT
> >         bool "SVPBMT extension support"
> >         depends on 64BIT && MMU
> > diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> > index d0e24aaa2aa0..69b8f1f4b80c 100644
> > --- a/arch/riscv/include/asm/barrier.h
> > +++ b/arch/riscv/include/asm/barrier.h
> > @@ -12,6 +12,8 @@
> >
> >  #ifndef __ASSEMBLY__
> >
> > +#include <asm/errata_list.h>
> > +
> >  #define nop()          __asm__ __volatile__ ("nop")
> >
> >  #define RISCV_FENCE(p, s) \
> > @@ -42,6 +44,69 @@ do {                                                                 \
> >         ___p1;                                                          \
> >  })
> >
> > +#if __riscv_xlen == 64
> > +
> > +#define __riscv_lrsc_word(t)                                           \
> > +       (sizeof(t) == sizeof(int) ||                                    \
> > +        sizeof(t) == sizeof(long))
> > +
> > +#define __riscv_lr(ptr)                                                        \
> > +       sizeof(*ptr) == sizeof(int) ? "lr.w" : "lr.d"
> > +
> > +#elif __riscv_xlen == 32
> > +
> > +#define __riscv_lrsc_word(ptr)                                         \
> > +       (sizeof(*ptr) == sizeof(int))
> > +
> > +#define __riscv_lr(t) "lr.w"
> > +
> > +#else
> > +#error "Unexpected __riscv_xlen"
> > +#endif /* __riscv_xlen */
> > +
> > +#define compiletime_assert_atomic_lrsc_type(t)                         \
> > +       compiletime_assert(__riscv_lrsc_word(t),                        \
> > +               "Need type compatible with LR/SC instructions.")
> > +
> > +#define ___smp_load_reservedN(pfx, ptr)                                        \
> > +({                                                                     \
> > +       typeof(*ptr) ___p1;                                             \
> > +       __asm__ __volatile__ ("lr." pfx "       %[p], %[c]\n"           \
> > +                             : [p]"=&r" (___p1), [c]"+A"(*ptr));       \
> > +       ___p1;                                                          \
> > +})
> > +
> > +#define ___smp_load_reserved32(ptr)                                    \
> > +       ___smp_load_reservedN("w", ptr)
> > +
> > +#define ___smp_load_reserved64(ptr)                                    \
> > +       ___smp_load_reservedN("d", ptr)
> > +
> > +#define __smp_load_reserved_relaxed(ptr)                               \
> > +({                                                                     \
> > +       typeof(*ptr) ___p1;                                             \
> > +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +       if (sizeof(*ptr) == 32) {                                       \
> > +               ___p1 = ___smp_load_reserved32(ptr);                    \
> > +       } else {                                                        \
> > +               ___p1 = ___smp_load_reserved64(ptr);                    \
> > +       }                                                               \
> > +       ___p1;                                                          \
> > +})
> > +
> > +#define __smp_load_reserved_acquire(ptr)                               \
> > +({                                                                     \
> > +       typeof(*ptr) ___p1;                                             \
> > +       compiletime_assert_atomic_lrsc_type(*ptr);                      \
> > +       if (sizeof(*ptr) == 32) {                                       \
> > +               ___p1 = ___smp_load_reserved32(ptr);                    \
> > +       } else {                                                        \
> > +               ___p1 = ___smp_load_reserved64(ptr);                    \
> > +       }                                                               \
> > +       RISCV_FENCE(r,rw);                                              \
> > +       ___p1;                                                          \
> > +})
> > +
> >  /*
> >   * This is a very specific barrier: it's currently only used in two places in
> >   * the kernel, both in the scheduler.  See include/linux/spinlock.h for the two
> > @@ -69,6 +134,38 @@ do {                                                                        \
> >   */
> >  #define smp_mb__after_spinlock()       RISCV_FENCE(iorw,iorw)
> >
> > +#define smp_cond_load_relaxed(ptr, cond_expr)                          \
> > +({                                                                     \
> > +       typeof(ptr) __PTR = (ptr);                                      \
> > +       __unqual_scalar_typeof(*ptr) VAL;                               \
> > +       VAL = READ_ONCE(*__PTR);                                        \
> > +       if (!cond_expr) {                                               \
> > +               for (;;) {                                              \
> > +                       VAL = __smp_load_reserved_relaxed(__PTR);       \
> > +                       if (cond_expr)                                  \
> > +                               break;                                  \
> > +                       ALT_WRS();                                      \
> > +               }                                                       \
> > +       }                                                               \
> > +       (typeof(*ptr))VAL;                                              \
> > +})
> > +
> > +#define smp_cond_load_acquire(ptr, cond_expr)                          \
> > +({                                                                     \
> > +       typeof(ptr) __PTR = (ptr);                                      \
> > +       __unqual_scalar_typeof(*ptr) VAL;                               \
> > +       VAL = smp_load_acquire(__PTR);                                  \
> > +       if (!cond_expr) {                                               \
> > +               for (;;) {                                              \
> > +                       VAL = __smp_load_reserved_acquire(__PTR);       \
> > +                       if (cond_expr)                                  \
> > +                               break;                                  \
> > +                       ALT_WRS();                                      \
> > +               }                                                       \
> > +       }                                                               \
> > +       (typeof(*ptr))VAL;                                              \
> > +})
> > +
> >  #include <asm-generic/barrier.h>
> >
> >  #endif /* __ASSEMBLY__ */
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index 9e2888dbb5b1..b9aa0b346493 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -19,8 +19,9 @@
> >  #define        ERRATA_THEAD_NUMBER 1
> >  #endif
> >
> > -#define        CPUFEATURE_SVPBMT 0
> > -#define        CPUFEATURE_NUMBER 1
> > +#define        CPUFEATURE_ZAWRS 0
> > +#define        CPUFEATURE_SVPBMT 1
> > +#define        CPUFEATURE_NUMBER 2
> >
> >  #ifdef __ASSEMBLY__
> >
> > @@ -42,6 +43,13 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,     \
> >                 ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)  \
> >                 : : "r" (addr) : "memory")
> >
> > +#define ZAWRS_WRS      ".long 0x1000073"
> > +#define ALT_WRS()                                                      \
> > +asm volatile(ALTERNATIVE(                                              \
> > +       "nop\n\t",                                                      \
> > +       ZAWRS_WRS "\n\t",                                               \
> > +       0, CPUFEATURE_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> > +
> >  /*
> >   * _val is marked as "will be overwritten", so need to set it to 0
> >   * in the default case.
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 4e2486881840..c7dd8cc38bec 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -51,7 +51,8 @@ extern unsigned long elf_hwcap;
> >   * available logical extension id.
> >   */
> >  enum riscv_isa_ext_id {
> > -       RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
> > +       RISCV_ISA_EXT_ZAWRS = RISCV_ISA_EXT_BASE,
> > +       RISCV_ISA_EXT_SSCOFPMF,
> >         RISCV_ISA_EXT_SVPBMT,
> >         RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> >  };
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index fba9e9f46a8c..6c3a10ff5358 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -87,6 +87,7 @@ int riscv_of_parent_hartid(struct device_node *node)
> >   *    extensions by an underscore.
> >   */
> >  static struct riscv_isa_ext_data isa_ext_arr[] = {
> > +       __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> >         __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> >         __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >         __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index dea3ea19deee..fc2c47a1784b 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -199,6 +199,7 @@ void __init riscv_fill_hwcap(void)
> >                         } else {
> >                                 SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> >                                 SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > +                               SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
> >                         }
> >  #undef SET_ISA_EXT_MAP
> >                 }
> > @@ -251,6 +252,14 @@ struct cpufeature_info {
> >         bool (*check_func)(unsigned int stage);
> >  };
> >
> > +static bool __init_or_module cpufeature_zawrs_check_func(unsigned int stage)
> > +{
> > +       if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > +               return false;
> > +
> > +       return riscv_isa_extension_available(NULL, ZAWRS);
> > +}
> > +
> >  static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >  {
> >  #ifdef CONFIG_RISCV_ISA_SVPBMT
> > @@ -267,6 +276,10 @@ static bool __init_or_module cpufeature_svpbmt_check_func(unsigned int stage)
> >
> >  static const struct cpufeature_info __initdata_or_module
> >  cpufeature_list[CPUFEATURE_NUMBER] = {
> > +       {
> > +               .name = "zawrs",
> > +               .check_func = cpufeature_zawrs_check_func
> > +       },
> >         {
> >                 .name = "svpbmt",
> >                 .check_func = cpufeature_svpbmt_check_func
> > --
> > 2.35.3
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> 
> 
> 





_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-06-14 16:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-02 14:10 [RFC PATCH] riscv: Add Zawrs support for spinlocks Christoph Muellner
2022-06-02 14:10 ` Christoph Muellner
2022-06-02 14:21 ` Heiko Stübner
2022-06-02 14:21   ` Heiko Stübner
2022-06-02 14:24   ` Christoph Müllner
2022-06-02 14:24     ` Christoph Müllner
2022-06-02 16:24 ` Randy Dunlap
2022-06-02 16:24   ` Randy Dunlap
2022-06-02 16:32   ` Christoph Müllner
2022-06-02 16:32     ` Christoph Müllner
2022-06-04 22:20 ` kernel test robot
2022-06-14 15:34 ` Atish Patra
2022-06-14 15:34   ` Atish Patra
2022-06-14 16:29   ` Heiko Stübner
2022-06-14 16:29     ` Heiko Stübner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.