* [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2023-05-21 11:47 ` Heiko Stuebner
0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stuebner @ 2023-05-21 11:47 UTC (permalink / raw)
To: linux-riscv, palmer, paul.walmsley
Cc: linux-kernel, christoph.muellner, David.Laight, Heiko Stuebner
From: Heiko Stuebner <heiko.stuebner@vrull.eu>
Zawrs [0] was ratified in november 2022 [1], so I've resurrect the patch
adding Zawrs support for spinlocks and adapted it to recent kernel
changes.
Also incorporated are the nice comments David Laight provided on v2.
Changes since v2:
- Rebase on top of 6.4-rc1
- Adapt to changed alternatives Kconfig handling
- Adapt to changed cpufeature extension handling
- Address review comments from David Laight
- better handling for 32/64bit cases (less ifdefery)
- less macros calling macros
- don't duplicate __smp_load_reserved_relaxed in
__smp_load_reserved_aquire
Changes since v1:
- Fixing type checking code for 32/64-bit values
- Adjustments according to the specification change
- Adding "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS
[0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
[1] https://github.com/riscv/riscv-zawrs/commit/9ff54f7e7fcd95cf1b111d2e54276ff1183bcd37
Christoph Müllner (1):
riscv: Add Zawrs support for spinlocks
Heiko Stuebner (1):
riscv: don't include kernel.h into alternative.h
arch/riscv/Kconfig | 10 +++++
arch/riscv/include/asm/alternative.h | 1 -
arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
arch/riscv/include/asm/errata_list.h | 14 ++++++
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 1 +
7 files changed, 91 insertions(+), 1 deletion(-)
--
2.39.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2023-05-21 11:47 ` Heiko Stuebner
0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stuebner @ 2023-05-21 11:47 UTC (permalink / raw)
To: linux-riscv, palmer, paul.walmsley
Cc: linux-kernel, christoph.muellner, David.Laight, Heiko Stuebner
From: Heiko Stuebner <heiko.stuebner@vrull.eu>
Zawrs [0] was ratified in november 2022 [1], so I've resurrect the patch
adding Zawrs support for spinlocks and adapted it to recent kernel
changes.
Also incorporated are the nice comments David Laight provided on v2.
Changes since v2:
- Rebase on top of 6.4-rc1
- Adapt to changed alternatives Kconfig handling
- Adapt to changed cpufeature extension handling
- Address review comments from David Laight
- better handling for 32/64bit cases (less ifdefery)
- less macros calling macros
- don't duplicate __smp_load_reserved_relaxed in
__smp_load_reserved_aquire
Changes since v1:
- Fixing type checking code for 32/64-bit values
- Adjustments according to the specification change
- Adding "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS
[0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
[1] https://github.com/riscv/riscv-zawrs/commit/9ff54f7e7fcd95cf1b111d2e54276ff1183bcd37
Christoph Müllner (1):
riscv: Add Zawrs support for spinlocks
Heiko Stuebner (1):
riscv: don't include kernel.h into alternative.h
arch/riscv/Kconfig | 10 +++++
arch/riscv/include/asm/alternative.h | 1 -
arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
arch/riscv/include/asm/errata_list.h | 14 ++++++
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 1 +
7 files changed, 91 insertions(+), 1 deletion(-)
--
2.39.0
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h
2023-05-21 11:47 ` Heiko Stuebner
@ 2023-05-21 11:47 ` Heiko Stuebner
-1 siblings, 0 replies; 32+ messages in thread
From: Heiko Stuebner @ 2023-05-21 11:47 UTC (permalink / raw)
To: linux-riscv, palmer, paul.walmsley
Cc: linux-kernel, christoph.muellner, David.Laight, Heiko Stuebner
From: Heiko Stuebner <heiko.stuebner@vrull.eu>
This include is not currently needed for alternatives and creates
possible issues when we want to add alternatives to deeper kernel
infrastructure.
The issue in question came from trying to introduce Zawrs alternatives,
which resulted in a somewhat circular dependency like:
In file included from ../include/linux/bitops.h:34,
from ../include/linux/kernel.h:22,
from ../arch/riscv/include/asm/alternative.h:16,
from ../arch/riscv/include/asm/errata_list.h:8,
from ../arch/riscv/include/asm/barrier.h:15,
from ../include/linux/list.h:11,
from ../include/linux/preempt.h:11,
from ../include/linux/spinlock.h:56,
from ../include/linux/mmzone.h:8,
from ../include/linux/gfp.h:7,
from ../include/linux/mm.h:7,
from ../arch/riscv/kernel/asm-offsets.c:10:
../include/asm-generic/bitops/generic-non-atomic.h: In function ‘generic_test_bit_acquire’:
../include/asm-generic/bitops/generic-non-atomic.h:140:23: error: implicit declaration of function ‘smp_load_acquire’ [-Werror=implicit-function-declaration]
140 | return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1)));
| ^~~~~~~~~~~~~~~~
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
arch/riscv/include/asm/alternative.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index 6a41537826a7..05885de6048c 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -13,7 +13,6 @@
#ifdef CONFIG_RISCV_ALTERNATIVE
#include <linux/init.h>
-#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/stddef.h>
#include <asm/hwcap.h>
--
2.39.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h
@ 2023-05-21 11:47 ` Heiko Stuebner
0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stuebner @ 2023-05-21 11:47 UTC (permalink / raw)
To: linux-riscv, palmer, paul.walmsley
Cc: linux-kernel, christoph.muellner, David.Laight, Heiko Stuebner
From: Heiko Stuebner <heiko.stuebner@vrull.eu>
This include is not currently needed for alternatives and creates
possible issues when we want to add alternatives to deeper kernel
infrastructure.
The issue in question came from trying to introduce Zawrs alternatives,
which resulted in a somewhat circular dependency like:
In file included from ../include/linux/bitops.h:34,
from ../include/linux/kernel.h:22,
from ../arch/riscv/include/asm/alternative.h:16,
from ../arch/riscv/include/asm/errata_list.h:8,
from ../arch/riscv/include/asm/barrier.h:15,
from ../include/linux/list.h:11,
from ../include/linux/preempt.h:11,
from ../include/linux/spinlock.h:56,
from ../include/linux/mmzone.h:8,
from ../include/linux/gfp.h:7,
from ../include/linux/mm.h:7,
from ../arch/riscv/kernel/asm-offsets.c:10:
../include/asm-generic/bitops/generic-non-atomic.h: In function ‘generic_test_bit_acquire’:
../include/asm-generic/bitops/generic-non-atomic.h:140:23: error: implicit declaration of function ‘smp_load_acquire’ [-Werror=implicit-function-declaration]
140 | return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1)));
| ^~~~~~~~~~~~~~~~
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
arch/riscv/include/asm/alternative.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index 6a41537826a7..05885de6048c 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -13,7 +13,6 @@
#ifdef CONFIG_RISCV_ALTERNATIVE
#include <linux/init.h>
-#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/stddef.h>
#include <asm/hwcap.h>
--
2.39.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
2023-05-21 11:47 ` Heiko Stuebner
@ 2023-05-21 11:47 ` Heiko Stuebner
-1 siblings, 0 replies; 32+ messages in thread
From: Heiko Stuebner @ 2023-05-21 11:47 UTC (permalink / raw)
To: linux-riscv, palmer, paul.walmsley
Cc: linux-kernel, christoph.muellner, David.Laight, Heiko Stuebner
From: Christoph Müllner <christoph.muellner@vrull.eu>
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.STO 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 presence of Zawrs at run-time.
If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
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
Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
[rebase, update to review comments]
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
arch/riscv/Kconfig | 10 +++++
arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
arch/riscv/include/asm/errata_list.h | 14 ++++++
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 1 +
6 files changed, 91 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 348c0fa1fc8c..60ff303ff58a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -460,6 +460,16 @@ config RISCV_ISA_SVPBMT
If you don't know what to do here, say Y.
+config RISCV_ISA_ZAWRS
+ bool "Zawrs extension support"
+ depends on 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 TOOLCHAIN_HAS_ZBB
bool
default y
diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index 110752594228..bd64cfe5ae10 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 __nops(n) ".rept " #n "\nnop\n.endr\n"
#define nops(n) __asm__ __volatile__ (__nops(n))
@@ -44,6 +46,36 @@ do { \
___p1; \
})
+#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_reserved_relaxed(ptr) \
+({ \
+ typeof(*ptr) ___p1; \
+ if (sizeof(*ptr) == sizeof(int)) \
+ ___p1 = ___smp_load_reservedN("w", ptr); \
+ else if (sizeof(*ptr) == sizeof(long)) \
+ ___p1 = ___smp_load_reservedN("d", ptr); \
+ else \
+ compiletime_assert(0, \
+ "Need type compatible with LR/SC instructions " \
+ "for " __stringify(ptr)); \
+ ___p1; \
+})
+
+#define __smp_load_reserved_acquire(ptr) \
+({ \
+ typeof(*ptr) ___p1; \
+ ___p1 = __smp_load_reserved_relaxed(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
@@ -71,6 +103,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_STO(); \
+ } \
+ } \
+ (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_STO(); \
+ } \
+ } \
+ (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 fb1a810f3d8c..36a72a07d62c 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -44,6 +44,20 @@ 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_NTO ".long 0x00d00073"
+#define ALT_WRS_NTO() \
+asm volatile(ALTERNATIVE( \
+ "nop\n\t", \
+ ZAWRS_WRS_NTO "\n\t", \
+ 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
+
+#define ZAWRS_WRS_STO ".long 0x01d00073"
+#define ALT_WRS_STO() \
+asm volatile(ALTERNATIVE( \
+ "nop\n\t", \
+ ZAWRS_WRS_STO "\n\t", \
+ 0, RISCV_ISA_EXT_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 e0c40a4c63d5..4233d87539ab 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -46,6 +46,7 @@
#define RISCV_ISA_EXT_ZICBOZ 34
#define RISCV_ISA_EXT_SMAIA 35
#define RISCV_ISA_EXT_SSAIA 36
+#define RISCV_ISA_EXT_ZAWRS 37
#define RISCV_ISA_EXT_MAX 64
#define RISCV_ISA_EXT_NAME_LEN_MAX 32
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index c96aa56cf1c7..ac9c604d8936 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -184,6 +184,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
+ __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
__RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
__RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b1d6b7e4b829..d4a22a7aed99 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -236,6 +236,7 @@ void __init riscv_fill_hwcap(void)
SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
+ SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
--
2.39.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
@ 2023-05-21 11:47 ` Heiko Stuebner
0 siblings, 0 replies; 32+ messages in thread
From: Heiko Stuebner @ 2023-05-21 11:47 UTC (permalink / raw)
To: linux-riscv, palmer, paul.walmsley
Cc: linux-kernel, christoph.muellner, David.Laight, Heiko Stuebner
From: Christoph Müllner <christoph.muellner@vrull.eu>
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.STO 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 presence of Zawrs at run-time.
If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
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
Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
[rebase, update to review comments]
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
arch/riscv/Kconfig | 10 +++++
arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
arch/riscv/include/asm/errata_list.h | 14 ++++++
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 1 +
6 files changed, 91 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 348c0fa1fc8c..60ff303ff58a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -460,6 +460,16 @@ config RISCV_ISA_SVPBMT
If you don't know what to do here, say Y.
+config RISCV_ISA_ZAWRS
+ bool "Zawrs extension support"
+ depends on 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 TOOLCHAIN_HAS_ZBB
bool
default y
diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index 110752594228..bd64cfe5ae10 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 __nops(n) ".rept " #n "\nnop\n.endr\n"
#define nops(n) __asm__ __volatile__ (__nops(n))
@@ -44,6 +46,36 @@ do { \
___p1; \
})
+#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_reserved_relaxed(ptr) \
+({ \
+ typeof(*ptr) ___p1; \
+ if (sizeof(*ptr) == sizeof(int)) \
+ ___p1 = ___smp_load_reservedN("w", ptr); \
+ else if (sizeof(*ptr) == sizeof(long)) \
+ ___p1 = ___smp_load_reservedN("d", ptr); \
+ else \
+ compiletime_assert(0, \
+ "Need type compatible with LR/SC instructions " \
+ "for " __stringify(ptr)); \
+ ___p1; \
+})
+
+#define __smp_load_reserved_acquire(ptr) \
+({ \
+ typeof(*ptr) ___p1; \
+ ___p1 = __smp_load_reserved_relaxed(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
@@ -71,6 +103,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_STO(); \
+ } \
+ } \
+ (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_STO(); \
+ } \
+ } \
+ (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 fb1a810f3d8c..36a72a07d62c 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -44,6 +44,20 @@ 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_NTO ".long 0x00d00073"
+#define ALT_WRS_NTO() \
+asm volatile(ALTERNATIVE( \
+ "nop\n\t", \
+ ZAWRS_WRS_NTO "\n\t", \
+ 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
+
+#define ZAWRS_WRS_STO ".long 0x01d00073"
+#define ALT_WRS_STO() \
+asm volatile(ALTERNATIVE( \
+ "nop\n\t", \
+ ZAWRS_WRS_STO "\n\t", \
+ 0, RISCV_ISA_EXT_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 e0c40a4c63d5..4233d87539ab 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -46,6 +46,7 @@
#define RISCV_ISA_EXT_ZICBOZ 34
#define RISCV_ISA_EXT_SMAIA 35
#define RISCV_ISA_EXT_SSAIA 36
+#define RISCV_ISA_EXT_ZAWRS 37
#define RISCV_ISA_EXT_MAX 64
#define RISCV_ISA_EXT_NAME_LEN_MAX 32
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index c96aa56cf1c7..ac9c604d8936 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -184,6 +184,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
+ __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
__RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
__RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b1d6b7e4b829..d4a22a7aed99 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -236,6 +236,7 @@ void __init riscv_fill_hwcap(void)
SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
+ SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
--
2.39.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
2023-05-21 11:47 ` Heiko Stuebner
@ 2023-05-22 17:43 ` Conor Dooley
-1 siblings, 0 replies; 32+ messages in thread
From: Conor Dooley @ 2023-05-22 17:43 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]
Hey Heiko,
On Sun, May 21, 2023 at 01:47:15PM +0200, Heiko Stuebner wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> 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.STO 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 presence of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
>
> 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
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> [rebase, update to review comments]
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
I do intend actually reviewing these two, but busy this week with dt
stuff! In the interim, got some build complaints..
gcc-13 & clang-16 allmodconfig:
kernel/rcu/rcuscale.c:819:3: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
k210_nommu_defconfig:
include/linux/atomic/atomic-arch-fallback.h:249:23: error: implicit declaration of function 'smp_load_acquire' [-Werror=implicit-function-declaration]
include/linux/atomic/atomic-arch-fallback.h:265:17: error: implicit declaration of function 'smp_store_release' [-Werror=implicit-function-declaration]
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
@ 2023-05-22 17:43 ` Conor Dooley
0 siblings, 0 replies; 32+ messages in thread
From: Conor Dooley @ 2023-05-22 17:43 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
[-- Attachment #1.1: Type: text/plain, Size: 2088 bytes --]
Hey Heiko,
On Sun, May 21, 2023 at 01:47:15PM +0200, Heiko Stuebner wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> 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.STO 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 presence of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
>
> 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
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> [rebase, update to review comments]
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
I do intend actually reviewing these two, but busy this week with dt
stuff! In the interim, got some build complaints..
gcc-13 & clang-16 allmodconfig:
kernel/rcu/rcuscale.c:819:3: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
k210_nommu_defconfig:
include/linux/atomic/atomic-arch-fallback.h:249:23: error: implicit declaration of function 'smp_load_acquire' [-Werror=implicit-function-declaration]
include/linux/atomic/atomic-arch-fallback.h:265:17: error: implicit declaration of function 'smp_store_release' [-Werror=implicit-function-declaration]
Thanks,
Conor.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h
2023-05-21 11:47 ` Heiko Stuebner
@ 2023-05-24 14:01 ` Andrew Jones
-1 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2023-05-24 14:01 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
On Sun, May 21, 2023 at 01:47:14PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> This include is not currently needed for alternatives and creates
> possible issues when we want to add alternatives to deeper kernel
> infrastructure.
>
> The issue in question came from trying to introduce Zawrs alternatives,
> which resulted in a somewhat circular dependency like:
>
> In file included from ../include/linux/bitops.h:34,
> from ../include/linux/kernel.h:22,
> from ../arch/riscv/include/asm/alternative.h:16,
> from ../arch/riscv/include/asm/errata_list.h:8,
> from ../arch/riscv/include/asm/barrier.h:15,
> from ../include/linux/list.h:11,
> from ../include/linux/preempt.h:11,
> from ../include/linux/spinlock.h:56,
> from ../include/linux/mmzone.h:8,
> from ../include/linux/gfp.h:7,
> from ../include/linux/mm.h:7,
> from ../arch/riscv/kernel/asm-offsets.c:10:
> ../include/asm-generic/bitops/generic-non-atomic.h: In function ‘generic_test_bit_acquire’:
> ../include/asm-generic/bitops/generic-non-atomic.h:140:23: error: implicit declaration of function ‘smp_load_acquire’ [-Werror=implicit-function-declaration]
> 140 | return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1)));
> | ^~~~~~~~~~~~~~~~
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
> arch/riscv/include/asm/alternative.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 6a41537826a7..05885de6048c 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -13,7 +13,6 @@
> #ifdef CONFIG_RISCV_ALTERNATIVE
>
> #include <linux/init.h>
> -#include <linux/kernel.h>
> #include <linux/types.h>
> #include <linux/stddef.h>
> #include <asm/hwcap.h>
> --
> 2.39.0
>
Removing this include doesn't break compilation because the only callers
of PATCH_ID_CPUFEATURE_ID() and PATCH_ID_CPUFEATURE_VALUE(), which are
defined with lower/upper_16_bits(), are in arch/riscv/kernel/cpufeature.c,
which includes at least one thing which eventually includes linux/kernel.h
(the first path I found was linux/module.h -> linux/moduleparam.h ->
linux/kernel.h). Ideally we wouldn't rely on that luck. We can open
code the PATCH_ID_* macros to drop the lower/upper_16_bits() dependencies
or move the macros elsewhere, maybe, for now, just to
arch/riscv/kernel/cpufeature.c
Thanks,
drew
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h
@ 2023-05-24 14:01 ` Andrew Jones
0 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2023-05-24 14:01 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
On Sun, May 21, 2023 at 01:47:14PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> This include is not currently needed for alternatives and creates
> possible issues when we want to add alternatives to deeper kernel
> infrastructure.
>
> The issue in question came from trying to introduce Zawrs alternatives,
> which resulted in a somewhat circular dependency like:
>
> In file included from ../include/linux/bitops.h:34,
> from ../include/linux/kernel.h:22,
> from ../arch/riscv/include/asm/alternative.h:16,
> from ../arch/riscv/include/asm/errata_list.h:8,
> from ../arch/riscv/include/asm/barrier.h:15,
> from ../include/linux/list.h:11,
> from ../include/linux/preempt.h:11,
> from ../include/linux/spinlock.h:56,
> from ../include/linux/mmzone.h:8,
> from ../include/linux/gfp.h:7,
> from ../include/linux/mm.h:7,
> from ../arch/riscv/kernel/asm-offsets.c:10:
> ../include/asm-generic/bitops/generic-non-atomic.h: In function ‘generic_test_bit_acquire’:
> ../include/asm-generic/bitops/generic-non-atomic.h:140:23: error: implicit declaration of function ‘smp_load_acquire’ [-Werror=implicit-function-declaration]
> 140 | return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1)));
> | ^~~~~~~~~~~~~~~~
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
> arch/riscv/include/asm/alternative.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 6a41537826a7..05885de6048c 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -13,7 +13,6 @@
> #ifdef CONFIG_RISCV_ALTERNATIVE
>
> #include <linux/init.h>
> -#include <linux/kernel.h>
> #include <linux/types.h>
> #include <linux/stddef.h>
> #include <asm/hwcap.h>
> --
> 2.39.0
>
Removing this include doesn't break compilation because the only callers
of PATCH_ID_CPUFEATURE_ID() and PATCH_ID_CPUFEATURE_VALUE(), which are
defined with lower/upper_16_bits(), are in arch/riscv/kernel/cpufeature.c,
which includes at least one thing which eventually includes linux/kernel.h
(the first path I found was linux/module.h -> linux/moduleparam.h ->
linux/kernel.h). Ideally we wouldn't rely on that luck. We can open
code the PATCH_ID_* macros to drop the lower/upper_16_bits() dependencies
or move the macros elsewhere, maybe, for now, just to
arch/riscv/kernel/cpufeature.c
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
2023-05-21 11:47 ` Heiko Stuebner
@ 2023-05-24 17:05 ` Andrew Jones
-1 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2023-05-24 17:05 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
On Sun, May 21, 2023 at 01:47:15PM +0200, Heiko Stuebner wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> 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
I guess this peeling off of the first iteration is because it's expected
that the load generated by READ_ONCE() is more efficient than lr.w/d? If
we're worried about unnecessary use of lr.w/d, then shouldn't we look
for a solution that doesn't issue those instructions when we don't have
the Zawrs extension?
> and modifies the waiting
> loop such, that it is possible to use the WRS.STO 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 presence of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
>
> 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
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> [rebase, update to review comments]
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
> arch/riscv/Kconfig | 10 +++++
> arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
> arch/riscv/include/asm/errata_list.h | 14 ++++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 1 +
> 6 files changed, 91 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 348c0fa1fc8c..60ff303ff58a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -460,6 +460,16 @@ config RISCV_ISA_SVPBMT
>
> If you don't know what to do here, say Y.
>
> +config RISCV_ISA_ZAWRS
> + bool "Zawrs extension support"
This should be "Zawrs extension support for ..."
> + depends on 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 TOOLCHAIN_HAS_ZBB
> bool
> default y
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index 110752594228..bd64cfe5ae10 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 __nops(n) ".rept " #n "\nnop\n.endr\n"
> #define nops(n) __asm__ __volatile__ (__nops(n))
> @@ -44,6 +46,36 @@ do { \
> ___p1; \
> })
>
> +#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_reserved_relaxed(ptr) \
> +({ \
> + typeof(*ptr) ___p1; \
> + if (sizeof(*ptr) == sizeof(int)) \
> + ___p1 = ___smp_load_reservedN("w", ptr); \
> + else if (sizeof(*ptr) == sizeof(long)) \
> + ___p1 = ___smp_load_reservedN("d", ptr); \
> + else \
> + compiletime_assert(0, \
> + "Need type compatible with LR/SC instructions " \
> + "for " __stringify(ptr)); \
> + ___p1; \
It's more common to use a switch for these things, embedding the
lr.w/d asm in each case, something like the macros in
arch/riscv/include/asm/cmpxchg.h. Can we stick with that pattern?
> +})
> +
> +#define __smp_load_reserved_acquire(ptr) \
> +({ \
> + typeof(*ptr) ___p1; \
> + ___p1 = __smp_load_reserved_relaxed(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
> @@ -71,6 +103,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_STO(); \
> + } \
> + } \
> + (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_STO(); \
> + } \
> + } \
> + (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 fb1a810f3d8c..36a72a07d62c 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -44,6 +44,20 @@ 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_NTO ".long 0x00d00073"
> +#define ALT_WRS_NTO() \
> +asm volatile(ALTERNATIVE( \
> + "nop\n\t", \
> + ZAWRS_WRS_NTO "\n\t", \
> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
We don't use this. Do we need to define it now?
> +
> +#define ZAWRS_WRS_STO ".long 0x01d00073"
I'd prefer we use insn-def.h to define instructions, rather than scatter
.long's around, but since this instruction doesn't have any inputs, then
I guess it's not so important to use insn-def.h.
> +#define ALT_WRS_STO() \
> +asm volatile(ALTERNATIVE( \
> + "nop\n\t", \
> + ZAWRS_WRS_STO "\n\t", \
> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> +
These alternatives aren't supporting errata, so putting them in
errata_list.h doesn't seem appropriate. I think cpufeature.h
would be better.
> /*
> * _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 e0c40a4c63d5..4233d87539ab 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -46,6 +46,7 @@
> #define RISCV_ISA_EXT_ZICBOZ 34
> #define RISCV_ISA_EXT_SMAIA 35
> #define RISCV_ISA_EXT_SSAIA 36
> +#define RISCV_ISA_EXT_ZAWRS 37
>
> #define RISCV_ISA_EXT_MAX 64
> #define RISCV_ISA_EXT_NAME_LEN_MAX 32
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index c96aa56cf1c7..ac9c604d8936 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -184,6 +184,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> + __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b1d6b7e4b829..d4a22a7aed99 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -236,6 +236,7 @@ void __init riscv_fill_hwcap(void)
> SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> + SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
> SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> --
> 2.39.0
>
Thanks,
drew
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
@ 2023-05-24 17:05 ` Andrew Jones
0 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2023-05-24 17:05 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
On Sun, May 21, 2023 at 01:47:15PM +0200, Heiko Stuebner wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> 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
I guess this peeling off of the first iteration is because it's expected
that the load generated by READ_ONCE() is more efficient than lr.w/d? If
we're worried about unnecessary use of lr.w/d, then shouldn't we look
for a solution that doesn't issue those instructions when we don't have
the Zawrs extension?
> and modifies the waiting
> loop such, that it is possible to use the WRS.STO 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 presence of Zawrs at run-time.
> If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
>
> 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
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> [rebase, update to review comments]
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
> arch/riscv/Kconfig | 10 +++++
> arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
> arch/riscv/include/asm/errata_list.h | 14 ++++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 1 +
> 6 files changed, 91 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 348c0fa1fc8c..60ff303ff58a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -460,6 +460,16 @@ config RISCV_ISA_SVPBMT
>
> If you don't know what to do here, say Y.
>
> +config RISCV_ISA_ZAWRS
> + bool "Zawrs extension support"
This should be "Zawrs extension support for ..."
> + depends on 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 TOOLCHAIN_HAS_ZBB
> bool
> default y
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index 110752594228..bd64cfe5ae10 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 __nops(n) ".rept " #n "\nnop\n.endr\n"
> #define nops(n) __asm__ __volatile__ (__nops(n))
> @@ -44,6 +46,36 @@ do { \
> ___p1; \
> })
>
> +#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_reserved_relaxed(ptr) \
> +({ \
> + typeof(*ptr) ___p1; \
> + if (sizeof(*ptr) == sizeof(int)) \
> + ___p1 = ___smp_load_reservedN("w", ptr); \
> + else if (sizeof(*ptr) == sizeof(long)) \
> + ___p1 = ___smp_load_reservedN("d", ptr); \
> + else \
> + compiletime_assert(0, \
> + "Need type compatible with LR/SC instructions " \
> + "for " __stringify(ptr)); \
> + ___p1; \
It's more common to use a switch for these things, embedding the
lr.w/d asm in each case, something like the macros in
arch/riscv/include/asm/cmpxchg.h. Can we stick with that pattern?
> +})
> +
> +#define __smp_load_reserved_acquire(ptr) \
> +({ \
> + typeof(*ptr) ___p1; \
> + ___p1 = __smp_load_reserved_relaxed(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
> @@ -71,6 +103,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_STO(); \
> + } \
> + } \
> + (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_STO(); \
> + } \
> + } \
> + (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 fb1a810f3d8c..36a72a07d62c 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -44,6 +44,20 @@ 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_NTO ".long 0x00d00073"
> +#define ALT_WRS_NTO() \
> +asm volatile(ALTERNATIVE( \
> + "nop\n\t", \
> + ZAWRS_WRS_NTO "\n\t", \
> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
We don't use this. Do we need to define it now?
> +
> +#define ZAWRS_WRS_STO ".long 0x01d00073"
I'd prefer we use insn-def.h to define instructions, rather than scatter
.long's around, but since this instruction doesn't have any inputs, then
I guess it's not so important to use insn-def.h.
> +#define ALT_WRS_STO() \
> +asm volatile(ALTERNATIVE( \
> + "nop\n\t", \
> + ZAWRS_WRS_STO "\n\t", \
> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
> +
These alternatives aren't supporting errata, so putting them in
errata_list.h doesn't seem appropriate. I think cpufeature.h
would be better.
> /*
> * _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 e0c40a4c63d5..4233d87539ab 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -46,6 +46,7 @@
> #define RISCV_ISA_EXT_ZICBOZ 34
> #define RISCV_ISA_EXT_SMAIA 35
> #define RISCV_ISA_EXT_SSAIA 36
> +#define RISCV_ISA_EXT_ZAWRS 37
>
> #define RISCV_ISA_EXT_MAX 64
> #define RISCV_ISA_EXT_NAME_LEN_MAX 32
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index c96aa56cf1c7..ac9c604d8936 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -184,6 +184,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> + __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
> __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b1d6b7e4b829..d4a22a7aed99 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -236,6 +236,7 @@ void __init riscv_fill_hwcap(void)
> SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
> SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> + SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
> SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
> --
> 2.39.0
>
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
2023-05-24 17:05 ` Andrew Jones
@ 2023-05-24 23:00 ` Palmer Dabbelt
-1 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2023-05-24 23:00 UTC (permalink / raw)
To: ajones
Cc: heiko, linux-riscv, Paul Walmsley, linux-kernel,
christoph.muellner, David.Laight, heiko.stuebner
On Wed, 24 May 2023 10:05:52 PDT (-0700), ajones@ventanamicro.com wrote:
> On Sun, May 21, 2023 at 01:47:15PM +0200, Heiko Stuebner wrote:
>> From: Christoph Müllner <christoph.muellner@vrull.eu>
>>
>> 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
>
> I guess this peeling off of the first iteration is because it's expected
> that the load generated by READ_ONCE() is more efficient than lr.w/d? If
> we're worried about unnecessary use of lr.w/d, then shouldn't we look
> for a solution that doesn't issue those instructions when we don't have
> the Zawrs extension?
It's actually just a consequence of how the Linux hooks are described:
they're macros that take a C expression to test in the loop, and we
can't handle C expressions in LR/SC loops as that'd require compiler
support and nobody's figured out how to do that correctly yet (there
were some patches, but they had issues). So we need to do this awkward
bit of checking without the reservation and then waiting with the
reservation.
That's kind of a clunky pattern, so happy to hear if you've got a better
idea. It's been a year or so since I looked at this last, and from
looking at the spec I'd need to go dig into the "valid reservations in
non-constrained loops" bit to see if we can get away with something.
The semantics aren't clear from just one read of the spec.
Another option would be to call out the common cases of smp_cond_load_*
as explicit hooks, with generic implementations that just do the C
expression. There's only a few of these calls in the kernel and they
fall into a few patterns, so we might just need ==0, !=0, and
!=long-in-register.
>> and modifies the waiting
>> loop such, that it is possible to use the WRS.STO 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 presence of Zawrs at run-time.
>> If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
I haven't looked at the patch, but I'd expect we NOP out the whole
LR/WRS sequence? I don't remember any reason to have the load
reservation without the WRS, but it's been a while so I might be
forgetting something.
If we do need the LR, we should really then we replacing the WRS with an
SC instead to avoid a dangling reservation as IIRC those have a
performance hit on the SiFive cores (and presumably anyone else who
locks cache lines).
>>
>> 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
IIUC it's ratified? HEAD is kind of an odd commit saying it's ratified
and changing some text:
https://github.com/riscv/riscv-zawrs/commit/861483d99eedc60cbe9fd2ceb18dbf28d0905c9c
>>
>> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
>> [rebase, update to review comments]
>> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
>> ---
>> arch/riscv/Kconfig | 10 +++++
>> arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
>> arch/riscv/include/asm/errata_list.h | 14 ++++++
>> arch/riscv/include/asm/hwcap.h | 1 +
>> arch/riscv/kernel/cpu.c | 1 +
>> arch/riscv/kernel/cpufeature.c | 1 +
>> 6 files changed, 91 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 348c0fa1fc8c..60ff303ff58a 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -460,6 +460,16 @@ config RISCV_ISA_SVPBMT
>>
>> If you don't know what to do here, say Y.
>>
>> +config RISCV_ISA_ZAWRS
>> + bool "Zawrs extension support"
>
> This should be "Zawrs extension support for ..."
>
>> + depends on 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 TOOLCHAIN_HAS_ZBB
>> bool
>> default y
>> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
>> index 110752594228..bd64cfe5ae10 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 __nops(n) ".rept " #n "\nnop\n.endr\n"
>> #define nops(n) __asm__ __volatile__ (__nops(n))
>> @@ -44,6 +46,36 @@ do { \
>> ___p1; \
>> })
>>
>> +#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_reserved_relaxed(ptr) \
>> +({ \
>> + typeof(*ptr) ___p1; \
>> + if (sizeof(*ptr) == sizeof(int)) \
>> + ___p1 = ___smp_load_reservedN("w", ptr); \
>> + else if (sizeof(*ptr) == sizeof(long)) \
>> + ___p1 = ___smp_load_reservedN("d", ptr); \
>> + else \
>> + compiletime_assert(0, \
>> + "Need type compatible with LR/SC instructions " \
>> + "for " __stringify(ptr)); \
>> + ___p1; \
>
> It's more common to use a switch for these things, embedding the
> lr.w/d asm in each case, something like the macros in
> arch/riscv/include/asm/cmpxchg.h. Can we stick with that pattern?
>
>> +})
>> +
>> +#define __smp_load_reserved_acquire(ptr) \
>> +({ \
>> + typeof(*ptr) ___p1; \
>> + ___p1 = __smp_load_reserved_relaxed(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
>> @@ -71,6 +103,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_STO(); \
>> + } \
>> + } \
>> + (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_STO(); \
>> + } \
>> + } \
>> + (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 fb1a810f3d8c..36a72a07d62c 100644
>> --- a/arch/riscv/include/asm/errata_list.h
>> +++ b/arch/riscv/include/asm/errata_list.h
>> @@ -44,6 +44,20 @@ 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_NTO ".long 0x00d00073"
>> +#define ALT_WRS_NTO() \
>> +asm volatile(ALTERNATIVE( \
>> + "nop\n\t", \
>> + ZAWRS_WRS_NTO "\n\t", \
>> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
>
> We don't use this. Do we need to define it now?
>
>> +
>> +#define ZAWRS_WRS_STO ".long 0x01d00073"
>
> I'd prefer we use insn-def.h to define instructions, rather than scatter
> .long's around, but since this instruction doesn't have any inputs, then
> I guess it's not so important to use insn-def.h.
>
>> +#define ALT_WRS_STO() \
>> +asm volatile(ALTERNATIVE( \
>> + "nop\n\t", \
>> + ZAWRS_WRS_STO "\n\t", \
>> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
>> +
>
> These alternatives aren't supporting errata, so putting them in
> errata_list.h doesn't seem appropriate. I think cpufeature.h
> would be better.
>
>> /*
>> * _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 e0c40a4c63d5..4233d87539ab 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -46,6 +46,7 @@
>> #define RISCV_ISA_EXT_ZICBOZ 34
>> #define RISCV_ISA_EXT_SMAIA 35
>> #define RISCV_ISA_EXT_SSAIA 36
>> +#define RISCV_ISA_EXT_ZAWRS 37
>>
>> #define RISCV_ISA_EXT_MAX 64
>> #define RISCV_ISA_EXT_NAME_LEN_MAX 32
>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>> index c96aa56cf1c7..ac9c604d8936 100644
>> --- a/arch/riscv/kernel/cpu.c
>> +++ b/arch/riscv/kernel/cpu.c
>> @@ -184,6 +184,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
>> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
>> __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
>> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>> + __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
>> __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
>> __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
>> __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index b1d6b7e4b829..d4a22a7aed99 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -236,6 +236,7 @@ void __init riscv_fill_hwcap(void)
>> SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
>> SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
>> + SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
>> SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
>> SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
>> SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
>> --
>> 2.39.0
>>
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
@ 2023-05-24 23:00 ` Palmer Dabbelt
0 siblings, 0 replies; 32+ messages in thread
From: Palmer Dabbelt @ 2023-05-24 23:00 UTC (permalink / raw)
To: ajones
Cc: heiko, linux-riscv, Paul Walmsley, linux-kernel,
christoph.muellner, David.Laight, heiko.stuebner
On Wed, 24 May 2023 10:05:52 PDT (-0700), ajones@ventanamicro.com wrote:
> On Sun, May 21, 2023 at 01:47:15PM +0200, Heiko Stuebner wrote:
>> From: Christoph Müllner <christoph.muellner@vrull.eu>
>>
>> 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
>
> I guess this peeling off of the first iteration is because it's expected
> that the load generated by READ_ONCE() is more efficient than lr.w/d? If
> we're worried about unnecessary use of lr.w/d, then shouldn't we look
> for a solution that doesn't issue those instructions when we don't have
> the Zawrs extension?
It's actually just a consequence of how the Linux hooks are described:
they're macros that take a C expression to test in the loop, and we
can't handle C expressions in LR/SC loops as that'd require compiler
support and nobody's figured out how to do that correctly yet (there
were some patches, but they had issues). So we need to do this awkward
bit of checking without the reservation and then waiting with the
reservation.
That's kind of a clunky pattern, so happy to hear if you've got a better
idea. It's been a year or so since I looked at this last, and from
looking at the spec I'd need to go dig into the "valid reservations in
non-constrained loops" bit to see if we can get away with something.
The semantics aren't clear from just one read of the spec.
Another option would be to call out the common cases of smp_cond_load_*
as explicit hooks, with generic implementations that just do the C
expression. There's only a few of these calls in the kernel and they
fall into a few patterns, so we might just need ==0, !=0, and
!=long-in-register.
>> and modifies the waiting
>> loop such, that it is possible to use the WRS.STO 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 presence of Zawrs at run-time.
>> If available a NOP instruction will be replaced by WRS.NTO or WRS.STO.
I haven't looked at the patch, but I'd expect we NOP out the whole
LR/WRS sequence? I don't remember any reason to have the load
reservation without the WRS, but it's been a while so I might be
forgetting something.
If we do need the LR, we should really then we replacing the WRS with an
SC instead to avoid a dangling reservation as IIRC those have a
performance hit on the SiFive cores (and presumably anyone else who
locks cache lines).
>>
>> 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
IIUC it's ratified? HEAD is kind of an odd commit saying it's ratified
and changing some text:
https://github.com/riscv/riscv-zawrs/commit/861483d99eedc60cbe9fd2ceb18dbf28d0905c9c
>>
>> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
>> [rebase, update to review comments]
>> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
>> ---
>> arch/riscv/Kconfig | 10 +++++
>> arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
>> arch/riscv/include/asm/errata_list.h | 14 ++++++
>> arch/riscv/include/asm/hwcap.h | 1 +
>> arch/riscv/kernel/cpu.c | 1 +
>> arch/riscv/kernel/cpufeature.c | 1 +
>> 6 files changed, 91 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 348c0fa1fc8c..60ff303ff58a 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -460,6 +460,16 @@ config RISCV_ISA_SVPBMT
>>
>> If you don't know what to do here, say Y.
>>
>> +config RISCV_ISA_ZAWRS
>> + bool "Zawrs extension support"
>
> This should be "Zawrs extension support for ..."
>
>> + depends on 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 TOOLCHAIN_HAS_ZBB
>> bool
>> default y
>> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
>> index 110752594228..bd64cfe5ae10 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 __nops(n) ".rept " #n "\nnop\n.endr\n"
>> #define nops(n) __asm__ __volatile__ (__nops(n))
>> @@ -44,6 +46,36 @@ do { \
>> ___p1; \
>> })
>>
>> +#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_reserved_relaxed(ptr) \
>> +({ \
>> + typeof(*ptr) ___p1; \
>> + if (sizeof(*ptr) == sizeof(int)) \
>> + ___p1 = ___smp_load_reservedN("w", ptr); \
>> + else if (sizeof(*ptr) == sizeof(long)) \
>> + ___p1 = ___smp_load_reservedN("d", ptr); \
>> + else \
>> + compiletime_assert(0, \
>> + "Need type compatible with LR/SC instructions " \
>> + "for " __stringify(ptr)); \
>> + ___p1; \
>
> It's more common to use a switch for these things, embedding the
> lr.w/d asm in each case, something like the macros in
> arch/riscv/include/asm/cmpxchg.h. Can we stick with that pattern?
>
>> +})
>> +
>> +#define __smp_load_reserved_acquire(ptr) \
>> +({ \
>> + typeof(*ptr) ___p1; \
>> + ___p1 = __smp_load_reserved_relaxed(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
>> @@ -71,6 +103,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_STO(); \
>> + } \
>> + } \
>> + (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_STO(); \
>> + } \
>> + } \
>> + (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 fb1a810f3d8c..36a72a07d62c 100644
>> --- a/arch/riscv/include/asm/errata_list.h
>> +++ b/arch/riscv/include/asm/errata_list.h
>> @@ -44,6 +44,20 @@ 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_NTO ".long 0x00d00073"
>> +#define ALT_WRS_NTO() \
>> +asm volatile(ALTERNATIVE( \
>> + "nop\n\t", \
>> + ZAWRS_WRS_NTO "\n\t", \
>> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
>
> We don't use this. Do we need to define it now?
>
>> +
>> +#define ZAWRS_WRS_STO ".long 0x01d00073"
>
> I'd prefer we use insn-def.h to define instructions, rather than scatter
> .long's around, but since this instruction doesn't have any inputs, then
> I guess it's not so important to use insn-def.h.
>
>> +#define ALT_WRS_STO() \
>> +asm volatile(ALTERNATIVE( \
>> + "nop\n\t", \
>> + ZAWRS_WRS_STO "\n\t", \
>> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS))
>> +
>
> These alternatives aren't supporting errata, so putting them in
> errata_list.h doesn't seem appropriate. I think cpufeature.h
> would be better.
>
>> /*
>> * _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 e0c40a4c63d5..4233d87539ab 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -46,6 +46,7 @@
>> #define RISCV_ISA_EXT_ZICBOZ 34
>> #define RISCV_ISA_EXT_SMAIA 35
>> #define RISCV_ISA_EXT_SSAIA 36
>> +#define RISCV_ISA_EXT_ZAWRS 37
>>
>> #define RISCV_ISA_EXT_MAX 64
>> #define RISCV_ISA_EXT_NAME_LEN_MAX 32
>> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>> index c96aa56cf1c7..ac9c604d8936 100644
>> --- a/arch/riscv/kernel/cpu.c
>> +++ b/arch/riscv/kernel/cpu.c
>> @@ -184,6 +184,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
>> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
>> __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
>> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>> + __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
>> __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
>> __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
>> __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index b1d6b7e4b829..d4a22a7aed99 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -236,6 +236,7 @@ void __init riscv_fill_hwcap(void)
>> SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
>> SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
>> SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
>> + SET_ISA_EXT_MAP("zawrs", RISCV_ISA_EXT_ZAWRS);
>> SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
>> SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
>> SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
>> --
>> 2.39.0
>>
>
> Thanks,
> drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
2023-05-24 23:00 ` Palmer Dabbelt
@ 2023-05-30 18:45 ` Andrea Parri
-1 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2023-05-30 18:45 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: ajones, heiko, linux-riscv, Paul Walmsley, linux-kernel,
christoph.muellner, David.Laight, heiko.stuebner
On Wed, May 24, 2023 at 04:00:43PM -0700, Palmer Dabbelt wrote:
> On Wed, 24 May 2023 10:05:52 PDT (-0700), ajones@ventanamicro.com wrote:
> > I guess this peeling off of the first iteration is because it's expected
> > that the load generated by READ_ONCE() is more efficient than lr.w/d? If
> > we're worried about unnecessary use of lr.w/d, then shouldn't we look
> > for a solution that doesn't issue those instructions when we don't have
> > the Zawrs extension?
>
> It's actually just a consequence of how the Linux hooks are described:
> they're macros that take a C expression to test in the loop, and we can't
> handle C expressions in LR/SC loops as that'd require compiler support and
> nobody's figured out how to do that correctly yet (there were some patches,
> but they had issues). So we need to do this awkward bit of checking without
> the reservation and then waiting with the reservation.
I believe Andrew was really just hinting to something like (from
arch/arm64/):
#define smp_cond_load_relaxed(ptr, cond_expr) \
({ \
typeof(ptr) __PTR = (ptr); \
__unqual_scalar_typeof(*ptr) VAL; \
for (;;) { \
VAL = READ_ONCE(*__PTR); \
if (cond_expr) \
break; \
__cmpwait_relaxed(__PTR, VAL); \
} \
(typeof(*ptr))VAL; \
})
where the __cmpwait_relaxed() would issue NOPs without Zawrs, a
sequence "lr.* ; beq ; wrs.sto" otherwise. (with the "dangling
reservation" when we branch, similarly to CMPXCHG)?
Andrea
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks
@ 2023-05-30 18:45 ` Andrea Parri
0 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2023-05-30 18:45 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: ajones, heiko, linux-riscv, Paul Walmsley, linux-kernel,
christoph.muellner, David.Laight, heiko.stuebner
On Wed, May 24, 2023 at 04:00:43PM -0700, Palmer Dabbelt wrote:
> On Wed, 24 May 2023 10:05:52 PDT (-0700), ajones@ventanamicro.com wrote:
> > I guess this peeling off of the first iteration is because it's expected
> > that the load generated by READ_ONCE() is more efficient than lr.w/d? If
> > we're worried about unnecessary use of lr.w/d, then shouldn't we look
> > for a solution that doesn't issue those instructions when we don't have
> > the Zawrs extension?
>
> It's actually just a consequence of how the Linux hooks are described:
> they're macros that take a C expression to test in the loop, and we can't
> handle C expressions in LR/SC loops as that'd require compiler support and
> nobody's figured out how to do that correctly yet (there were some patches,
> but they had issues). So we need to do this awkward bit of checking without
> the reservation and then waiting with the reservation.
I believe Andrew was really just hinting to something like (from
arch/arm64/):
#define smp_cond_load_relaxed(ptr, cond_expr) \
({ \
typeof(ptr) __PTR = (ptr); \
__unqual_scalar_typeof(*ptr) VAL; \
for (;;) { \
VAL = READ_ONCE(*__PTR); \
if (cond_expr) \
break; \
__cmpwait_relaxed(__PTR, VAL); \
} \
(typeof(*ptr))VAL; \
})
where the __cmpwait_relaxed() would issue NOPs without Zawrs, a
sequence "lr.* ; beq ; wrs.sto" otherwise. (with the "dangling
reservation" when we branch, similarly to CMPXCHG)?
Andrea
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2023-05-21 11:47 ` Heiko Stuebner
@ 2023-10-19 14:21 ` Andrea Parri
-1 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2023-10-19 14:21 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
On Sun, May 21, 2023 at 01:47:13PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Zawrs [0] was ratified in november 2022 [1], so I've resurrect the patch
> adding Zawrs support for spinlocks and adapted it to recent kernel
> changes.
>
> Also incorporated are the nice comments David Laight provided on v2.
>
>
> Changes since v2:
> - Rebase on top of 6.4-rc1
> - Adapt to changed alternatives Kconfig handling
> - Adapt to changed cpufeature extension handling
> - Address review comments from David Laight
> - better handling for 32/64bit cases (less ifdefery)
> - less macros calling macros
> - don't duplicate __smp_load_reserved_relaxed in
> __smp_load_reserved_aquire
>
> Changes since v1:
> - Fixing type checking code for 32/64-bit values
> - Adjustments according to the specification change
> - Adding "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS
>
>
> [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> [1] https://github.com/riscv/riscv-zawrs/commit/9ff54f7e7fcd95cf1b111d2e54276ff1183bcd37
>
> Christoph Müllner (1):
> riscv: Add Zawrs support for spinlocks
>
> Heiko Stuebner (1):
> riscv: don't include kernel.h into alternative.h
>
> arch/riscv/Kconfig | 10 +++++
> arch/riscv/include/asm/alternative.h | 1 -
> arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
> arch/riscv/include/asm/errata_list.h | 14 ++++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 1 +
> 7 files changed, 91 insertions(+), 1 deletion(-)
Hi Heiko and Christoph,
I was wondering about the plan for this series: am I missing some update to
this discussion? do we have a new version to review?
I actually went ahead (as Palmer suggested in private :-) ) and spent some
time trying to integrate feedback provided in this thread into your changes,
obtaining the diff below (on 6.6-rc6, the #include removal fixes some nommu
builds and should/can be splitted into a separate patch); thoughts?
Andrea
From 79d25361ddd4a14db6ce94aec140efbdf2d89684 Mon Sep 17 00:00:00 2001
From: Andrea Parri <parri.andrea@gmail.com>
Date: Wed, 18 Oct 2023 02:22:28 +0200
Subject: [PATCH] riscv: Implement LR+WRS-based smp_cond_load_{relaxed,acquire}()
The Zawrs extension defines instructions allowing a core to enter
a low-power state and wait on a store to a memory location.
Introduce the Zawrs-instruction WRS.STO and use it in the polling
loops of the macros smp_cond_load_{relaxed,acquire}().
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
[based on the arm64 implementation and work from Heiko and Christoph]
---
arch/riscv/Kconfig | 14 +++++++++
arch/riscv/include/asm/barrier.h | 26 ++++++++++++++++
arch/riscv/include/asm/cmpxchg.h | 51 +++++++++++++++++++++++++++++++
arch/riscv/include/asm/hwcap.h | 3 +-
arch/riscv/include/asm/insn-def.h | 2 ++
arch/riscv/kernel/cpufeature.c | 1 +
6 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d607ab0f7c6da..ff49f90d175ba 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -489,6 +489,20 @@ config RISCV_ISA_SVPBMT
If you don't know what to do here, say Y.
+config RISCV_ISA_ZAWRS
+ bool "Zawrs extension support for wait-on-reservation-set instructions"
+ depends on RISCV_ALTERNATIVE
+ default y
+ help
+ Adds support to dynamically detect the presence of the Zawrs
+ extension and enable its usage.
+
+ The Zawrs extension defines a pair of instructions to be used
+ in polling loops that allows a core to enter a low-power state
+ and wait on a store to a memory location.
+
+ If you don't know what to do here, say Y.
+
config TOOLCHAIN_HAS_V
bool
default y
diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index 110752594228e..7ab7a28e72210 100644
--- a/arch/riscv/include/asm/barrier.h
+++ b/arch/riscv/include/asm/barrier.h
@@ -71,6 +71,32 @@ 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; \
+ for (;;) { \
+ VAL = READ_ONCE(*__PTR); \
+ if (cond_expr) \
+ break; \
+ __cmpwait_relaxed(__PTR, VAL); \
+ } \
+ (typeof(*ptr))VAL; \
+})
+
+#define smp_cond_load_acquire(ptr, cond_expr) \
+({ \
+ typeof(ptr) __PTR = (ptr); \
+ __unqual_scalar_typeof(*ptr) VAL; \
+ for (;;) { \
+ VAL = smp_load_acquire(__PTR); \
+ if (cond_expr) \
+ break; \
+ __cmpwait_relaxed(__PTR, VAL); \
+ } \
+ (typeof(*ptr))VAL; \
+})
+
#include <asm-generic/barrier.h>
#endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 2f4726d3cfcc2..65bc21379f40e 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -8,8 +8,11 @@
#include <linux/bug.h>
+#include <asm/alternative-macros.h>
#include <asm/barrier.h>
#include <asm/fence.h>
+#include <asm/hwcap.h>
+#include <asm/insn-def.h>
#define __xchg_relaxed(ptr, new, size) \
({ \
@@ -360,4 +363,52 @@
arch_cmpxchg_relaxed((ptr), (o), (n)); \
})
+#define __CMPWAIT_CASE(w, sz) \
+static inline void __cmpwait_case_##sz(volatile void *ptr, \
+ unsigned long val) \
+{ \
+ unsigned long tmp; \
+ \
+ asm volatile(ALTERNATIVE( \
+ __nops(4), \
+ "lr." #w "\t" "%[tmp], %[v]\n\t" \
+ "xor %[tmp], %[tmp], %[val]\n\t" \
+ "bnez %[tmp], 1f\n\t" \
+ WRS_sto "\n\t" \
+ "1:\n", \
+ 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS) \
+ : [tmp] "=&r" (tmp), [v] "+A" (*(u##sz *)ptr) \
+ : [val] "rJ" (val) \
+ : "memory"); \
+}
+
+__CMPWAIT_CASE(w, 32);
+__CMPWAIT_CASE(d, 64);
+
+#undef __CMPWAIT_CASE
+
+#define __CMPWAIT_GEN() \
+static __always_inline void __cmpwait(volatile void *ptr, \
+ unsigned long val, \
+ int size) \
+{ \
+ switch (size) { \
+ case 4: \
+ return __cmpwait_case_32(ptr, val); \
+ case 8: \
+ return __cmpwait_case_64(ptr, val); \
+ default: \
+ BUILD_BUG(); \
+ } \
+ \
+ unreachable(); \
+}
+
+__CMPWAIT_GEN()
+
+#undef __CMPWAIT_GEN
+
+#define __cmpwait_relaxed(ptr, val) \
+ __cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr)))
+
#endif /* _ASM_RISCV_CMPXCHG_H */
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index b7b58258f6c7c..afabcbf849526 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -58,6 +58,7 @@
#define RISCV_ISA_EXT_ZICSR 40
#define RISCV_ISA_EXT_ZIFENCEI 41
#define RISCV_ISA_EXT_ZIHPM 42
+#define RISCV_ISA_EXT_ZAWRS 43
#define RISCV_ISA_EXT_MAX 64
@@ -69,8 +70,6 @@
#ifndef __ASSEMBLY__
-#include <linux/jump_label.h>
-
unsigned long riscv_get_elf_hwcap(void);
struct riscv_isa_ext_data {
diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
index 6960beb75f329..fecb744ed7b29 100644
--- a/arch/riscv/include/asm/insn-def.h
+++ b/arch/riscv/include/asm/insn-def.h
@@ -196,4 +196,6 @@
INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \
RS1(base), SIMM12(4))
+#define WRS_sto ".long 0x01d00073"
+
#endif /* __ASM_INSN_DEF_H */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1cfbba65d11ae..044682f54f78f 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -181,6 +181,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
+ __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
};
const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
--
2.34.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2023-10-19 14:21 ` Andrea Parri
0 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2023-10-19 14:21 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, paul.walmsley, linux-kernel,
christoph.muellner, David.Laight, Heiko Stuebner
On Sun, May 21, 2023 at 01:47:13PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Zawrs [0] was ratified in november 2022 [1], so I've resurrect the patch
> adding Zawrs support for spinlocks and adapted it to recent kernel
> changes.
>
> Also incorporated are the nice comments David Laight provided on v2.
>
>
> Changes since v2:
> - Rebase on top of 6.4-rc1
> - Adapt to changed alternatives Kconfig handling
> - Adapt to changed cpufeature extension handling
> - Address review comments from David Laight
> - better handling for 32/64bit cases (less ifdefery)
> - less macros calling macros
> - don't duplicate __smp_load_reserved_relaxed in
> __smp_load_reserved_aquire
>
> Changes since v1:
> - Fixing type checking code for 32/64-bit values
> - Adjustments according to the specification change
> - Adding "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS
>
>
> [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> [1] https://github.com/riscv/riscv-zawrs/commit/9ff54f7e7fcd95cf1b111d2e54276ff1183bcd37
>
> Christoph Müllner (1):
> riscv: Add Zawrs support for spinlocks
>
> Heiko Stuebner (1):
> riscv: don't include kernel.h into alternative.h
>
> arch/riscv/Kconfig | 10 +++++
> arch/riscv/include/asm/alternative.h | 1 -
> arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
> arch/riscv/include/asm/errata_list.h | 14 ++++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 1 +
> 7 files changed, 91 insertions(+), 1 deletion(-)
Hi Heiko and Christoph,
I was wondering about the plan for this series: am I missing some update to
this discussion? do we have a new version to review?
I actually went ahead (as Palmer suggested in private :-) ) and spent some
time trying to integrate feedback provided in this thread into your changes,
obtaining the diff below (on 6.6-rc6, the #include removal fixes some nommu
builds and should/can be splitted into a separate patch); thoughts?
Andrea
From 79d25361ddd4a14db6ce94aec140efbdf2d89684 Mon Sep 17 00:00:00 2001
From: Andrea Parri <parri.andrea@gmail.com>
Date: Wed, 18 Oct 2023 02:22:28 +0200
Subject: [PATCH] riscv: Implement LR+WRS-based smp_cond_load_{relaxed,acquire}()
The Zawrs extension defines instructions allowing a core to enter
a low-power state and wait on a store to a memory location.
Introduce the Zawrs-instruction WRS.STO and use it in the polling
loops of the macros smp_cond_load_{relaxed,acquire}().
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
[based on the arm64 implementation and work from Heiko and Christoph]
---
arch/riscv/Kconfig | 14 +++++++++
arch/riscv/include/asm/barrier.h | 26 ++++++++++++++++
arch/riscv/include/asm/cmpxchg.h | 51 +++++++++++++++++++++++++++++++
arch/riscv/include/asm/hwcap.h | 3 +-
arch/riscv/include/asm/insn-def.h | 2 ++
arch/riscv/kernel/cpufeature.c | 1 +
6 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d607ab0f7c6da..ff49f90d175ba 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -489,6 +489,20 @@ config RISCV_ISA_SVPBMT
If you don't know what to do here, say Y.
+config RISCV_ISA_ZAWRS
+ bool "Zawrs extension support for wait-on-reservation-set instructions"
+ depends on RISCV_ALTERNATIVE
+ default y
+ help
+ Adds support to dynamically detect the presence of the Zawrs
+ extension and enable its usage.
+
+ The Zawrs extension defines a pair of instructions to be used
+ in polling loops that allows a core to enter a low-power state
+ and wait on a store to a memory location.
+
+ If you don't know what to do here, say Y.
+
config TOOLCHAIN_HAS_V
bool
default y
diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index 110752594228e..7ab7a28e72210 100644
--- a/arch/riscv/include/asm/barrier.h
+++ b/arch/riscv/include/asm/barrier.h
@@ -71,6 +71,32 @@ 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; \
+ for (;;) { \
+ VAL = READ_ONCE(*__PTR); \
+ if (cond_expr) \
+ break; \
+ __cmpwait_relaxed(__PTR, VAL); \
+ } \
+ (typeof(*ptr))VAL; \
+})
+
+#define smp_cond_load_acquire(ptr, cond_expr) \
+({ \
+ typeof(ptr) __PTR = (ptr); \
+ __unqual_scalar_typeof(*ptr) VAL; \
+ for (;;) { \
+ VAL = smp_load_acquire(__PTR); \
+ if (cond_expr) \
+ break; \
+ __cmpwait_relaxed(__PTR, VAL); \
+ } \
+ (typeof(*ptr))VAL; \
+})
+
#include <asm-generic/barrier.h>
#endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 2f4726d3cfcc2..65bc21379f40e 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -8,8 +8,11 @@
#include <linux/bug.h>
+#include <asm/alternative-macros.h>
#include <asm/barrier.h>
#include <asm/fence.h>
+#include <asm/hwcap.h>
+#include <asm/insn-def.h>
#define __xchg_relaxed(ptr, new, size) \
({ \
@@ -360,4 +363,52 @@
arch_cmpxchg_relaxed((ptr), (o), (n)); \
})
+#define __CMPWAIT_CASE(w, sz) \
+static inline void __cmpwait_case_##sz(volatile void *ptr, \
+ unsigned long val) \
+{ \
+ unsigned long tmp; \
+ \
+ asm volatile(ALTERNATIVE( \
+ __nops(4), \
+ "lr." #w "\t" "%[tmp], %[v]\n\t" \
+ "xor %[tmp], %[tmp], %[val]\n\t" \
+ "bnez %[tmp], 1f\n\t" \
+ WRS_sto "\n\t" \
+ "1:\n", \
+ 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS) \
+ : [tmp] "=&r" (tmp), [v] "+A" (*(u##sz *)ptr) \
+ : [val] "rJ" (val) \
+ : "memory"); \
+}
+
+__CMPWAIT_CASE(w, 32);
+__CMPWAIT_CASE(d, 64);
+
+#undef __CMPWAIT_CASE
+
+#define __CMPWAIT_GEN() \
+static __always_inline void __cmpwait(volatile void *ptr, \
+ unsigned long val, \
+ int size) \
+{ \
+ switch (size) { \
+ case 4: \
+ return __cmpwait_case_32(ptr, val); \
+ case 8: \
+ return __cmpwait_case_64(ptr, val); \
+ default: \
+ BUILD_BUG(); \
+ } \
+ \
+ unreachable(); \
+}
+
+__CMPWAIT_GEN()
+
+#undef __CMPWAIT_GEN
+
+#define __cmpwait_relaxed(ptr, val) \
+ __cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr)))
+
#endif /* _ASM_RISCV_CMPXCHG_H */
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index b7b58258f6c7c..afabcbf849526 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -58,6 +58,7 @@
#define RISCV_ISA_EXT_ZICSR 40
#define RISCV_ISA_EXT_ZIFENCEI 41
#define RISCV_ISA_EXT_ZIHPM 42
+#define RISCV_ISA_EXT_ZAWRS 43
#define RISCV_ISA_EXT_MAX 64
@@ -69,8 +70,6 @@
#ifndef __ASSEMBLY__
-#include <linux/jump_label.h>
-
unsigned long riscv_get_elf_hwcap(void);
struct riscv_isa_ext_data {
diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
index 6960beb75f329..fecb744ed7b29 100644
--- a/arch/riscv/include/asm/insn-def.h
+++ b/arch/riscv/include/asm/insn-def.h
@@ -196,4 +196,6 @@
INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \
RS1(base), SIMM12(4))
+#define WRS_sto ".long 0x01d00073"
+
#endif /* __ASM_INSN_DEF_H */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1cfbba65d11ae..044682f54f78f 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -181,6 +181,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
__RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
__RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
+ __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
};
const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2023-10-19 14:21 ` Andrea Parri
@ 2023-10-19 16:22 ` Christoph Müllner
-1 siblings, 0 replies; 32+ messages in thread
From: Christoph Müllner @ 2023-10-19 16:22 UTC (permalink / raw)
To: Andrea Parri
Cc: Heiko Stuebner, linux-riscv, palmer, paul.walmsley, linux-kernel,
David.Laight, Heiko Stuebner, Andrew Jones, Conor Dooley
On Thu, Oct 19, 2023 at 4:21 PM Andrea Parri <parri.andrea@gmail.com> wrote:
>
> On Sun, May 21, 2023 at 01:47:13PM +0200, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> >
> > Zawrs [0] was ratified in november 2022 [1], so I've resurrect the patch
> > adding Zawrs support for spinlocks and adapted it to recent kernel
> > changes.
> >
> > Also incorporated are the nice comments David Laight provided on v2.
> >
> >
> > Changes since v2:
> > - Rebase on top of 6.4-rc1
> > - Adapt to changed alternatives Kconfig handling
> > - Adapt to changed cpufeature extension handling
> > - Address review comments from David Laight
> > - better handling for 32/64bit cases (less ifdefery)
> > - less macros calling macros
> > - don't duplicate __smp_load_reserved_relaxed in
> > __smp_load_reserved_aquire
> >
> > Changes since v1:
> > - Fixing type checking code for 32/64-bit values
> > - Adjustments according to the specification change
> > - Adding "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS
> >
> >
> > [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> > [1] https://github.com/riscv/riscv-zawrs/commit/9ff54f7e7fcd95cf1b111d2e54276ff1183bcd37
> >
> > Christoph Müllner (1):
> > riscv: Add Zawrs support for spinlocks
> >
> > Heiko Stuebner (1):
> > riscv: don't include kernel.h into alternative.h
> >
> > arch/riscv/Kconfig | 10 +++++
> > arch/riscv/include/asm/alternative.h | 1 -
> > arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
> > arch/riscv/include/asm/errata_list.h | 14 ++++++
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/kernel/cpu.c | 1 +
> > arch/riscv/kernel/cpufeature.c | 1 +
> > 7 files changed, 91 insertions(+), 1 deletion(-)
>
>
> Hi Heiko and Christoph,
>
> I was wondering about the plan for this series: am I missing some update to
> this discussion? do we have a new version to review?
Hi Andrea,
Thanks for reaching out!
We have this on the list of tasks that we would like to work on,
but as this currently has low priority for us, we don't have a date
when we can come up with a revised patch.
> I actually went ahead (as Palmer suggested in private :-) ) and spent some
> time trying to integrate feedback provided in this thread into your changes,
> obtaining the diff below (on 6.6-rc6, the #include removal fixes some nommu
> builds and should/can be splitted into a separate patch); thoughts?
I had a quick look at your changes, and they look good to me.
Did you agree with Palmer about testing requirements?
I.e., do we need to run this on hardware that implements Zawrs in a
non-trivial way?
I can try to raise the priority on this here, but can't promise anything.
For me it is also ok if you take over this patchset.
BR
Christoph
>
> Andrea
>
>
> From 79d25361ddd4a14db6ce94aec140efbdf2d89684 Mon Sep 17 00:00:00 2001
> From: Andrea Parri <parri.andrea@gmail.com>
> Date: Wed, 18 Oct 2023 02:22:28 +0200
> Subject: [PATCH] riscv: Implement LR+WRS-based smp_cond_load_{relaxed,acquire}()
>
> The Zawrs extension defines instructions allowing a core to enter
> a low-power state and wait on a store to a memory location.
>
> Introduce the Zawrs-instruction WRS.STO and use it in the polling
> loops of the macros smp_cond_load_{relaxed,acquire}().
>
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> [based on the arm64 implementation and work from Heiko and Christoph]
> ---
> arch/riscv/Kconfig | 14 +++++++++
> arch/riscv/include/asm/barrier.h | 26 ++++++++++++++++
> arch/riscv/include/asm/cmpxchg.h | 51 +++++++++++++++++++++++++++++++
> arch/riscv/include/asm/hwcap.h | 3 +-
> arch/riscv/include/asm/insn-def.h | 2 ++
> arch/riscv/kernel/cpufeature.c | 1 +
> 6 files changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index d607ab0f7c6da..ff49f90d175ba 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -489,6 +489,20 @@ config RISCV_ISA_SVPBMT
>
> If you don't know what to do here, say Y.
>
> +config RISCV_ISA_ZAWRS
> + bool "Zawrs extension support for wait-on-reservation-set instructions"
> + depends on RISCV_ALTERNATIVE
> + default y
> + help
> + Adds support to dynamically detect the presence of the Zawrs
> + extension and enable its usage.
> +
> + The Zawrs extension defines a pair of instructions to be used
> + in polling loops that allows a core to enter a low-power state
> + and wait on a store to a memory location.
> +
> + If you don't know what to do here, say Y.
> +
> config TOOLCHAIN_HAS_V
> bool
> default y
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index 110752594228e..7ab7a28e72210 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -71,6 +71,32 @@ 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; \
> + for (;;) { \
> + VAL = READ_ONCE(*__PTR); \
> + if (cond_expr) \
> + break; \
> + __cmpwait_relaxed(__PTR, VAL); \
> + } \
> + (typeof(*ptr))VAL; \
> +})
> +
> +#define smp_cond_load_acquire(ptr, cond_expr) \
> +({ \
> + typeof(ptr) __PTR = (ptr); \
> + __unqual_scalar_typeof(*ptr) VAL; \
> + for (;;) { \
> + VAL = smp_load_acquire(__PTR); \
> + if (cond_expr) \
> + break; \
> + __cmpwait_relaxed(__PTR, VAL); \
> + } \
> + (typeof(*ptr))VAL; \
> +})
> +
> #include <asm-generic/barrier.h>
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 2f4726d3cfcc2..65bc21379f40e 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -8,8 +8,11 @@
>
> #include <linux/bug.h>
>
> +#include <asm/alternative-macros.h>
> #include <asm/barrier.h>
> #include <asm/fence.h>
> +#include <asm/hwcap.h>
> +#include <asm/insn-def.h>
>
> #define __xchg_relaxed(ptr, new, size) \
> ({ \
> @@ -360,4 +363,52 @@
> arch_cmpxchg_relaxed((ptr), (o), (n)); \
> })
>
> +#define __CMPWAIT_CASE(w, sz) \
> +static inline void __cmpwait_case_##sz(volatile void *ptr, \
> + unsigned long val) \
> +{ \
> + unsigned long tmp; \
> + \
> + asm volatile(ALTERNATIVE( \
> + __nops(4), \
> + "lr." #w "\t" "%[tmp], %[v]\n\t" \
> + "xor %[tmp], %[tmp], %[val]\n\t" \
> + "bnez %[tmp], 1f\n\t" \
> + WRS_sto "\n\t" \
> + "1:\n", \
> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS) \
> + : [tmp] "=&r" (tmp), [v] "+A" (*(u##sz *)ptr) \
> + : [val] "rJ" (val) \
> + : "memory"); \
> +}
> +
> +__CMPWAIT_CASE(w, 32);
> +__CMPWAIT_CASE(d, 64);
> +
> +#undef __CMPWAIT_CASE
> +
> +#define __CMPWAIT_GEN() \
> +static __always_inline void __cmpwait(volatile void *ptr, \
> + unsigned long val, \
> + int size) \
> +{ \
> + switch (size) { \
> + case 4: \
> + return __cmpwait_case_32(ptr, val); \
> + case 8: \
> + return __cmpwait_case_64(ptr, val); \
> + default: \
> + BUILD_BUG(); \
> + } \
> + \
> + unreachable(); \
> +}
> +
> +__CMPWAIT_GEN()
> +
> +#undef __CMPWAIT_GEN
> +
> +#define __cmpwait_relaxed(ptr, val) \
> + __cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr)))
> +
> #endif /* _ASM_RISCV_CMPXCHG_H */
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index b7b58258f6c7c..afabcbf849526 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -58,6 +58,7 @@
> #define RISCV_ISA_EXT_ZICSR 40
> #define RISCV_ISA_EXT_ZIFENCEI 41
> #define RISCV_ISA_EXT_ZIHPM 42
> +#define RISCV_ISA_EXT_ZAWRS 43
>
> #define RISCV_ISA_EXT_MAX 64
>
> @@ -69,8 +70,6 @@
>
> #ifndef __ASSEMBLY__
>
> -#include <linux/jump_label.h>
> -
> unsigned long riscv_get_elf_hwcap(void);
>
> struct riscv_isa_ext_data {
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index 6960beb75f329..fecb744ed7b29 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -196,4 +196,6 @@
> INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \
> RS1(base), SIMM12(4))
>
> +#define WRS_sto ".long 0x01d00073"
> +
> #endif /* __ASM_INSN_DEF_H */
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1cfbba65d11ae..044682f54f78f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -181,6 +181,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
> __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> + __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> };
>
> const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2023-10-19 16:22 ` Christoph Müllner
0 siblings, 0 replies; 32+ messages in thread
From: Christoph Müllner @ 2023-10-19 16:22 UTC (permalink / raw)
To: Andrea Parri
Cc: Heiko Stuebner, linux-riscv, palmer, paul.walmsley, linux-kernel,
David.Laight, Heiko Stuebner, Andrew Jones, Conor Dooley
On Thu, Oct 19, 2023 at 4:21 PM Andrea Parri <parri.andrea@gmail.com> wrote:
>
> On Sun, May 21, 2023 at 01:47:13PM +0200, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> >
> > Zawrs [0] was ratified in november 2022 [1], so I've resurrect the patch
> > adding Zawrs support for spinlocks and adapted it to recent kernel
> > changes.
> >
> > Also incorporated are the nice comments David Laight provided on v2.
> >
> >
> > Changes since v2:
> > - Rebase on top of 6.4-rc1
> > - Adapt to changed alternatives Kconfig handling
> > - Adapt to changed cpufeature extension handling
> > - Address review comments from David Laight
> > - better handling for 32/64bit cases (less ifdefery)
> > - less macros calling macros
> > - don't duplicate __smp_load_reserved_relaxed in
> > __smp_load_reserved_aquire
> >
> > Changes since v1:
> > - Fixing type checking code for 32/64-bit values
> > - Adjustments according to the specification change
> > - Adding "depends on !XIP_KERNEL" to RISCV_ISA_ZAWRS
> >
> >
> > [0] https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> > [1] https://github.com/riscv/riscv-zawrs/commit/9ff54f7e7fcd95cf1b111d2e54276ff1183bcd37
> >
> > Christoph Müllner (1):
> > riscv: Add Zawrs support for spinlocks
> >
> > Heiko Stuebner (1):
> > riscv: don't include kernel.h into alternative.h
> >
> > arch/riscv/Kconfig | 10 +++++
> > arch/riscv/include/asm/alternative.h | 1 -
> > arch/riscv/include/asm/barrier.h | 64 ++++++++++++++++++++++++++++
> > arch/riscv/include/asm/errata_list.h | 14 ++++++
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/kernel/cpu.c | 1 +
> > arch/riscv/kernel/cpufeature.c | 1 +
> > 7 files changed, 91 insertions(+), 1 deletion(-)
>
>
> Hi Heiko and Christoph,
>
> I was wondering about the plan for this series: am I missing some update to
> this discussion? do we have a new version to review?
Hi Andrea,
Thanks for reaching out!
We have this on the list of tasks that we would like to work on,
but as this currently has low priority for us, we don't have a date
when we can come up with a revised patch.
> I actually went ahead (as Palmer suggested in private :-) ) and spent some
> time trying to integrate feedback provided in this thread into your changes,
> obtaining the diff below (on 6.6-rc6, the #include removal fixes some nommu
> builds and should/can be splitted into a separate patch); thoughts?
I had a quick look at your changes, and they look good to me.
Did you agree with Palmer about testing requirements?
I.e., do we need to run this on hardware that implements Zawrs in a
non-trivial way?
I can try to raise the priority on this here, but can't promise anything.
For me it is also ok if you take over this patchset.
BR
Christoph
>
> Andrea
>
>
> From 79d25361ddd4a14db6ce94aec140efbdf2d89684 Mon Sep 17 00:00:00 2001
> From: Andrea Parri <parri.andrea@gmail.com>
> Date: Wed, 18 Oct 2023 02:22:28 +0200
> Subject: [PATCH] riscv: Implement LR+WRS-based smp_cond_load_{relaxed,acquire}()
>
> The Zawrs extension defines instructions allowing a core to enter
> a low-power state and wait on a store to a memory location.
>
> Introduce the Zawrs-instruction WRS.STO and use it in the polling
> loops of the macros smp_cond_load_{relaxed,acquire}().
>
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> [based on the arm64 implementation and work from Heiko and Christoph]
> ---
> arch/riscv/Kconfig | 14 +++++++++
> arch/riscv/include/asm/barrier.h | 26 ++++++++++++++++
> arch/riscv/include/asm/cmpxchg.h | 51 +++++++++++++++++++++++++++++++
> arch/riscv/include/asm/hwcap.h | 3 +-
> arch/riscv/include/asm/insn-def.h | 2 ++
> arch/riscv/kernel/cpufeature.c | 1 +
> 6 files changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index d607ab0f7c6da..ff49f90d175ba 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -489,6 +489,20 @@ config RISCV_ISA_SVPBMT
>
> If you don't know what to do here, say Y.
>
> +config RISCV_ISA_ZAWRS
> + bool "Zawrs extension support for wait-on-reservation-set instructions"
> + depends on RISCV_ALTERNATIVE
> + default y
> + help
> + Adds support to dynamically detect the presence of the Zawrs
> + extension and enable its usage.
> +
> + The Zawrs extension defines a pair of instructions to be used
> + in polling loops that allows a core to enter a low-power state
> + and wait on a store to a memory location.
> +
> + If you don't know what to do here, say Y.
> +
> config TOOLCHAIN_HAS_V
> bool
> default y
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index 110752594228e..7ab7a28e72210 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -71,6 +71,32 @@ 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; \
> + for (;;) { \
> + VAL = READ_ONCE(*__PTR); \
> + if (cond_expr) \
> + break; \
> + __cmpwait_relaxed(__PTR, VAL); \
> + } \
> + (typeof(*ptr))VAL; \
> +})
> +
> +#define smp_cond_load_acquire(ptr, cond_expr) \
> +({ \
> + typeof(ptr) __PTR = (ptr); \
> + __unqual_scalar_typeof(*ptr) VAL; \
> + for (;;) { \
> + VAL = smp_load_acquire(__PTR); \
> + if (cond_expr) \
> + break; \
> + __cmpwait_relaxed(__PTR, VAL); \
> + } \
> + (typeof(*ptr))VAL; \
> +})
> +
> #include <asm-generic/barrier.h>
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 2f4726d3cfcc2..65bc21379f40e 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -8,8 +8,11 @@
>
> #include <linux/bug.h>
>
> +#include <asm/alternative-macros.h>
> #include <asm/barrier.h>
> #include <asm/fence.h>
> +#include <asm/hwcap.h>
> +#include <asm/insn-def.h>
>
> #define __xchg_relaxed(ptr, new, size) \
> ({ \
> @@ -360,4 +363,52 @@
> arch_cmpxchg_relaxed((ptr), (o), (n)); \
> })
>
> +#define __CMPWAIT_CASE(w, sz) \
> +static inline void __cmpwait_case_##sz(volatile void *ptr, \
> + unsigned long val) \
> +{ \
> + unsigned long tmp; \
> + \
> + asm volatile(ALTERNATIVE( \
> + __nops(4), \
> + "lr." #w "\t" "%[tmp], %[v]\n\t" \
> + "xor %[tmp], %[tmp], %[val]\n\t" \
> + "bnez %[tmp], 1f\n\t" \
> + WRS_sto "\n\t" \
> + "1:\n", \
> + 0, RISCV_ISA_EXT_ZAWRS, CONFIG_RISCV_ISA_ZAWRS) \
> + : [tmp] "=&r" (tmp), [v] "+A" (*(u##sz *)ptr) \
> + : [val] "rJ" (val) \
> + : "memory"); \
> +}
> +
> +__CMPWAIT_CASE(w, 32);
> +__CMPWAIT_CASE(d, 64);
> +
> +#undef __CMPWAIT_CASE
> +
> +#define __CMPWAIT_GEN() \
> +static __always_inline void __cmpwait(volatile void *ptr, \
> + unsigned long val, \
> + int size) \
> +{ \
> + switch (size) { \
> + case 4: \
> + return __cmpwait_case_32(ptr, val); \
> + case 8: \
> + return __cmpwait_case_64(ptr, val); \
> + default: \
> + BUILD_BUG(); \
> + } \
> + \
> + unreachable(); \
> +}
> +
> +__CMPWAIT_GEN()
> +
> +#undef __CMPWAIT_GEN
> +
> +#define __cmpwait_relaxed(ptr, val) \
> + __cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr)))
> +
> #endif /* _ASM_RISCV_CMPXCHG_H */
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index b7b58258f6c7c..afabcbf849526 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -58,6 +58,7 @@
> #define RISCV_ISA_EXT_ZICSR 40
> #define RISCV_ISA_EXT_ZIFENCEI 41
> #define RISCV_ISA_EXT_ZIHPM 42
> +#define RISCV_ISA_EXT_ZAWRS 43
>
> #define RISCV_ISA_EXT_MAX 64
>
> @@ -69,8 +70,6 @@
>
> #ifndef __ASSEMBLY__
>
> -#include <linux/jump_label.h>
> -
> unsigned long riscv_get_elf_hwcap(void);
>
> struct riscv_isa_ext_data {
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index 6960beb75f329..fecb744ed7b29 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -196,4 +196,6 @@
> INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0), \
> RS1(base), SIMM12(4))
>
> +#define WRS_sto ".long 0x01d00073"
> +
> #endif /* __ASM_INSN_DEF_H */
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1cfbba65d11ae..044682f54f78f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -181,6 +181,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
> __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> + __RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> };
>
> const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
> --
> 2.34.1
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2023-10-19 16:22 ` Christoph Müllner
@ 2023-10-20 10:19 ` Andrea Parri
-1 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2023-10-20 10:19 UTC (permalink / raw)
To: Christoph Müllner
Cc: Heiko Stuebner, linux-riscv, palmer, paul.walmsley, linux-kernel,
David.Laight, Andrew Jones, Conor Dooley
(Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
his @sntech address.)
> I had a quick look at your changes, and they look good to me.
Great. Thank you for looking them over.
> Did you agree with Palmer about testing requirements?
> I.e., do we need to run this on hardware that implements Zawrs in a
> non-trivial way?
I didn't quite discuss such specific requirements or hardware implementations,
but I agree that's a valid concern. Not that I currently have access to such
hardware; any further inputs/data will be appreciated.
> I can try to raise the priority on this here, but can't promise anything.
> For me it is also ok if you take over this patchset.
Thanks. Either way works for me. No urgency from my side. I'd say - let us
leave this up to the community/other reviewers. (IIUC, Palmer was recovering
from a certain flu and might need more time than usual to get back here.)
Andrea
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2023-10-20 10:19 ` Andrea Parri
0 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2023-10-20 10:19 UTC (permalink / raw)
To: Christoph Müllner
Cc: Heiko Stuebner, linux-riscv, palmer, paul.walmsley, linux-kernel,
David.Laight, Andrew Jones, Conor Dooley
(Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
his @sntech address.)
> I had a quick look at your changes, and they look good to me.
Great. Thank you for looking them over.
> Did you agree with Palmer about testing requirements?
> I.e., do we need to run this on hardware that implements Zawrs in a
> non-trivial way?
I didn't quite discuss such specific requirements or hardware implementations,
but I agree that's a valid concern. Not that I currently have access to such
hardware; any further inputs/data will be appreciated.
> I can try to raise the priority on this here, but can't promise anything.
> For me it is also ok if you take over this patchset.
Thanks. Either way works for me. No urgency from my side. I'd say - let us
leave this up to the community/other reviewers. (IIUC, Palmer was recovering
from a certain flu and might need more time than usual to get back here.)
Andrea
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2023-10-20 10:19 ` Andrea Parri
@ 2024-01-08 11:35 ` Andrew Jones
-1 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2024-01-08 11:35 UTC (permalink / raw)
To: Andrea Parri
Cc: Christoph Müllner, Heiko Stuebner, linux-riscv, palmer,
paul.walmsley, linux-kernel, David.Laight, Conor Dooley
On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> his @sntech address.)
>
> > I had a quick look at your changes, and they look good to me.
>
> Great. Thank you for looking them over.
>
> > Did you agree with Palmer about testing requirements?
> > I.e., do we need to run this on hardware that implements Zawrs in a
> > non-trivial way?
>
> I didn't quite discuss such specific requirements or hardware implementations,
> but I agree that's a valid concern. Not that I currently have access to such
> hardware; any further inputs/data will be appreciated.
>
> > I can try to raise the priority on this here, but can't promise anything.
> > For me it is also ok if you take over this patchset.
>
> Thanks. Either way works for me. No urgency from my side. I'd say - let us
> leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> from a certain flu and might need more time than usual to get back here.)
>
Hi everyone,
I'm also interested in seeing this series resurrected and making progress
again. I'd be happy to help out in any way. It's not clear to me if it has
a current owner. If not, then I could start shepherding the patches with
their authorships intact.
I may be able to do some testing on an FPGA too.
Thanks,
drew
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2024-01-08 11:35 ` Andrew Jones
0 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2024-01-08 11:35 UTC (permalink / raw)
To: Andrea Parri
Cc: Christoph Müllner, Heiko Stuebner, linux-riscv, palmer,
paul.walmsley, linux-kernel, David.Laight, Conor Dooley
On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> his @sntech address.)
>
> > I had a quick look at your changes, and they look good to me.
>
> Great. Thank you for looking them over.
>
> > Did you agree with Palmer about testing requirements?
> > I.e., do we need to run this on hardware that implements Zawrs in a
> > non-trivial way?
>
> I didn't quite discuss such specific requirements or hardware implementations,
> but I agree that's a valid concern. Not that I currently have access to such
> hardware; any further inputs/data will be appreciated.
>
> > I can try to raise the priority on this here, but can't promise anything.
> > For me it is also ok if you take over this patchset.
>
> Thanks. Either way works for me. No urgency from my side. I'd say - let us
> leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> from a certain flu and might need more time than usual to get back here.)
>
Hi everyone,
I'm also interested in seeing this series resurrected and making progress
again. I'd be happy to help out in any way. It's not clear to me if it has
a current owner. If not, then I could start shepherding the patches with
their authorships intact.
I may be able to do some testing on an FPGA too.
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2024-01-08 11:35 ` Andrew Jones
@ 2024-01-08 13:38 ` Andrea Parri
-1 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2024-01-08 13:38 UTC (permalink / raw)
To: Andrew Jones
Cc: Christoph Müllner, Heiko Stuebner, linux-riscv, palmer,
paul.walmsley, linux-kernel, David.Laight, Conor Dooley
Hi Andrew,
> > > I can try to raise the priority on this here, but can't promise anything.
> > > For me it is also ok if you take over this patchset.
> >
> > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > from a certain flu and might need more time than usual to get back here.)
> >
>
> Hi everyone,
>
> I'm also interested in seeing this series resurrected and making progress
> again. I'd be happy to help out in any way. It's not clear to me if it has
> a current owner. If not, then I could start shepherding the patches with
> their authorships intact.
This sounds great to me - please do!
I don't have additional information to provide about this matter.
Thanks!
Andrea
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2024-01-08 13:38 ` Andrea Parri
0 siblings, 0 replies; 32+ messages in thread
From: Andrea Parri @ 2024-01-08 13:38 UTC (permalink / raw)
To: Andrew Jones
Cc: Christoph Müllner, Heiko Stuebner, linux-riscv, palmer,
paul.walmsley, linux-kernel, David.Laight, Conor Dooley
Hi Andrew,
> > > I can try to raise the priority on this here, but can't promise anything.
> > > For me it is also ok if you take over this patchset.
> >
> > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > from a certain flu and might need more time than usual to get back here.)
> >
>
> Hi everyone,
>
> I'm also interested in seeing this series resurrected and making progress
> again. I'd be happy to help out in any way. It's not clear to me if it has
> a current owner. If not, then I could start shepherding the patches with
> their authorships intact.
This sounds great to me - please do!
I don't have additional information to provide about this matter.
Thanks!
Andrea
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2024-01-08 11:35 ` Andrew Jones
@ 2024-01-08 14:00 ` Christoph Müllner
-1 siblings, 0 replies; 32+ messages in thread
From: Christoph Müllner @ 2024-01-08 14:00 UTC (permalink / raw)
To: Andrew Jones
Cc: Andrea Parri, Heiko Stuebner, linux-riscv, palmer, paul.walmsley,
linux-kernel, David.Laight, Conor Dooley
On Mon, Jan 8, 2024 at 12:35 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> > (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> > his @sntech address.)
> >
> > > I had a quick look at your changes, and they look good to me.
> >
> > Great. Thank you for looking them over.
> >
> > > Did you agree with Palmer about testing requirements?
> > > I.e., do we need to run this on hardware that implements Zawrs in a
> > > non-trivial way?
> >
> > I didn't quite discuss such specific requirements or hardware implementations,
> > but I agree that's a valid concern. Not that I currently have access to such
> > hardware; any further inputs/data will be appreciated.
> >
> > > I can try to raise the priority on this here, but can't promise anything.
> > > For me it is also ok if you take over this patchset.
> >
> > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > from a certain flu and might need more time than usual to get back here.)
> >
>
> Hi everyone,
>
> I'm also interested in seeing this series resurrected and making progress
> again. I'd be happy to help out in any way. It's not clear to me if it has
> a current owner. If not, then I could start shepherding the patches with
> their authorships intact.
Sounds good to me!
Thanks for working on this!
>
> I may be able to do some testing on an FPGA too.
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2024-01-08 14:00 ` Christoph Müllner
0 siblings, 0 replies; 32+ messages in thread
From: Christoph Müllner @ 2024-01-08 14:00 UTC (permalink / raw)
To: Andrew Jones
Cc: Andrea Parri, Heiko Stuebner, linux-riscv, palmer, paul.walmsley,
linux-kernel, David.Laight, Conor Dooley
On Mon, Jan 8, 2024 at 12:35 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> > (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> > his @sntech address.)
> >
> > > I had a quick look at your changes, and they look good to me.
> >
> > Great. Thank you for looking them over.
> >
> > > Did you agree with Palmer about testing requirements?
> > > I.e., do we need to run this on hardware that implements Zawrs in a
> > > non-trivial way?
> >
> > I didn't quite discuss such specific requirements or hardware implementations,
> > but I agree that's a valid concern. Not that I currently have access to such
> > hardware; any further inputs/data will be appreciated.
> >
> > > I can try to raise the priority on this here, but can't promise anything.
> > > For me it is also ok if you take over this patchset.
> >
> > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > from a certain flu and might need more time than usual to get back here.)
> >
>
> Hi everyone,
>
> I'm also interested in seeing this series resurrected and making progress
> again. I'd be happy to help out in any way. It's not clear to me if it has
> a current owner. If not, then I could start shepherding the patches with
> their authorships intact.
Sounds good to me!
Thanks for working on this!
>
> I may be able to do some testing on an FPGA too.
>
> Thanks,
> drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2024-01-08 14:00 ` Christoph Müllner
@ 2024-01-08 14:10 ` Andrew Jones
-1 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2024-01-08 14:10 UTC (permalink / raw)
To: Christoph Müllner
Cc: Andrea Parri, Heiko Stuebner, linux-riscv, palmer, paul.walmsley,
linux-kernel, David.Laight, Conor Dooley
On Mon, Jan 08, 2024 at 03:00:29PM +0100, Christoph Müllner wrote:
> On Mon, Jan 8, 2024 at 12:35 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> > > (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> > > his @sntech address.)
> > >
> > > > I had a quick look at your changes, and they look good to me.
> > >
> > > Great. Thank you for looking them over.
> > >
> > > > Did you agree with Palmer about testing requirements?
> > > > I.e., do we need to run this on hardware that implements Zawrs in a
> > > > non-trivial way?
> > >
> > > I didn't quite discuss such specific requirements or hardware implementations,
> > > but I agree that's a valid concern. Not that I currently have access to such
> > > hardware; any further inputs/data will be appreciated.
> > >
> > > > I can try to raise the priority on this here, but can't promise anything.
> > > > For me it is also ok if you take over this patchset.
> > >
> > > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > > from a certain flu and might need more time than usual to get back here.)
> > >
> >
> > Hi everyone,
> >
> > I'm also interested in seeing this series resurrected and making progress
> > again. I'd be happy to help out in any way. It's not clear to me if it has
> > a current owner. If not, then I could start shepherding the patches with
> > their authorships intact.
>
> Sounds good to me!
> Thanks for working on this!
Thanks for the quick replies! I'll try pull something together in the very
near future.
drew
>
> >
> > I may be able to do some testing on an FPGA too.
> >
> > Thanks,
> > drew
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2024-01-08 14:10 ` Andrew Jones
0 siblings, 0 replies; 32+ messages in thread
From: Andrew Jones @ 2024-01-08 14:10 UTC (permalink / raw)
To: Christoph Müllner
Cc: Andrea Parri, Heiko Stuebner, linux-riscv, palmer, paul.walmsley,
linux-kernel, David.Laight, Conor Dooley
On Mon, Jan 08, 2024 at 03:00:29PM +0100, Christoph Müllner wrote:
> On Mon, Jan 8, 2024 at 12:35 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> > > (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> > > his @sntech address.)
> > >
> > > > I had a quick look at your changes, and they look good to me.
> > >
> > > Great. Thank you for looking them over.
> > >
> > > > Did you agree with Palmer about testing requirements?
> > > > I.e., do we need to run this on hardware that implements Zawrs in a
> > > > non-trivial way?
> > >
> > > I didn't quite discuss such specific requirements or hardware implementations,
> > > but I agree that's a valid concern. Not that I currently have access to such
> > > hardware; any further inputs/data will be appreciated.
> > >
> > > > I can try to raise the priority on this here, but can't promise anything.
> > > > For me it is also ok if you take over this patchset.
> > >
> > > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > > from a certain flu and might need more time than usual to get back here.)
> > >
> >
> > Hi everyone,
> >
> > I'm also interested in seeing this series resurrected and making progress
> > again. I'd be happy to help out in any way. It's not clear to me if it has
> > a current owner. If not, then I could start shepherding the patches with
> > their authorships intact.
>
> Sounds good to me!
> Thanks for working on this!
Thanks for the quick replies! I'll try pull something together in the very
near future.
drew
>
> >
> > I may be able to do some testing on an FPGA too.
> >
> > Thanks,
> > drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
2024-01-08 14:10 ` Andrew Jones
@ 2024-03-05 23:31 ` Charlie Jenkins
-1 siblings, 0 replies; 32+ messages in thread
From: Charlie Jenkins @ 2024-03-05 23:31 UTC (permalink / raw)
To: Andrew Jones
Cc: Christoph Müllner, Andrea Parri, Heiko Stuebner,
linux-riscv, palmer, paul.walmsley, linux-kernel, David.Laight,
Conor Dooley
On Mon, Jan 08, 2024 at 03:10:12PM +0100, Andrew Jones wrote:
> On Mon, Jan 08, 2024 at 03:00:29PM +0100, Christoph Müllner wrote:
> > On Mon, Jan 8, 2024 at 12:35 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> > >
> > > On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> > > > (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> > > > his @sntech address.)
> > > >
> > > > > I had a quick look at your changes, and they look good to me.
> > > >
> > > > Great. Thank you for looking them over.
> > > >
> > > > > Did you agree with Palmer about testing requirements?
> > > > > I.e., do we need to run this on hardware that implements Zawrs in a
> > > > > non-trivial way?
> > > >
> > > > I didn't quite discuss such specific requirements or hardware implementations,
> > > > but I agree that's a valid concern. Not that I currently have access to such
> > > > hardware; any further inputs/data will be appreciated.
> > > >
> > > > > I can try to raise the priority on this here, but can't promise anything.
> > > > > For me it is also ok if you take over this patchset.
> > > >
> > > > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > > > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > > > from a certain flu and might need more time than usual to get back here.)
> > > >
> > >
> > > Hi everyone,
> > >
> > > I'm also interested in seeing this series resurrected and making progress
> > > again. I'd be happy to help out in any way. It's not clear to me if it has
> > > a current owner. If not, then I could start shepherding the patches with
> > > their authorships intact.
> >
> > Sounds good to me!
> > Thanks for working on this!
>
> Thanks for the quick replies! I'll try pull something together in the very
> near future.
>
> drew
I am interested in this as well, please copy me when you send out the
patch Drew!
- Charlie
>
> >
> > >
> > > I may be able to do some testing on an FPGA too.
> > >
> > > Thanks,
> > > drew
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Re: Re: [PATCH v3 0/2] Add Zawrs support and use it for spinlocks
@ 2024-03-05 23:31 ` Charlie Jenkins
0 siblings, 0 replies; 32+ messages in thread
From: Charlie Jenkins @ 2024-03-05 23:31 UTC (permalink / raw)
To: Andrew Jones
Cc: Christoph Müllner, Andrea Parri, Heiko Stuebner,
linux-riscv, palmer, paul.walmsley, linux-kernel, David.Laight,
Conor Dooley
On Mon, Jan 08, 2024 at 03:10:12PM +0100, Andrew Jones wrote:
> On Mon, Jan 08, 2024 at 03:00:29PM +0100, Christoph Müllner wrote:
> > On Mon, Jan 8, 2024 at 12:35 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> > >
> > > On Fri, Oct 20, 2023 at 12:19:38PM +0200, Andrea Parri wrote:
> > > > (Removing Heiko's @vrull address from Cc:, since it seemed to bounce, keeping
> > > > his @sntech address.)
> > > >
> > > > > I had a quick look at your changes, and they look good to me.
> > > >
> > > > Great. Thank you for looking them over.
> > > >
> > > > > Did you agree with Palmer about testing requirements?
> > > > > I.e., do we need to run this on hardware that implements Zawrs in a
> > > > > non-trivial way?
> > > >
> > > > I didn't quite discuss such specific requirements or hardware implementations,
> > > > but I agree that's a valid concern. Not that I currently have access to such
> > > > hardware; any further inputs/data will be appreciated.
> > > >
> > > > > I can try to raise the priority on this here, but can't promise anything.
> > > > > For me it is also ok if you take over this patchset.
> > > >
> > > > Thanks. Either way works for me. No urgency from my side. I'd say - let us
> > > > leave this up to the community/other reviewers. (IIUC, Palmer was recovering
> > > > from a certain flu and might need more time than usual to get back here.)
> > > >
> > >
> > > Hi everyone,
> > >
> > > I'm also interested in seeing this series resurrected and making progress
> > > again. I'd be happy to help out in any way. It's not clear to me if it has
> > > a current owner. If not, then I could start shepherding the patches with
> > > their authorships intact.
> >
> > Sounds good to me!
> > Thanks for working on this!
>
> Thanks for the quick replies! I'll try pull something together in the very
> near future.
>
> drew
I am interested in this as well, please copy me when you send out the
patch Drew!
- Charlie
>
> >
> > >
> > > I may be able to do some testing on an FPGA too.
> > >
> > > Thanks,
> > > drew
>
> _______________________________________________
> 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] 32+ messages in thread
end of thread, other threads:[~2024-03-05 23:31 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-21 11:47 [PATCH v3 0/2] Add Zawrs support and use it for spinlocks Heiko Stuebner
2023-05-21 11:47 ` Heiko Stuebner
2023-05-21 11:47 ` [PATCH v3 1/2] riscv: don't include kernel.h into alternative.h Heiko Stuebner
2023-05-21 11:47 ` Heiko Stuebner
2023-05-24 14:01 ` Andrew Jones
2023-05-24 14:01 ` Andrew Jones
2023-05-21 11:47 ` [PATCH v3 2/2] riscv: Add Zawrs support for spinlocks Heiko Stuebner
2023-05-21 11:47 ` Heiko Stuebner
2023-05-22 17:43 ` Conor Dooley
2023-05-22 17:43 ` Conor Dooley
2023-05-24 17:05 ` Andrew Jones
2023-05-24 17:05 ` Andrew Jones
2023-05-24 23:00 ` Palmer Dabbelt
2023-05-24 23:00 ` Palmer Dabbelt
2023-05-30 18:45 ` Andrea Parri
2023-05-30 18:45 ` Andrea Parri
2023-10-19 14:21 ` [PATCH v3 0/2] Add Zawrs support and use it " Andrea Parri
2023-10-19 14:21 ` Andrea Parri
2023-10-19 16:22 ` Christoph Müllner
2023-10-19 16:22 ` Christoph Müllner
2023-10-20 10:19 ` Andrea Parri
2023-10-20 10:19 ` Andrea Parri
2024-01-08 11:35 ` Andrew Jones
2024-01-08 11:35 ` Andrew Jones
2024-01-08 13:38 ` Andrea Parri
2024-01-08 13:38 ` Andrea Parri
2024-01-08 14:00 ` Christoph Müllner
2024-01-08 14:00 ` Christoph Müllner
2024-01-08 14:10 ` Andrew Jones
2024-01-08 14:10 ` Andrew Jones
2024-03-05 23:31 ` Charlie Jenkins
2024-03-05 23:31 ` Charlie Jenkins
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.