linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation
@ 2025-04-11  2:46 Xi Ruoyao
  2025-04-11  8:04 ` Thomas Weißschuh
  0 siblings, 1 reply; 16+ messages in thread
From: Xi Ruoyao @ 2025-04-11  2:46 UTC (permalink / raw)
  To: Jason A. Donenfeld, Thomas Weißschuh, Paul Walmsley,
	Palmer Dabbelt, Guo Ren
  Cc: linux-riscv, linux-kernel, Xi Ruoyao

Hook up the generic vDSO implementation to the generic vDSO getrandom
implementation by providing the required __arch_chacha20_blocks_nostack
and getrandom_syscall implementations. Also wire up the selftests.

The benchmark result:

	vdso: 25000000 times in 2.466341333 seconds
	libc: 25000000 times in 41.447720005 seconds
	syscall: 25000000 times in 41.043926672 seconds

	vdso: 25000000 x 256 times in 162.286219353 seconds
	libc: 25000000 x 256 times in 2953.855018685 seconds
	syscall: 25000000 x 256 times in 2796.268546000 seconds

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---

[v1]->v2:
- Fix the commit message.
- Only build the vDSO getrandom code if CONFIG_VDSO_GETRANDOM, to
  unbreak RV32 build.
- Likewise, only enable the selftest if __riscv_xlen == 64.

[v1]: https://lore.kernel.org/all/20250224122541.65045-1-xry111@xry111.site/

 arch/riscv/Kconfig                            |   1 +
 arch/riscv/include/asm/vdso/getrandom.h       |  30 +++
 arch/riscv/kernel/vdso/Makefile               |  12 +
 arch/riscv/kernel/vdso/getrandom.c            |  10 +
 arch/riscv/kernel/vdso/vdso.lds.S             |   1 +
 arch/riscv/kernel/vdso/vgetrandom-chacha.S    | 244 ++++++++++++++++++
 .../selftests/vDSO/vgetrandom-chacha.S        |   2 +
 7 files changed, 300 insertions(+)
 create mode 100644 arch/riscv/include/asm/vdso/getrandom.h
 create mode 100644 arch/riscv/kernel/vdso/getrandom.c
 create mode 100644 arch/riscv/kernel/vdso/vgetrandom-chacha.S

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index bbec87b79309..fbca724302ab 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -218,6 +218,7 @@ config RISCV
 	select THREAD_INFO_IN_TASK
 	select TRACE_IRQFLAGS_SUPPORT
 	select UACCESS_MEMCPY if !MMU
+	select VDSO_GETRANDOM if HAVE_GENERIC_VDSO
 	select USER_STACKTRACE_SUPPORT
 	select ZONE_DMA32 if 64BIT
 
diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
new file mode 100644
index 000000000000..8dc92441702a
--- /dev/null
+++ b/arch/riscv/include/asm/vdso/getrandom.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 Xi Ruoyao <xry111@xry111.site>. All Rights Reserved.
+ */
+#ifndef __ASM_VDSO_GETRANDOM_H
+#define __ASM_VDSO_GETRANDOM_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/unistd.h>
+
+static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, unsigned int _flags)
+{
+	register long ret asm("a0");
+	register long nr asm("a7") = __NR_getrandom;
+	register void *buffer asm("a0") = _buffer;
+	register size_t len asm("a1") = _len;
+	register unsigned int flags asm("a2") = _flags;
+
+	asm volatile ("ecall\n"
+		      : "+r" (ret)
+		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
+		      : "memory");
+
+	return ret;
+}
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_GETRANDOM_H */
diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
index 4a5d131506fc..8d12f5646eb5 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -13,9 +13,17 @@ vdso-syms += flush_icache
 vdso-syms += hwprobe
 vdso-syms += sys_hwprobe
 
+ifdef CONFIG_VDSO_GETRANDOM
+vdso-syms += getrandom
+endif
+
 # Files to link into the vdso
 obj-vdso = $(patsubst %, %.o, $(vdso-syms)) note.o
 
+ifdef CONFIG_VDSO_GETRANDOM
+obj-vdso += vgetrandom-chacha.o
+endif
+
 ccflags-y := -fno-stack-protector
 ccflags-y += -DDISABLE_BRANCH_PROFILING
 ccflags-y += -fno-builtin
@@ -24,6 +32,10 @@ ifneq ($(c-gettimeofday-y),)
   CFLAGS_vgettimeofday.o += -fPIC -include $(c-gettimeofday-y)
 endif
 
+ifneq ($(c-getrandom-y),)
+  CFLAGS_getrandom.o += -fPIC -include $(c-getrandom-y)
+endif
+
 CFLAGS_hwprobe.o += -fPIC
 
 # Build rules
diff --git a/arch/riscv/kernel/vdso/getrandom.c b/arch/riscv/kernel/vdso/getrandom.c
new file mode 100644
index 000000000000..f21922e8cebd
--- /dev/null
+++ b/arch/riscv/kernel/vdso/getrandom.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Xi Ruoyao <xry111@xry111.site>. All Rights Reserved.
+ */
+#include <linux/types.h>
+
+ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len)
+{
+	return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len);
+}
diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
index 8e86965a8aae..abc69cda0445 100644
--- a/arch/riscv/kernel/vdso/vdso.lds.S
+++ b/arch/riscv/kernel/vdso/vdso.lds.S
@@ -80,6 +80,7 @@ VERSION
 #ifndef COMPAT_VDSO
 		__vdso_riscv_hwprobe;
 #endif
+		__vdso_getrandom;
 	local: *;
 	};
 }
diff --git a/arch/riscv/kernel/vdso/vgetrandom-chacha.S b/arch/riscv/kernel/vdso/vgetrandom-chacha.S
new file mode 100644
index 000000000000..d793cadc78a6
--- /dev/null
+++ b/arch/riscv/kernel/vdso/vgetrandom-chacha.S
@@ -0,0 +1,244 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2025 Xi Ruoyao <xry111@xry111.site>. All Rights Reserved.
+ *
+ * Based on arch/loongarch/vdso/vgetrandom-chacha.S.
+ */
+
+#include <asm/asm.h>
+#include <linux/linkage.h>
+
+.text
+
+.macro	ROTRI	rd rs imm
+	slliw	t0, \rs, 32 - \imm
+	srliw	\rd, \rs, \imm
+	or	\rd, \rd, t0
+.endm
+
+.macro	OP_4REG	op d0 d1 d2 d3 s0 s1 s2 s3
+	\op	\d0, \d0, \s0
+	\op	\d1, \d1, \s1
+	\op	\d2, \d2, \s2
+	\op	\d3, \d3, \s3
+.endm
+
+/*
+ *	a0: output bytes
+ * 	a1: 32-byte key input
+ *	a2: 8-byte counter input/output
+ *	a3: number of 64-byte blocks to write to output
+ */
+SYM_FUNC_START(__arch_chacha20_blocks_nostack)
+
+#define output		a0
+#define key		a1
+#define counter		a2
+#define nblocks		a3
+#define i		a4
+#define state0		s0
+#define state1		s1
+#define state2		s2
+#define state3		s3
+#define state4		s4
+#define state5		s5
+#define state6		s6
+#define state7		s7
+#define state8		s8
+#define state9		s9
+#define state10		s10
+#define state11		s11
+#define state12		a5
+#define state13		a6
+#define state14		a7
+#define state15		t1
+#define cnt		t2
+#define copy0		t3
+#define copy1		t4
+#define copy2		t5
+#define copy3		t6
+
+/* Packs to be used with OP_4REG */
+#define line0		state0, state1, state2, state3
+#define line1		state4, state5, state6, state7
+#define line2		state8, state9, state10, state11
+#define line3		state12, state13, state14, state15
+
+#define line1_perm	state5, state6, state7, state4
+#define line2_perm	state10, state11, state8, state9
+#define line3_perm	state15, state12, state13, state14
+
+#define copy		copy0, copy1, copy2, copy3
+
+#define _16		16, 16, 16, 16
+#define _20		20, 20, 20, 20
+#define _24		24, 24, 24, 24
+#define _25		25, 25, 25, 25
+
+	addi		sp, sp, -12*SZREG
+	REG_S		s0,         (sp)
+	REG_S		s1,    SZREG(sp)
+	REG_S		s2,  2*SZREG(sp)
+	REG_S		s3,  3*SZREG(sp)
+	REG_S		s4,  4*SZREG(sp)
+	REG_S		s5,  5*SZREG(sp)
+	REG_S		s6,  6*SZREG(sp)
+	REG_S		s7,  7*SZREG(sp)
+	REG_S		s8,  8*SZREG(sp)
+	REG_S		s9,  9*SZREG(sp)
+	REG_S		s10, 10*SZREG(sp)
+	REG_S		s11, 11*SZREG(sp)
+
+	ld		cnt, (counter)
+
+	li		copy0, 0x61707865
+	li		copy1, 0x3320646e
+	li		copy2, 0x79622d32
+	li		copy3, 0x6b206574
+
+.Lblock:
+	/* state[0,1,2,3] = "expand 32-byte k" */
+	mv		state0, copy0
+	mv		state1, copy1
+	mv		state2, copy2
+	mv		state3, copy3
+
+	/* state[4,5,..,11] = key */
+	lw		state4,   (key)
+	lw		state5,  4(key)
+	lw		state6,  8(key)
+	lw		state7,  12(key)
+	lw		state8,  16(key)
+	lw		state9,  20(key)
+	lw		state10, 24(key)
+	lw		state11, 28(key)
+
+	/* state[12,13] = counter */
+	mv		state12, cnt
+	srli		state13, cnt, 32
+
+	/* state[14,15] = 0 */
+	mv		state14, zero
+	mv		state15, zero
+
+	li		i, 10
+.Lpermute:
+	/* odd round */
+	OP_4REG	addw	line0, line1
+	OP_4REG	xor	line3, line0
+	OP_4REG	ROTRI	line3, _16
+
+	OP_4REG	addw	line2, line3
+	OP_4REG	xor	line1, line2
+	OP_4REG	ROTRI	line1, _20
+
+	OP_4REG	addw	line0, line1
+	OP_4REG	xor	line3, line0
+	OP_4REG	ROTRI	line3, _24
+
+	OP_4REG	addw	line2, line3
+	OP_4REG	xor	line1, line2
+	OP_4REG	ROTRI	line1, _25
+
+	/* even round */
+	OP_4REG	addw	line0, line1_perm
+	OP_4REG	xor	line3_perm, line0
+	OP_4REG	ROTRI	line3_perm, _16
+
+	OP_4REG	addw	line2_perm, line3_perm
+	OP_4REG	xor	line1_perm, line2_perm
+	OP_4REG	ROTRI	line1_perm, _20
+
+	OP_4REG	addw	line0, line1_perm
+	OP_4REG	xor	line3_perm, line0
+	OP_4REG	ROTRI	line3_perm, _24
+
+	OP_4REG	addw	line2_perm, line3_perm
+	OP_4REG	xor	line1_perm, line2_perm
+	OP_4REG	ROTRI	line1_perm, _25
+
+	addi		i, i, -1
+	bnez		i, .Lpermute
+
+	/* output[0,1,2,3] = copy[0,1,2,3] + state[0,1,2,3] */
+	OP_4REG	addw	line0, copy
+	sw		state0,   (output)
+	sw		state1,  4(output)
+	sw		state2,  8(output)
+	sw		state3, 12(output)
+
+	/* from now on state[0,1,2,3] are scratch registers  */
+
+	/* state[0,1,2,3] = lo(key) */
+	lw		state0,   (key)
+	lw		state1,  4(key)
+	lw		state2,  8(key)
+	lw		state3, 12(key)
+
+	/* output[4,5,6,7] = state[0,1,2,3] + state[4,5,6,7] */
+	OP_4REG	addw	line1, line0
+	sw		state4, 16(output)
+	sw		state5, 20(output)
+	sw		state6, 24(output)
+	sw		state7, 28(output)
+
+	/* state[0,1,2,3] = hi(key) */
+	lw		state0, 16(key)
+	lw		state1, 20(key)
+	lw		state2, 24(key)
+	lw		state3, 28(key)
+
+	/* output[8,9,10,11] = tmp[0,1,2,3] + state[8,9,10,11] */
+	OP_4REG	addw	line2, line0
+	sw		state8,  32(output)
+	sw		state9,  36(output)
+	sw		state10, 40(output)
+	sw		state11, 44(output)
+
+	/* output[12,13,14,15] = state[12,13,14,15] + [cnt_lo, cnt_hi, 0, 0] */
+	addw		state12, state12, cnt
+	srli		state0, cnt, 32
+	addw		state13, state13, state0
+	sw		state12, 48(output)
+	sw		state13, 52(output)
+	sw		state14, 56(output)
+	sw		state15, 60(output)
+
+	/* ++counter */
+	addi		cnt, cnt, 1
+
+	/* output += 64 */
+	addi		output, output, 64
+	/* --nblocks */
+	addi		nblocks, nblocks, -1
+	bnez		nblocks, .Lblock
+
+	/* counter = [cnt_lo, cnt_hi] */
+	sd		cnt, (counter)
+
+	/* Zero out the potentially sensitive regs, in case nothing uses these
+	 * again.  As at now copy[0,1,2,3] just contains "expand 32-byte k" and
+	 * state[0,...,11] are s0-s11 those we'll restore in the epilogue, we
+	 * only need to zero state[12,...,15].
+	 */
+	mv		state12, zero
+	mv		state13, zero
+	mv		state14, zero
+	mv		state15, zero
+
+	REG_L		s0,         (sp)
+	REG_L		s1,    SZREG(sp)
+	REG_L		s2,  2*SZREG(sp)
+	REG_L		s3,  3*SZREG(sp)
+	REG_L		s4,  4*SZREG(sp)
+	REG_L		s5,  5*SZREG(sp)
+	REG_L		s6,  6*SZREG(sp)
+	REG_L		s7,  7*SZREG(sp)
+	REG_L		s8,  8*SZREG(sp)
+	REG_L		s9,  9*SZREG(sp)
+	REG_L		s10, 10*SZREG(sp)
+	REG_L		s11, 11*SZREG(sp)
+	addi		sp, sp, 12*SZREG
+
+	ret
+SYM_FUNC_END(__arch_chacha20_blocks_nostack)
diff --git a/tools/testing/selftests/vDSO/vgetrandom-chacha.S b/tools/testing/selftests/vDSO/vgetrandom-chacha.S
index d6e09af7c0a9..a4a82e1c28a9 100644
--- a/tools/testing/selftests/vDSO/vgetrandom-chacha.S
+++ b/tools/testing/selftests/vDSO/vgetrandom-chacha.S
@@ -11,6 +11,8 @@
 #include "../../../../arch/loongarch/vdso/vgetrandom-chacha.S"
 #elif defined(__powerpc__) || defined(__powerpc64__)
 #include "../../../../arch/powerpc/kernel/vdso/vgetrandom-chacha.S"
+#elif defined(__riscv) && __riscv_xlen == 64
+#include "../../../../arch/riscv/kernel/vdso/vgetrandom-chacha.S"
 #elif defined(__s390x__)
 #include "../../../../arch/s390/kernel/vdso64/vgetrandom-chacha.S"
 #elif defined(__x86_64__)
-- 
2.49.0


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

* Re: [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation
  2025-04-11  2:46 [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation Xi Ruoyao
@ 2025-04-11  8:04 ` Thomas Weißschuh
  2025-05-23  8:01   ` Alexandre Ghiti
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Weißschuh @ 2025-04-11  8:04 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: Jason A. Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

On Fri, Apr 11, 2025 at 10:46:00AM +0800, Xi Ruoyao wrote:
> Hook up the generic vDSO implementation to the generic vDSO getrandom
> implementation by providing the required __arch_chacha20_blocks_nostack
> and getrandom_syscall implementations. Also wire up the selftests.
> 
> The benchmark result:
> 
> 	vdso: 25000000 times in 2.466341333 seconds
> 	libc: 25000000 times in 41.447720005 seconds
> 	syscall: 25000000 times in 41.043926672 seconds
> 
> 	vdso: 25000000 x 256 times in 162.286219353 seconds
> 	libc: 25000000 x 256 times in 2953.855018685 seconds
> 	syscall: 25000000 x 256 times in 2796.268546000 seconds
> 
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> 
> [v1]->v2:
> - Fix the commit message.
> - Only build the vDSO getrandom code if CONFIG_VDSO_GETRANDOM, to
>   unbreak RV32 build.
> - Likewise, only enable the selftest if __riscv_xlen == 64.
> 
> [v1]: https://lore.kernel.org/all/20250224122541.65045-1-xry111@xry111.site/
> 
>  arch/riscv/Kconfig                            |   1 +
>  arch/riscv/include/asm/vdso/getrandom.h       |  30 +++
>  arch/riscv/kernel/vdso/Makefile               |  12 +
>  arch/riscv/kernel/vdso/getrandom.c            |  10 +
>  arch/riscv/kernel/vdso/vdso.lds.S             |   1 +
>  arch/riscv/kernel/vdso/vgetrandom-chacha.S    | 244 ++++++++++++++++++
>  .../selftests/vDSO/vgetrandom-chacha.S        |   2 +
>  7 files changed, 300 insertions(+)
>  create mode 100644 arch/riscv/include/asm/vdso/getrandom.h
>  create mode 100644 arch/riscv/kernel/vdso/getrandom.c
>  create mode 100644 arch/riscv/kernel/vdso/vgetrandom-chacha.S

<snip>

> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> index 8e86965a8aae..abc69cda0445 100644
> --- a/arch/riscv/kernel/vdso/vdso.lds.S
> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> @@ -80,6 +80,7 @@ VERSION
>  #ifndef COMPAT_VDSO
>  		__vdso_riscv_hwprobe;
>  #endif
> +		__vdso_getrandom;

For consistency this could be gated behind CONFIG_VDSO_GETRANDOM.

>  	local: *;
>  	};
>  }
> diff --git a/arch/riscv/kernel/vdso/vgetrandom-chacha.S b/arch/riscv/kernel/vdso/vgetrandom-chacha.S
> new file mode 100644
> index 000000000000..d793cadc78a6
> --- /dev/null
> +++ b/arch/riscv/kernel/vdso/vgetrandom-chacha.S
> @@ -0,0 +1,244 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2025 Xi Ruoyao <xry111@xry111.site>. All Rights Reserved.
> + *
> + * Based on arch/loongarch/vdso/vgetrandom-chacha.S.
> + */
> +
> +#include <asm/asm.h>
> +#include <linux/linkage.h>
> +
> +.text
> +
> +.macro	ROTRI	rd rs imm
> +	slliw	t0, \rs, 32 - \imm
> +	srliw	\rd, \rs, \imm
> +	or	\rd, \rd, t0
> +.endm
> +
> +.macro	OP_4REG	op d0 d1 d2 d3 s0 s1 s2 s3
> +	\op	\d0, \d0, \s0
> +	\op	\d1, \d1, \s1
> +	\op	\d2, \d2, \s2
> +	\op	\d3, \d3, \s3
> +.endm
> +
> +/*
> + *	a0: output bytes
> + * 	a1: 32-byte key input
> + *	a2: 8-byte counter input/output
> + *	a3: number of 64-byte blocks to write to output
> + */
> +SYM_FUNC_START(__arch_chacha20_blocks_nostack)
> +
> +#define output		a0
> +#define key		a1
> +#define counter		a2
> +#define nblocks		a3
> +#define i		a4
> +#define state0		s0
> +#define state1		s1
> +#define state2		s2
> +#define state3		s3
> +#define state4		s4
> +#define state5		s5
> +#define state6		s6
> +#define state7		s7
> +#define state8		s8
> +#define state9		s9
> +#define state10		s10
> +#define state11		s11
> +#define state12		a5
> +#define state13		a6
> +#define state14		a7
> +#define state15		t1
> +#define cnt		t2
> +#define copy0		t3
> +#define copy1		t4
> +#define copy2		t5
> +#define copy3		t6
> +
> +/* Packs to be used with OP_4REG */
> +#define line0		state0, state1, state2, state3
> +#define line1		state4, state5, state6, state7
> +#define line2		state8, state9, state10, state11
> +#define line3		state12, state13, state14, state15
> +
> +#define line1_perm	state5, state6, state7, state4
> +#define line2_perm	state10, state11, state8, state9
> +#define line3_perm	state15, state12, state13, state14
> +
> +#define copy		copy0, copy1, copy2, copy3
> +
> +#define _16		16, 16, 16, 16
> +#define _20		20, 20, 20, 20
> +#define _24		24, 24, 24, 24
> +#define _25		25, 25, 25, 25
> +
> +	addi		sp, sp, -12*SZREG
> +	REG_S		s0,         (sp)
> +	REG_S		s1,    SZREG(sp)
> +	REG_S		s2,  2*SZREG(sp)
> +	REG_S		s3,  3*SZREG(sp)
> +	REG_S		s4,  4*SZREG(sp)
> +	REG_S		s5,  5*SZREG(sp)
> +	REG_S		s6,  6*SZREG(sp)
> +	REG_S		s7,  7*SZREG(sp)
> +	REG_S		s8,  8*SZREG(sp)
> +	REG_S		s9,  9*SZREG(sp)
> +	REG_S		s10, 10*SZREG(sp)
> +	REG_S		s11, 11*SZREG(sp)

This should have the same comment as the loongarch implementation that it is
fine to store to the stack here. Contrary to the general claim of the
documentation for __arch_chacha20_blocks_nostack() in include/linux/getrandom.h.

<snip>

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

* Re: [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation
  2025-04-11  8:04 ` Thomas Weißschuh
@ 2025-05-23  8:01   ` Alexandre Ghiti
  2025-05-23  8:02     ` Xi Ruoyao
  0 siblings, 1 reply; 16+ messages in thread
From: Alexandre Ghiti @ 2025-05-23  8:01 UTC (permalink / raw)
  To: Thomas Weißschuh, Xi Ruoyao, Nathan Chancellor
  Cc: Jason A. Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

Hi Xi,

On 4/11/25 10:04, Thomas Weißschuh wrote:
> On Fri, Apr 11, 2025 at 10:46:00AM +0800, Xi Ruoyao wrote:
>> Hook up the generic vDSO implementation to the generic vDSO getrandom
>> implementation by providing the required __arch_chacha20_blocks_nostack
>> and getrandom_syscall implementations. Also wire up the selftests.
>>
>> The benchmark result:
>>
>> 	vdso: 25000000 times in 2.466341333 seconds
>> 	libc: 25000000 times in 41.447720005 seconds
>> 	syscall: 25000000 times in 41.043926672 seconds
>>
>> 	vdso: 25000000 x 256 times in 162.286219353 seconds
>> 	libc: 25000000 x 256 times in 2953.855018685 seconds
>> 	syscall: 25000000 x 256 times in 2796.268546000 seconds
>>
>> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
>> ---
>>
>> [v1]->v2:
>> - Fix the commit message.
>> - Only build the vDSO getrandom code if CONFIG_VDSO_GETRANDOM, to
>>    unbreak RV32 build.
>> - Likewise, only enable the selftest if __riscv_xlen == 64.
>>
>> [v1]: https://lore.kernel.org/all/20250224122541.65045-1-xry111@xry111.site/
>>
>>   arch/riscv/Kconfig                            |   1 +
>>   arch/riscv/include/asm/vdso/getrandom.h       |  30 +++
>>   arch/riscv/kernel/vdso/Makefile               |  12 +
>>   arch/riscv/kernel/vdso/getrandom.c            |  10 +
>>   arch/riscv/kernel/vdso/vdso.lds.S             |   1 +
>>   arch/riscv/kernel/vdso/vgetrandom-chacha.S    | 244 ++++++++++++++++++
>>   .../selftests/vDSO/vgetrandom-chacha.S        |   2 +
>>   7 files changed, 300 insertions(+)
>>   create mode 100644 arch/riscv/include/asm/vdso/getrandom.h
>>   create mode 100644 arch/riscv/kernel/vdso/getrandom.c
>>   create mode 100644 arch/riscv/kernel/vdso/vgetrandom-chacha.S
> <snip>
>
>> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
>> index 8e86965a8aae..abc69cda0445 100644
>> --- a/arch/riscv/kernel/vdso/vdso.lds.S
>> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
>> @@ -80,6 +80,7 @@ VERSION
>>   #ifndef COMPAT_VDSO
>>   		__vdso_riscv_hwprobe;
>>   #endif
>> +		__vdso_getrandom;
> For consistency this could be gated behind CONFIG_VDSO_GETRANDOM.


Nathan sent a fix for this here:

https://lore.kernel.org/all/20250423-riscv-fix-compat_vdso-lld-v2-1-b7bbbc244501@kernel.org/


>
>>   	local: *;
>>   	};
>>   }
>> diff --git a/arch/riscv/kernel/vdso/vgetrandom-chacha.S b/arch/riscv/kernel/vdso/vgetrandom-chacha.S
>> new file mode 100644
>> index 000000000000..d793cadc78a6
>> --- /dev/null
>> +++ b/arch/riscv/kernel/vdso/vgetrandom-chacha.S
>> @@ -0,0 +1,244 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (C) 2025 Xi Ruoyao <xry111@xry111.site>. All Rights Reserved.
>> + *
>> + * Based on arch/loongarch/vdso/vgetrandom-chacha.S.
>> + */
>> +
>> +#include <asm/asm.h>
>> +#include <linux/linkage.h>
>> +
>> +.text
>> +
>> +.macro	ROTRI	rd rs imm
>> +	slliw	t0, \rs, 32 - \imm
>> +	srliw	\rd, \rs, \imm
>> +	or	\rd, \rd, t0
>> +.endm
>> +
>> +.macro	OP_4REG	op d0 d1 d2 d3 s0 s1 s2 s3
>> +	\op	\d0, \d0, \s0
>> +	\op	\d1, \d1, \s1
>> +	\op	\d2, \d2, \s2
>> +	\op	\d3, \d3, \s3
>> +.endm
>> +
>> +/*
>> + *	a0: output bytes
>> + * 	a1: 32-byte key input
>> + *	a2: 8-byte counter input/output
>> + *	a3: number of 64-byte blocks to write to output
>> + */
>> +SYM_FUNC_START(__arch_chacha20_blocks_nostack)
>> +
>> +#define output		a0
>> +#define key		a1
>> +#define counter		a2
>> +#define nblocks		a3
>> +#define i		a4
>> +#define state0		s0
>> +#define state1		s1
>> +#define state2		s2
>> +#define state3		s3
>> +#define state4		s4
>> +#define state5		s5
>> +#define state6		s6
>> +#define state7		s7
>> +#define state8		s8
>> +#define state9		s9
>> +#define state10		s10
>> +#define state11		s11
>> +#define state12		a5
>> +#define state13		a6
>> +#define state14		a7
>> +#define state15		t1
>> +#define cnt		t2
>> +#define copy0		t3
>> +#define copy1		t4
>> +#define copy2		t5
>> +#define copy3		t6
>> +
>> +/* Packs to be used with OP_4REG */
>> +#define line0		state0, state1, state2, state3
>> +#define line1		state4, state5, state6, state7
>> +#define line2		state8, state9, state10, state11
>> +#define line3		state12, state13, state14, state15
>> +
>> +#define line1_perm	state5, state6, state7, state4
>> +#define line2_perm	state10, state11, state8, state9
>> +#define line3_perm	state15, state12, state13, state14
>> +
>> +#define copy		copy0, copy1, copy2, copy3
>> +
>> +#define _16		16, 16, 16, 16
>> +#define _20		20, 20, 20, 20
>> +#define _24		24, 24, 24, 24
>> +#define _25		25, 25, 25, 25
>> +
>> +	addi		sp, sp, -12*SZREG
>> +	REG_S		s0,         (sp)
>> +	REG_S		s1,    SZREG(sp)
>> +	REG_S		s2,  2*SZREG(sp)
>> +	REG_S		s3,  3*SZREG(sp)
>> +	REG_S		s4,  4*SZREG(sp)
>> +	REG_S		s5,  5*SZREG(sp)
>> +	REG_S		s6,  6*SZREG(sp)
>> +	REG_S		s7,  7*SZREG(sp)
>> +	REG_S		s8,  8*SZREG(sp)
>> +	REG_S		s9,  9*SZREG(sp)
>> +	REG_S		s10, 10*SZREG(sp)
>> +	REG_S		s11, 11*SZREG(sp)
> This should have the same comment as the loongarch implementation that it is
> fine to store to the stack here. Contrary to the general claim of the
> documentation for __arch_chacha20_blocks_nostack() in include/linux/getrandom.h.


I agree, let's add the same comment.

In addition, I had to fix the presence of dynamic relocations (_mcount) 
with the following diff:

diff --git a/arch/riscv/kernel/vdso/Makefile 
b/arch/riscv/kernel/vdso/Makefile
index 7575ef088adc5..dca888852d93b 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -50,6 +50,7 @@ endif

  # Disable -pg to prevent insert call site
  CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
+CFLAGS_REMOVE_getrandom.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
  CFLAGS_REMOVE_hwprobe.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)

  # Force dependency

I squashed all those changes into your patch before merging it, so no 
need to resend a new version.

vdso_test_chacha passes on my end so:

Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


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

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

* Re: [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation
  2025-05-23  8:01   ` Alexandre Ghiti
@ 2025-05-23  8:02     ` Xi Ruoyao
  2025-05-23 10:06       ` Alexandre Ghiti
  0 siblings, 1 reply; 16+ messages in thread
From: Xi Ruoyao @ 2025-05-23  8:02 UTC (permalink / raw)
  To: Alexandre Ghiti, Thomas Weißschuh, Nathan Chancellor
  Cc: Jason A. Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

On Fri, 2025-05-23 at 10:01 +0200, Alexandre Ghiti wrote:
> Hi Xi,
> 
> On 4/11/25 10:04, Thomas Weißschuh wrote:
> > On Fri, Apr 11, 2025 at 10:46:00AM +0800, Xi Ruoyao wrote:
> > > Hook up the generic vDSO implementation to the generic vDSO
> > > getrandom
> > > implementation by providing the required
> > > __arch_chacha20_blocks_nostack
> > > and getrandom_syscall implementations. Also wire up the selftests.
> > > 
> > > The benchmark result:
> > > 
> > > 	vdso: 25000000 times in 2.466341333 seconds
> > > 	libc: 25000000 times in 41.447720005 seconds
> > > 	syscall: 25000000 times in 41.043926672 seconds
> > > 
> > > 	vdso: 25000000 x 256 times in 162.286219353 seconds
> > > 	libc: 25000000 x 256 times in 2953.855018685 seconds
> > > 	syscall: 25000000 x 256 times in 2796.268546000 seconds
> > > 
> > > Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> > > ---
> > > 
> > > [v1]->v2:
> > > - Fix the commit message.
> > > - Only build the vDSO getrandom code if CONFIG_VDSO_GETRANDOM, to
> > >    unbreak RV32 build.
> > > - Likewise, only enable the selftest if __riscv_xlen == 64.
> > > 
> > > [v1]:
> > > https://lore.kernel.org/all/20250224122541.65045-1-xry111@xry111.site/
> > > 
> > >   arch/riscv/Kconfig                            |   1 +
> > >   arch/riscv/include/asm/vdso/getrandom.h       |  30 +++
> > >   arch/riscv/kernel/vdso/Makefile               |  12 +
> > >   arch/riscv/kernel/vdso/getrandom.c            |  10 +
> > >   arch/riscv/kernel/vdso/vdso.lds.S             |   1 +
> > >   arch/riscv/kernel/vdso/vgetrandom-chacha.S    | 244
> > > ++++++++++++++++++
> > >   .../selftests/vDSO/vgetrandom-chacha.S        |   2 +
> > >   7 files changed, 300 insertions(+)
> > >   create mode 100644 arch/riscv/include/asm/vdso/getrandom.h
> > >   create mode 100644 arch/riscv/kernel/vdso/getrandom.c
> > >   create mode 100644 arch/riscv/kernel/vdso/vgetrandom-chacha.S
> > <snip>
> > 
> > > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S
> > > b/arch/riscv/kernel/vdso/vdso.lds.S
> > > index 8e86965a8aae..abc69cda0445 100644
> > > --- a/arch/riscv/kernel/vdso/vdso.lds.S
> > > +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> > > @@ -80,6 +80,7 @@ VERSION
> > >   #ifndef COMPAT_VDSO
> > >   		__vdso_riscv_hwprobe;
> > >   #endif
> > > +		__vdso_getrandom;
> > For consistency this could be gated behind CONFIG_VDSO_GETRANDOM.
> 
> 
> Nathan sent a fix for this here:
> 
> https://lore.kernel.org/all/20250423-riscv-fix-compat_vdso-lld-v2-1-b7bbbc244501@kernel.org/

I've given it an R-b.  Do you prefer me to squash the patches and keep
the SoB of both I and Nathan?

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

* Re: [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation
  2025-05-23  8:02     ` Xi Ruoyao
@ 2025-05-23 10:06       ` Alexandre Ghiti
  2025-06-03 12:48         ` Xi Ruoyao
  0 siblings, 1 reply; 16+ messages in thread
From: Alexandre Ghiti @ 2025-05-23 10:06 UTC (permalink / raw)
  To: Xi Ruoyao, Thomas Weißschuh, Nathan Chancellor
  Cc: Jason A. Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

On 5/23/25 10:02, Xi Ruoyao wrote:
> On Fri, 2025-05-23 at 10:01 +0200, Alexandre Ghiti wrote:
>> Hi Xi,
>>
>> On 4/11/25 10:04, Thomas Weißschuh wrote:
>>> On Fri, Apr 11, 2025 at 10:46:00AM +0800, Xi Ruoyao wrote:
>>>> Hook up the generic vDSO implementation to the generic vDSO
>>>> getrandom
>>>> implementation by providing the required
>>>> __arch_chacha20_blocks_nostack
>>>> and getrandom_syscall implementations. Also wire up the selftests.
>>>>
>>>> The benchmark result:
>>>>
>>>> 	vdso: 25000000 times in 2.466341333 seconds
>>>> 	libc: 25000000 times in 41.447720005 seconds
>>>> 	syscall: 25000000 times in 41.043926672 seconds
>>>>
>>>> 	vdso: 25000000 x 256 times in 162.286219353 seconds
>>>> 	libc: 25000000 x 256 times in 2953.855018685 seconds
>>>> 	syscall: 25000000 x 256 times in 2796.268546000 seconds
>>>>
>>>> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
>>>> ---
>>>>
>>>> [v1]->v2:
>>>> - Fix the commit message.
>>>> - Only build the vDSO getrandom code if CONFIG_VDSO_GETRANDOM, to
>>>>     unbreak RV32 build.
>>>> - Likewise, only enable the selftest if __riscv_xlen == 64.
>>>>
>>>> [v1]:
>>>> https://lore.kernel.org/all/20250224122541.65045-1-xry111@xry111.site/
>>>>
>>>>    arch/riscv/Kconfig                            |   1 +
>>>>    arch/riscv/include/asm/vdso/getrandom.h       |  30 +++
>>>>    arch/riscv/kernel/vdso/Makefile               |  12 +
>>>>    arch/riscv/kernel/vdso/getrandom.c            |  10 +
>>>>    arch/riscv/kernel/vdso/vdso.lds.S             |   1 +
>>>>    arch/riscv/kernel/vdso/vgetrandom-chacha.S    | 244
>>>> ++++++++++++++++++
>>>>    .../selftests/vDSO/vgetrandom-chacha.S        |   2 +
>>>>    7 files changed, 300 insertions(+)
>>>>    create mode 100644 arch/riscv/include/asm/vdso/getrandom.h
>>>>    create mode 100644 arch/riscv/kernel/vdso/getrandom.c
>>>>    create mode 100644 arch/riscv/kernel/vdso/vgetrandom-chacha.S
>>> <snip>
>>>
>>>> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S
>>>> b/arch/riscv/kernel/vdso/vdso.lds.S
>>>> index 8e86965a8aae..abc69cda0445 100644
>>>> --- a/arch/riscv/kernel/vdso/vdso.lds.S
>>>> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
>>>> @@ -80,6 +80,7 @@ VERSION
>>>>    #ifndef COMPAT_VDSO
>>>>    		__vdso_riscv_hwprobe;
>>>>    #endif
>>>> +		__vdso_getrandom;
>>> For consistency this could be gated behind CONFIG_VDSO_GETRANDOM.
>>
>> Nathan sent a fix for this here:
>>
>> https://lore.kernel.org/all/20250423-riscv-fix-compat_vdso-lld-v2-1-b7bbbc244501@kernel.org/
> I've given it an R-b.  Do you prefer me to squash the patches and keep
> the SoB of both I and Nathan?

Hmm I was about to send a new PR today after the CI passes, I mentioned 
Nathan's patch in the squash so he keeps credit for the fix. Unless you 
can send something today, I'll keep my squashed patch.

Thanks,

Alex


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

* Re: [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation
  2025-05-23 10:06       ` Alexandre Ghiti
@ 2025-06-03 12:48         ` Xi Ruoyao
  2025-06-04 11:45           ` Alexandre Ghiti
  0 siblings, 1 reply; 16+ messages in thread
From: Xi Ruoyao @ 2025-06-03 12:48 UTC (permalink / raw)
  To: Alexandre Ghiti, Thomas Weißschuh, Nathan Chancellor
  Cc: Jason A. Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

On Fri, 2025-05-23 at 12:06 +0200, Alexandre Ghiti wrote:
> On 5/23/25 10:02, Xi Ruoyao wrote:
> > On Fri, 2025-05-23 at 10:01 +0200, Alexandre Ghiti wrote:
> > > Hi Xi,
> > > 
> > > On 4/11/25 10:04, Thomas Weißschuh wrote:
> > > > On Fri, Apr 11, 2025 at 10:46:00AM +0800, Xi Ruoyao wrote:
> > > > > Hook up the generic vDSO implementation to the generic vDSO
> > > > > getrandom
> > > > > implementation by providing the required
> > > > > __arch_chacha20_blocks_nostack
> > > > > and getrandom_syscall implementations. Also wire up the selftests.
> > > > > 
> > > > > The benchmark result:
> > > > > 
> > > > > 	vdso: 25000000 times in 2.466341333 seconds
> > > > > 	libc: 25000000 times in 41.447720005 seconds
> > > > > 	syscall: 25000000 times in 41.043926672 seconds
> > > > > 
> > > > > 	vdso: 25000000 x 256 times in 162.286219353 seconds
> > > > > 	libc: 25000000 x 256 times in 2953.855018685 seconds
> > > > > 	syscall: 25000000 x 256 times in 2796.268546000 seconds
> > > > > 
> > > > > Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> > > > > ---
> > > > > 
> > > > > [v1]->v2:
> > > > > - Fix the commit message.
> > > > > - Only build the vDSO getrandom code if CONFIG_VDSO_GETRANDOM, to
> > > > >     unbreak RV32 build.
> > > > > - Likewise, only enable the selftest if __riscv_xlen == 64.
> > > > > 
> > > > > [v1]:
> > > > > https://lore.kernel.org/all/20250224122541.65045-1-xry111@xry111.site/
> > > > > 
> > > > >    arch/riscv/Kconfig                            |   1 +
> > > > >    arch/riscv/include/asm/vdso/getrandom.h       |  30 +++
> > > > >    arch/riscv/kernel/vdso/Makefile               |  12 +
> > > > >    arch/riscv/kernel/vdso/getrandom.c            |  10 +
> > > > >    arch/riscv/kernel/vdso/vdso.lds.S             |   1 +
> > > > >    arch/riscv/kernel/vdso/vgetrandom-chacha.S    | 244
> > > > > ++++++++++++++++++
> > > > >    .../selftests/vDSO/vgetrandom-chacha.S        |   2 +
> > > > >    7 files changed, 300 insertions(+)
> > > > >    create mode 100644 arch/riscv/include/asm/vdso/getrandom.h
> > > > >    create mode 100644 arch/riscv/kernel/vdso/getrandom.c
> > > > >    create mode 100644 arch/riscv/kernel/vdso/vgetrandom-chacha.S
> > > > <snip>
> > > > 
> > > > > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S
> > > > > b/arch/riscv/kernel/vdso/vdso.lds.S
> > > > > index 8e86965a8aae..abc69cda0445 100644
> > > > > --- a/arch/riscv/kernel/vdso/vdso.lds.S
> > > > > +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> > > > > @@ -80,6 +80,7 @@ VERSION
> > > > >    #ifndef COMPAT_VDSO
> > > > >    		__vdso_riscv_hwprobe;
> > > > >    #endif
> > > > > +		__vdso_getrandom;
> > > > For consistency this could be gated behind CONFIG_VDSO_GETRANDOM.
> > > 
> > > Nathan sent a fix for this here:
> > > 
> > > https://lore.kernel.org/all/20250423-riscv-fix-compat_vdso-lld-v2-1-b7bbbc244501@kernel.org/
> > I've given it an R-b.  Do you prefer me to squash the patches and keep
> > the SoB of both I and Nathan?
> 
> Hmm I was about to send a new PR today after the CI passes, I mentioned 
> Nathan's patch in the squash so he keeps credit for the fix. Unless you 
> can send something today, I'll keep my squashed patch.

Palmer has reverted this in for-next and Thomas just informed me another
mistake in the code at https://lore.kernel.org/all/20250603-loongarch-
vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/.

I'll try to sort things up and send v3 in the week.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

* Re: [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation
  2025-06-03 12:48         ` Xi Ruoyao
@ 2025-06-04 11:45           ` Alexandre Ghiti
  2025-06-04 15:09             ` [PATCH] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper Xi Ruoyao
  0 siblings, 1 reply; 16+ messages in thread
From: Alexandre Ghiti @ 2025-06-04 11:45 UTC (permalink / raw)
  To: Xi Ruoyao, Thomas Weißschuh, Nathan Chancellor
  Cc: Jason A. Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

Hi Xi,

On 6/3/25 14:48, Xi Ruoyao wrote:
> On Fri, 2025-05-23 at 12:06 +0200, Alexandre Ghiti wrote:
>> On 5/23/25 10:02, Xi Ruoyao wrote:
>>> On Fri, 2025-05-23 at 10:01 +0200, Alexandre Ghiti wrote:
>>>> Hi Xi,
>>>>
>>>> On 4/11/25 10:04, Thomas Weißschuh wrote:
>>>>> On Fri, Apr 11, 2025 at 10:46:00AM +0800, Xi Ruoyao wrote:
>>>>>> Hook up the generic vDSO implementation to the generic vDSO
>>>>>> getrandom
>>>>>> implementation by providing the required
>>>>>> __arch_chacha20_blocks_nostack
>>>>>> and getrandom_syscall implementations. Also wire up the selftests.
>>>>>>
>>>>>> The benchmark result:
>>>>>>
>>>>>> 	vdso: 25000000 times in 2.466341333 seconds
>>>>>> 	libc: 25000000 times in 41.447720005 seconds
>>>>>> 	syscall: 25000000 times in 41.043926672 seconds
>>>>>>
>>>>>> 	vdso: 25000000 x 256 times in 162.286219353 seconds
>>>>>> 	libc: 25000000 x 256 times in 2953.855018685 seconds
>>>>>> 	syscall: 25000000 x 256 times in 2796.268546000 seconds
>>>>>>
>>>>>> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
>>>>>> ---
>>>>>>
>>>>>> [v1]->v2:
>>>>>> - Fix the commit message.
>>>>>> - Only build the vDSO getrandom code if CONFIG_VDSO_GETRANDOM, to
>>>>>>      unbreak RV32 build.
>>>>>> - Likewise, only enable the selftest if __riscv_xlen == 64.
>>>>>>
>>>>>> [v1]:
>>>>>> https://lore.kernel.org/all/20250224122541.65045-1-xry111@xry111.site/
>>>>>>
>>>>>>     arch/riscv/Kconfig                            |   1 +
>>>>>>     arch/riscv/include/asm/vdso/getrandom.h       |  30 +++
>>>>>>     arch/riscv/kernel/vdso/Makefile               |  12 +
>>>>>>     arch/riscv/kernel/vdso/getrandom.c            |  10 +
>>>>>>     arch/riscv/kernel/vdso/vdso.lds.S             |   1 +
>>>>>>     arch/riscv/kernel/vdso/vgetrandom-chacha.S    | 244
>>>>>> ++++++++++++++++++
>>>>>>     .../selftests/vDSO/vgetrandom-chacha.S        |   2 +
>>>>>>     7 files changed, 300 insertions(+)
>>>>>>     create mode 100644 arch/riscv/include/asm/vdso/getrandom.h
>>>>>>     create mode 100644 arch/riscv/kernel/vdso/getrandom.c
>>>>>>     create mode 100644 arch/riscv/kernel/vdso/vgetrandom-chacha.S
>>>>> <snip>
>>>>>
>>>>>> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S
>>>>>> b/arch/riscv/kernel/vdso/vdso.lds.S
>>>>>> index 8e86965a8aae..abc69cda0445 100644
>>>>>> --- a/arch/riscv/kernel/vdso/vdso.lds.S
>>>>>> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
>>>>>> @@ -80,6 +80,7 @@ VERSION
>>>>>>     #ifndef COMPAT_VDSO
>>>>>>     		__vdso_riscv_hwprobe;
>>>>>>     #endif
>>>>>> +		__vdso_getrandom;
>>>>> For consistency this could be gated behind CONFIG_VDSO_GETRANDOM.
>>>> Nathan sent a fix for this here:
>>>>
>>>> https://lore.kernel.org/all/20250423-riscv-fix-compat_vdso-lld-v2-1-b7bbbc244501@kernel.org/
>>> I've given it an R-b.  Do you prefer me to squash the patches and keep
>>> the SoB of both I and Nathan?
>> Hmm I was about to send a new PR today after the CI passes, I mentioned
>> Nathan's patch in the squash so he keeps credit for the fix. Unless you
>> can send something today, I'll keep my squashed patch.
> Palmer has reverted this in for-next and Thomas just informed me another
> mistake in the code at https://lore.kernel.org/all/20250603-loongarch-
> vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/.
>
> I'll try to sort things up and send v3 in the week.


I already sent this patch with a few fixes in my second PR for 6.16 
(https://git.kernel.org/pub/scm/linux/kernel/git/alexghiti/linux.git/commit/?h=alex-for-next-sbi-3.0-rebase-6.15-rc6&id=dc5240f09bca7b5fc72ad8894d6b9321bce51139)

Can you just send the fix? I'll merge it next week in -rc2.

Thanks,

Alex


>

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

* [PATCH] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-04 11:45           ` Alexandre Ghiti
@ 2025-06-04 15:09             ` Xi Ruoyao
  2025-06-05  7:15               ` Thomas Weißschuh
  0 siblings, 1 reply; 16+ messages in thread
From: Xi Ruoyao @ 2025-06-04 15:09 UTC (permalink / raw)
  To: Alexandre Ghiti, Thomas Weißschuh, Nathan Chancellor
  Cc: Jason A. Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

As recently pointed out by Thomas, if a register is forced for two
different register variables, among them one is used as "+" (both input
and output) and another is only used as input, Clang would treat the
conflicting input parameters as undefined behaviour and optimize away
the argument assignment.

Per an example in the GCC documentation, for this purpose we can use "="
(only output) for the output, and "0" for the input for that we must
reuse the same register as the output.  I'm not sure if using a simple
"r" for the input is safe or not here, so just follow the documentation.

Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Cc: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 arch/riscv/include/asm/vdso/getrandom.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
index 8dc92441702a..0d84a38e79da 100644
--- a/arch/riscv/include/asm/vdso/getrandom.h
+++ b/arch/riscv/include/asm/vdso/getrandom.h
@@ -18,8 +18,8 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
 	register unsigned int flags asm("a2") = _flags;
 
 	asm volatile ("ecall\n"
-		      : "+r" (ret)
-		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
+		      : "=r" (ret)
+		      : "r" (nr), "0" (buffer), "r" (len), "r" (flags)
 		      : "memory");
 
 	return ret;

base-commit: dc5240f09bca7b5fc72ad8894d6b9321bce51139
-- 
2.49.0

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

* Re: [PATCH] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-04 15:09             ` [PATCH] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper Xi Ruoyao
@ 2025-06-05  7:15               ` Thomas Weißschuh
  2025-06-06  9:24                 ` [PATCH v2] " Xi Ruoyao
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Weißschuh @ 2025-06-05  7:15 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: Alexandre Ghiti, Nathan Chancellor, Jason A. Donenfeld,
	Paul Walmsley, Palmer Dabbelt, Guo Ren, linux-riscv, linux-kernel

Hi Xi,

On Wed, Jun 04, 2025 at 11:09:52PM +0800, Xi Ruoyao wrote:
> As recently pointed out by Thomas, if a register is forced for two
> different register variables, among them one is used as "+" (both input
> and output) and another is only used as input, Clang would treat the
> conflicting input parameters as undefined behaviour and optimize away
> the argument assignment.

Good catch, I forgot to check the getrandom() implementations.
 
> Per an example in the GCC documentation, for this purpose we can use "="
> (only output) for the output, and "0" for the input for that we must
> reuse the same register as the output.  I'm not sure if using a simple
> "r" for the input is safe or not here, so just follow the documentation.

This looks good in general. However the usage of the "0" constraint now differs
from the "r" constraint used by all other vDSO implementations, even the RISC-V
gettimeofday() itself.
I'd like to hold off applying this commit and wait for the conclusion of the
discussion linked below and then implement the result of that everywhere.

> Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
> Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>  arch/riscv/include/asm/vdso/getrandom.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
> index 8dc92441702a..0d84a38e79da 100644
> --- a/arch/riscv/include/asm/vdso/getrandom.h
> +++ b/arch/riscv/include/asm/vdso/getrandom.h
> @@ -18,8 +18,8 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>  	register unsigned int flags asm("a2") = _flags;
>  
>  	asm volatile ("ecall\n"
> -		      : "+r" (ret)
> -		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
> +		      : "=r" (ret)
> +		      : "r" (nr), "0" (buffer), "r" (len), "r" (flags)
>  		      : "memory");
>  
>  	return ret;
> 
> base-commit: dc5240f09bca7b5fc72ad8894d6b9321bce51139
> -- 
> 2.49.0

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

* [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-05  7:15               ` Thomas Weißschuh
@ 2025-06-06  9:24                 ` Xi Ruoyao
  2025-06-06  9:50                   ` Thomas Weißschuh
                                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Xi Ruoyao @ 2025-06-06  9:24 UTC (permalink / raw)
  To: Alexandre Ghiti, Thomas Weißschuh, Nathan Chancellor
  Cc: Jason A . Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel, Xi Ruoyao

As recently pointed out by Thomas, if a register is forced for two
different register variables, among them one is used as "+" (both input
and output) and another is only used as input, Clang would treat the
conflicting input parameters as undefined behaviour and optimize away
the argument assignment.

Per an example in the GCC documentation, for this purpose we can use "="
(only output) for the output, and "0" for the input for that we must
reuse the same register as the output.  And GCC developers have
confirmed using a simple "r" (that we use for most vDSO implementations)
instead of "0" is also fine.

Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
Link: https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Cc: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---

v1 -> v2: Keep using "r" for buffer to follow the existing convention
(that the GCC developers have confirmed fine).

 arch/riscv/include/asm/vdso/getrandom.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
index 8dc92441702a..c6d66895c1f5 100644
--- a/arch/riscv/include/asm/vdso/getrandom.h
+++ b/arch/riscv/include/asm/vdso/getrandom.h
@@ -18,7 +18,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
 	register unsigned int flags asm("a2") = _flags;
 
 	asm volatile ("ecall\n"
-		      : "+r" (ret)
+		      : "=r" (ret)
 		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
 		      : "memory");
 

base-commit: dc5240f09bca7b5fc72ad8894d6b9321bce51139
-- 
2.49.0


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

* Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-06  9:24                 ` [PATCH v2] " Xi Ruoyao
@ 2025-06-06  9:50                   ` Thomas Weißschuh
  2025-06-06 22:01                   ` Vineet Gupta
  2025-06-12 20:10                   ` patchwork-bot+linux-riscv
  2 siblings, 0 replies; 16+ messages in thread
From: Thomas Weißschuh @ 2025-06-06  9:50 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: Alexandre Ghiti, Nathan Chancellor, Jason A . Donenfeld,
	Paul Walmsley, Palmer Dabbelt, Guo Ren, linux-riscv, linux-kernel

On Fri, Jun 06, 2025 at 05:24:44PM +0800, Xi Ruoyao wrote:
> As recently pointed out by Thomas, if a register is forced for two
> different register variables, among them one is used as "+" (both input
> and output) and another is only used as input, Clang would treat the
> conflicting input parameters as undefined behaviour and optimize away
> the argument assignment.
> 
> Per an example in the GCC documentation, for this purpose we can use "="
> (only output) for the output, and "0" for the input for that we must
> reuse the same register as the output.  And GCC developers have
> confirmed using a simple "r" (that we use for most vDSO implementations)
> instead of "0" is also fine.

The wording is a bit confusing. Maybe this is better:

	Instead use "=r" (only output) for the output parameter and "r" (only input)
	for the input parameter.
	While the example from the GCC documentation uses "0" for the input parameter,
	this is not necessary as confirmed by the GCC developers and "r"
	matches what the other architectures' vDSO implementations are using.

> Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
> Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
> Link: https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html
> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

In any case, thanks and

Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

> Cc: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> 
> v1 -> v2: Keep using "r" for buffer to follow the existing convention
> (that the GCC developers have confirmed fine).
> 
>  arch/riscv/include/asm/vdso/getrandom.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
> index 8dc92441702a..c6d66895c1f5 100644
> --- a/arch/riscv/include/asm/vdso/getrandom.h
> +++ b/arch/riscv/include/asm/vdso/getrandom.h
> @@ -18,7 +18,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>  	register unsigned int flags asm("a2") = _flags;
>  
>  	asm volatile ("ecall\n"
> -		      : "+r" (ret)
> +		      : "=r" (ret)
>  		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>  		      : "memory");
>  
> 
> base-commit: dc5240f09bca7b5fc72ad8894d6b9321bce51139
> -- 
> 2.49.0
> 

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

* Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-06  9:24                 ` [PATCH v2] " Xi Ruoyao
  2025-06-06  9:50                   ` Thomas Weißschuh
@ 2025-06-06 22:01                   ` Vineet Gupta
  2025-06-07 14:16                     ` Xi Ruoyao
  2025-06-12 20:10                   ` patchwork-bot+linux-riscv
  2 siblings, 1 reply; 16+ messages in thread
From: Vineet Gupta @ 2025-06-06 22:01 UTC (permalink / raw)
  To: Xi Ruoyao, Alexandre Ghiti, Thomas Weißschuh,
	Nathan Chancellor
  Cc: Jason A . Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

On 6/6/25 02:24, Xi Ruoyao wrote:
> As recently pointed out by Thomas, if a register is forced for two
> different register variables, among them one is used as "+" (both input
> and output) and another is only used as input, Clang would treat the
> conflicting input parameters as undefined behaviour and optimize away
> the argument assignment.
>
> Per an example in the GCC documentation, for this purpose we can use "="
> (only output) for the output, and "0" for the input for that we must
> reuse the same register as the output.  And GCC developers have
> confirmed using a simple "r" (that we use for most vDSO implementations)
> instead of "0" is also fine.
>
> Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
> Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
> Link: https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html
> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>
> v1 -> v2: Keep using "r" for buffer to follow the existing convention
> (that the GCC developers have confirmed fine).
>
>  arch/riscv/include/asm/vdso/getrandom.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
> index 8dc92441702a..c6d66895c1f5 100644
> --- a/arch/riscv/include/asm/vdso/getrandom.h
> +++ b/arch/riscv/include/asm/vdso/getrandom.h
> @@ -18,7 +18,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>  	register unsigned int flags asm("a2") = _flags;
>  
>  	asm volatile ("ecall\n"
> -		      : "+r" (ret)
> +		      : "=r" (ret)
>  		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>  		      : "memory");

My 2 cents as I've dabbled into this for ARC glibc syscall macros [1] where r0
is both the first syscall/function arg and also the function/syscall return.

The v2 approach still keeps 2 different variables in same local reg which has
potential for any future compiler shenanigans.
Segher's example avoided specifying the same reg.
What about something like the following: seems to generate the right code (with
gcc 15)

       register long ret asm("a0");
       register long nr asm("a7") = __NR_getrandom;
       register size_t len asm("a1") = _len;
       register unsigned int flags asm("a2") = _flags;
       ret = (unsigned long) _buffer;

       asm volatile ("ecall\n"
                     : "+r" (ret)                                 // keep "+r"
for input _buffer / output ret
                     : "r" (nr), "r" (len), "r" (flags)
                     : "memory");

       return ret;

Thx,
-Vineet

[1] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/arc/sysdep.h

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

* Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-06 22:01                   ` Vineet Gupta
@ 2025-06-07 14:16                     ` Xi Ruoyao
  2025-06-10  7:11                       ` Thomas Weißschuh
  0 siblings, 1 reply; 16+ messages in thread
From: Xi Ruoyao @ 2025-06-07 14:16 UTC (permalink / raw)
  To: Vineet Gupta, Alexandre Ghiti, Thomas Weißschuh,
	Nathan Chancellor
  Cc: Jason A . Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

On Fri, 2025-06-06 at 15:01 -0700, Vineet Gupta wrote:
> On 6/6/25 02:24, Xi Ruoyao wrote:
> > As recently pointed out by Thomas, if a register is forced for two
> > different register variables, among them one is used as "+" (both input
> > and output) and another is only used as input, Clang would treat the
> > conflicting input parameters as undefined behaviour and optimize away
> > the argument assignment.
> > 
> > Per an example in the GCC documentation, for this purpose we can use "="
> > (only output) for the output, and "0" for the input for that we must
> > reuse the same register as the output.  And GCC developers have
> > confirmed using a simple "r" (that we use for most vDSO implementations)
> > instead of "0" is also fine.
> > 
> > Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
> > Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
> > Link: https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html
> > Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> > ---
> > 
> > v1 -> v2: Keep using "r" for buffer to follow the existing convention
> > (that the GCC developers have confirmed fine).
> > 
> >  arch/riscv/include/asm/vdso/getrandom.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
> > index 8dc92441702a..c6d66895c1f5 100644
> > --- a/arch/riscv/include/asm/vdso/getrandom.h
> > +++ b/arch/riscv/include/asm/vdso/getrandom.h
> > @@ -18,7 +18,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
> >  	register unsigned int flags asm("a2") = _flags;
> >  
> >  	asm volatile ("ecall\n"
> > -		      : "+r" (ret)
> > +		      : "=r" (ret)
> >  		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
> >  		      : "memory");
> 
> My 2 cents as I've dabbled into this for ARC glibc syscall macros [1] where r0
> is both the first syscall/function arg and also the function/syscall return.
> 
> The v2 approach still keeps 2 different variables in same local reg which has
> potential for any future compiler shenanigans.
> Segher's example avoided specifying the same reg.
> What about something like the following: seems to generate the right code (with
> gcc 15)
> 
>        register long ret asm("a0");

Then it would be better to rename this variable to just "a0".  And I
guess Thomas doesn't want a new convention different from all other
syscall wrappers in vDSO...

>        register long nr asm("a7") = __NR_getrandom;
>        register size_t len asm("a1") = _len;
>        register unsigned int flags asm("a2") = _flags;
>        ret = (unsigned long) _buffer;
> 
>        asm volatile ("ecall\n"
>                      : "+r" (ret)                                 // keep "+r"
> for input _buffer / output ret
>                      : "r" (nr), "r" (len), "r" (flags)
>                      : "memory");
> 
>        return ret;
> 
> Thx,
> -Vineet
> 
> [1] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/arc/sysdep.h

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

* Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-07 14:16                     ` Xi Ruoyao
@ 2025-06-10  7:11                       ` Thomas Weißschuh
  2025-06-10  8:15                         ` Alexandre Ghiti
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Weißschuh @ 2025-06-10  7:11 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: Vineet Gupta, Alexandre Ghiti, Nathan Chancellor,
	Jason A . Donenfeld, Paul Walmsley, Palmer Dabbelt, Guo Ren,
	linux-riscv, linux-kernel

On Sat, Jun 07, 2025 at 10:16:34PM +0800, Xi Ruoyao wrote:
> On Fri, 2025-06-06 at 15:01 -0700, Vineet Gupta wrote:
> > On 6/6/25 02:24, Xi Ruoyao wrote:
> > > As recently pointed out by Thomas, if a register is forced for two
> > > different register variables, among them one is used as "+" (both input
> > > and output) and another is only used as input, Clang would treat the
> > > conflicting input parameters as undefined behaviour and optimize away
> > > the argument assignment.
> > > 
> > > Per an example in the GCC documentation, for this purpose we can use "="
> > > (only output) for the output, and "0" for the input for that we must
> > > reuse the same register as the output.  And GCC developers have
> > > confirmed using a simple "r" (that we use for most vDSO implementations)
> > > instead of "0" is also fine.
> > > 
> > > Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
> > > Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
> > > Link: https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html
> > > Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > > Cc: Nathan Chancellor <nathan@kernel.org>
> > > Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> > > ---
> > > 
> > > v1 -> v2: Keep using "r" for buffer to follow the existing convention
> > > (that the GCC developers have confirmed fine).
> > > 
> > >  arch/riscv/include/asm/vdso/getrandom.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
> > > index 8dc92441702a..c6d66895c1f5 100644
> > > --- a/arch/riscv/include/asm/vdso/getrandom.h
> > > +++ b/arch/riscv/include/asm/vdso/getrandom.h
> > > @@ -18,7 +18,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
> > >  	register unsigned int flags asm("a2") = _flags;
> > >  
> > >  	asm volatile ("ecall\n"
> > > -		      : "+r" (ret)
> > > +		      : "=r" (ret)
> > >  		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
> > >  		      : "memory");
> > 
> > My 2 cents as I've dabbled into this for ARC glibc syscall macros [1] where r0
> > is both the first syscall/function arg and also the function/syscall return.
> > 
> > The v2 approach still keeps 2 different variables in same local reg which has
> > potential for any future compiler shenanigans.
> > Segher's example avoided specifying the same reg.
> > What about something like the following: seems to generate the right code (with
> > gcc 15)
> > 
> >        register long ret asm("a0");
> 
> Then it would be better to rename this variable to just "a0".  And I
> guess Thomas doesn't want a new convention different from all other
> syscall wrappers in vDSO...

Indeed. I want to keep it consistent. Especially for a bugfix.
Speaking of which, IMO this patch should have a Fixes tag.

Then we could start a new discussion about changing it to something else everywhere.
Although I don't think that the single-variable variant is better.

> >        register long nr asm("a7") = __NR_getrandom;
> >        register size_t len asm("a1") = _len;
> >        register unsigned int flags asm("a2") = _flags;
> >        ret = (unsigned long) _buffer;
> > 
> >        asm volatile ("ecall\n"
> >                      : "+r" (ret)                                 // keep "+r"
> > for input _buffer / output ret
> >                      : "r" (nr), "r" (len), "r" (flags)
> >                      : "memory");
> > 
> >        return ret;
> > 
> > Thx,
> > -Vineet
> > 
> > [1] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/arc/sysdep.h
> 
> -- 
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

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

* Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-10  7:11                       ` Thomas Weißschuh
@ 2025-06-10  8:15                         ` Alexandre Ghiti
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandre Ghiti @ 2025-06-10  8:15 UTC (permalink / raw)
  To: Thomas Weißschuh, Xi Ruoyao
  Cc: Vineet Gupta, Nathan Chancellor, Jason A . Donenfeld,
	Paul Walmsley, Palmer Dabbelt, Guo Ren, linux-riscv, linux-kernel

Hi,

On 6/10/25 09:11, Thomas Weißschuh wrote:
> On Sat, Jun 07, 2025 at 10:16:34PM +0800, Xi Ruoyao wrote:
>> On Fri, 2025-06-06 at 15:01 -0700, Vineet Gupta wrote:
>>> On 6/6/25 02:24, Xi Ruoyao wrote:
>>>> As recently pointed out by Thomas, if a register is forced for two
>>>> different register variables, among them one is used as "+" (both input
>>>> and output) and another is only used as input, Clang would treat the
>>>> conflicting input parameters as undefined behaviour and optimize away
>>>> the argument assignment.
>>>>
>>>> Per an example in the GCC documentation, for this purpose we can use "="
>>>> (only output) for the output, and "0" for the input for that we must
>>>> reuse the same register as the output.  And GCC developers have
>>>> confirmed using a simple "r" (that we use for most vDSO implementations)
>>>> instead of "0" is also fine.
>>>>
>>>> Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
>>>> Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
>>>> Link: https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html
>>>> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>>>> Cc: Nathan Chancellor <nathan@kernel.org>
>>>> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
>>>> ---
>>>>
>>>> v1 -> v2: Keep using "r" for buffer to follow the existing convention
>>>> (that the GCC developers have confirmed fine).
>>>>
>>>>   arch/riscv/include/asm/vdso/getrandom.h | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
>>>> index 8dc92441702a..c6d66895c1f5 100644
>>>> --- a/arch/riscv/include/asm/vdso/getrandom.h
>>>> +++ b/arch/riscv/include/asm/vdso/getrandom.h
>>>> @@ -18,7 +18,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>>>>   	register unsigned int flags asm("a2") = _flags;
>>>>   
>>>>   	asm volatile ("ecall\n"
>>>> -		      : "+r" (ret)
>>>> +		      : "=r" (ret)
>>>>   		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>>>>   		      : "memory");
>>> My 2 cents as I've dabbled into this for ARC glibc syscall macros [1] where r0
>>> is both the first syscall/function arg and also the function/syscall return.
>>>
>>> The v2 approach still keeps 2 different variables in same local reg which has
>>> potential for any future compiler shenanigans.
>>> Segher's example avoided specifying the same reg.
>>> What about something like the following: seems to generate the right code (with
>>> gcc 15)
>>>
>>>         register long ret asm("a0");
>> Then it would be better to rename this variable to just "a0".  And I
>> guess Thomas doesn't want a new convention different from all other
>> syscall wrappers in vDSO...
> Indeed. I want to keep it consistent. Especially for a bugfix.
> Speaking of which, IMO this patch should have a Fixes tag.


Yes, here it is:

Fixes: ee0d03053e70 ("RISC-V: vDSO: Wire up getrandom() vDSO 
implementation")


>
> Then we could start a new discussion about changing it to something else everywhere.
> Although I don't think that the single-variable variant is better.


Vineet feel free to propose something for all architectures if you think 
that's better.

For now, I'll merge this version for inclusion in -rc2,

Thanks,

Alex


>
>>>         register long nr asm("a7") = __NR_getrandom;
>>>         register size_t len asm("a1") = _len;
>>>         register unsigned int flags asm("a2") = _flags;
>>>         ret = (unsigned long) _buffer;
>>>
>>>         asm volatile ("ecall\n"
>>>                       : "+r" (ret)                                 // keep "+r"
>>> for input _buffer / output ret
>>>                       : "r" (nr), "r" (len), "r" (flags)
>>>                       : "memory");
>>>
>>>         return ret;
>>>
>>> Thx,
>>> -Vineet
>>>
>>> [1] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/arc/sysdep.h
>> -- 
>> Xi Ruoyao <xry111@xry111.site>
>> School of Aerospace Science and Technology, Xidian University
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
  2025-06-06  9:24                 ` [PATCH v2] " Xi Ruoyao
  2025-06-06  9:50                   ` Thomas Weißschuh
  2025-06-06 22:01                   ` Vineet Gupta
@ 2025-06-12 20:10                   ` patchwork-bot+linux-riscv
  2 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+linux-riscv @ 2025-06-12 20:10 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: linux-riscv, alex, thomas.weissschuh, nathan, Jason,
	paul.walmsley, palmer, guoren, linux-kernel

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@dabbelt.com>:

On Fri,  6 Jun 2025 17:24:44 +0800 you wrote:
> As recently pointed out by Thomas, if a register is forced for two
> different register variables, among them one is used as "+" (both input
> and output) and another is only used as input, Clang would treat the
> conflicting input parameters as undefined behaviour and optimize away
> the argument assignment.
> 
> Per an example in the GCC documentation, for this purpose we can use "="
> (only output) for the output, and "0" for the input for that we must
> reuse the same register as the output.  And GCC developers have
> confirmed using a simple "r" (that we use for most vDSO implementations)
> instead of "0" is also fine.
> 
> [...]

Here is the summary with links:
  - [v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
    https://git.kernel.org/riscv/c/2b9518684f85

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



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

end of thread, other threads:[~2025-06-12 20:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-11  2:46 [PATCH v2] RISC-V: vDSO: Wire up getrandom() vDSO implementation Xi Ruoyao
2025-04-11  8:04 ` Thomas Weißschuh
2025-05-23  8:01   ` Alexandre Ghiti
2025-05-23  8:02     ` Xi Ruoyao
2025-05-23 10:06       ` Alexandre Ghiti
2025-06-03 12:48         ` Xi Ruoyao
2025-06-04 11:45           ` Alexandre Ghiti
2025-06-04 15:09             ` [PATCH] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper Xi Ruoyao
2025-06-05  7:15               ` Thomas Weißschuh
2025-06-06  9:24                 ` [PATCH v2] " Xi Ruoyao
2025-06-06  9:50                   ` Thomas Weißschuh
2025-06-06 22:01                   ` Vineet Gupta
2025-06-07 14:16                     ` Xi Ruoyao
2025-06-10  7:11                       ` Thomas Weißschuh
2025-06-10  8:15                         ` Alexandre Ghiti
2025-06-12 20:10                   ` patchwork-bot+linux-riscv

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).