* [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
_______________________________________________
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-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>
_______________________________________________
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: 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
_______________________________________________
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
_______________________________________________
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: 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
_______________________________________________
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: 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
_______________________________________________
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: 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
>
_______________________________________________
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
* [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
_______________________________________________
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] 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
_______________________________________________
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
* [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
_______________________________________________
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: 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
>
_______________________________________________
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-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
_______________________________________________
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 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
_______________________________________________
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-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
_______________________________________________
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-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
_______________________________________________
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
_______________________________________________
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
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).