[PATCH v3 0/6] riscv: Apply Zawrs when available

[PATCH v3 0/6] riscv: Apply Zawrs when available

[Andrew Jones]

Zawrs provides two instructions (wrs.nto and wrs.sto), where both are
meant to allow the hart to enter a low-power state while waiting on a
store to a memory location. The instructions also both wait an
implementation-defined "short" duration (unless the implementation
terminates the stall for another reason). The difference is that while
wrs.sto will terminate when the duration elapses, wrs.nto, depending on
configuration, will either just keep waiting or an ILL exception will be
raised. Linux will use wrs.nto, so if platforms have an implementation
which falls in the "just keep waiting" category (which is not expected),
then it should _not_ advertise Zawrs in the hardware description.

Like wfi (and with the same {m,h}status bits to configure it), when
wrs.nto is configured to raise exceptions it's expected that the higher
privilege level will see the instruction was a wait instruction, do
something, and then resume execution following the instruction. For
example, KVM does configure exceptions for wfi (hstatus.VTW=1) and
therefore also for wrs.nto. KVM does this for wfi since it's better to
allow other tasks to be scheduled while a VCPU waits for an interrupt.
For waits such as those where wrs.nto/sto would be used, which are
typically locks, it is also a good idea for KVM to be involved, as it
can attempt to schedule the lock holding VCPU.

This series starts with Christoph's addition of the riscv
smp_cond_load_relaxed function which applies wrs.sto when available.
That patch has been reworked to use wrs.nto and to use the same approach
as Arm for the wait loop, since we can't have arbitrary C code between
the load-reserved and the wrs. Then, hwprobe support is added (since the
instructions are also usable from usermode), and finally KVM is
taught about wrs.nto, allowing guests to see and use the Zawrs
extension.

We still don't have test results from hardware, and it's not possible to
prove that using Zawrs is a win when testing on QEMU, not even when
oversubscribing VCPUs to guests. However, it is possible to use KVM
selftests to force a scenario where we can prove Zawrs does its job and
does it well. [4] is a test which does this and, on my machine, without
Zawrs it takes 16 seconds to complete and with Zawrs it takes 0.25
seconds.

This series is also available here [1]. In order to use QEMU for testing
a build with [2] is needed. In order to enable guests to use Zawrs with
KVM using kvmtool, the branch at [3] may be used.

[1] https://github.com/jones-drew/linux/commits/riscv/zawrs-v3/
[2] https://lore.kernel.org/all/20240312152901.512001-2-ajones@ventanamicro.com/
[3] https://github.com/jones-drew/kvmtool/commits/riscv/zawrs/
[4] https://github.com/jones-drew/linux/commit/cb2beccebcece10881db842ed69bdd5715cfab5d

Thanks,
drew

v3:
 - Moved comment about expected termination from the DT binding text
   to a code comment.

v2:
 - Added DT bindings patch with additional Linux specifications due
   to wrs.nto potentially never terminating, as suggested by Palmer
 - Added patch to share pause insn definition
 - Rework main Zawrs support patch to use Arm approach (which is
   also the approach that Andrea Parri suggested)
 - Dropped the riscv implementation of smp_cond_load_acquire().
   afaict, the generic implementation, which will use the riscv
   implementation of smp_cond_load_relaxed() is sufficient for riscv.
 - The rework was large enough (IMO) to drop Heiko's s-o-b and to
   add myself as a co-developer


Andrew Jones (5):
  riscv: Provide a definition for 'pause'
  dt-bindings: riscv: Add Zawrs ISA extension description
  riscv: hwprobe: export Zawrs ISA extension
  KVM: riscv: Support guest wrs.nto
  KVM: riscv: selftests: Add Zawrs extension to get-reg-list test

Christoph Müllner (1):
  riscv: Add Zawrs support for spinlocks

 Documentation/arch/riscv/hwprobe.rst          |  4 ++
 .../devicetree/bindings/riscv/extensions.yaml |  7 +++
 arch/riscv/Kconfig                            | 20 ++++---
 arch/riscv/Makefile                           |  3 -
 arch/riscv/include/asm/barrier.h              | 45 +++++++++-----
 arch/riscv/include/asm/cmpxchg.h              | 58 +++++++++++++++++++
 arch/riscv/include/asm/hwcap.h                |  1 +
 arch/riscv/include/asm/insn-def.h             |  4 ++
 arch/riscv/include/asm/kvm_host.h             |  1 +
 arch/riscv/include/asm/vdso/processor.h       |  8 +--
 arch/riscv/include/uapi/asm/hwprobe.h         |  1 +
 arch/riscv/include/uapi/asm/kvm.h             |  1 +
 arch/riscv/kernel/cpufeature.c                |  1 +
 arch/riscv/kernel/sys_hwprobe.c               |  1 +
 arch/riscv/kvm/vcpu.c                         |  1 +
 arch/riscv/kvm/vcpu_insn.c                    | 15 +++++
 arch/riscv/kvm/vcpu_onereg.c                  |  2 +
 .../selftests/kvm/riscv/get-reg-list.c        |  4 ++
 18 files changed, 146 insertions(+), 31 deletions(-)

-- 
2.44.0

[PATCH v3 1/6] riscv: Provide a definition for 'pause'

[Andrew Jones]

If we're going to provide the encoding for 'pause' in cpu_relax()
anyway, then we can drop the toolchain checks and just always use
it. The advantage of doing this is that other code that need
pause don't need to also define it (yes, another use is coming).
Add the definition to insn-def.h since it's an instruction
definition and also because insn-def.h doesn't include much, so
it's safe to include from asm/vdso/processor.h without concern for
circular dependencies.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/Kconfig                      | 7 -------
 arch/riscv/Makefile                     | 3 ---
 arch/riscv/include/asm/insn-def.h       | 2 ++
 arch/riscv/include/asm/vdso/processor.h | 8 ++------
 4 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index be09c8836d56..7427d8088337 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -639,13 +639,6 @@ config RISCV_ISA_ZICBOZ
 
 	   If you don't know what to do here, say Y.
 
-config TOOLCHAIN_HAS_ZIHINTPAUSE
-	bool
-	default y
-	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zihintpause)
-	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zihintpause)
-	depends on LLD_VERSION >= 150000 || LD_VERSION >= 23600
-
 config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
 	def_bool y
 	# https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 252d63942f34..f1792ac03335 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -75,9 +75,6 @@ else
 riscv-march-$(CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI) := $(riscv-march-y)_zicsr_zifencei
 endif
 
-# Check if the toolchain supports Zihintpause extension
-riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
-
 # Remove F,D,V from isa string for all. Keep extensions between "fd" and "v" by
 # matching non-v and non-multi-letter extensions out with the filter ([^v_]*)
 KBUILD_CFLAGS += -march=$(shell echo $(riscv-march-y) | sed -E 's/(rv32ima|rv64ima)fd([^v_]*)v?/\1\2/')
diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
index e27179b26086..64dffaa21bfa 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 RISCV_PAUSE	".4byte 0x100000f"
+
 #endif /* __ASM_INSN_DEF_H */
diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
index 96b65a5396df..8f383f05a290 100644
--- a/arch/riscv/include/asm/vdso/processor.h
+++ b/arch/riscv/include/asm/vdso/processor.h
@@ -5,6 +5,7 @@
 #ifndef __ASSEMBLY__
 
 #include <asm/barrier.h>
+#include <asm/insn-def.h>
 
 static inline void cpu_relax(void)
 {
@@ -14,16 +15,11 @@ static inline void cpu_relax(void)
 	__asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
 #endif
 
-#ifdef CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE
 	/*
 	 * Reduce instruction retirement.
 	 * This assumes the PC changes.
 	 */
-	__asm__ __volatile__ ("pause");
-#else
-	/* Encoding of the pause instruction */
-	__asm__ __volatile__ (".4byte 0x100000F");
-#endif
+	__asm__ __volatile__ (RISCV_PAUSE);
 	barrier();
 }
 
-- 
2.44.0

[PATCH v3 2/6] dt-bindings: riscv: Add Zawrs ISA extension description

[Andrew Jones]

Add description for the Zawrs (Wait-on-Reservation-Set) ISA extension
which was ratified in commit 98918c844281 of riscv-isa-manual.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 Documentation/devicetree/bindings/riscv/extensions.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
index 468c646247aa..8594e7c33ea2 100644
--- a/Documentation/devicetree/bindings/riscv/extensions.yaml
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -177,6 +177,13 @@ properties:
             is supported as ratified at commit 5059e0ca641c ("update to
             ratified") of the riscv-zacas.
 
+        - const: zawrs
+          description: |
+            The Zawrs extension for entering a low-power state or for trapping
+            to a hypervisor while waiting on a store to a memory location, as
+            ratified in commit 98918c844281 ("Merge pull request #1217 from
+            riscv/zawrs") of riscv-isa-manual.
+
         - const: zba
           description: |
             The standard Zba bit-manipulation extension for address generation
-- 
2.44.0

Re: [PATCH v3 2/6] dt-bindings: riscv: Add Zawrs ISA extension description

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

On Fri, Apr 26, 2024 at 12:08:22PM +0200, Andrew Jones wrote:
> Add description for the Zawrs (Wait-on-Reservation-Set) ISA extension
> which was ratified in commit 98918c844281 of riscv-isa-manual.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 3/6] riscv: Add Zawrs support for spinlocks

[Andrew Jones]

From: Christoph Müllner <christoph.muellner@vrull.eu>

RISC-V code uses the generic ticket lock implementation, which calls
the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
Introduce a RISC-V specific implementation of smp_cond_load_relaxed()
which applies WRS.NTO of the Zawrs extension in order to reduce power
consumption while waiting and allows hypervisors to enable guests to
trap while waiting. smp_cond_load_acquire() doesn't need a RISC-V
specific implementation as the generic implementation is based on
smp_cond_load_relaxed() and smp_acquire__after_ctrl_dep() sufficiently
provides the acquire semantics.

This implementation is heavily based on Arm's approach which is the
approach Andrea Parri also suggested.

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>
Co-developed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/Kconfig                | 13 +++++++
 arch/riscv/include/asm/barrier.h  | 45 ++++++++++++++++--------
 arch/riscv/include/asm/cmpxchg.h  | 58 +++++++++++++++++++++++++++++++
 arch/riscv/include/asm/hwcap.h    |  1 +
 arch/riscv/include/asm/insn-def.h |  2 ++
 arch/riscv/kernel/cpufeature.c    |  1 +
 6 files changed, 105 insertions(+), 15 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 7427d8088337..34bbe6b70546 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -578,6 +578,19 @@ config RISCV_ISA_V_PREEMPTIVE
 	  preemption. Enabling this config will result in higher memory
 	  consumption due to the allocation of per-task's kernel Vector context.
 
+config RISCV_ISA_ZAWRS
+	bool "Zawrs extension support for more efficient busy waiting"
+	depends on RISCV_ALTERNATIVE
+	default y
+	help
+	  The Zawrs extension defines instructions to be used in polling loops
+	  which allow a hart to enter a low-power state or to trap to the
+	  hypervisor while waiting on a store to a memory location. Enable the
+	  use of these instructions in the kernel when the Zawrs extension is
+	  detected at boot.
+
+	  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 880b56d8480d..e1d9bf1deca6 100644
--- a/arch/riscv/include/asm/barrier.h
+++ b/arch/riscv/include/asm/barrier.h
@@ -11,6 +11,7 @@
 #define _ASM_RISCV_BARRIER_H
 
 #ifndef __ASSEMBLY__
+#include <asm/cmpxchg.h>
 #include <asm/fence.h>
 
 #define nop()		__asm__ __volatile__ ("nop")
@@ -28,21 +29,6 @@
 #define __smp_rmb()	RISCV_FENCE(r, r)
 #define __smp_wmb()	RISCV_FENCE(w, w)
 
-#define __smp_store_release(p, v)					\
-do {									\
-	compiletime_assert_atomic_type(*p);				\
-	RISCV_FENCE(rw, w);						\
-	WRITE_ONCE(*p, v);						\
-} while (0)
-
-#define __smp_load_acquire(p)						\
-({									\
-	typeof(*p) ___p1 = READ_ONCE(*p);				\
-	compiletime_assert_atomic_type(*p);				\
-	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
@@ -70,6 +56,35 @@ do {									\
  */
 #define smp_mb__after_spinlock()	RISCV_FENCE(iorw, iorw)
 
+#define __smp_store_release(p, v)					\
+do {									\
+	compiletime_assert_atomic_type(*p);				\
+	RISCV_FENCE(rw, w);						\
+	WRITE_ONCE(*p, v);						\
+} while (0)
+
+#define __smp_load_acquire(p)						\
+({									\
+	typeof(*p) ___p1 = READ_ONCE(*p);				\
+	compiletime_assert_atomic_type(*p);				\
+	RISCV_FENCE(r, rw);						\
+	___p1;								\
+})
+
+#ifdef CONFIG_RISCV_ISA_ZAWRS
+#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;						\
+})
+#endif
+
 #include <asm-generic/barrier.h>
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index 2fee65cc8443..725276dcb996 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -8,7 +8,10 @@
 
 #include <linux/bug.h>
 
+#include <asm/alternative-macros.h>
 #include <asm/fence.h>
+#include <asm/hwcap.h>
+#include <asm/insn-def.h>
 
 #define __xchg_relaxed(ptr, new, size)					\
 ({									\
@@ -359,4 +362,59 @@
 	arch_cmpxchg_relaxed((ptr), (o), (n));				\
 })
 
+#ifdef CONFIG_RISCV_ISA_ZAWRS
+/*
+ * Despite wrs.nto being "WRS-with-no-timeout", in the absence of changes to
+ * @val we expect it to still terminate within a "reasonable" amount of time
+ * for an implementation-specific other reason, a pending, locally-enabled
+ * interrupt, or because it has been configured to raise an illegal
+ * instruction exception.
+ */
+static __always_inline void __cmpwait(volatile void *ptr,
+				      unsigned long val,
+				      int size)
+{
+	unsigned long tmp;
+
+	asm goto(ALTERNATIVE("j %l[no_zawrs]", "nop",
+			     0, RISCV_ISA_EXT_ZAWRS, 1)
+		 : : : : no_zawrs);
+
+	switch (size) {
+	case 4:
+		asm volatile(
+		"	lr.w	%0, %1\n"
+		"	xor	%0, %0, %2\n"
+		"	bnez	%0, 1f\n"
+			ZAWRS_WRS_NTO "\n"
+		"1:"
+		: "=&r" (tmp), "+A" (*(u32 *)ptr)
+		: "r" (val));
+		break;
+#if __riscv_xlen == 64
+	case 8:
+		asm volatile(
+		"	lr.d	%0, %1\n"
+		"	xor	%0, %0, %2\n"
+		"	bnez	%0, 1f\n"
+			ZAWRS_WRS_NTO "\n"
+		"1:"
+		: "=&r" (tmp), "+A" (*(u64 *)ptr)
+		: "r" (val));
+		break;
+#endif
+	default:
+		BUILD_BUG();
+	}
+
+	return;
+
+no_zawrs:
+	asm volatile(RISCV_PAUSE : : : "memory");
+}
+
+#define __cmpwait_relaxed(ptr, val) \
+	__cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr)))
+#endif
+
 #endif /* _ASM_RISCV_CMPXCHG_H */
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..5b358c3cf212 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,7 @@
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
 #define RISCV_ISA_EXT_XANDESPMU		74
+#define RISCV_ISA_EXT_ZAWRS		75
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
index 64dffaa21bfa..9a913010cdd9 100644
--- a/arch/riscv/include/asm/insn-def.h
+++ b/arch/riscv/include/asm/insn-def.h
@@ -197,5 +197,7 @@
 	       RS1(base), SIMM12(4))
 
 #define RISCV_PAUSE	".4byte 0x100000f"
+#define ZAWRS_WRS_NTO	".4byte 0x00d00073"
+#define ZAWRS_WRS_STO	".4byte 0x01d00073"
 
 #endif /* __ASM_INSN_DEF_H */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 3ed2359eae35..02de9eaa3f42 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -257,6 +257,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
 	__RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
 	__RISCV_ISA_EXT_DATA(zacas, RISCV_ISA_EXT_ZACAS),
+	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
 	__RISCV_ISA_EXT_DATA(zfa, RISCV_ISA_EXT_ZFA),
 	__RISCV_ISA_EXT_DATA(zfh, RISCV_ISA_EXT_ZFH),
 	__RISCV_ISA_EXT_DATA(zfhmin, RISCV_ISA_EXT_ZFHMIN),
-- 
2.44.0

Re: [PATCH v3 3/6] riscv: Add Zawrs support for spinlocks

[Alexandre Ghiti]

Hi Drew,

On 26/04/2024 12:08, Andrew Jones wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> RISC-V code uses the generic ticket lock implementation, which calls
> the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> Introduce a RISC-V specific implementation of smp_cond_load_relaxed()
> which applies WRS.NTO of the Zawrs extension in order to reduce power
> consumption while waiting and allows hypervisors to enable guests to
> trap while waiting. smp_cond_load_acquire() doesn't need a RISC-V
> specific implementation as the generic implementation is based on
> smp_cond_load_relaxed() and smp_acquire__after_ctrl_dep() sufficiently
> provides the acquire semantics.
>
> This implementation is heavily based on Arm's approach which is the
> approach Andrea Parri also suggested.
>
> 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>
> Co-developed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>   arch/riscv/Kconfig                | 13 +++++++
>   arch/riscv/include/asm/barrier.h  | 45 ++++++++++++++++--------
>   arch/riscv/include/asm/cmpxchg.h  | 58 +++++++++++++++++++++++++++++++
>   arch/riscv/include/asm/hwcap.h    |  1 +
>   arch/riscv/include/asm/insn-def.h |  2 ++
>   arch/riscv/kernel/cpufeature.c    |  1 +
>   6 files changed, 105 insertions(+), 15 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 7427d8088337..34bbe6b70546 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -578,6 +578,19 @@ config RISCV_ISA_V_PREEMPTIVE
>   	  preemption. Enabling this config will result in higher memory
>   	  consumption due to the allocation of per-task's kernel Vector context.
>   
> +config RISCV_ISA_ZAWRS
> +	bool "Zawrs extension support for more efficient busy waiting"
> +	depends on RISCV_ALTERNATIVE
> +	default y
> +	help
> +	  The Zawrs extension defines instructions to be used in polling loops
> +	  which allow a hart to enter a low-power state or to trap to the
> +	  hypervisor while waiting on a store to a memory location. Enable the
> +	  use of these instructions in the kernel when the Zawrs extension is
> +	  detected at boot.
> +
> +	  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 880b56d8480d..e1d9bf1deca6 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -11,6 +11,7 @@
>   #define _ASM_RISCV_BARRIER_H
>   
>   #ifndef __ASSEMBLY__
> +#include <asm/cmpxchg.h>
>   #include <asm/fence.h>
>   
>   #define nop()		__asm__ __volatile__ ("nop")
> @@ -28,21 +29,6 @@
>   #define __smp_rmb()	RISCV_FENCE(r, r)
>   #define __smp_wmb()	RISCV_FENCE(w, w)
>   
> -#define __smp_store_release(p, v)					\
> -do {									\
> -	compiletime_assert_atomic_type(*p);				\
> -	RISCV_FENCE(rw, w);						\
> -	WRITE_ONCE(*p, v);						\
> -} while (0)
> -
> -#define __smp_load_acquire(p)						\
> -({									\
> -	typeof(*p) ___p1 = READ_ONCE(*p);				\
> -	compiletime_assert_atomic_type(*p);				\
> -	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
> @@ -70,6 +56,35 @@ do {									\
>    */
>   #define smp_mb__after_spinlock()	RISCV_FENCE(iorw, iorw)
>   
> +#define __smp_store_release(p, v)					\
> +do {									\
> +	compiletime_assert_atomic_type(*p);				\
> +	RISCV_FENCE(rw, w);						\
> +	WRITE_ONCE(*p, v);						\
> +} while (0)
> +
> +#define __smp_load_acquire(p)						\
> +({									\
> +	typeof(*p) ___p1 = READ_ONCE(*p);				\
> +	compiletime_assert_atomic_type(*p);				\
> +	RISCV_FENCE(r, rw);						\
> +	___p1;								\
> +})
> +
> +#ifdef CONFIG_RISCV_ISA_ZAWRS
> +#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;						\
> +})
> +#endif
> +
>   #include <asm-generic/barrier.h>
>   
>   #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 2fee65cc8443..725276dcb996 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -8,7 +8,10 @@
>   
>   #include <linux/bug.h>
>   
> +#include <asm/alternative-macros.h>
>   #include <asm/fence.h>
> +#include <asm/hwcap.h>
> +#include <asm/insn-def.h>
>   
>   #define __xchg_relaxed(ptr, new, size)					\
>   ({									\
> @@ -359,4 +362,59 @@
>   	arch_cmpxchg_relaxed((ptr), (o), (n));				\
>   })
>   
> +#ifdef CONFIG_RISCV_ISA_ZAWRS
> +/*
> + * Despite wrs.nto being "WRS-with-no-timeout", in the absence of changes to
> + * @val we expect it to still terminate within a "reasonable" amount of time
> + * for an implementation-specific other reason, a pending, locally-enabled
> + * interrupt, or because it has been configured to raise an illegal
> + * instruction exception.
> + */
> +static __always_inline void __cmpwait(volatile void *ptr,
> +				      unsigned long val,
> +				      int size)
> +{
> +	unsigned long tmp;
> +
> +	asm goto(ALTERNATIVE("j %l[no_zawrs]", "nop",
> +			     0, RISCV_ISA_EXT_ZAWRS, 1)
> +		 : : : : no_zawrs);
> +
> +	switch (size) {
> +	case 4:
> +		asm volatile(
> +		"	lr.w	%0, %1\n"
> +		"	xor	%0, %0, %2\n"
> +		"	bnez	%0, 1f\n"
> +			ZAWRS_WRS_NTO "\n"
> +		"1:"
> +		: "=&r" (tmp), "+A" (*(u32 *)ptr)
> +		: "r" (val));
> +		break;
> +#if __riscv_xlen == 64
> +	case 8:
> +		asm volatile(
> +		"	lr.d	%0, %1\n"
> +		"	xor	%0, %0, %2\n"
> +		"	bnez	%0, 1f\n"
> +			ZAWRS_WRS_NTO "\n"
> +		"1:"
> +		: "=&r" (tmp), "+A" (*(u64 *)ptr)
> +		: "r" (val));
> +		break;
> +#endif
> +	default:
> +		BUILD_BUG();
> +	}
> +
> +	return;
> +
> +no_zawrs:
> +	asm volatile(RISCV_PAUSE : : : "memory");


Shouldn't we fallback to the previous implementation (cpu_relax()) here? 
Not sure this is really important, but I want to make sure it was not an 
oversight.

Thanks,

Alex


> +}
> +
> +#define __cmpwait_relaxed(ptr, val) \
> +	__cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr)))
> +#endif
> +
>   #endif /* _ASM_RISCV_CMPXCHG_H */
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e17d0078a651..5b358c3cf212 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,7 @@
>   #define RISCV_ISA_EXT_ZTSO		72
>   #define RISCV_ISA_EXT_ZACAS		73
>   #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_ZAWRS		75
>   
>   #define RISCV_ISA_EXT_XLINUXENVCFG	127
>   
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index 64dffaa21bfa..9a913010cdd9 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -197,5 +197,7 @@
>   	       RS1(base), SIMM12(4))
>   
>   #define RISCV_PAUSE	".4byte 0x100000f"
> +#define ZAWRS_WRS_NTO	".4byte 0x00d00073"
> +#define ZAWRS_WRS_STO	".4byte 0x01d00073"
>   
>   #endif /* __ASM_INSN_DEF_H */
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 3ed2359eae35..02de9eaa3f42 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -257,6 +257,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>   	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>   	__RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
>   	__RISCV_ISA_EXT_DATA(zacas, RISCV_ISA_EXT_ZACAS),
> +	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
>   	__RISCV_ISA_EXT_DATA(zfa, RISCV_ISA_EXT_ZFA),
>   	__RISCV_ISA_EXT_DATA(zfh, RISCV_ISA_EXT_ZFH),
>   	__RISCV_ISA_EXT_DATA(zfhmin, RISCV_ISA_EXT_ZFHMIN),

Re: [PATCH v3 3/6] riscv: Add Zawrs support for spinlocks

[Andrew Jones]

On Mon, Jul 29, 2024 at 03:43:30PM GMT, Alexandre Ghiti wrote:
...
> > +static __always_inline void __cmpwait(volatile void *ptr,
> > +				      unsigned long val,
> > +				      int size)
> > +{
> > +	unsigned long tmp;
> > +
> > +	asm goto(ALTERNATIVE("j %l[no_zawrs]", "nop",
> > +			     0, RISCV_ISA_EXT_ZAWRS, 1)
> > +		 : : : : no_zawrs);
> > +
> > +	switch (size) {
> > +	case 4:
> > +		asm volatile(
> > +		"	lr.w	%0, %1\n"
> > +		"	xor	%0, %0, %2\n"
> > +		"	bnez	%0, 1f\n"
> > +			ZAWRS_WRS_NTO "\n"
> > +		"1:"
> > +		: "=&r" (tmp), "+A" (*(u32 *)ptr)
> > +		: "r" (val));
> > +		break;
> > +#if __riscv_xlen == 64
> > +	case 8:
> > +		asm volatile(
> > +		"	lr.d	%0, %1\n"
> > +		"	xor	%0, %0, %2\n"
> > +		"	bnez	%0, 1f\n"
> > +			ZAWRS_WRS_NTO "\n"
> > +		"1:"
> > +		: "=&r" (tmp), "+A" (*(u64 *)ptr)
> > +		: "r" (val));
> > +		break;
> > +#endif
> > +	default:
> > +		BUILD_BUG();
> > +	}
> > +
> > +	return;
> > +
> > +no_zawrs:
> > +	asm volatile(RISCV_PAUSE : : : "memory");
> 
> 
> Shouldn't we fallback to the previous implementation (cpu_relax()) here? Not
> sure this is really important, but I want to make sure it was not an
> oversight.
>

Hi Alex,

It was intentional. We can't easily call cpu_relax() from here because
asm/vdso/processor.h includes asm/barrier.h which includes asm/cmpxchg.h.
We've mostly reproduced cpu_relax() here since we're only skipping the
div, and, as __cmpwait will be used in loops which load memory for the
comparison, I didn't think we needed the extra div stalls.

Thanks,
drew

Re: [PATCH v3 3/6] riscv: Add Zawrs support for spinlocks

[Alexandre Ghiti]

On 29/07/2024 16:01, Andrew Jones wrote:
> On Mon, Jul 29, 2024 at 03:43:30PM GMT, Alexandre Ghiti wrote:
> ...
>>> +static __always_inline void __cmpwait(volatile void *ptr,
>>> +				      unsigned long val,
>>> +				      int size)
>>> +{
>>> +	unsigned long tmp;
>>> +
>>> +	asm goto(ALTERNATIVE("j %l[no_zawrs]", "nop",
>>> +			     0, RISCV_ISA_EXT_ZAWRS, 1)
>>> +		 : : : : no_zawrs);
>>> +
>>> +	switch (size) {
>>> +	case 4:
>>> +		asm volatile(
>>> +		"	lr.w	%0, %1\n"
>>> +		"	xor	%0, %0, %2\n"
>>> +		"	bnez	%0, 1f\n"
>>> +			ZAWRS_WRS_NTO "\n"
>>> +		"1:"
>>> +		: "=&r" (tmp), "+A" (*(u32 *)ptr)
>>> +		: "r" (val));
>>> +		break;
>>> +#if __riscv_xlen == 64
>>> +	case 8:
>>> +		asm volatile(
>>> +		"	lr.d	%0, %1\n"
>>> +		"	xor	%0, %0, %2\n"
>>> +		"	bnez	%0, 1f\n"
>>> +			ZAWRS_WRS_NTO "\n"
>>> +		"1:"
>>> +		: "=&r" (tmp), "+A" (*(u64 *)ptr)
>>> +		: "r" (val));
>>> +		break;
>>> +#endif
>>> +	default:
>>> +		BUILD_BUG();
>>> +	}
>>> +
>>> +	return;
>>> +
>>> +no_zawrs:
>>> +	asm volatile(RISCV_PAUSE : : : "memory");
>>
>> Shouldn't we fallback to the previous implementation (cpu_relax()) here? Not
>> sure this is really important, but I want to make sure it was not an
>> oversight.
>>
> Hi Alex,
>
> It was intentional. We can't easily call cpu_relax() from here because
> asm/vdso/processor.h includes asm/barrier.h which includes asm/cmpxchg.h.
> We've mostly reproduced cpu_relax() here since we're only skipping the
> div, and, as __cmpwait will be used in loops which load memory for the
> comparison, I didn't think we needed the extra div stalls.


Ok thanks for your quick answer. Actually I'm asking because I fell onto 
the issue with the headers and I managed to fix it by replacing 
asm/barrier.h with linux/compiler.h, which is actually where is defined 
barrier().

But anyway, it makes sense.

Thanks,

Alex


> Thanks,
> drew

[PATCH v3 4/6] riscv: hwprobe: export Zawrs ISA extension

[Andrew Jones]

Export Zawrs ISA extension through hwprobe.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 Documentation/arch/riscv/hwprobe.rst  | 4 ++++
 arch/riscv/include/uapi/asm/hwprobe.h | 1 +
 arch/riscv/kernel/sys_hwprobe.c       | 1 +
 3 files changed, 6 insertions(+)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index b2bcc9eed9aa..e072ce8285d8 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -188,6 +188,10 @@ The following keys are defined:
        manual starting from commit 95cf1f9 ("Add changes requested by Ved
        during signoff")
 
+  * :c:macro:`RISCV_HWPROBE_EXT_ZAWRS`: The Zawrs extension is supported as
+       ratified in commit 98918c844281 ("Merge pull request #1217 from
+       riscv/zawrs") of riscv-isa-manual.
+
 * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
   information about the selected set of processors.
 
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 9f2a8e3ff204..a5fca3878a32 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -59,6 +59,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
 #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
 #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
+#define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 36)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
 #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index 8cae41a502dd..b86e3531a45a 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -111,6 +111,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 		EXT_KEY(ZTSO);
 		EXT_KEY(ZACAS);
 		EXT_KEY(ZICOND);
+		EXT_KEY(ZAWRS);
 
 		if (has_vector()) {
 			EXT_KEY(ZVBB);
-- 
2.44.0

Re: [PATCH v3 4/6] riscv: hwprobe: export Zawrs ISA extension

[Clément Léger]

On 26/04/2024 12:08, Andrew Jones wrote:
> Export Zawrs ISA extension through hwprobe.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  Documentation/arch/riscv/hwprobe.rst  | 4 ++++
>  arch/riscv/include/uapi/asm/hwprobe.h | 1 +
>  arch/riscv/kernel/sys_hwprobe.c       | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> index b2bcc9eed9aa..e072ce8285d8 100644
> --- a/Documentation/arch/riscv/hwprobe.rst
> +++ b/Documentation/arch/riscv/hwprobe.rst
> @@ -188,6 +188,10 @@ The following keys are defined:
>         manual starting from commit 95cf1f9 ("Add changes requested by Ved
>         during signoff")
>  
> +  * :c:macro:`RISCV_HWPROBE_EXT_ZAWRS`: The Zawrs extension is supported as
> +       ratified in commit 98918c844281 ("Merge pull request #1217 from
> +       riscv/zawrs") of riscv-isa-manual.
> +
>  * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
>    information about the selected set of processors.
>  
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 9f2a8e3ff204..a5fca3878a32 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -59,6 +59,7 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
>  #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
>  #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
> +#define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 36)
>  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
>  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index 8cae41a502dd..b86e3531a45a 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -111,6 +111,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  		EXT_KEY(ZTSO);
>  		EXT_KEY(ZACAS);
>  		EXT_KEY(ZICOND);
> +		EXT_KEY(ZAWRS);
>  
>  		if (has_vector()) {
>  			EXT_KEY(ZVBB);

AFAIU, when used in userspace, this will actually "stall" the processor
until an interrupt/timeout happens, so the current process will keep
occupying the processor doing nothing (up to the next interrupt/timeout)
right ?

BTW, the spec also states that "When the TW (Timeout Wait) bit in
mstatus is set and WRS.NTO is executed in any privilege mode other than
M mode, and it does not complete within an implementation-specific
bounded time limit, the WRS.NTO instruction will cause an illegal
instruction exception." so I guess the process will be killed in this case ?

If this is not a concern:

Reviewed-by: Clément Léger <cleger@rivosinc.com>

Thanks,

Clément

Re: [PATCH v3 4/6] riscv: hwprobe: export Zawrs ISA extension

[Andrew Jones]

On Tue, Jun 18, 2024 at 03:48:59PM GMT, Clément Léger wrote:
> 
> 
> On 26/04/2024 12:08, Andrew Jones wrote:
> > Export Zawrs ISA extension through hwprobe.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  Documentation/arch/riscv/hwprobe.rst  | 4 ++++
> >  arch/riscv/include/uapi/asm/hwprobe.h | 1 +
> >  arch/riscv/kernel/sys_hwprobe.c       | 1 +
> >  3 files changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > index b2bcc9eed9aa..e072ce8285d8 100644
> > --- a/Documentation/arch/riscv/hwprobe.rst
> > +++ b/Documentation/arch/riscv/hwprobe.rst
> > @@ -188,6 +188,10 @@ The following keys are defined:
> >         manual starting from commit 95cf1f9 ("Add changes requested by Ved
> >         during signoff")
> >  
> > +  * :c:macro:`RISCV_HWPROBE_EXT_ZAWRS`: The Zawrs extension is supported as
> > +       ratified in commit 98918c844281 ("Merge pull request #1217 from
> > +       riscv/zawrs") of riscv-isa-manual.
> > +
> >  * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
> >    information about the selected set of processors.
> >  
> > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > index 9f2a8e3ff204..a5fca3878a32 100644
> > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > @@ -59,6 +59,7 @@ struct riscv_hwprobe {
> >  #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
> >  #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
> >  #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
> > +#define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 36)
> >  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
> >  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
> >  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > index 8cae41a502dd..b86e3531a45a 100644
> > --- a/arch/riscv/kernel/sys_hwprobe.c
> > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > @@ -111,6 +111,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
> >  		EXT_KEY(ZTSO);
> >  		EXT_KEY(ZACAS);
> >  		EXT_KEY(ZICOND);
> > +		EXT_KEY(ZAWRS);
> >  
> >  		if (has_vector()) {
> >  			EXT_KEY(ZVBB);
> 
> AFAIU, when used in userspace, this will actually "stall" the processor
> until an interrupt/timeout happens, so the current process will keep
> occupying the processor doing nothing (up to the next interrupt/timeout)
> right ?

Yes, but of course the OS can always preempt the task as well.

> 
> BTW, the spec also states that "When the TW (Timeout Wait) bit in
> mstatus is set and WRS.NTO is executed in any privilege mode other than
> M mode, and it does not complete within an implementation-specific
> bounded time limit, the WRS.NTO instruction will cause an illegal
> instruction exception." so I guess the process will be killed in this case ?

We don't expect mstatus.TW to be set. If it is, then wfi would likely kill
the kernel before wrs.nto gets a chance to, but one or the other will
certainly ensure usermode never gets a chance to try it :-)

We have a handful of these assumptions about how M-mode has configured
things prior to Linux starting. It'd be good if we documented them all
somewhere.

> 
> If this is not a concern:
> 
> Reviewed-by: Clément Léger <cleger@rivosinc.com>

Thanks,
drew

Re: [PATCH v3 4/6] riscv: hwprobe: export Zawrs ISA extension

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3646 bytes --]

On Tue, Jun 18, 2024 at 06:01:04PM +0200, Andrew Jones wrote:
> On Tue, Jun 18, 2024 at 03:48:59PM GMT, Clément Léger wrote:
> > 
> > 
> > On 26/04/2024 12:08, Andrew Jones wrote:
> > > Export Zawrs ISA extension through hwprobe.
> > > 
> > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > > ---
> > >  Documentation/arch/riscv/hwprobe.rst  | 4 ++++
> > >  arch/riscv/include/uapi/asm/hwprobe.h | 1 +
> > >  arch/riscv/kernel/sys_hwprobe.c       | 1 +
> > >  3 files changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > > index b2bcc9eed9aa..e072ce8285d8 100644
> > > --- a/Documentation/arch/riscv/hwprobe.rst
> > > +++ b/Documentation/arch/riscv/hwprobe.rst
> > > @@ -188,6 +188,10 @@ The following keys are defined:
> > >         manual starting from commit 95cf1f9 ("Add changes requested by Ved
> > >         during signoff")
> > >  
> > > +  * :c:macro:`RISCV_HWPROBE_EXT_ZAWRS`: The Zawrs extension is supported as
> > > +       ratified in commit 98918c844281 ("Merge pull request #1217 from
> > > +       riscv/zawrs") of riscv-isa-manual.
> > > +
> > >  * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
> > >    information about the selected set of processors.
> > >  
> > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > > index 9f2a8e3ff204..a5fca3878a32 100644
> > > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > > @@ -59,6 +59,7 @@ struct riscv_hwprobe {
> > >  #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
> > >  #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
> > >  #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
> > > +#define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 36)
> > >  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
> > >  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
> > >  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> > > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > > index 8cae41a502dd..b86e3531a45a 100644
> > > --- a/arch/riscv/kernel/sys_hwprobe.c
> > > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > > @@ -111,6 +111,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
> > >  		EXT_KEY(ZTSO);
> > >  		EXT_KEY(ZACAS);
> > >  		EXT_KEY(ZICOND);
> > > +		EXT_KEY(ZAWRS);
> > >  
> > >  		if (has_vector()) {
> > >  			EXT_KEY(ZVBB);
> > 
> > AFAIU, when used in userspace, this will actually "stall" the processor
> > until an interrupt/timeout happens, so the current process will keep
> > occupying the processor doing nothing (up to the next interrupt/timeout)
> > right ?
> 
> Yes, but of course the OS can always preempt the task as well.
> 
> > 
> > BTW, the spec also states that "When the TW (Timeout Wait) bit in
> > mstatus is set and WRS.NTO is executed in any privilege mode other than
> > M mode, and it does not complete within an implementation-specific
> > bounded time limit, the WRS.NTO instruction will cause an illegal
> > instruction exception." so I guess the process will be killed in this case ?
> 
> We don't expect mstatus.TW to be set. If it is, then wfi would likely kill
> the kernel before wrs.nto gets a chance to, but one or the other will
> certainly ensure usermode never gets a chance to try it :-)
> 
> We have a handful of these assumptions about how M-mode has configured
> things prior to Linux starting. It'd be good if we documented them all
> somewhere.

Boot.rst :) If you're adding a new assumption, I think it should go
there.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 4/6] riscv: hwprobe: export Zawrs ISA extension

[Andrew Jones]

On Tue, Jun 18, 2024 at 05:59:22PM GMT, Conor Dooley wrote:
> On Tue, Jun 18, 2024 at 06:01:04PM +0200, Andrew Jones wrote:
> > On Tue, Jun 18, 2024 at 03:48:59PM GMT, Clément Léger wrote:
> > > 
> > > 
> > > On 26/04/2024 12:08, Andrew Jones wrote:
> > > > Export Zawrs ISA extension through hwprobe.
> > > > 
> > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > > > ---
> > > >  Documentation/arch/riscv/hwprobe.rst  | 4 ++++
> > > >  arch/riscv/include/uapi/asm/hwprobe.h | 1 +
> > > >  arch/riscv/kernel/sys_hwprobe.c       | 1 +
> > > >  3 files changed, 6 insertions(+)
> > > > 
> > > > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > > > index b2bcc9eed9aa..e072ce8285d8 100644
> > > > --- a/Documentation/arch/riscv/hwprobe.rst
> > > > +++ b/Documentation/arch/riscv/hwprobe.rst
> > > > @@ -188,6 +188,10 @@ The following keys are defined:
> > > >         manual starting from commit 95cf1f9 ("Add changes requested by Ved
> > > >         during signoff")
> > > >  
> > > > +  * :c:macro:`RISCV_HWPROBE_EXT_ZAWRS`: The Zawrs extension is supported as
> > > > +       ratified in commit 98918c844281 ("Merge pull request #1217 from
> > > > +       riscv/zawrs") of riscv-isa-manual.
> > > > +
> > > >  * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
> > > >    information about the selected set of processors.
> > > >  
> > > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > > > index 9f2a8e3ff204..a5fca3878a32 100644
> > > > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > > > @@ -59,6 +59,7 @@ struct riscv_hwprobe {
> > > >  #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
> > > >  #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
> > > >  #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
> > > > +#define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 36)
> > > >  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
> > > >  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
> > > >  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> > > > diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> > > > index 8cae41a502dd..b86e3531a45a 100644
> > > > --- a/arch/riscv/kernel/sys_hwprobe.c
> > > > +++ b/arch/riscv/kernel/sys_hwprobe.c
> > > > @@ -111,6 +111,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
> > > >  		EXT_KEY(ZTSO);
> > > >  		EXT_KEY(ZACAS);
> > > >  		EXT_KEY(ZICOND);
> > > > +		EXT_KEY(ZAWRS);
> > > >  
> > > >  		if (has_vector()) {
> > > >  			EXT_KEY(ZVBB);
> > > 
> > > AFAIU, when used in userspace, this will actually "stall" the processor
> > > until an interrupt/timeout happens, so the current process will keep
> > > occupying the processor doing nothing (up to the next interrupt/timeout)
> > > right ?
> > 
> > Yes, but of course the OS can always preempt the task as well.
> > 
> > > 
> > > BTW, the spec also states that "When the TW (Timeout Wait) bit in
> > > mstatus is set and WRS.NTO is executed in any privilege mode other than
> > > M mode, and it does not complete within an implementation-specific
> > > bounded time limit, the WRS.NTO instruction will cause an illegal
> > > instruction exception." so I guess the process will be killed in this case ?
> > 
> > We don't expect mstatus.TW to be set. If it is, then wfi would likely kill
> > the kernel before wrs.nto gets a chance to, but one or the other will
> > certainly ensure usermode never gets a chance to try it :-)
> > 
> > We have a handful of these assumptions about how M-mode has configured
> > things prior to Linux starting. It'd be good if we documented them all
> > somewhere.
> 
> Boot.rst :) If you're adding a new assumption, I think it should go
> there.

It's an old assumption (wfi has been counting on it since the beginning of
time). But documenting it, along with anything else similar to it, in
boot.rst is a good idea. I've added that to my TODO.

Thanks,
drew

Re: [PATCH v3 4/6] riscv: hwprobe: export Zawrs ISA extension

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

On Tue, Jun 18, 2024 at 07:05:41PM +0200, Andrew Jones wrote:
> On Tue, Jun 18, 2024 at 05:59:22PM GMT, Conor Dooley wrote:
> > On Tue, Jun 18, 2024 at 06:01:04PM +0200, Andrew Jones wrote:
> > > On Tue, Jun 18, 2024 at 03:48:59PM GMT, Clément Léger wrote:
> > > > On 26/04/2024 12:08, Andrew Jones wrote:

> > > > 
> > > > BTW, the spec also states that "When the TW (Timeout Wait) bit in
> > > > mstatus is set and WRS.NTO is executed in any privilege mode other than
> > > > M mode, and it does not complete within an implementation-specific
> > > > bounded time limit, the WRS.NTO instruction will cause an illegal
> > > > instruction exception." so I guess the process will be killed in this case ?
> > > 
> > > We don't expect mstatus.TW to be set. If it is, then wfi would likely kill
> > > the kernel before wrs.nto gets a chance to, but one or the other will
> > > certainly ensure usermode never gets a chance to try it :-)
> > > 
> > > We have a handful of these assumptions about how M-mode has configured
> > > things prior to Linux starting. It'd be good if we documented them all
> > > somewhere.
> > 
> > Boot.rst :) If you're adding a new assumption, I think it should go
> > there.
> 
> It's an old assumption (wfi has been counting on it since the beginning of
> time). But documenting it, along with anything else similar to it, in
> boot.rst is a good idea. I've added that to my TODO.

Misunderstanding of "new" I think, I meant new to documentation not just
new to the kernel :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 4/6] riscv: hwprobe: export Zawrs ISA extension

[Palmer Dabbelt]

On Fri, 26 Apr 2024 03:08:24 PDT (-0700), ajones@ventanamicro.com wrote:
> Export Zawrs ISA extension through hwprobe.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  Documentation/arch/riscv/hwprobe.rst  | 4 ++++
>  arch/riscv/include/uapi/asm/hwprobe.h | 1 +
>  arch/riscv/kernel/sys_hwprobe.c       | 1 +
>  3 files changed, 6 insertions(+)
>
> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> index b2bcc9eed9aa..e072ce8285d8 100644
> --- a/Documentation/arch/riscv/hwprobe.rst
> +++ b/Documentation/arch/riscv/hwprobe.rst
> @@ -188,6 +188,10 @@ The following keys are defined:
>         manual starting from commit 95cf1f9 ("Add changes requested by Ved
>         during signoff")
>
> +  * :c:macro:`RISCV_HWPROBE_EXT_ZAWRS`: The Zawrs extension is supported as
> +       ratified in commit 98918c844281 ("Merge pull request #1217 from
> +       riscv/zawrs") of riscv-isa-manual.
> +
>  * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
>    information about the selected set of processors.
>
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 9f2a8e3ff204..a5fca3878a32 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -59,6 +59,7 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
>  #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
>  #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
> +#define		RISCV_HWPROBE_EXT_ZAWRS		(1ULL << 36)

I bumped this to a non-conflicting key when I picked up the patch, I 
figured that'd be better as then we don't have a uABI hiccup.  There 
were some other merge coflicts that looked easy to be, but LMK if I 
screwed something up as I'm not operating on a ton of sleep...

>  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
>  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index 8cae41a502dd..b86e3531a45a 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -111,6 +111,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  		EXT_KEY(ZTSO);
>  		EXT_KEY(ZACAS);
>  		EXT_KEY(ZICOND);
> +		EXT_KEY(ZAWRS);
>
>  		if (has_vector()) {
>  			EXT_KEY(ZVBB);

[PATCH v3 5/6] KVM: riscv: Support guest wrs.nto

[Andrew Jones]

When a guest traps on wrs.nto, call kvm_vcpu_on_spin() to attempt
to yield to the lock holding VCPU. Also extend the KVM ISA extension
ONE_REG interface to allow KVM userspace to detect and enable the
Zawrs extension for the Guest/VM.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/asm/kvm_host.h |  1 +
 arch/riscv/include/uapi/asm/kvm.h |  1 +
 arch/riscv/kvm/vcpu.c             |  1 +
 arch/riscv/kvm/vcpu_insn.c        | 15 +++++++++++++++
 arch/riscv/kvm/vcpu_onereg.c      |  2 ++
 5 files changed, 20 insertions(+)

diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 484d04a92fa6..e27c56e44783 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -69,6 +69,7 @@ struct kvm_vcpu_stat {
 	struct kvm_vcpu_stat_generic generic;
 	u64 ecall_exit_stat;
 	u64 wfi_exit_stat;
+	u64 wrs_exit_stat;
 	u64 mmio_exit_user;
 	u64 mmio_exit_kernel;
 	u64 csr_exit_user;
diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index b1c503c2959c..89ea06bd07c2 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -167,6 +167,7 @@ enum KVM_RISCV_ISA_EXT_ID {
 	KVM_RISCV_ISA_EXT_ZFA,
 	KVM_RISCV_ISA_EXT_ZTSO,
 	KVM_RISCV_ISA_EXT_ZACAS,
+	KVM_RISCV_ISA_EXT_ZAWRS,
 	KVM_RISCV_ISA_EXT_MAX,
 };
 
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index b5ca9f2e98ac..abcdc78671e0 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -25,6 +25,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
 	KVM_GENERIC_VCPU_STATS(),
 	STATS_DESC_COUNTER(VCPU, ecall_exit_stat),
 	STATS_DESC_COUNTER(VCPU, wfi_exit_stat),
+	STATS_DESC_COUNTER(VCPU, wrs_exit_stat),
 	STATS_DESC_COUNTER(VCPU, mmio_exit_user),
 	STATS_DESC_COUNTER(VCPU, mmio_exit_kernel),
 	STATS_DESC_COUNTER(VCPU, csr_exit_user),
diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
index ee7215f4071f..97dec18e6989 100644
--- a/arch/riscv/kvm/vcpu_insn.c
+++ b/arch/riscv/kvm/vcpu_insn.c
@@ -16,6 +16,9 @@
 #define INSN_MASK_WFI		0xffffffff
 #define INSN_MATCH_WFI		0x10500073
 
+#define INSN_MASK_WRS		0xffffffff
+#define INSN_MATCH_WRS		0x00d00073
+
 #define INSN_MATCH_CSRRW	0x1073
 #define INSN_MASK_CSRRW		0x707f
 #define INSN_MATCH_CSRRS	0x2073
@@ -203,6 +206,13 @@ static int wfi_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
 	return KVM_INSN_CONTINUE_NEXT_SEPC;
 }
 
+static int wrs_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
+{
+	vcpu->stat.wrs_exit_stat++;
+	kvm_vcpu_on_spin(vcpu, vcpu->arch.guest_context.sstatus & SR_SPP);
+	return KVM_INSN_CONTINUE_NEXT_SEPC;
+}
+
 struct csr_func {
 	unsigned int base;
 	unsigned int count;
@@ -378,6 +388,11 @@ static const struct insn_func system_opcode_funcs[] = {
 		.match = INSN_MATCH_WFI,
 		.func  = wfi_insn,
 	},
+	{
+		.mask  = INSN_MASK_WRS,
+		.match = INSN_MATCH_WRS,
+		.func  = wrs_insn,
+	},
 };
 
 static int system_opcode_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index f4a6124d25c9..67c5794af3b6 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -41,6 +41,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
 	KVM_ISA_EXT_ARR(SVNAPOT),
 	KVM_ISA_EXT_ARR(SVPBMT),
 	KVM_ISA_EXT_ARR(ZACAS),
+	KVM_ISA_EXT_ARR(ZAWRS),
 	KVM_ISA_EXT_ARR(ZBA),
 	KVM_ISA_EXT_ARR(ZBB),
 	KVM_ISA_EXT_ARR(ZBC),
@@ -120,6 +121,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
 	case KVM_RISCV_ISA_EXT_SVINVAL:
 	case KVM_RISCV_ISA_EXT_SVNAPOT:
 	case KVM_RISCV_ISA_EXT_ZACAS:
+	case KVM_RISCV_ISA_EXT_ZAWRS:
 	case KVM_RISCV_ISA_EXT_ZBA:
 	case KVM_RISCV_ISA_EXT_ZBB:
 	case KVM_RISCV_ISA_EXT_ZBC:
-- 
2.44.0

Re: [PATCH v3 5/6] KVM: riscv: Support guest wrs.nto

[Anup Patel]

On Fri, Apr 26, 2024 at 3:38 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> When a guest traps on wrs.nto, call kvm_vcpu_on_spin() to attempt
> to yield to the lock holding VCPU. Also extend the KVM ISA extension
> ONE_REG interface to allow KVM userspace to detect and enable the
> Zawrs extension for the Guest/VM.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

LGTM.

Acked-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/include/asm/kvm_host.h |  1 +
>  arch/riscv/include/uapi/asm/kvm.h |  1 +
>  arch/riscv/kvm/vcpu.c             |  1 +
>  arch/riscv/kvm/vcpu_insn.c        | 15 +++++++++++++++
>  arch/riscv/kvm/vcpu_onereg.c      |  2 ++
>  5 files changed, 20 insertions(+)
>
> diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
> index 484d04a92fa6..e27c56e44783 100644
> --- a/arch/riscv/include/asm/kvm_host.h
> +++ b/arch/riscv/include/asm/kvm_host.h
> @@ -69,6 +69,7 @@ struct kvm_vcpu_stat {
>         struct kvm_vcpu_stat_generic generic;
>         u64 ecall_exit_stat;
>         u64 wfi_exit_stat;
> +       u64 wrs_exit_stat;
>         u64 mmio_exit_user;
>         u64 mmio_exit_kernel;
>         u64 csr_exit_user;
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index b1c503c2959c..89ea06bd07c2 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -167,6 +167,7 @@ enum KVM_RISCV_ISA_EXT_ID {
>         KVM_RISCV_ISA_EXT_ZFA,
>         KVM_RISCV_ISA_EXT_ZTSO,
>         KVM_RISCV_ISA_EXT_ZACAS,
> +       KVM_RISCV_ISA_EXT_ZAWRS,
>         KVM_RISCV_ISA_EXT_MAX,
>  };
>
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index b5ca9f2e98ac..abcdc78671e0 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -25,6 +25,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
>         KVM_GENERIC_VCPU_STATS(),
>         STATS_DESC_COUNTER(VCPU, ecall_exit_stat),
>         STATS_DESC_COUNTER(VCPU, wfi_exit_stat),
> +       STATS_DESC_COUNTER(VCPU, wrs_exit_stat),
>         STATS_DESC_COUNTER(VCPU, mmio_exit_user),
>         STATS_DESC_COUNTER(VCPU, mmio_exit_kernel),
>         STATS_DESC_COUNTER(VCPU, csr_exit_user),
> diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
> index ee7215f4071f..97dec18e6989 100644
> --- a/arch/riscv/kvm/vcpu_insn.c
> +++ b/arch/riscv/kvm/vcpu_insn.c
> @@ -16,6 +16,9 @@
>  #define INSN_MASK_WFI          0xffffffff
>  #define INSN_MATCH_WFI         0x10500073
>
> +#define INSN_MASK_WRS          0xffffffff
> +#define INSN_MATCH_WRS         0x00d00073
> +
>  #define INSN_MATCH_CSRRW       0x1073
>  #define INSN_MASK_CSRRW                0x707f
>  #define INSN_MATCH_CSRRS       0x2073
> @@ -203,6 +206,13 @@ static int wfi_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
>         return KVM_INSN_CONTINUE_NEXT_SEPC;
>  }
>
> +static int wrs_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
> +{
> +       vcpu->stat.wrs_exit_stat++;
> +       kvm_vcpu_on_spin(vcpu, vcpu->arch.guest_context.sstatus & SR_SPP);
> +       return KVM_INSN_CONTINUE_NEXT_SEPC;
> +}
> +
>  struct csr_func {
>         unsigned int base;
>         unsigned int count;
> @@ -378,6 +388,11 @@ static const struct insn_func system_opcode_funcs[] = {
>                 .match = INSN_MATCH_WFI,
>                 .func  = wfi_insn,
>         },
> +       {
> +               .mask  = INSN_MASK_WRS,
> +               .match = INSN_MATCH_WRS,
> +               .func  = wrs_insn,
> +       },
>  };
>
>  static int system_opcode_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index f4a6124d25c9..67c5794af3b6 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c
> @@ -41,6 +41,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
>         KVM_ISA_EXT_ARR(SVNAPOT),
>         KVM_ISA_EXT_ARR(SVPBMT),
>         KVM_ISA_EXT_ARR(ZACAS),
> +       KVM_ISA_EXT_ARR(ZAWRS),
>         KVM_ISA_EXT_ARR(ZBA),
>         KVM_ISA_EXT_ARR(ZBB),
>         KVM_ISA_EXT_ARR(ZBC),
> @@ -120,6 +121,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
>         case KVM_RISCV_ISA_EXT_SVINVAL:
>         case KVM_RISCV_ISA_EXT_SVNAPOT:
>         case KVM_RISCV_ISA_EXT_ZACAS:
> +       case KVM_RISCV_ISA_EXT_ZAWRS:
>         case KVM_RISCV_ISA_EXT_ZBA:
>         case KVM_RISCV_ISA_EXT_ZBB:
>         case KVM_RISCV_ISA_EXT_ZBC:
> --
> 2.44.0
>

[PATCH v3 6/6] KVM: riscv: selftests: Add Zawrs extension to get-reg-list test

[Andrew Jones]

KVM RISC-V allows the Zawrs extension for the Guest/VM, so add it
to the get-reg-list test.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 tools/testing/selftests/kvm/riscv/get-reg-list.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
index b882b7b9b785..8c4c27bd4b88 100644
--- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
+++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
@@ -48,6 +48,7 @@ bool filter_reg(__u64 reg)
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVNAPOT:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVPBMT:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZACAS:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZAWRS:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBA:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBB:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBC:
@@ -413,6 +414,7 @@ static const char *isa_ext_single_id_to_str(__u64 reg_off)
 		KVM_ISA_EXT_ARR(SVNAPOT),
 		KVM_ISA_EXT_ARR(SVPBMT),
 		KVM_ISA_EXT_ARR(ZACAS),
+		KVM_ISA_EXT_ARR(ZAWRS),
 		KVM_ISA_EXT_ARR(ZBA),
 		KVM_ISA_EXT_ARR(ZBB),
 		KVM_ISA_EXT_ARR(ZBC),
@@ -936,6 +938,7 @@ KVM_ISA_EXT_SIMPLE_CONFIG(svinval, SVINVAL);
 KVM_ISA_EXT_SIMPLE_CONFIG(svnapot, SVNAPOT);
 KVM_ISA_EXT_SIMPLE_CONFIG(svpbmt, SVPBMT);
 KVM_ISA_EXT_SIMPLE_CONFIG(zacas, ZACAS);
+KVM_ISA_EXT_SIMPLE_CONFIG(zawrs, ZAWRS);
 KVM_ISA_EXT_SIMPLE_CONFIG(zba, ZBA);
 KVM_ISA_EXT_SIMPLE_CONFIG(zbb, ZBB);
 KVM_ISA_EXT_SIMPLE_CONFIG(zbc, ZBC);
@@ -991,6 +994,7 @@ struct vcpu_reg_list *vcpu_configs[] = {
 	&config_svnapot,
 	&config_svpbmt,
 	&config_zacas,
+	&config_zawrs,
 	&config_zba,
 	&config_zbb,
 	&config_zbc,
-- 
2.44.0

Re: [PATCH v3 6/6] KVM: riscv: selftests: Add Zawrs extension to get-reg-list test

[Anup Patel]

On Fri, Apr 26, 2024 at 3:38 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> KVM RISC-V allows the Zawrs extension for the Guest/VM, so add it
> to the get-reg-list test.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

LGTM.

Acked-by: Anup Patel <anup@brainfault.org>

Thanks,
Anup

> ---
>  tools/testing/selftests/kvm/riscv/get-reg-list.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> index b882b7b9b785..8c4c27bd4b88 100644
> --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> @@ -48,6 +48,7 @@ bool filter_reg(__u64 reg)
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVNAPOT:
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVPBMT:
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZACAS:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZAWRS:
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBA:
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBB:
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBC:
> @@ -413,6 +414,7 @@ static const char *isa_ext_single_id_to_str(__u64 reg_off)
>                 KVM_ISA_EXT_ARR(SVNAPOT),
>                 KVM_ISA_EXT_ARR(SVPBMT),
>                 KVM_ISA_EXT_ARR(ZACAS),
> +               KVM_ISA_EXT_ARR(ZAWRS),
>                 KVM_ISA_EXT_ARR(ZBA),
>                 KVM_ISA_EXT_ARR(ZBB),
>                 KVM_ISA_EXT_ARR(ZBC),
> @@ -936,6 +938,7 @@ KVM_ISA_EXT_SIMPLE_CONFIG(svinval, SVINVAL);
>  KVM_ISA_EXT_SIMPLE_CONFIG(svnapot, SVNAPOT);
>  KVM_ISA_EXT_SIMPLE_CONFIG(svpbmt, SVPBMT);
>  KVM_ISA_EXT_SIMPLE_CONFIG(zacas, ZACAS);
> +KVM_ISA_EXT_SIMPLE_CONFIG(zawrs, ZAWRS);
>  KVM_ISA_EXT_SIMPLE_CONFIG(zba, ZBA);
>  KVM_ISA_EXT_SIMPLE_CONFIG(zbb, ZBB);
>  KVM_ISA_EXT_SIMPLE_CONFIG(zbc, ZBC);
> @@ -991,6 +994,7 @@ struct vcpu_reg_list *vcpu_configs[] = {
>         &config_svnapot,
>         &config_svpbmt,
>         &config_zacas,
> +       &config_zawrs,
>         &config_zba,
>         &config_zbb,
>         &config_zbc,
> --
> 2.44.0
>

Re: [PATCH v3 0/6] riscv: Apply Zawrs when available

[Andrew Jones]

Hi Palmer,

I submit our concerns of wrs.nto to RVI ARC for consideration. They
discussed it but don't believe there's a need for concern. The
expectation is that there will always be enough interrupt activity
and that those interrupts with activity will not likely be locally
disabled.

Can we consider this series for 6.10?

Thanks,
drew

On Fri, Apr 26, 2024 at 12:08:20PM GMT, Andrew Jones wrote:
> Zawrs provides two instructions (wrs.nto and wrs.sto), where both are
> meant to allow the hart to enter a low-power state while waiting on a
> store to a memory location. The instructions also both wait an
> implementation-defined "short" duration (unless the implementation
> terminates the stall for another reason). The difference is that while
> wrs.sto will terminate when the duration elapses, wrs.nto, depending on
> configuration, will either just keep waiting or an ILL exception will be
> raised. Linux will use wrs.nto, so if platforms have an implementation
> which falls in the "just keep waiting" category (which is not expected),
> then it should _not_ advertise Zawrs in the hardware description.
> 
> Like wfi (and with the same {m,h}status bits to configure it), when
> wrs.nto is configured to raise exceptions it's expected that the higher
> privilege level will see the instruction was a wait instruction, do
> something, and then resume execution following the instruction. For
> example, KVM does configure exceptions for wfi (hstatus.VTW=1) and
> therefore also for wrs.nto. KVM does this for wfi since it's better to
> allow other tasks to be scheduled while a VCPU waits for an interrupt.
> For waits such as those where wrs.nto/sto would be used, which are
> typically locks, it is also a good idea for KVM to be involved, as it
> can attempt to schedule the lock holding VCPU.
> 
> This series starts with Christoph's addition of the riscv
> smp_cond_load_relaxed function which applies wrs.sto when available.
> That patch has been reworked to use wrs.nto and to use the same approach
> as Arm for the wait loop, since we can't have arbitrary C code between
> the load-reserved and the wrs. Then, hwprobe support is added (since the
> instructions are also usable from usermode), and finally KVM is
> taught about wrs.nto, allowing guests to see and use the Zawrs
> extension.
> 
> We still don't have test results from hardware, and it's not possible to
> prove that using Zawrs is a win when testing on QEMU, not even when
> oversubscribing VCPUs to guests. However, it is possible to use KVM
> selftests to force a scenario where we can prove Zawrs does its job and
> does it well. [4] is a test which does this and, on my machine, without
> Zawrs it takes 16 seconds to complete and with Zawrs it takes 0.25
> seconds.
> 
> This series is also available here [1]. In order to use QEMU for testing
> a build with [2] is needed. In order to enable guests to use Zawrs with
> KVM using kvmtool, the branch at [3] may be used.
> 
> [1] https://github.com/jones-drew/linux/commits/riscv/zawrs-v3/
> [2] https://lore.kernel.org/all/20240312152901.512001-2-ajones@ventanamicro.com/
> [3] https://github.com/jones-drew/kvmtool/commits/riscv/zawrs/
> [4] https://github.com/jones-drew/linux/commit/cb2beccebcece10881db842ed69bdd5715cfab5d
> 
> Thanks,
> drew
> 
> v3:
>  - Moved comment about expected termination from the DT binding text
>    to a code comment.
> 
> v2:
>  - Added DT bindings patch with additional Linux specifications due
>    to wrs.nto potentially never terminating, as suggested by Palmer
>  - Added patch to share pause insn definition
>  - Rework main Zawrs support patch to use Arm approach (which is
>    also the approach that Andrea Parri suggested)
>  - Dropped the riscv implementation of smp_cond_load_acquire().
>    afaict, the generic implementation, which will use the riscv
>    implementation of smp_cond_load_relaxed() is sufficient for riscv.
>  - The rework was large enough (IMO) to drop Heiko's s-o-b and to
>    add myself as a co-developer
> 
> 
> Andrew Jones (5):
>   riscv: Provide a definition for 'pause'
>   dt-bindings: riscv: Add Zawrs ISA extension description
>   riscv: hwprobe: export Zawrs ISA extension
>   KVM: riscv: Support guest wrs.nto
>   KVM: riscv: selftests: Add Zawrs extension to get-reg-list test
> 
> Christoph M??llner (1):
>   riscv: Add Zawrs support for spinlocks
> 
>  Documentation/arch/riscv/hwprobe.rst          |  4 ++
>  .../devicetree/bindings/riscv/extensions.yaml |  7 +++
>  arch/riscv/Kconfig                            | 20 ++++---
>  arch/riscv/Makefile                           |  3 -
>  arch/riscv/include/asm/barrier.h              | 45 +++++++++-----
>  arch/riscv/include/asm/cmpxchg.h              | 58 +++++++++++++++++++
>  arch/riscv/include/asm/hwcap.h                |  1 +
>  arch/riscv/include/asm/insn-def.h             |  4 ++
>  arch/riscv/include/asm/kvm_host.h             |  1 +
>  arch/riscv/include/asm/vdso/processor.h       |  8 +--
>  arch/riscv/include/uapi/asm/hwprobe.h         |  1 +
>  arch/riscv/include/uapi/asm/kvm.h             |  1 +
>  arch/riscv/kernel/cpufeature.c                |  1 +
>  arch/riscv/kernel/sys_hwprobe.c               |  1 +
>  arch/riscv/kvm/vcpu.c                         |  1 +
>  arch/riscv/kvm/vcpu_insn.c                    | 15 +++++
>  arch/riscv/kvm/vcpu_onereg.c                  |  2 +
>  .../selftests/kvm/riscv/get-reg-list.c        |  4 ++
>  18 files changed, 146 insertions(+), 31 deletions(-)
> 
> -- 
> 2.44.0
> 

Re: [PATCH v3 0/6] riscv: Apply Zawrs when available

[Palmer Dabbelt]

On Mon, 03 Jun 2024 05:36:42 PDT (-0700), ajones@ventanamicro.com wrote:
> Hi Palmer,
>
> I submit our concerns of wrs.nto to RVI ARC for consideration. They
> discussed it but don't believe there's a need for concern. The
> expectation is that there will always be enough interrupt activity
> and that those interrupts with activity will not likely be locally
> disabled.
>
> Can we consider this series for 6.10?

Sorry I forgot about this one, Anup just brought it up in the patchwork 
meeting.  He's going to ack the KVM bits, then I'll pick it up.

>
> Thanks,
> drew
>
> On Fri, Apr 26, 2024 at 12:08:20PM GMT, Andrew Jones wrote:
>> Zawrs provides two instructions (wrs.nto and wrs.sto), where both are
>> meant to allow the hart to enter a low-power state while waiting on a
>> store to a memory location. The instructions also both wait an
>> implementation-defined "short" duration (unless the implementation
>> terminates the stall for another reason). The difference is that while
>> wrs.sto will terminate when the duration elapses, wrs.nto, depending on
>> configuration, will either just keep waiting or an ILL exception will be
>> raised. Linux will use wrs.nto, so if platforms have an implementation
>> which falls in the "just keep waiting" category (which is not expected),
>> then it should _not_ advertise Zawrs in the hardware description.
>>
>> Like wfi (and with the same {m,h}status bits to configure it), when
>> wrs.nto is configured to raise exceptions it's expected that the higher
>> privilege level will see the instruction was a wait instruction, do
>> something, and then resume execution following the instruction. For
>> example, KVM does configure exceptions for wfi (hstatus.VTW=1) and
>> therefore also for wrs.nto. KVM does this for wfi since it's better to
>> allow other tasks to be scheduled while a VCPU waits for an interrupt.
>> For waits such as those where wrs.nto/sto would be used, which are
>> typically locks, it is also a good idea for KVM to be involved, as it
>> can attempt to schedule the lock holding VCPU.
>>
>> This series starts with Christoph's addition of the riscv
>> smp_cond_load_relaxed function which applies wrs.sto when available.
>> That patch has been reworked to use wrs.nto and to use the same approach
>> as Arm for the wait loop, since we can't have arbitrary C code between
>> the load-reserved and the wrs. Then, hwprobe support is added (since the
>> instructions are also usable from usermode), and finally KVM is
>> taught about wrs.nto, allowing guests to see and use the Zawrs
>> extension.
>>
>> We still don't have test results from hardware, and it's not possible to
>> prove that using Zawrs is a win when testing on QEMU, not even when
>> oversubscribing VCPUs to guests. However, it is possible to use KVM
>> selftests to force a scenario where we can prove Zawrs does its job and
>> does it well. [4] is a test which does this and, on my machine, without
>> Zawrs it takes 16 seconds to complete and with Zawrs it takes 0.25
>> seconds.
>>
>> This series is also available here [1]. In order to use QEMU for testing
>> a build with [2] is needed. In order to enable guests to use Zawrs with
>> KVM using kvmtool, the branch at [3] may be used.
>>
>> [1] https://github.com/jones-drew/linux/commits/riscv/zawrs-v3/
>> [2] https://lore.kernel.org/all/20240312152901.512001-2-ajones@ventanamicro.com/
>> [3] https://github.com/jones-drew/kvmtool/commits/riscv/zawrs/
>> [4] https://github.com/jones-drew/linux/commit/cb2beccebcece10881db842ed69bdd5715cfab5d
>>
>> Thanks,
>> drew
>>
>> v3:
>>  - Moved comment about expected termination from the DT binding text
>>    to a code comment.
>>
>> v2:
>>  - Added DT bindings patch with additional Linux specifications due
>>    to wrs.nto potentially never terminating, as suggested by Palmer
>>  - Added patch to share pause insn definition
>>  - Rework main Zawrs support patch to use Arm approach (which is
>>    also the approach that Andrea Parri suggested)
>>  - Dropped the riscv implementation of smp_cond_load_acquire().
>>    afaict, the generic implementation, which will use the riscv
>>    implementation of smp_cond_load_relaxed() is sufficient for riscv.
>>  - The rework was large enough (IMO) to drop Heiko's s-o-b and to
>>    add myself as a co-developer
>>
>>
>> Andrew Jones (5):
>>   riscv: Provide a definition for 'pause'
>>   dt-bindings: riscv: Add Zawrs ISA extension description
>>   riscv: hwprobe: export Zawrs ISA extension
>>   KVM: riscv: Support guest wrs.nto
>>   KVM: riscv: selftests: Add Zawrs extension to get-reg-list test
>>
>> Christoph M??llner (1):
>>   riscv: Add Zawrs support for spinlocks
>>
>>  Documentation/arch/riscv/hwprobe.rst          |  4 ++
>>  .../devicetree/bindings/riscv/extensions.yaml |  7 +++
>>  arch/riscv/Kconfig                            | 20 ++++---
>>  arch/riscv/Makefile                           |  3 -
>>  arch/riscv/include/asm/barrier.h              | 45 +++++++++-----
>>  arch/riscv/include/asm/cmpxchg.h              | 58 +++++++++++++++++++
>>  arch/riscv/include/asm/hwcap.h                |  1 +
>>  arch/riscv/include/asm/insn-def.h             |  4 ++
>>  arch/riscv/include/asm/kvm_host.h             |  1 +
>>  arch/riscv/include/asm/vdso/processor.h       |  8 +--
>>  arch/riscv/include/uapi/asm/hwprobe.h         |  1 +
>>  arch/riscv/include/uapi/asm/kvm.h             |  1 +
>>  arch/riscv/kernel/cpufeature.c                |  1 +
>>  arch/riscv/kernel/sys_hwprobe.c               |  1 +
>>  arch/riscv/kvm/vcpu.c                         |  1 +
>>  arch/riscv/kvm/vcpu_insn.c                    | 15 +++++
>>  arch/riscv/kvm/vcpu_onereg.c                  |  2 +
>>  .../selftests/kvm/riscv/get-reg-list.c        |  4 ++
>>  18 files changed, 146 insertions(+), 31 deletions(-)
>>
>> --
>> 2.44.0
>>

Re: [PATCH v3 0/6] riscv: Apply Zawrs when available

[Andrew Jones]

On Wed, Jun 26, 2024 at 07:34:48AM GMT, Palmer Dabbelt wrote:
> On Mon, 03 Jun 2024 05:36:42 PDT (-0700), ajones@ventanamicro.com wrote:
> > Hi Palmer,
> > 
> > I submit our concerns of wrs.nto to RVI ARC for consideration. They
> > discussed it but don't believe there's a need for concern. The
> > expectation is that there will always be enough interrupt activity
> > and that those interrupts with activity will not likely be locally
> > disabled.
> > 
> > Can we consider this series for 6.10?
> 
> Sorry I forgot about this one, Anup just brought it up in the patchwork
> meeting.  He's going to ack the KVM bits, then I'll pick it up.

Thanks Palmer!

drew


> 
> > 
> > Thanks,
> > drew
> > 
> > On Fri, Apr 26, 2024 at 12:08:20PM GMT, Andrew Jones wrote:
> > > Zawrs provides two instructions (wrs.nto and wrs.sto), where both are
> > > meant to allow the hart to enter a low-power state while waiting on a
> > > store to a memory location. The instructions also both wait an
> > > implementation-defined "short" duration (unless the implementation
> > > terminates the stall for another reason). The difference is that while
> > > wrs.sto will terminate when the duration elapses, wrs.nto, depending on
> > > configuration, will either just keep waiting or an ILL exception will be
> > > raised. Linux will use wrs.nto, so if platforms have an implementation
> > > which falls in the "just keep waiting" category (which is not expected),
> > > then it should _not_ advertise Zawrs in the hardware description.
> > > 
> > > Like wfi (and with the same {m,h}status bits to configure it), when
> > > wrs.nto is configured to raise exceptions it's expected that the higher
> > > privilege level will see the instruction was a wait instruction, do
> > > something, and then resume execution following the instruction. For
> > > example, KVM does configure exceptions for wfi (hstatus.VTW=1) and
> > > therefore also for wrs.nto. KVM does this for wfi since it's better to
> > > allow other tasks to be scheduled while a VCPU waits for an interrupt.
> > > For waits such as those where wrs.nto/sto would be used, which are
> > > typically locks, it is also a good idea for KVM to be involved, as it
> > > can attempt to schedule the lock holding VCPU.
> > > 
> > > This series starts with Christoph's addition of the riscv
> > > smp_cond_load_relaxed function which applies wrs.sto when available.
> > > That patch has been reworked to use wrs.nto and to use the same approach
> > > as Arm for the wait loop, since we can't have arbitrary C code between
> > > the load-reserved and the wrs. Then, hwprobe support is added (since the
> > > instructions are also usable from usermode), and finally KVM is
> > > taught about wrs.nto, allowing guests to see and use the Zawrs
> > > extension.
> > > 
> > > We still don't have test results from hardware, and it's not possible to
> > > prove that using Zawrs is a win when testing on QEMU, not even when
> > > oversubscribing VCPUs to guests. However, it is possible to use KVM
> > > selftests to force a scenario where we can prove Zawrs does its job and
> > > does it well. [4] is a test which does this and, on my machine, without
> > > Zawrs it takes 16 seconds to complete and with Zawrs it takes 0.25
> > > seconds.
> > > 
> > > This series is also available here [1]. In order to use QEMU for testing
> > > a build with [2] is needed. In order to enable guests to use Zawrs with
> > > KVM using kvmtool, the branch at [3] may be used.
> > > 
> > > [1] https://github.com/jones-drew/linux/commits/riscv/zawrs-v3/
> > > [2] https://lore.kernel.org/all/20240312152901.512001-2-ajones@ventanamicro.com/
> > > [3] https://github.com/jones-drew/kvmtool/commits/riscv/zawrs/
> > > [4] https://github.com/jones-drew/linux/commit/cb2beccebcece10881db842ed69bdd5715cfab5d
> > > 
> > > Thanks,
> > > drew
> > > 
> > > v3:
> > >  - Moved comment about expected termination from the DT binding text
> > >    to a code comment.
> > > 
> > > v2:
> > >  - Added DT bindings patch with additional Linux specifications due
> > >    to wrs.nto potentially never terminating, as suggested by Palmer
> > >  - Added patch to share pause insn definition
> > >  - Rework main Zawrs support patch to use Arm approach (which is
> > >    also the approach that Andrea Parri suggested)
> > >  - Dropped the riscv implementation of smp_cond_load_acquire().
> > >    afaict, the generic implementation, which will use the riscv
> > >    implementation of smp_cond_load_relaxed() is sufficient for riscv.
> > >  - The rework was large enough (IMO) to drop Heiko's s-o-b and to
> > >    add myself as a co-developer
> > > 
> > > 
> > > Andrew Jones (5):
> > >   riscv: Provide a definition for 'pause'
> > >   dt-bindings: riscv: Add Zawrs ISA extension description
> > >   riscv: hwprobe: export Zawrs ISA extension
> > >   KVM: riscv: Support guest wrs.nto
> > >   KVM: riscv: selftests: Add Zawrs extension to get-reg-list test
> > > 
> > > Christoph M??llner (1):
> > >   riscv: Add Zawrs support for spinlocks
> > > 
> > >  Documentation/arch/riscv/hwprobe.rst          |  4 ++
> > >  .../devicetree/bindings/riscv/extensions.yaml |  7 +++
> > >  arch/riscv/Kconfig                            | 20 ++++---
> > >  arch/riscv/Makefile                           |  3 -
> > >  arch/riscv/include/asm/barrier.h              | 45 +++++++++-----
> > >  arch/riscv/include/asm/cmpxchg.h              | 58 +++++++++++++++++++
> > >  arch/riscv/include/asm/hwcap.h                |  1 +
> > >  arch/riscv/include/asm/insn-def.h             |  4 ++
> > >  arch/riscv/include/asm/kvm_host.h             |  1 +
> > >  arch/riscv/include/asm/vdso/processor.h       |  8 +--
> > >  arch/riscv/include/uapi/asm/hwprobe.h         |  1 +
> > >  arch/riscv/include/uapi/asm/kvm.h             |  1 +
> > >  arch/riscv/kernel/cpufeature.c                |  1 +
> > >  arch/riscv/kernel/sys_hwprobe.c               |  1 +
> > >  arch/riscv/kvm/vcpu.c                         |  1 +
> > >  arch/riscv/kvm/vcpu_insn.c                    | 15 +++++
> > >  arch/riscv/kvm/vcpu_onereg.c                  |  2 +
> > >  .../selftests/kvm/riscv/get-reg-list.c        |  4 ++
> > >  18 files changed, 146 insertions(+), 31 deletions(-)
> > > 
> > > --
> > > 2.44.0
> > > 

Re: [PATCH v3 0/6] riscv: Apply Zawrs when available

[Anup Patel]

Hi Palmer,

On Wed, Jun 26, 2024 at 8:04 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Mon, 03 Jun 2024 05:36:42 PDT (-0700), ajones@ventanamicro.com wrote:
> > Hi Palmer,
> >
> > I submit our concerns of wrs.nto to RVI ARC for consideration. They
> > discussed it but don't believe there's a need for concern. The
> > expectation is that there will always be enough interrupt activity
> > and that those interrupts with activity will not likely be locally
> > disabled.
> >
> > Can we consider this series for 6.10?
>
> Sorry I forgot about this one, Anup just brought it up in the patchwork
> meeting.  He's going to ack the KVM bits, then I'll pick it up.

I have Acked the KVM bits so please go ahead with this series.

Regards,
Anup

>
> >
> > Thanks,
> > drew
> >
> > On Fri, Apr 26, 2024 at 12:08:20PM GMT, Andrew Jones wrote:
> >> Zawrs provides two instructions (wrs.nto and wrs.sto), where both are
> >> meant to allow the hart to enter a low-power state while waiting on a
> >> store to a memory location. The instructions also both wait an
> >> implementation-defined "short" duration (unless the implementation
> >> terminates the stall for another reason). The difference is that while
> >> wrs.sto will terminate when the duration elapses, wrs.nto, depending on
> >> configuration, will either just keep waiting or an ILL exception will be
> >> raised. Linux will use wrs.nto, so if platforms have an implementation
> >> which falls in the "just keep waiting" category (which is not expected),
> >> then it should _not_ advertise Zawrs in the hardware description.
> >>
> >> Like wfi (and with the same {m,h}status bits to configure it), when
> >> wrs.nto is configured to raise exceptions it's expected that the higher
> >> privilege level will see the instruction was a wait instruction, do
> >> something, and then resume execution following the instruction. For
> >> example, KVM does configure exceptions for wfi (hstatus.VTW=1) and
> >> therefore also for wrs.nto. KVM does this for wfi since it's better to
> >> allow other tasks to be scheduled while a VCPU waits for an interrupt.
> >> For waits such as those where wrs.nto/sto would be used, which are
> >> typically locks, it is also a good idea for KVM to be involved, as it
> >> can attempt to schedule the lock holding VCPU.
> >>
> >> This series starts with Christoph's addition of the riscv
> >> smp_cond_load_relaxed function which applies wrs.sto when available.
> >> That patch has been reworked to use wrs.nto and to use the same approach
> >> as Arm for the wait loop, since we can't have arbitrary C code between
> >> the load-reserved and the wrs. Then, hwprobe support is added (since the
> >> instructions are also usable from usermode), and finally KVM is
> >> taught about wrs.nto, allowing guests to see and use the Zawrs
> >> extension.
> >>
> >> We still don't have test results from hardware, and it's not possible to
> >> prove that using Zawrs is a win when testing on QEMU, not even when
> >> oversubscribing VCPUs to guests. However, it is possible to use KVM
> >> selftests to force a scenario where we can prove Zawrs does its job and
> >> does it well. [4] is a test which does this and, on my machine, without
> >> Zawrs it takes 16 seconds to complete and with Zawrs it takes 0.25
> >> seconds.
> >>
> >> This series is also available here [1]. In order to use QEMU for testing
> >> a build with [2] is needed. In order to enable guests to use Zawrs with
> >> KVM using kvmtool, the branch at [3] may be used.
> >>
> >> [1] https://github.com/jones-drew/linux/commits/riscv/zawrs-v3/
> >> [2] https://lore.kernel.org/all/20240312152901.512001-2-ajones@ventanamicro.com/
> >> [3] https://github.com/jones-drew/kvmtool/commits/riscv/zawrs/
> >> [4] https://github.com/jones-drew/linux/commit/cb2beccebcece10881db842ed69bdd5715cfab5d
> >>
> >> Thanks,
> >> drew
> >>
> >> v3:
> >>  - Moved comment about expected termination from the DT binding text
> >>    to a code comment.
> >>
> >> v2:
> >>  - Added DT bindings patch with additional Linux specifications due
> >>    to wrs.nto potentially never terminating, as suggested by Palmer
> >>  - Added patch to share pause insn definition
> >>  - Rework main Zawrs support patch to use Arm approach (which is
> >>    also the approach that Andrea Parri suggested)
> >>  - Dropped the riscv implementation of smp_cond_load_acquire().
> >>    afaict, the generic implementation, which will use the riscv
> >>    implementation of smp_cond_load_relaxed() is sufficient for riscv.
> >>  - The rework was large enough (IMO) to drop Heiko's s-o-b and to
> >>    add myself as a co-developer
> >>
> >>
> >> Andrew Jones (5):
> >>   riscv: Provide a definition for 'pause'
> >>   dt-bindings: riscv: Add Zawrs ISA extension description
> >>   riscv: hwprobe: export Zawrs ISA extension
> >>   KVM: riscv: Support guest wrs.nto
> >>   KVM: riscv: selftests: Add Zawrs extension to get-reg-list test
> >>
> >> Christoph M??llner (1):
> >>   riscv: Add Zawrs support for spinlocks
> >>
> >>  Documentation/arch/riscv/hwprobe.rst          |  4 ++
> >>  .../devicetree/bindings/riscv/extensions.yaml |  7 +++
> >>  arch/riscv/Kconfig                            | 20 ++++---
> >>  arch/riscv/Makefile                           |  3 -
> >>  arch/riscv/include/asm/barrier.h              | 45 +++++++++-----
> >>  arch/riscv/include/asm/cmpxchg.h              | 58 +++++++++++++++++++
> >>  arch/riscv/include/asm/hwcap.h                |  1 +
> >>  arch/riscv/include/asm/insn-def.h             |  4 ++
> >>  arch/riscv/include/asm/kvm_host.h             |  1 +
> >>  arch/riscv/include/asm/vdso/processor.h       |  8 +--
> >>  arch/riscv/include/uapi/asm/hwprobe.h         |  1 +
> >>  arch/riscv/include/uapi/asm/kvm.h             |  1 +
> >>  arch/riscv/kernel/cpufeature.c                |  1 +
> >>  arch/riscv/kernel/sys_hwprobe.c               |  1 +
> >>  arch/riscv/kvm/vcpu.c                         |  1 +
> >>  arch/riscv/kvm/vcpu_insn.c                    | 15 +++++
> >>  arch/riscv/kvm/vcpu_onereg.c                  |  2 +
> >>  .../selftests/kvm/riscv/get-reg-list.c        |  4 ++
> >>  18 files changed, 146 insertions(+), 31 deletions(-)
> >>
> >> --
> >> 2.44.0
> >>

Re: [PATCH v3 0/6] riscv: Apply Zawrs when available

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Fri, 26 Apr 2024 12:08:20 +0200 you wrote:
> Zawrs provides two instructions (wrs.nto and wrs.sto), where both are
> meant to allow the hart to enter a low-power state while waiting on a
> store to a memory location. The instructions also both wait an
> implementation-defined "short" duration (unless the implementation
> terminates the stall for another reason). The difference is that while
> wrs.sto will terminate when the duration elapses, wrs.nto, depending on
> configuration, will either just keep waiting or an ILL exception will be
> raised. Linux will use wrs.nto, so if platforms have an implementation
> which falls in the "just keep waiting" category (which is not expected),
> then it should _not_ advertise Zawrs in the hardware description.
> 
> [...]

Here is the summary with links:
  - [v3,1/6] riscv: Provide a definition for 'pause'
    https://git.kernel.org/riscv/c/6da111574baf
  - [v3,2/6] dt-bindings: riscv: Add Zawrs ISA extension description
    https://git.kernel.org/riscv/c/6d5852811600
  - [v3,3/6] riscv: Add Zawrs support for spinlocks
    https://git.kernel.org/riscv/c/b8ddb0df30f9
  - [v3,4/6] riscv: hwprobe: export Zawrs ISA extension
    https://git.kernel.org/riscv/c/244c18fbf64a
  - [v3,5/6] KVM: riscv: Support guest wrs.nto
    https://git.kernel.org/riscv/c/86d6a86e59e3
  - [v3,6/6] KVM: riscv: selftests: Add Zawrs extension to get-reg-list test
    https://git.kernel.org/riscv/c/f2c43c61160e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html