linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests
@ 2025-04-22 16:23 Clément Léger
  2025-04-22 16:23 ` [PATCH v2 1/5] riscv: misaligned: factorize trap handling Clément Léger
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Clément Léger @ 2025-04-22 16:23 UTC (permalink / raw)
  To: open list:DOCUMENTATION, open list, open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Shuah Khan,
	Andrew Jones, Samuel Holland

This series fixes misaligned access handling when in non interruptible
context by reenabling interrupts when possible. A previous commit
changed raw_copy_from_user() with copy_from_user() which enables page
faulting and thus can sleep. While correct, a warning is now triggered
due to being called in an invalid context (sleeping in
non-interruptible). This series fixes that problem by factorizing
misaligned load/store entry in a single function than reenables
interrupt if the interrupted context had interrupts enabled.
In order for misaligned handling problems to be caught sooner, add a
kselftest for all the currently supported instructions .

Note: these commits were actually part of another larger series for
misaligned request delegation but was split since it isn't directly
required.

V2:
 - Use an array of struct to simplify misaligned load/store selection
 - Revert use of irqentry_enter()/exit() to irqentry_nmi_enter() for
   kernel space.

Clément Léger (5):
  riscv: misaligned: factorize trap handling
  riscv: misaligned: enable IRQs while handling misaligned accesses
  riscv: misaligned: use get_user() instead of __get_user()
  Documentation/sysctl: add riscv to unaligned-trap supported archs
  selftests: riscv: add misaligned access testing

 Documentation/admin-guide/sysctl/kernel.rst   |   4 +-
 arch/riscv/kernel/traps.c                     |  64 +++--
 arch/riscv/kernel/traps_misaligned.c          |   2 +-
 .../selftests/riscv/misaligned/.gitignore     |   1 +
 .../selftests/riscv/misaligned/Makefile       |  12 +
 .../selftests/riscv/misaligned/common.S       |  33 +++
 .../testing/selftests/riscv/misaligned/fpu.S  | 180 +++++++++++++
 tools/testing/selftests/riscv/misaligned/gp.S | 103 +++++++
 .../selftests/riscv/misaligned/misaligned.c   | 254 ++++++++++++++++++
 9 files changed, 623 insertions(+), 30 deletions(-)
 create mode 100644 tools/testing/selftests/riscv/misaligned/.gitignore
 create mode 100644 tools/testing/selftests/riscv/misaligned/Makefile
 create mode 100644 tools/testing/selftests/riscv/misaligned/common.S
 create mode 100644 tools/testing/selftests/riscv/misaligned/fpu.S
 create mode 100644 tools/testing/selftests/riscv/misaligned/gp.S
 create mode 100644 tools/testing/selftests/riscv/misaligned/misaligned.c

-- 
2.49.0


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

* [PATCH v2 1/5] riscv: misaligned: factorize trap handling
  2025-04-22 16:23 [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests Clément Léger
@ 2025-04-22 16:23 ` Clément Léger
  2025-05-06 10:58   ` Alexandre Ghiti
  2025-04-22 16:23 ` [PATCH v2 2/5] riscv: misaligned: enable IRQs while handling misaligned accesses Clément Léger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Clément Léger @ 2025-04-22 16:23 UTC (permalink / raw)
  To: open list:DOCUMENTATION, open list, open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Shuah Khan,
	Andrew Jones, Samuel Holland

Since both load/store and user/kernel should use almost the same path and
that we are going to add some code around that, factorize it.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/kernel/traps.c | 66 +++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 8ff8e8b36524..b1d991c78a23 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -198,47 +198,53 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
 DO_ERROR_INFO(do_trap_load_fault,
 	SIGSEGV, SEGV_ACCERR, "load access fault");
 
-asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs)
+enum misaligned_access_type {
+	MISALIGNED_STORE,
+	MISALIGNED_LOAD,
+};
+static const struct {
+	const char *type_str;
+	int (*handler)(struct pt_regs *regs);
+} misaligned_handler[] = {
+	[MISALIGNED_STORE] = {
+		.type_str = "Oops - store (or AMO) address misaligned",
+		.handler = handle_misaligned_store,
+	},
+	[MISALIGNED_LOAD] = {
+		.type_str = "Oops - load address misaligned",
+		.handler = handle_misaligned_load,
+	},
+};
+
+static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type)
 {
-	if (user_mode(regs)) {
+	irqentry_state_t state;
+
+	if (user_mode(regs))
 		irqentry_enter_from_user_mode(regs);
+	else
+		state = irqentry_nmi_enter(regs);
 
-		if (handle_misaligned_load(regs))
-			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-			      "Oops - load address misaligned");
+	if (misaligned_handler[type].handler(regs))
+		do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+			      misaligned_handler[type].type_str);
 
+	if (user_mode(regs))
 		irqentry_exit_to_user_mode(regs);
-	} else {
-		irqentry_state_t state = irqentry_nmi_enter(regs);
-
-		if (handle_misaligned_load(regs))
-			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-			      "Oops - load address misaligned");
-
+	else
 		irqentry_nmi_exit(regs, state);
-	}
 }
 
-asmlinkage __visible __trap_section void do_trap_store_misaligned(struct pt_regs *regs)
+asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs)
 {
-	if (user_mode(regs)) {
-		irqentry_enter_from_user_mode(regs);
-
-		if (handle_misaligned_store(regs))
-			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-				"Oops - store (or AMO) address misaligned");
-
-		irqentry_exit_to_user_mode(regs);
-	} else {
-		irqentry_state_t state = irqentry_nmi_enter(regs);
-
-		if (handle_misaligned_store(regs))
-			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-				"Oops - store (or AMO) address misaligned");
+	do_trap_misaligned(regs, MISALIGNED_LOAD);
+}
 
-		irqentry_nmi_exit(regs, state);
-	}
+asmlinkage __visible __trap_section void do_trap_store_misaligned(struct pt_regs *regs)
+{
+	do_trap_misaligned(regs, MISALIGNED_STORE);
 }
+
 DO_ERROR_INFO(do_trap_store_fault,
 	SIGSEGV, SEGV_ACCERR, "store (or AMO) access fault");
 DO_ERROR_INFO(do_trap_ecall_s,
-- 
2.49.0


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

* [PATCH v2 2/5] riscv: misaligned: enable IRQs while handling misaligned accesses
  2025-04-22 16:23 [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests Clément Léger
  2025-04-22 16:23 ` [PATCH v2 1/5] riscv: misaligned: factorize trap handling Clément Léger
@ 2025-04-22 16:23 ` Clément Léger
  2025-05-06 11:07   ` Alexandre Ghiti
  2025-04-22 16:23 ` [PATCH v2 3/5] riscv: misaligned: use get_user() instead of __get_user() Clément Léger
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Clément Léger @ 2025-04-22 16:23 UTC (permalink / raw)
  To: open list:DOCUMENTATION, open list, open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Shuah Khan,
	Andrew Jones, Samuel Holland

We can safely reenable IRQs if coming from userspace. This allows to
access user memory that could potentially trigger a page fault.

Fixes: b686ecdeacf6 ("riscv: misaligned: Restrict user access to kernel memory")
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/kernel/traps.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index b1d991c78a23..9c83848797a7 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -220,19 +220,23 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type
 {
 	irqentry_state_t state;
 
-	if (user_mode(regs))
+	if (user_mode(regs)) {
 		irqentry_enter_from_user_mode(regs);
-	else
+		local_irq_enable();
+	} else {
 		state = irqentry_nmi_enter(regs);
+	}
 
 	if (misaligned_handler[type].handler(regs))
 		do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
 			      misaligned_handler[type].type_str);
 
-	if (user_mode(regs))
+	if (user_mode(regs)) {
+		local_irq_disable();
 		irqentry_exit_to_user_mode(regs);
-	else
+	} else {
 		irqentry_nmi_exit(regs, state);
+	}
 }
 
 asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs)
-- 
2.49.0


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

* [PATCH v2 3/5] riscv: misaligned: use get_user() instead of __get_user()
  2025-04-22 16:23 [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests Clément Léger
  2025-04-22 16:23 ` [PATCH v2 1/5] riscv: misaligned: factorize trap handling Clément Léger
  2025-04-22 16:23 ` [PATCH v2 2/5] riscv: misaligned: enable IRQs while handling misaligned accesses Clément Léger
@ 2025-04-22 16:23 ` Clément Léger
  2025-04-22 16:23 ` [PATCH v2 4/5] Documentation/sysctl: add riscv to unaligned-trap supported archs Clément Léger
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Clément Léger @ 2025-04-22 16:23 UTC (permalink / raw)
  To: open list:DOCUMENTATION, open list, open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Shuah Khan,
	Andrew Jones, Samuel Holland, Alexandre Ghiti

Now that we can safely handle user memory accesses while in the
misaligned access handlers, use get_user() instead of __get_user() to
have user memory access checks.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 arch/riscv/kernel/traps_misaligned.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
index 4354c87c0376..97c674d7d34f 100644
--- a/arch/riscv/kernel/traps_misaligned.c
+++ b/arch/riscv/kernel/traps_misaligned.c
@@ -268,7 +268,7 @@ static unsigned long get_f32_rs(unsigned long insn, u8 fp_reg_offset,
 	int __ret;					\
 							\
 	if (user_mode(regs)) {				\
-		__ret = __get_user(insn, (type __user *) insn_addr); \
+		__ret = get_user(insn, (type __user *) insn_addr); \
 	} else {					\
 		insn = *(type *)insn_addr;		\
 		__ret = 0;				\
-- 
2.49.0


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

* [PATCH v2 4/5] Documentation/sysctl: add riscv to unaligned-trap supported archs
  2025-04-22 16:23 [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests Clément Léger
                   ` (2 preceding siblings ...)
  2025-04-22 16:23 ` [PATCH v2 3/5] riscv: misaligned: use get_user() instead of __get_user() Clément Léger
@ 2025-04-22 16:23 ` Clément Léger
  2025-04-22 16:23 ` [PATCH v2 5/5] selftests: riscv: add misaligned access testing Clément Léger
  2025-05-08 16:52 ` [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests patchwork-bot+linux-riscv
  5 siblings, 0 replies; 11+ messages in thread
From: Clément Léger @ 2025-04-22 16:23 UTC (permalink / raw)
  To: open list:DOCUMENTATION, open list, open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Shuah Khan,
	Andrew Jones, Samuel Holland, Alexandre Ghiti

riscv supports the "unaligned-trap" sysctl variable, add it to the list
of supported architectures.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 Documentation/admin-guide/sysctl/kernel.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index dd49a89a62d3..a38e91c4d92c 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -1595,8 +1595,8 @@ unaligned-trap
 
 On architectures where unaligned accesses cause traps, and where this
 feature is supported (``CONFIG_SYSCTL_ARCH_UNALIGN_ALLOW``; currently,
-``arc``, ``parisc`` and ``loongarch``), controls whether unaligned traps
-are caught and emulated (instead of failing).
+``arc``, ``parisc``, ``loongarch`` and ``riscv``), controls whether unaligned
+traps are caught and emulated (instead of failing).
 
 = ========================================================
 0 Do not emulate unaligned accesses.
-- 
2.49.0


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

* [PATCH v2 5/5] selftests: riscv: add misaligned access testing
  2025-04-22 16:23 [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests Clément Léger
                   ` (3 preceding siblings ...)
  2025-04-22 16:23 ` [PATCH v2 4/5] Documentation/sysctl: add riscv to unaligned-trap supported archs Clément Léger
@ 2025-04-22 16:23 ` Clément Léger
  2025-05-09  8:22   ` Alexandre Ghiti
  2025-05-08 16:52 ` [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests patchwork-bot+linux-riscv
  5 siblings, 1 reply; 11+ messages in thread
From: Clément Léger @ 2025-04-22 16:23 UTC (permalink / raw)
  To: open list:DOCUMENTATION, open list, open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Shuah Khan,
	Andrew Jones, Samuel Holland

This selftest tests (almost) all the currently emulated instruction
(except for the RV32 compressed ones which are left as a future
exercise for a RV32 user). For the FPU instructions, all the FPU
registers are tested.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 .../selftests/riscv/misaligned/.gitignore     |   1 +
 .../selftests/riscv/misaligned/Makefile       |  12 +
 .../selftests/riscv/misaligned/common.S       |  33 +++
 .../testing/selftests/riscv/misaligned/fpu.S  | 180 +++++++++++++
 tools/testing/selftests/riscv/misaligned/gp.S | 103 +++++++
 .../selftests/riscv/misaligned/misaligned.c   | 254 ++++++++++++++++++
 6 files changed, 583 insertions(+)
 create mode 100644 tools/testing/selftests/riscv/misaligned/.gitignore
 create mode 100644 tools/testing/selftests/riscv/misaligned/Makefile
 create mode 100644 tools/testing/selftests/riscv/misaligned/common.S
 create mode 100644 tools/testing/selftests/riscv/misaligned/fpu.S
 create mode 100644 tools/testing/selftests/riscv/misaligned/gp.S
 create mode 100644 tools/testing/selftests/riscv/misaligned/misaligned.c

diff --git a/tools/testing/selftests/riscv/misaligned/.gitignore b/tools/testing/selftests/riscv/misaligned/.gitignore
new file mode 100644
index 000000000000..5eff15a1f981
--- /dev/null
+++ b/tools/testing/selftests/riscv/misaligned/.gitignore
@@ -0,0 +1 @@
+misaligned
diff --git a/tools/testing/selftests/riscv/misaligned/Makefile b/tools/testing/selftests/riscv/misaligned/Makefile
new file mode 100644
index 000000000000..1aa40110c50d
--- /dev/null
+++ b/tools/testing/selftests/riscv/misaligned/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 ARM Limited
+# Originally tools/testing/arm64/abi/Makefile
+
+CFLAGS += -I$(top_srcdir)/tools/include
+
+TEST_GEN_PROGS := misaligned
+
+include ../../lib.mk
+
+$(OUTPUT)/misaligned: misaligned.c fpu.S gp.S
+	$(CC) -g3 -static -o$@ -march=rv64imafdc $(CFLAGS) $(LDFLAGS) $^
diff --git a/tools/testing/selftests/riscv/misaligned/common.S b/tools/testing/selftests/riscv/misaligned/common.S
new file mode 100644
index 000000000000..8fa00035bd5d
--- /dev/null
+++ b/tools/testing/selftests/riscv/misaligned/common.S
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Rivos Inc.
+ *
+ * Authors:
+ *     Clément Léger <cleger@rivosinc.com>
+ */
+
+.macro lb_sb temp, offset, src, dst
+	lb \temp, \offset(\src)
+	sb \temp, \offset(\dst)
+.endm
+
+.macro copy_long_to temp, src, dst
+	lb_sb \temp, 0, \src, \dst,
+	lb_sb \temp, 1, \src, \dst,
+	lb_sb \temp, 2, \src, \dst,
+	lb_sb \temp, 3, \src, \dst,
+	lb_sb \temp, 4, \src, \dst,
+	lb_sb \temp, 5, \src, \dst,
+	lb_sb \temp, 6, \src, \dst,
+	lb_sb \temp, 7, \src, \dst,
+.endm
+
+.macro sp_stack_prologue offset
+	addi sp, sp, -8
+	sub sp, sp, \offset
+.endm
+
+.macro sp_stack_epilogue offset
+	add sp, sp, \offset
+	addi sp, sp, 8
+.endm
diff --git a/tools/testing/selftests/riscv/misaligned/fpu.S b/tools/testing/selftests/riscv/misaligned/fpu.S
new file mode 100644
index 000000000000..d008bff58310
--- /dev/null
+++ b/tools/testing/selftests/riscv/misaligned/fpu.S
@@ -0,0 +1,180 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Rivos Inc.
+ *
+ * Authors:
+ *     Clément Léger <cleger@rivosinc.com>
+ */
+
+#include "common.S"
+
+#define CASE_ALIGN		4
+
+.macro fpu_load_inst fpreg, inst, precision, load_reg
+.align CASE_ALIGN
+	\inst \fpreg, 0(\load_reg)
+	fmv.\precision fa0, \fpreg
+	j 2f
+.endm
+
+#define flw(__fpreg) fpu_load_inst __fpreg, flw, s, s1
+#define fld(__fpreg) fpu_load_inst __fpreg, fld, d, s1
+#define c_flw(__fpreg) fpu_load_inst __fpreg, c.flw, s, s1
+#define c_fld(__fpreg) fpu_load_inst __fpreg, c.fld, d, s1
+#define c_fldsp(__fpreg) fpu_load_inst __fpreg, c.fldsp, d, sp
+
+.macro fpu_store_inst fpreg, inst, precision, store_reg
+.align CASE_ALIGN
+	fmv.\precision \fpreg, fa0
+	\inst \fpreg, 0(\store_reg)
+	j 2f
+.endm
+
+#define fsw(__fpreg) fpu_store_inst __fpreg, fsw, s, s1
+#define fsd(__fpreg) fpu_store_inst __fpreg, fsd, d, s1
+#define c_fsw(__fpreg) fpu_store_inst __fpreg, c.fsw, s, s1
+#define c_fsd(__fpreg) fpu_store_inst __fpreg, c.fsd, d, s1
+#define c_fsdsp(__fpreg) fpu_store_inst __fpreg, c.fsdsp, d, sp
+
+.macro fp_test_prologue
+	move s1, a1
+	/*
+	 * Compute jump offset to store the correct FP register since we don't
+	 * have indirect FP register access (or at least we don't use this
+	 * extension so that works on all archs)
+	 */
+	sll t0, a0, CASE_ALIGN
+	la t2, 1f
+	add t0, t0, t2
+	jr t0
+.align	CASE_ALIGN
+1:
+.endm
+
+.macro fp_test_prologue_compressed
+	/* FP registers for compressed instructions starts from 8 to 16 */
+	addi a0, a0, -8
+	fp_test_prologue
+.endm
+
+#define fp_test_body_compressed(__inst_func) \
+	__inst_func(f8); \
+	__inst_func(f9); \
+	__inst_func(f10); \
+	__inst_func(f11); \
+	__inst_func(f12); \
+	__inst_func(f13); \
+	__inst_func(f14); \
+	__inst_func(f15); \
+2:
+
+#define fp_test_body(__inst_func) \
+	__inst_func(f0); \
+	__inst_func(f1); \
+	__inst_func(f2); \
+	__inst_func(f3); \
+	__inst_func(f4); \
+	__inst_func(f5); \
+	__inst_func(f6); \
+	__inst_func(f7); \
+	__inst_func(f8); \
+	__inst_func(f9); \
+	__inst_func(f10); \
+	__inst_func(f11); \
+	__inst_func(f12); \
+	__inst_func(f13); \
+	__inst_func(f14); \
+	__inst_func(f15); \
+	__inst_func(f16); \
+	__inst_func(f17); \
+	__inst_func(f18); \
+	__inst_func(f19); \
+	__inst_func(f20); \
+	__inst_func(f21); \
+	__inst_func(f22); \
+	__inst_func(f23); \
+	__inst_func(f24); \
+	__inst_func(f25); \
+	__inst_func(f26); \
+	__inst_func(f27); \
+	__inst_func(f28); \
+	__inst_func(f29); \
+	__inst_func(f30); \
+	__inst_func(f31); \
+2:
+.text
+
+#define __gen_test_inst(__inst, __suffix) \
+.global test_ ## __inst; \
+test_ ## __inst:; \
+	fp_test_prologue ## __suffix; \
+	fp_test_body ## __suffix(__inst); \
+	ret
+
+#define gen_test_inst_compressed(__inst) \
+	.option arch,+c; \
+	__gen_test_inst(c_ ## __inst, _compressed)
+
+#define gen_test_inst(__inst) \
+	.balign 16; \
+	.option push; \
+	.option arch,-c; \
+	__gen_test_inst(__inst, ); \
+	.option pop
+
+.macro fp_test_prologue_load_compressed_sp
+	copy_long_to t0, a1, sp
+.endm
+
+.macro fp_test_epilogue_load_compressed_sp
+.endm
+
+.macro fp_test_prologue_store_compressed_sp
+.endm
+
+.macro fp_test_epilogue_store_compressed_sp
+	copy_long_to t0, sp, a1
+.endm
+
+#define gen_inst_compressed_sp(__inst, __type) \
+	.global test_c_ ## __inst ## sp; \
+	test_c_ ## __inst ## sp:; \
+		sp_stack_prologue a2; \
+		fp_test_prologue_## __type ## _compressed_sp; \
+		fp_test_prologue_compressed; \
+		fp_test_body_compressed(c_ ## __inst ## sp); \
+		fp_test_epilogue_## __type ## _compressed_sp; \
+		sp_stack_epilogue a2; \
+		ret
+
+#define gen_test_load_compressed_sp(__inst) gen_inst_compressed_sp(__inst, load)
+#define gen_test_store_compressed_sp(__inst) gen_inst_compressed_sp(__inst, store)
+
+/*
+ * float_fsw_reg - Set a FP register from a register containing the value
+ * a0 = FP register index to be set
+ * a1 = addr where to store register value
+ * a2 = address offset
+ * a3 = value to be store
+ */
+gen_test_inst(fsw)
+
+/*
+ * float_flw_reg - Get a FP register value and return it
+ * a0 = FP register index to be retrieved
+ * a1 = addr to load register from
+ * a2 = address offset
+ */
+gen_test_inst(flw)
+
+gen_test_inst(fsd)
+#ifdef __riscv_compressed
+gen_test_inst_compressed(fsd)
+gen_test_store_compressed_sp(fsd)
+#endif
+
+gen_test_inst(fld)
+#ifdef __riscv_compressed
+gen_test_inst_compressed(fld)
+gen_test_load_compressed_sp(fld)
+#endif
diff --git a/tools/testing/selftests/riscv/misaligned/gp.S b/tools/testing/selftests/riscv/misaligned/gp.S
new file mode 100644
index 000000000000..f53f4c6d81dd
--- /dev/null
+++ b/tools/testing/selftests/riscv/misaligned/gp.S
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Rivos Inc.
+ *
+ * Authors:
+ *     Clément Léger <cleger@rivosinc.com>
+ */
+
+#include "common.S"
+
+.text
+
+.macro __gen_test_inst inst, src_reg
+	\inst a2, 0(\src_reg)
+	move a0, a2
+.endm
+
+.macro gen_func_header func_name, rvc
+	.option arch,\rvc
+	.global test_\func_name
+	test_\func_name:
+.endm
+
+.macro gen_test_inst inst
+	.option push
+	gen_func_header \inst, -c
+	__gen_test_inst \inst, a0
+	.option pop
+	ret
+.endm
+
+.macro __gen_test_inst_c name, src_reg
+	.option push
+	gen_func_header c_\name, +c
+	 __gen_test_inst c.\name, \src_reg
+	.option pop
+	ret
+.endm
+
+.macro gen_test_inst_c name
+ 	__gen_test_inst_c \name, a0
+.endm
+
+
+.macro gen_test_inst_load_c_sp name
+	.option push
+	gen_func_header c_\name\()sp, +c
+	sp_stack_prologue a1
+	copy_long_to t0, a0, sp
+	c.ldsp a0, 0(sp)
+	sp_stack_epilogue a1
+	.option pop
+	ret
+.endm
+
+.macro lb_sp_sb_a0 reg, offset
+	lb_sb \reg, \offset, sp, a0
+.endm
+
+.macro gen_test_inst_store_c_sp inst_name
+	.option push
+	gen_func_header c_\inst_name\()sp, +c
+	/* Misalign stack pointer */
+	sp_stack_prologue a1
+	/* Misalign access */
+	c.sdsp a2, 0(sp)
+	copy_long_to t0, sp, a0
+	sp_stack_epilogue a1
+	.option pop
+	ret
+.endm
+
+
+ /*
+ * a0 = addr to load from
+ * a1 = address offset
+ * a2 = value to be loaded
+ */
+gen_test_inst lh
+gen_test_inst lhu
+gen_test_inst lw
+gen_test_inst lwu
+gen_test_inst ld
+#ifdef __riscv_compressed
+gen_test_inst_c lw
+gen_test_inst_c ld
+gen_test_inst_load_c_sp ld
+#endif
+
+/*
+ * a0 = addr where to store value
+ * a1 = address offset
+ * a2 = value to be stored
+ */
+gen_test_inst sh
+gen_test_inst sw
+gen_test_inst sd
+#ifdef __riscv_compressed
+gen_test_inst_c sw
+gen_test_inst_c sd
+gen_test_inst_store_c_sp sd
+#endif
+
diff --git a/tools/testing/selftests/riscv/misaligned/misaligned.c b/tools/testing/selftests/riscv/misaligned/misaligned.c
new file mode 100644
index 000000000000..c66aa87ec03e
--- /dev/null
+++ b/tools/testing/selftests/riscv/misaligned/misaligned.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Rivos Inc.
+ *
+ * Authors:
+ *     Clément Léger <cleger@rivosinc.com>
+ */
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <linux/ptrace.h>
+#include "../../kselftest_harness.h"
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <float.h>
+#include <errno.h>
+#include <math.h>
+#include <string.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <inttypes.h>
+#include <ucontext.h>
+
+#include <sys/prctl.h>
+
+#define stringify(s) __stringify(s)
+#define __stringify(s) #s
+
+#define VAL16	0x1234
+#define VAL32	0xDEADBEEF
+#define VAL64	0x45674321D00DF789
+
+#define VAL_float	78951.234375
+#define VAL_double	567890.512396965789589290
+
+static bool float_equal(float a, float b)
+{
+	float scaled_epsilon;
+	float difference = fabsf(a - b);
+
+	// Scale to the largest value.
+	a = fabsf(a);
+	b = fabsf(b);
+	if (a > b)
+		scaled_epsilon = FLT_EPSILON * a;
+	else
+		scaled_epsilon = FLT_EPSILON * b;
+
+	return difference <= scaled_epsilon;
+}
+
+static bool double_equal(double a, double b)
+{
+	double scaled_epsilon;
+	double difference = fabsf(a - b);
+
+	// Scale to the largest value.
+	a = fabs(a);
+	b = fabs(b);
+	if (a > b)
+		scaled_epsilon = DBL_EPSILON * a;
+	else
+		scaled_epsilon = DBL_EPSILON * b;
+
+	return difference <= scaled_epsilon;
+}
+
+#define fpu_load_proto(__inst, __type) \
+extern __type test_ ## __inst(unsigned long fp_reg, void *addr, unsigned long offset, __type value)
+
+fpu_load_proto(flw, float);
+fpu_load_proto(fld, double);
+fpu_load_proto(c_flw, float);
+fpu_load_proto(c_fld, double);
+fpu_load_proto(c_fldsp, double);
+
+#define fpu_store_proto(__inst, __type) \
+extern void test_ ## __inst(unsigned long fp_reg, void *addr, unsigned long offset, __type value)
+
+fpu_store_proto(fsw, float);
+fpu_store_proto(fsd, double);
+fpu_store_proto(c_fsw, float);
+fpu_store_proto(c_fsd, double);
+fpu_store_proto(c_fsdsp, double);
+
+#define gp_load_proto(__inst, __type) \
+extern __type test_ ## __inst(void *addr, unsigned long offset, __type value)
+
+gp_load_proto(lh, uint16_t);
+gp_load_proto(lhu, uint16_t);
+gp_load_proto(lw, uint32_t);
+gp_load_proto(lwu, uint32_t);
+gp_load_proto(ld, uint64_t);
+gp_load_proto(c_lw, uint32_t);
+gp_load_proto(c_ld, uint64_t);
+gp_load_proto(c_ldsp, uint64_t);
+
+#define gp_store_proto(__inst, __type) \
+extern void test_ ## __inst(void *addr, unsigned long offset, __type value)
+
+gp_store_proto(sh, uint16_t);
+gp_store_proto(sw, uint32_t);
+gp_store_proto(sd, uint64_t);
+gp_store_proto(c_sw, uint32_t);
+gp_store_proto(c_sd, uint64_t);
+gp_store_proto(c_sdsp, uint64_t);
+
+#define TEST_GP_LOAD(__inst, __type_size)					\
+TEST(gp_load_ ## __inst)							\
+{										\
+	int offset, ret;							\
+	uint8_t buf[16] __attribute__((aligned(16)));				\
+										\
+	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
+	ASSERT_EQ(ret, 0);							\
+										\
+	for (offset = 1; offset < __type_size / 8; offset++) {			\
+		uint ## __type_size ## _t val = VAL ## __type_size;		\
+		uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t *) (buf + offset); \
+		memcpy(ptr, &val, sizeof(val));					\
+		val = test_ ## __inst(ptr, offset, val);			\
+		EXPECT_EQ(VAL ## __type_size, val);				\
+	}									\
+}
+
+TEST_GP_LOAD(lh, 16);
+TEST_GP_LOAD(lhu, 16);
+TEST_GP_LOAD(lw, 32);
+TEST_GP_LOAD(lwu, 32);
+TEST_GP_LOAD(ld, 64);
+#ifdef __riscv_compressed
+TEST_GP_LOAD(c_lw, 32);
+TEST_GP_LOAD(c_ld, 64);
+TEST_GP_LOAD(c_ldsp, 64);
+#endif
+
+#define TEST_GP_STORE(__inst, __type_size)					\
+TEST(gp_load_ ## __inst)							\
+{										\
+	int offset, ret;							\
+	uint8_t buf[16] __attribute__((aligned(16)));				\
+										\
+	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
+	ASSERT_EQ(ret, 0);							\
+										\
+	for (offset = 1; offset < __type_size / 8; offset++) {			\
+		uint ## __type_size ## _t val = VAL ## __type_size;		\
+		uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t *) (buf + offset); \
+		memset(ptr, 0, sizeof(val));					\
+		test_ ## __inst(ptr, offset, val);				\
+		memcpy(&val, ptr, sizeof(val));					\
+		EXPECT_EQ(VAL ## __type_size, val);				\
+	}									\
+}
+TEST_GP_STORE(sh, 16);
+TEST_GP_STORE(sw, 32);
+TEST_GP_STORE(sd, 64);
+#ifdef __riscv_compressed
+TEST_GP_STORE(c_sw, 32);
+TEST_GP_STORE(c_sd, 64);
+TEST_GP_STORE(c_sdsp, 64);
+#endif
+
+#define __TEST_FPU_LOAD(__type, __inst, __reg_start, __reg_end)			\
+TEST(fpu_load_ ## __inst)							\
+{										\
+	int i, ret, offset, fp_reg;						\
+	uint8_t buf[16] __attribute__((aligned(16)));				\
+										\
+	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
+	ASSERT_EQ(ret, 0);							\
+										\
+	for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) {		\
+		for (offset = 1; offset < 4; offset++) {			\
+			void *load_addr = (buf + offset);			\
+			__type val = VAL_ ## __type ;				\
+										\
+			memcpy(load_addr, &val, sizeof(val));			\
+			val = test_ ## __inst(fp_reg, load_addr, offset, val);	\
+			EXPECT_TRUE(__type ##_equal(val, VAL_## __type));	\
+		}								\
+	}									\
+}
+#define TEST_FPU_LOAD(__type, __inst) \
+	__TEST_FPU_LOAD(__type, __inst, 0, 32)
+#define TEST_FPU_LOAD_COMPRESSED(__type, __inst) \
+	__TEST_FPU_LOAD(__type, __inst, 8, 16)
+
+TEST_FPU_LOAD(float, flw)
+TEST_FPU_LOAD(double, fld)
+#ifdef __riscv_compressed
+TEST_FPU_LOAD_COMPRESSED(double, c_fld)
+TEST_FPU_LOAD_COMPRESSED(double, c_fldsp)
+#endif
+
+#define __TEST_FPU_STORE(__type, __inst, __reg_start, __reg_end)		\
+TEST(fpu_store_ ## __inst)							\
+{										\
+	int i, ret, offset, fp_reg;						\
+	uint8_t buf[16] __attribute__((aligned(16)));				\
+										\
+	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
+	ASSERT_EQ(ret, 0);							\
+										\
+	for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) {		\
+		for (offset = 1; offset < 4; offset++) {			\
+										\
+			void *store_addr = (buf + offset);			\
+			__type val = VAL_ ## __type ;				\
+										\
+			test_ ## __inst(fp_reg, store_addr, offset, val);	\
+			memcpy(&val, store_addr, sizeof(val));			\
+			EXPECT_TRUE(__type ## _equal(val, VAL_## __type));	\
+		}								\
+	}									\
+}
+#define TEST_FPU_STORE(__type, __inst) \
+	__TEST_FPU_STORE(__type, __inst, 0, 32)
+#define TEST_FPU_STORE_COMPRESSED(__type, __inst) \
+	__TEST_FPU_STORE(__type, __inst, 8, 16)
+
+TEST_FPU_STORE(float, fsw)
+TEST_FPU_STORE(double, fsd)
+#ifdef __riscv_compressed
+TEST_FPU_STORE_COMPRESSED(double, c_fsd)
+TEST_FPU_STORE_COMPRESSED(double, c_fsdsp)
+#endif
+
+TEST_SIGNAL(gen_sigbus, SIGBUS)
+{
+	uint32_t *ptr;
+	uint8_t buf[16] __attribute__((aligned(16)));
+	int ret;
+
+	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_SIGBUS);
+	ASSERT_EQ(ret, 0);
+
+	ptr = (uint32_t *)(buf + 1);
+	*ptr = 0xDEADBEEFULL;
+}
+
+int main(int argc, char **argv)
+{
+	int ret, val;
+
+	ret = prctl(PR_GET_UNALIGN, &val);
+	if (ret == -1 && errno == EINVAL)
+		ksft_exit_skip("SKIP GET_UNALIGN_CTL not supported\n");
+
+	exit(test_harness_run(argc, argv));
+}
-- 
2.49.0


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

* Re: [PATCH v2 1/5] riscv: misaligned: factorize trap handling
  2025-04-22 16:23 ` [PATCH v2 1/5] riscv: misaligned: factorize trap handling Clément Léger
@ 2025-05-06 10:58   ` Alexandre Ghiti
  0 siblings, 0 replies; 11+ messages in thread
From: Alexandre Ghiti @ 2025-05-06 10:58 UTC (permalink / raw)
  To: Clément Léger, open list:DOCUMENTATION, open list,
	open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan, Andrew Jones, Samuel Holland

Hi Clément,

On 22/04/2025 18:23, Clément Léger wrote:
> Since both load/store and user/kernel should use almost the same path and
> that we are going to add some code around that, factorize it.
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>   arch/riscv/kernel/traps.c | 66 +++++++++++++++++++++------------------
>   1 file changed, 36 insertions(+), 30 deletions(-)
>
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 8ff8e8b36524..b1d991c78a23 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -198,47 +198,53 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
>   DO_ERROR_INFO(do_trap_load_fault,
>   	SIGSEGV, SEGV_ACCERR, "load access fault");
>   
> -asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs)
> +enum misaligned_access_type {
> +	MISALIGNED_STORE,
> +	MISALIGNED_LOAD,
> +};
> +static const struct {
> +	const char *type_str;
> +	int (*handler)(struct pt_regs *regs);
> +} misaligned_handler[] = {
> +	[MISALIGNED_STORE] = {
> +		.type_str = "Oops - store (or AMO) address misaligned",
> +		.handler = handle_misaligned_store,
> +	},
> +	[MISALIGNED_LOAD] = {
> +		.type_str = "Oops - load address misaligned",
> +		.handler = handle_misaligned_load,
> +	},
> +};
> +
> +static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type)
>   {
> -	if (user_mode(regs)) {
> +	irqentry_state_t state;
> +
> +	if (user_mode(regs))
>   		irqentry_enter_from_user_mode(regs);
> +	else
> +		state = irqentry_nmi_enter(regs);
>   
> -		if (handle_misaligned_load(regs))
> -			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
> -			      "Oops - load address misaligned");
> +	if (misaligned_handler[type].handler(regs))
> +		do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
> +			      misaligned_handler[type].type_str);
>   
> +	if (user_mode(regs))
>   		irqentry_exit_to_user_mode(regs);
> -	} else {
> -		irqentry_state_t state = irqentry_nmi_enter(regs);
> -
> -		if (handle_misaligned_load(regs))
> -			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
> -			      "Oops - load address misaligned");
> -
> +	else
>   		irqentry_nmi_exit(regs, state);
> -	}
>   }
>   
> -asmlinkage __visible __trap_section void do_trap_store_misaligned(struct pt_regs *regs)
> +asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs)
>   {
> -	if (user_mode(regs)) {
> -		irqentry_enter_from_user_mode(regs);
> -
> -		if (handle_misaligned_store(regs))
> -			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
> -				"Oops - store (or AMO) address misaligned");
> -
> -		irqentry_exit_to_user_mode(regs);
> -	} else {
> -		irqentry_state_t state = irqentry_nmi_enter(regs);
> -
> -		if (handle_misaligned_store(regs))
> -			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
> -				"Oops - store (or AMO) address misaligned");
> +	do_trap_misaligned(regs, MISALIGNED_LOAD);
> +}
>   
> -		irqentry_nmi_exit(regs, state);
> -	}
> +asmlinkage __visible __trap_section void do_trap_store_misaligned(struct pt_regs *regs)
> +{
> +	do_trap_misaligned(regs, MISALIGNED_STORE);
>   }
> +
>   DO_ERROR_INFO(do_trap_store_fault,
>   	SIGSEGV, SEGV_ACCERR, "store (or AMO) access fault");
>   DO_ERROR_INFO(do_trap_ecall_s,


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

Thanks,

Alex


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

* Re: [PATCH v2 2/5] riscv: misaligned: enable IRQs while handling misaligned accesses
  2025-04-22 16:23 ` [PATCH v2 2/5] riscv: misaligned: enable IRQs while handling misaligned accesses Clément Léger
@ 2025-05-06 11:07   ` Alexandre Ghiti
  0 siblings, 0 replies; 11+ messages in thread
From: Alexandre Ghiti @ 2025-05-06 11:07 UTC (permalink / raw)
  To: Clément Léger, open list:DOCUMENTATION, open list,
	open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan, Andrew Jones, Samuel Holland

On 22/04/2025 18:23, Clément Léger wrote:
> We can safely reenable IRQs if coming from userspace. This allows to
> access user memory that could potentially trigger a page fault.
>
> Fixes: b686ecdeacf6 ("riscv: misaligned: Restrict user access to kernel memory")
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>   arch/riscv/kernel/traps.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index b1d991c78a23..9c83848797a7 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -220,19 +220,23 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type
>   {
>   	irqentry_state_t state;
>   
> -	if (user_mode(regs))
> +	if (user_mode(regs)) {
>   		irqentry_enter_from_user_mode(regs);
> -	else
> +		local_irq_enable();
> +	} else {
>   		state = irqentry_nmi_enter(regs);
> +	}
>   
>   	if (misaligned_handler[type].handler(regs))
>   		do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
>   			      misaligned_handler[type].type_str);
>   
> -	if (user_mode(regs))
> +	if (user_mode(regs)) {
> +		local_irq_disable();
>   		irqentry_exit_to_user_mode(regs);
> -	else
> +	} else {
>   		irqentry_nmi_exit(regs, state);
> +	}
>   }
>   
>   asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs)


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

Thanks,

Alex


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

* Re: [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests
  2025-04-22 16:23 [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests Clément Léger
                   ` (4 preceding siblings ...)
  2025-04-22 16:23 ` [PATCH v2 5/5] selftests: riscv: add misaligned access testing Clément Léger
@ 2025-05-08 16:52 ` patchwork-bot+linux-riscv
  5 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+linux-riscv @ 2025-05-08 16:52 UTC (permalink / raw)
  To: =?utf-8?b?Q2zDqW1lbnQgTMOpZ2VyIDxjbGVnZXJAcml2b3NpbmMuY29tPg==?=
  Cc: linux-riscv, linux-doc, linux-kernel, linux-kselftest, corbet,
	paul.walmsley, palmer, aou, alex, shuah, ajones, samuel.holland

Hello:

This series was applied to riscv/linux.git (fixes)
by Alexandre Ghiti <alexghiti@rivosinc.com>:

On Tue, 22 Apr 2025 18:23:07 +0200 you wrote:
> This series fixes misaligned access handling when in non interruptible
> context by reenabling interrupts when possible. A previous commit
> changed raw_copy_from_user() with copy_from_user() which enables page
> faulting and thus can sleep. While correct, a warning is now triggered
> due to being called in an invalid context (sleeping in
> non-interruptible). This series fixes that problem by factorizing
> misaligned load/store entry in a single function than reenables
> interrupt if the interrupted context had interrupts enabled.
> In order for misaligned handling problems to be caught sooner, add a
> kselftest for all the currently supported instructions .
> 
> [...]

Here is the summary with links:
  - [v2,1/5] riscv: misaligned: factorize trap handling
    (no matching commit)
  - [v2,2/5] riscv: misaligned: enable IRQs while handling misaligned accesses
    https://git.kernel.org/riscv/c/453805f0a28f
  - [v2,3/5] riscv: misaligned: use get_user() instead of __get_user()
    https://git.kernel.org/riscv/c/897e8aece3c8
  - [v2,4/5] Documentation/sysctl: add riscv to unaligned-trap supported archs
    (no matching commit)
  - [v2,5/5] selftests: riscv: add misaligned access testing
    (no matching commit)

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] 11+ messages in thread

* Re: [PATCH v2 5/5] selftests: riscv: add misaligned access testing
  2025-04-22 16:23 ` [PATCH v2 5/5] selftests: riscv: add misaligned access testing Clément Léger
@ 2025-05-09  8:22   ` Alexandre Ghiti
  2025-05-12  7:18     ` Clément Léger
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandre Ghiti @ 2025-05-09  8:22 UTC (permalink / raw)
  To: Clément Léger, open list:DOCUMENTATION, open list,
	open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan, Andrew Jones, Samuel Holland

Hi Clément,

On 22/04/2025 18:23, Clément Léger wrote:
> This selftest tests (almost) all the currently emulated instruction
> (except for the RV32 compressed ones which are left as a future
> exercise for a RV32 user). For the FPU instructions, all the FPU
> registers are tested.
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>   .../selftests/riscv/misaligned/.gitignore     |   1 +
>   .../selftests/riscv/misaligned/Makefile       |  12 +
>   .../selftests/riscv/misaligned/common.S       |  33 +++
>   .../testing/selftests/riscv/misaligned/fpu.S  | 180 +++++++++++++
>   tools/testing/selftests/riscv/misaligned/gp.S | 103 +++++++
>   .../selftests/riscv/misaligned/misaligned.c   | 254 ++++++++++++++++++
>   6 files changed, 583 insertions(+)
>   create mode 100644 tools/testing/selftests/riscv/misaligned/.gitignore
>   create mode 100644 tools/testing/selftests/riscv/misaligned/Makefile
>   create mode 100644 tools/testing/selftests/riscv/misaligned/common.S
>   create mode 100644 tools/testing/selftests/riscv/misaligned/fpu.S
>   create mode 100644 tools/testing/selftests/riscv/misaligned/gp.S
>   create mode 100644 tools/testing/selftests/riscv/misaligned/misaligned.c
>
> diff --git a/tools/testing/selftests/riscv/misaligned/.gitignore b/tools/testing/selftests/riscv/misaligned/.gitignore
> new file mode 100644
> index 000000000000..5eff15a1f981
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/misaligned/.gitignore
> @@ -0,0 +1 @@
> +misaligned
> diff --git a/tools/testing/selftests/riscv/misaligned/Makefile b/tools/testing/selftests/riscv/misaligned/Makefile
> new file mode 100644
> index 000000000000..1aa40110c50d
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/misaligned/Makefile
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2021 ARM Limited
> +# Originally tools/testing/arm64/abi/Makefile
> +
> +CFLAGS += -I$(top_srcdir)/tools/include
> +
> +TEST_GEN_PROGS := misaligned
> +
> +include ../../lib.mk
> +
> +$(OUTPUT)/misaligned: misaligned.c fpu.S gp.S
> +	$(CC) -g3 -static -o$@ -march=rv64imafdc $(CFLAGS) $(LDFLAGS) $^
> diff --git a/tools/testing/selftests/riscv/misaligned/common.S b/tools/testing/selftests/riscv/misaligned/common.S
> new file mode 100644
> index 000000000000..8fa00035bd5d
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/misaligned/common.S
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Rivos Inc.
> + *
> + * Authors:
> + *     Clément Léger <cleger@rivosinc.com>
> + */
> +
> +.macro lb_sb temp, offset, src, dst
> +	lb \temp, \offset(\src)
> +	sb \temp, \offset(\dst)
> +.endm
> +
> +.macro copy_long_to temp, src, dst
> +	lb_sb \temp, 0, \src, \dst,
> +	lb_sb \temp, 1, \src, \dst,
> +	lb_sb \temp, 2, \src, \dst,
> +	lb_sb \temp, 3, \src, \dst,
> +	lb_sb \temp, 4, \src, \dst,
> +	lb_sb \temp, 5, \src, \dst,
> +	lb_sb \temp, 6, \src, \dst,
> +	lb_sb \temp, 7, \src, \dst,
> +.endm
> +
> +.macro sp_stack_prologue offset
> +	addi sp, sp, -8
> +	sub sp, sp, \offset
> +.endm
> +
> +.macro sp_stack_epilogue offset
> +	add sp, sp, \offset
> +	addi sp, sp, 8
> +.endm
> diff --git a/tools/testing/selftests/riscv/misaligned/fpu.S b/tools/testing/selftests/riscv/misaligned/fpu.S
> new file mode 100644
> index 000000000000..d008bff58310
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/misaligned/fpu.S
> @@ -0,0 +1,180 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Rivos Inc.
> + *
> + * Authors:
> + *     Clément Léger <cleger@rivosinc.com>
> + */
> +
> +#include "common.S"
> +
> +#define CASE_ALIGN		4
> +
> +.macro fpu_load_inst fpreg, inst, precision, load_reg
> +.align CASE_ALIGN
> +	\inst \fpreg, 0(\load_reg)
> +	fmv.\precision fa0, \fpreg
> +	j 2f
> +.endm
> +
> +#define flw(__fpreg) fpu_load_inst __fpreg, flw, s, s1
> +#define fld(__fpreg) fpu_load_inst __fpreg, fld, d, s1
> +#define c_flw(__fpreg) fpu_load_inst __fpreg, c.flw, s, s1
> +#define c_fld(__fpreg) fpu_load_inst __fpreg, c.fld, d, s1
> +#define c_fldsp(__fpreg) fpu_load_inst __fpreg, c.fldsp, d, sp
> +
> +.macro fpu_store_inst fpreg, inst, precision, store_reg
> +.align CASE_ALIGN
> +	fmv.\precision \fpreg, fa0
> +	\inst \fpreg, 0(\store_reg)
> +	j 2f
> +.endm
> +
> +#define fsw(__fpreg) fpu_store_inst __fpreg, fsw, s, s1
> +#define fsd(__fpreg) fpu_store_inst __fpreg, fsd, d, s1
> +#define c_fsw(__fpreg) fpu_store_inst __fpreg, c.fsw, s, s1
> +#define c_fsd(__fpreg) fpu_store_inst __fpreg, c.fsd, d, s1
> +#define c_fsdsp(__fpreg) fpu_store_inst __fpreg, c.fsdsp, d, sp
> +
> +.macro fp_test_prologue
> +	move s1, a1
> +	/*
> +	 * Compute jump offset to store the correct FP register since we don't
> +	 * have indirect FP register access (or at least we don't use this
> +	 * extension so that works on all archs)
> +	 */
> +	sll t0, a0, CASE_ALIGN
> +	la t2, 1f
> +	add t0, t0, t2
> +	jr t0
> +.align	CASE_ALIGN
> +1:
> +.endm
> +
> +.macro fp_test_prologue_compressed
> +	/* FP registers for compressed instructions starts from 8 to 16 */
> +	addi a0, a0, -8
> +	fp_test_prologue
> +.endm
> +
> +#define fp_test_body_compressed(__inst_func) \
> +	__inst_func(f8); \
> +	__inst_func(f9); \
> +	__inst_func(f10); \
> +	__inst_func(f11); \
> +	__inst_func(f12); \
> +	__inst_func(f13); \
> +	__inst_func(f14); \
> +	__inst_func(f15); \
> +2:
> +
> +#define fp_test_body(__inst_func) \
> +	__inst_func(f0); \
> +	__inst_func(f1); \
> +	__inst_func(f2); \
> +	__inst_func(f3); \
> +	__inst_func(f4); \
> +	__inst_func(f5); \
> +	__inst_func(f6); \
> +	__inst_func(f7); \
> +	__inst_func(f8); \
> +	__inst_func(f9); \
> +	__inst_func(f10); \
> +	__inst_func(f11); \
> +	__inst_func(f12); \
> +	__inst_func(f13); \
> +	__inst_func(f14); \
> +	__inst_func(f15); \
> +	__inst_func(f16); \
> +	__inst_func(f17); \
> +	__inst_func(f18); \
> +	__inst_func(f19); \
> +	__inst_func(f20); \
> +	__inst_func(f21); \
> +	__inst_func(f22); \
> +	__inst_func(f23); \
> +	__inst_func(f24); \
> +	__inst_func(f25); \
> +	__inst_func(f26); \
> +	__inst_func(f27); \
> +	__inst_func(f28); \
> +	__inst_func(f29); \
> +	__inst_func(f30); \
> +	__inst_func(f31); \
> +2:
> +.text
> +
> +#define __gen_test_inst(__inst, __suffix) \
> +.global test_ ## __inst; \
> +test_ ## __inst:; \
> +	fp_test_prologue ## __suffix; \
> +	fp_test_body ## __suffix(__inst); \
> +	ret
> +
> +#define gen_test_inst_compressed(__inst) \
> +	.option arch,+c; \
> +	__gen_test_inst(c_ ## __inst, _compressed)
> +
> +#define gen_test_inst(__inst) \
> +	.balign 16; \
> +	.option push; \
> +	.option arch,-c; \
> +	__gen_test_inst(__inst, ); \
> +	.option pop
> +
> +.macro fp_test_prologue_load_compressed_sp
> +	copy_long_to t0, a1, sp
> +.endm
> +
> +.macro fp_test_epilogue_load_compressed_sp
> +.endm
> +
> +.macro fp_test_prologue_store_compressed_sp
> +.endm
> +
> +.macro fp_test_epilogue_store_compressed_sp
> +	copy_long_to t0, sp, a1
> +.endm
> +
> +#define gen_inst_compressed_sp(__inst, __type) \
> +	.global test_c_ ## __inst ## sp; \
> +	test_c_ ## __inst ## sp:; \
> +		sp_stack_prologue a2; \
> +		fp_test_prologue_## __type ## _compressed_sp; \
> +		fp_test_prologue_compressed; \
> +		fp_test_body_compressed(c_ ## __inst ## sp); \
> +		fp_test_epilogue_## __type ## _compressed_sp; \
> +		sp_stack_epilogue a2; \
> +		ret
> +
> +#define gen_test_load_compressed_sp(__inst) gen_inst_compressed_sp(__inst, load)
> +#define gen_test_store_compressed_sp(__inst) gen_inst_compressed_sp(__inst, store)
> +
> +/*
> + * float_fsw_reg - Set a FP register from a register containing the value
> + * a0 = FP register index to be set
> + * a1 = addr where to store register value
> + * a2 = address offset
> + * a3 = value to be store
> + */
> +gen_test_inst(fsw)
> +
> +/*
> + * float_flw_reg - Get a FP register value and return it
> + * a0 = FP register index to be retrieved
> + * a1 = addr to load register from
> + * a2 = address offset
> + */
> +gen_test_inst(flw)
> +
> +gen_test_inst(fsd)
> +#ifdef __riscv_compressed
> +gen_test_inst_compressed(fsd)
> +gen_test_store_compressed_sp(fsd)
> +#endif
> +
> +gen_test_inst(fld)
> +#ifdef __riscv_compressed
> +gen_test_inst_compressed(fld)
> +gen_test_load_compressed_sp(fld)
> +#endif
> diff --git a/tools/testing/selftests/riscv/misaligned/gp.S b/tools/testing/selftests/riscv/misaligned/gp.S
> new file mode 100644
> index 000000000000..f53f4c6d81dd
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/misaligned/gp.S
> @@ -0,0 +1,103 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Rivos Inc.
> + *
> + * Authors:
> + *     Clément Léger <cleger@rivosinc.com>
> + */
> +
> +#include "common.S"
> +
> +.text
> +
> +.macro __gen_test_inst inst, src_reg
> +	\inst a2, 0(\src_reg)
> +	move a0, a2
> +.endm
> +
> +.macro gen_func_header func_name, rvc
> +	.option arch,\rvc
> +	.global test_\func_name
> +	test_\func_name:
> +.endm
> +
> +.macro gen_test_inst inst
> +	.option push
> +	gen_func_header \inst, -c
> +	__gen_test_inst \inst, a0
> +	.option pop
> +	ret
> +.endm
> +
> +.macro __gen_test_inst_c name, src_reg
> +	.option push
> +	gen_func_header c_\name, +c
> +	 __gen_test_inst c.\name, \src_reg
> +	.option pop
> +	ret
> +.endm
> +
> +.macro gen_test_inst_c name
> + 	__gen_test_inst_c \name, a0
> +.endm
> +
> +
> +.macro gen_test_inst_load_c_sp name
> +	.option push
> +	gen_func_header c_\name\()sp, +c
> +	sp_stack_prologue a1
> +	copy_long_to t0, a0, sp
> +	c.ldsp a0, 0(sp)
> +	sp_stack_epilogue a1
> +	.option pop
> +	ret
> +.endm
> +
> +.macro lb_sp_sb_a0 reg, offset
> +	lb_sb \reg, \offset, sp, a0
> +.endm
> +
> +.macro gen_test_inst_store_c_sp inst_name
> +	.option push
> +	gen_func_header c_\inst_name\()sp, +c
> +	/* Misalign stack pointer */
> +	sp_stack_prologue a1
> +	/* Misalign access */
> +	c.sdsp a2, 0(sp)
> +	copy_long_to t0, sp, a0
> +	sp_stack_epilogue a1
> +	.option pop
> +	ret
> +.endm
> +
> +
> + /*
> + * a0 = addr to load from
> + * a1 = address offset
> + * a2 = value to be loaded
> + */
> +gen_test_inst lh
> +gen_test_inst lhu
> +gen_test_inst lw
> +gen_test_inst lwu
> +gen_test_inst ld
> +#ifdef __riscv_compressed
> +gen_test_inst_c lw
> +gen_test_inst_c ld
> +gen_test_inst_load_c_sp ld
> +#endif
> +
> +/*
> + * a0 = addr where to store value
> + * a1 = address offset
> + * a2 = value to be stored
> + */
> +gen_test_inst sh
> +gen_test_inst sw
> +gen_test_inst sd
> +#ifdef __riscv_compressed
> +gen_test_inst_c sw
> +gen_test_inst_c sd
> +gen_test_inst_store_c_sp sd
> +#endif
> +
> diff --git a/tools/testing/selftests/riscv/misaligned/misaligned.c b/tools/testing/selftests/riscv/misaligned/misaligned.c
> new file mode 100644
> index 000000000000..c66aa87ec03e
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/misaligned/misaligned.c
> @@ -0,0 +1,254 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Rivos Inc.
> + *
> + * Authors:
> + *     Clément Léger <cleger@rivosinc.com>
> + */
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <linux/ptrace.h>
> +#include "../../kselftest_harness.h"
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <float.h>
> +#include <errno.h>
> +#include <math.h>
> +#include <string.h>
> +#include <signal.h>
> +#include <stdbool.h>
> +#include <unistd.h>
> +#include <inttypes.h>
> +#include <ucontext.h>
> +
> +#include <sys/prctl.h>
> +
> +#define stringify(s) __stringify(s)
> +#define __stringify(s) #s
> +
> +#define VAL16	0x1234
> +#define VAL32	0xDEADBEEF
> +#define VAL64	0x45674321D00DF789
> +
> +#define VAL_float	78951.234375
> +#define VAL_double	567890.512396965789589290
> +
> +static bool float_equal(float a, float b)
> +{
> +	float scaled_epsilon;
> +	float difference = fabsf(a - b);
> +
> +	// Scale to the largest value.
> +	a = fabsf(a);
> +	b = fabsf(b);
> +	if (a > b)
> +		scaled_epsilon = FLT_EPSILON * a;
> +	else
> +		scaled_epsilon = FLT_EPSILON * b;
> +
> +	return difference <= scaled_epsilon;
> +}
> +
> +static bool double_equal(double a, double b)
> +{
> +	double scaled_epsilon;
> +	double difference = fabsf(a - b);
> +
> +	// Scale to the largest value.
> +	a = fabs(a);
> +	b = fabs(b);
> +	if (a > b)
> +		scaled_epsilon = DBL_EPSILON * a;
> +	else
> +		scaled_epsilon = DBL_EPSILON * b;
> +
> +	return difference <= scaled_epsilon;
> +}
> +
> +#define fpu_load_proto(__inst, __type) \
> +extern __type test_ ## __inst(unsigned long fp_reg, void *addr, unsigned long offset, __type value)
> +
> +fpu_load_proto(flw, float);
> +fpu_load_proto(fld, double);
> +fpu_load_proto(c_flw, float);
> +fpu_load_proto(c_fld, double);
> +fpu_load_proto(c_fldsp, double);
> +
> +#define fpu_store_proto(__inst, __type) \
> +extern void test_ ## __inst(unsigned long fp_reg, void *addr, unsigned long offset, __type value)
> +
> +fpu_store_proto(fsw, float);
> +fpu_store_proto(fsd, double);
> +fpu_store_proto(c_fsw, float);
> +fpu_store_proto(c_fsd, double);
> +fpu_store_proto(c_fsdsp, double);
> +
> +#define gp_load_proto(__inst, __type) \
> +extern __type test_ ## __inst(void *addr, unsigned long offset, __type value)
> +
> +gp_load_proto(lh, uint16_t);
> +gp_load_proto(lhu, uint16_t);
> +gp_load_proto(lw, uint32_t);
> +gp_load_proto(lwu, uint32_t);
> +gp_load_proto(ld, uint64_t);
> +gp_load_proto(c_lw, uint32_t);
> +gp_load_proto(c_ld, uint64_t);
> +gp_load_proto(c_ldsp, uint64_t);
> +
> +#define gp_store_proto(__inst, __type) \
> +extern void test_ ## __inst(void *addr, unsigned long offset, __type value)
> +
> +gp_store_proto(sh, uint16_t);
> +gp_store_proto(sw, uint32_t);
> +gp_store_proto(sd, uint64_t);
> +gp_store_proto(c_sw, uint32_t);
> +gp_store_proto(c_sd, uint64_t);
> +gp_store_proto(c_sdsp, uint64_t);
> +
> +#define TEST_GP_LOAD(__inst, __type_size)					\
> +TEST(gp_load_ ## __inst)							\
> +{										\
> +	int offset, ret;							\
> +	uint8_t buf[16] __attribute__((aligned(16)));				\
> +										\
> +	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
> +	ASSERT_EQ(ret, 0);							\
> +										\
> +	for (offset = 1; offset < __type_size / 8; offset++) {			\
> +		uint ## __type_size ## _t val = VAL ## __type_size;		\
> +		uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t *) (buf + offset); \
> +		memcpy(ptr, &val, sizeof(val));					\
> +		val = test_ ## __inst(ptr, offset, val);			\
> +		EXPECT_EQ(VAL ## __type_size, val);				\
> +	}									\
> +}
> +
> +TEST_GP_LOAD(lh, 16);
> +TEST_GP_LOAD(lhu, 16);
> +TEST_GP_LOAD(lw, 32);
> +TEST_GP_LOAD(lwu, 32);
> +TEST_GP_LOAD(ld, 64);
> +#ifdef __riscv_compressed
> +TEST_GP_LOAD(c_lw, 32);
> +TEST_GP_LOAD(c_ld, 64);
> +TEST_GP_LOAD(c_ldsp, 64);
> +#endif
> +
> +#define TEST_GP_STORE(__inst, __type_size)					\
> +TEST(gp_load_ ## __inst)							\
> +{										\
> +	int offset, ret;							\
> +	uint8_t buf[16] __attribute__((aligned(16)));				\
> +										\
> +	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
> +	ASSERT_EQ(ret, 0);							\
> +										\
> +	for (offset = 1; offset < __type_size / 8; offset++) {			\
> +		uint ## __type_size ## _t val = VAL ## __type_size;		\
> +		uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t *) (buf + offset); \
> +		memset(ptr, 0, sizeof(val));					\
> +		test_ ## __inst(ptr, offset, val);				\
> +		memcpy(&val, ptr, sizeof(val));					\
> +		EXPECT_EQ(VAL ## __type_size, val);				\
> +	}									\
> +}
> +TEST_GP_STORE(sh, 16);
> +TEST_GP_STORE(sw, 32);
> +TEST_GP_STORE(sd, 64);
> +#ifdef __riscv_compressed
> +TEST_GP_STORE(c_sw, 32);
> +TEST_GP_STORE(c_sd, 64);
> +TEST_GP_STORE(c_sdsp, 64);
> +#endif
> +
> +#define __TEST_FPU_LOAD(__type, __inst, __reg_start, __reg_end)			\
> +TEST(fpu_load_ ## __inst)							\
> +{										\
> +	int i, ret, offset, fp_reg;						\
> +	uint8_t buf[16] __attribute__((aligned(16)));				\
> +										\
> +	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
> +	ASSERT_EQ(ret, 0);							\
> +										\
> +	for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) {		\
> +		for (offset = 1; offset < 4; offset++) {			\
> +			void *load_addr = (buf + offset);			\
> +			__type val = VAL_ ## __type ;				\
> +										\
> +			memcpy(load_addr, &val, sizeof(val));			\
> +			val = test_ ## __inst(fp_reg, load_addr, offset, val);	\
> +			EXPECT_TRUE(__type ##_equal(val, VAL_## __type));	\
> +		}								\
> +	}									\
> +}
> +#define TEST_FPU_LOAD(__type, __inst) \
> +	__TEST_FPU_LOAD(__type, __inst, 0, 32)
> +#define TEST_FPU_LOAD_COMPRESSED(__type, __inst) \
> +	__TEST_FPU_LOAD(__type, __inst, 8, 16)
> +
> +TEST_FPU_LOAD(float, flw)
> +TEST_FPU_LOAD(double, fld)
> +#ifdef __riscv_compressed
> +TEST_FPU_LOAD_COMPRESSED(double, c_fld)
> +TEST_FPU_LOAD_COMPRESSED(double, c_fldsp)
> +#endif
> +
> +#define __TEST_FPU_STORE(__type, __inst, __reg_start, __reg_end)		\
> +TEST(fpu_store_ ## __inst)							\
> +{										\
> +	int i, ret, offset, fp_reg;						\
> +	uint8_t buf[16] __attribute__((aligned(16)));				\
> +										\
> +	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);			\
> +	ASSERT_EQ(ret, 0);							\
> +										\
> +	for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) {		\
> +		for (offset = 1; offset < 4; offset++) {			\
> +										\
> +			void *store_addr = (buf + offset);			\
> +			__type val = VAL_ ## __type ;				\
> +										\
> +			test_ ## __inst(fp_reg, store_addr, offset, val);	\
> +			memcpy(&val, store_addr, sizeof(val));			\
> +			EXPECT_TRUE(__type ## _equal(val, VAL_## __type));	\
> +		}								\
> +	}									\
> +}
> +#define TEST_FPU_STORE(__type, __inst) \
> +	__TEST_FPU_STORE(__type, __inst, 0, 32)
> +#define TEST_FPU_STORE_COMPRESSED(__type, __inst) \
> +	__TEST_FPU_STORE(__type, __inst, 8, 16)
> +
> +TEST_FPU_STORE(float, fsw)
> +TEST_FPU_STORE(double, fsd)
> +#ifdef __riscv_compressed
> +TEST_FPU_STORE_COMPRESSED(double, c_fsd)
> +TEST_FPU_STORE_COMPRESSED(double, c_fsdsp)
> +#endif
> +
> +TEST_SIGNAL(gen_sigbus, SIGBUS)
> +{
> +	uint32_t *ptr;
> +	uint8_t buf[16] __attribute__((aligned(16)));
> +	int ret;
> +
> +	ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_SIGBUS);
> +	ASSERT_EQ(ret, 0);
> +
> +	ptr = (uint32_t *)(buf + 1);
> +	*ptr = 0xDEADBEEFULL;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int ret, val;
> +
> +	ret = prctl(PR_GET_UNALIGN, &val);
> +	if (ret == -1 && errno == EINVAL)
> +		ksft_exit_skip("SKIP GET_UNALIGN_CTL not supported\n");
> +
> +	exit(test_harness_run(argc, argv));
> +}


So I had to add the following to actually compile this selftest along 
with other riscv tests and fix some warnings:

diff --git a/tools/testing/selftests/riscv/Makefile 
b/tools/testing/selftests/riscv/Makefile
index 099b8c1f46f89..95a98ceeb3b3a 100644
--- a/tools/testing/selftests/riscv/Makefile
+++ b/tools/testing/selftests/riscv/Makefile
@@ -5,7 +5,7 @@
  ARCH ?= $(shell uname -m 2>/dev/null || echo not)

  ifneq (,$(filter $(ARCH),riscv))
-RISCV_SUBTARGETS ?= abi hwprobe mm sigreturn vector
+RISCV_SUBTARGETS ?= abi hwprobe mm sigreturn vector misaligned
  else
  RISCV_SUBTARGETS :=
  endif
diff --git a/tools/testing/selftests/riscv/misaligned/misaligned.c 
b/tools/testing/selftests/riscv/misaligned/misaligned.c
index c66aa87ec03ec..8fa5ad1a93d17 100644
--- a/tools/testing/selftests/riscv/misaligned/misaligned.c
+++ b/tools/testing/selftests/riscv/misaligned/misaligned.c
@@ -167,7 +167,7 @@ TEST_GP_STORE(c_sdsp, 64);
  #define __TEST_FPU_LOAD(__type, __inst, __reg_start, 
__reg_end)                             \
  TEST(fpu_load_ ## 
__inst)                                                      \
  { \
-       int i, ret, offset, 
fp_reg;                                             \
+       int ret, offset, 
fp_reg;                                                \
         uint8_t buf[16] 
__attribute__((aligned(16)));                           \
\
         ret = prctl(PR_SET_UNALIGN, 
PR_UNALIGN_NOPRINT);                        \
@@ -199,7 +199,7 @@ TEST_FPU_LOAD_COMPRESSED(double, c_fldsp)
  #define __TEST_FPU_STORE(__type, __inst, __reg_start, 
__reg_end)               \
  TEST(fpu_store_ ## 
__inst)                                                     \
  { \
-       int i, ret, offset, 
fp_reg;                                             \
+       int ret, offset, 
fp_reg;                                                \
         uint8_t buf[16] 
__attribute__((aligned(16)));                           \
\
         ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);


I already merged the first 3 commits of this patchset in fixes, so can 
you resend only the last 2 patches with the fixes above?

Thanks,

Alex



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

* Re: [PATCH v2 5/5] selftests: riscv: add misaligned access testing
  2025-05-09  8:22   ` Alexandre Ghiti
@ 2025-05-12  7:18     ` Clément Léger
  0 siblings, 0 replies; 11+ messages in thread
From: Clément Léger @ 2025-05-12  7:18 UTC (permalink / raw)
  To: Alexandre Ghiti, open list:DOCUMENTATION, open list,
	open list:RISC-V ARCHITECTURE,
	open list:KERNEL SELFTEST FRAMEWORK
  Cc: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan, Andrew Jones, Samuel Holland



On 09/05/2025 10:22, Alexandre Ghiti wrote:
> Hi Clément,
> 
> On 22/04/2025 18:23, Clément Léger wrote:
>> This selftest tests (almost) all the currently emulated instruction
>> (except for the RV32 compressed ones which are left as a future
>> exercise for a RV32 user). For the FPU instructions, all the FPU
>> registers are tested.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>>   .../selftests/riscv/misaligned/.gitignore     |   1 +
>>   .../selftests/riscv/misaligned/Makefile       |  12 +
>>   .../selftests/riscv/misaligned/common.S       |  33 +++
>>   .../testing/selftests/riscv/misaligned/fpu.S  | 180 +++++++++++++
>>   tools/testing/selftests/riscv/misaligned/gp.S | 103 +++++++
>>   .../selftests/riscv/misaligned/misaligned.c   | 254 ++++++++++++++++++
>>   6 files changed, 583 insertions(+)
>>   create mode 100644 tools/testing/selftests/riscv/misaligned/.gitignore
>>   create mode 100644 tools/testing/selftests/riscv/misaligned/Makefile
>>   create mode 100644 tools/testing/selftests/riscv/misaligned/common.S
>>   create mode 100644 tools/testing/selftests/riscv/misaligned/fpu.S
>>   create mode 100644 tools/testing/selftests/riscv/misaligned/gp.S
>>   create mode 100644 tools/testing/selftests/riscv/misaligned/
>> misaligned.c
>>
>> diff --git a/tools/testing/selftests/riscv/misaligned/.gitignore b/
>> tools/testing/selftests/riscv/misaligned/.gitignore
>> new file mode 100644
>> index 000000000000..5eff15a1f981
>> --- /dev/null
>> +++ b/tools/testing/selftests/riscv/misaligned/.gitignore
>> @@ -0,0 +1 @@
>> +misaligned
>> diff --git a/tools/testing/selftests/riscv/misaligned/Makefile b/
>> tools/testing/selftests/riscv/misaligned/Makefile
>> new file mode 100644
>> index 000000000000..1aa40110c50d
>> --- /dev/null
>> +++ b/tools/testing/selftests/riscv/misaligned/Makefile
>> @@ -0,0 +1,12 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2021 ARM Limited
>> +# Originally tools/testing/arm64/abi/Makefile
>> +
>> +CFLAGS += -I$(top_srcdir)/tools/include
>> +
>> +TEST_GEN_PROGS := misaligned
>> +
>> +include ../../lib.mk
>> +
>> +$(OUTPUT)/misaligned: misaligned.c fpu.S gp.S
>> +    $(CC) -g3 -static -o$@ -march=rv64imafdc $(CFLAGS) $(LDFLAGS) $^
>> diff --git a/tools/testing/selftests/riscv/misaligned/common.S b/
>> tools/testing/selftests/riscv/misaligned/common.S
>> new file mode 100644
>> index 000000000000..8fa00035bd5d
>> --- /dev/null
>> +++ b/tools/testing/selftests/riscv/misaligned/common.S
>> @@ -0,0 +1,33 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2025 Rivos Inc.
>> + *
>> + * Authors:
>> + *     Clément Léger <cleger@rivosinc.com>
>> + */
>> +
>> +.macro lb_sb temp, offset, src, dst
>> +    lb \temp, \offset(\src)
>> +    sb \temp, \offset(\dst)
>> +.endm
>> +
>> +.macro copy_long_to temp, src, dst
>> +    lb_sb \temp, 0, \src, \dst,
>> +    lb_sb \temp, 1, \src, \dst,
>> +    lb_sb \temp, 2, \src, \dst,
>> +    lb_sb \temp, 3, \src, \dst,
>> +    lb_sb \temp, 4, \src, \dst,
>> +    lb_sb \temp, 5, \src, \dst,
>> +    lb_sb \temp, 6, \src, \dst,
>> +    lb_sb \temp, 7, \src, \dst,
>> +.endm
>> +
>> +.macro sp_stack_prologue offset
>> +    addi sp, sp, -8
>> +    sub sp, sp, \offset
>> +.endm
>> +
>> +.macro sp_stack_epilogue offset
>> +    add sp, sp, \offset
>> +    addi sp, sp, 8
>> +.endm
>> diff --git a/tools/testing/selftests/riscv/misaligned/fpu.S b/tools/
>> testing/selftests/riscv/misaligned/fpu.S
>> new file mode 100644
>> index 000000000000..d008bff58310
>> --- /dev/null
>> +++ b/tools/testing/selftests/riscv/misaligned/fpu.S
>> @@ -0,0 +1,180 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2025 Rivos Inc.
>> + *
>> + * Authors:
>> + *     Clément Léger <cleger@rivosinc.com>
>> + */
>> +
>> +#include "common.S"
>> +
>> +#define CASE_ALIGN        4
>> +
>> +.macro fpu_load_inst fpreg, inst, precision, load_reg
>> +.align CASE_ALIGN
>> +    \inst \fpreg, 0(\load_reg)
>> +    fmv.\precision fa0, \fpreg
>> +    j 2f
>> +.endm
>> +
>> +#define flw(__fpreg) fpu_load_inst __fpreg, flw, s, s1
>> +#define fld(__fpreg) fpu_load_inst __fpreg, fld, d, s1
>> +#define c_flw(__fpreg) fpu_load_inst __fpreg, c.flw, s, s1
>> +#define c_fld(__fpreg) fpu_load_inst __fpreg, c.fld, d, s1
>> +#define c_fldsp(__fpreg) fpu_load_inst __fpreg, c.fldsp, d, sp
>> +
>> +.macro fpu_store_inst fpreg, inst, precision, store_reg
>> +.align CASE_ALIGN
>> +    fmv.\precision \fpreg, fa0
>> +    \inst \fpreg, 0(\store_reg)
>> +    j 2f
>> +.endm
>> +
>> +#define fsw(__fpreg) fpu_store_inst __fpreg, fsw, s, s1
>> +#define fsd(__fpreg) fpu_store_inst __fpreg, fsd, d, s1
>> +#define c_fsw(__fpreg) fpu_store_inst __fpreg, c.fsw, s, s1
>> +#define c_fsd(__fpreg) fpu_store_inst __fpreg, c.fsd, d, s1
>> +#define c_fsdsp(__fpreg) fpu_store_inst __fpreg, c.fsdsp, d, sp
>> +
>> +.macro fp_test_prologue
>> +    move s1, a1
>> +    /*
>> +     * Compute jump offset to store the correct FP register since we
>> don't
>> +     * have indirect FP register access (or at least we don't use this
>> +     * extension so that works on all archs)
>> +     */
>> +    sll t0, a0, CASE_ALIGN
>> +    la t2, 1f
>> +    add t0, t0, t2
>> +    jr t0
>> +.align    CASE_ALIGN
>> +1:
>> +.endm
>> +
>> +.macro fp_test_prologue_compressed
>> +    /* FP registers for compressed instructions starts from 8 to 16 */
>> +    addi a0, a0, -8
>> +    fp_test_prologue
>> +.endm
>> +
>> +#define fp_test_body_compressed(__inst_func) \
>> +    __inst_func(f8); \
>> +    __inst_func(f9); \
>> +    __inst_func(f10); \
>> +    __inst_func(f11); \
>> +    __inst_func(f12); \
>> +    __inst_func(f13); \
>> +    __inst_func(f14); \
>> +    __inst_func(f15); \
>> +2:
>> +
>> +#define fp_test_body(__inst_func) \
>> +    __inst_func(f0); \
>> +    __inst_func(f1); \
>> +    __inst_func(f2); \
>> +    __inst_func(f3); \
>> +    __inst_func(f4); \
>> +    __inst_func(f5); \
>> +    __inst_func(f6); \
>> +    __inst_func(f7); \
>> +    __inst_func(f8); \
>> +    __inst_func(f9); \
>> +    __inst_func(f10); \
>> +    __inst_func(f11); \
>> +    __inst_func(f12); \
>> +    __inst_func(f13); \
>> +    __inst_func(f14); \
>> +    __inst_func(f15); \
>> +    __inst_func(f16); \
>> +    __inst_func(f17); \
>> +    __inst_func(f18); \
>> +    __inst_func(f19); \
>> +    __inst_func(f20); \
>> +    __inst_func(f21); \
>> +    __inst_func(f22); \
>> +    __inst_func(f23); \
>> +    __inst_func(f24); \
>> +    __inst_func(f25); \
>> +    __inst_func(f26); \
>> +    __inst_func(f27); \
>> +    __inst_func(f28); \
>> +    __inst_func(f29); \
>> +    __inst_func(f30); \
>> +    __inst_func(f31); \
>> +2:
>> +.text
>> +
>> +#define __gen_test_inst(__inst, __suffix) \
>> +.global test_ ## __inst; \
>> +test_ ## __inst:; \
>> +    fp_test_prologue ## __suffix; \
>> +    fp_test_body ## __suffix(__inst); \
>> +    ret
>> +
>> +#define gen_test_inst_compressed(__inst) \
>> +    .option arch,+c; \
>> +    __gen_test_inst(c_ ## __inst, _compressed)
>> +
>> +#define gen_test_inst(__inst) \
>> +    .balign 16; \
>> +    .option push; \
>> +    .option arch,-c; \
>> +    __gen_test_inst(__inst, ); \
>> +    .option pop
>> +
>> +.macro fp_test_prologue_load_compressed_sp
>> +    copy_long_to t0, a1, sp
>> +.endm
>> +
>> +.macro fp_test_epilogue_load_compressed_sp
>> +.endm
>> +
>> +.macro fp_test_prologue_store_compressed_sp
>> +.endm
>> +
>> +.macro fp_test_epilogue_store_compressed_sp
>> +    copy_long_to t0, sp, a1
>> +.endm
>> +
>> +#define gen_inst_compressed_sp(__inst, __type) \
>> +    .global test_c_ ## __inst ## sp; \
>> +    test_c_ ## __inst ## sp:; \
>> +        sp_stack_prologue a2; \
>> +        fp_test_prologue_## __type ## _compressed_sp; \
>> +        fp_test_prologue_compressed; \
>> +        fp_test_body_compressed(c_ ## __inst ## sp); \
>> +        fp_test_epilogue_## __type ## _compressed_sp; \
>> +        sp_stack_epilogue a2; \
>> +        ret
>> +
>> +#define gen_test_load_compressed_sp(__inst)
>> gen_inst_compressed_sp(__inst, load)
>> +#define gen_test_store_compressed_sp(__inst)
>> gen_inst_compressed_sp(__inst, store)
>> +
>> +/*
>> + * float_fsw_reg - Set a FP register from a register containing the
>> value
>> + * a0 = FP register index to be set
>> + * a1 = addr where to store register value
>> + * a2 = address offset
>> + * a3 = value to be store
>> + */
>> +gen_test_inst(fsw)
>> +
>> +/*
>> + * float_flw_reg - Get a FP register value and return it
>> + * a0 = FP register index to be retrieved
>> + * a1 = addr to load register from
>> + * a2 = address offset
>> + */
>> +gen_test_inst(flw)
>> +
>> +gen_test_inst(fsd)
>> +#ifdef __riscv_compressed
>> +gen_test_inst_compressed(fsd)
>> +gen_test_store_compressed_sp(fsd)
>> +#endif
>> +
>> +gen_test_inst(fld)
>> +#ifdef __riscv_compressed
>> +gen_test_inst_compressed(fld)
>> +gen_test_load_compressed_sp(fld)
>> +#endif
>> diff --git a/tools/testing/selftests/riscv/misaligned/gp.S b/tools/
>> testing/selftests/riscv/misaligned/gp.S
>> new file mode 100644
>> index 000000000000..f53f4c6d81dd
>> --- /dev/null
>> +++ b/tools/testing/selftests/riscv/misaligned/gp.S
>> @@ -0,0 +1,103 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2025 Rivos Inc.
>> + *
>> + * Authors:
>> + *     Clément Léger <cleger@rivosinc.com>
>> + */
>> +
>> +#include "common.S"
>> +
>> +.text
>> +
>> +.macro __gen_test_inst inst, src_reg
>> +    \inst a2, 0(\src_reg)
>> +    move a0, a2
>> +.endm
>> +
>> +.macro gen_func_header func_name, rvc
>> +    .option arch,\rvc
>> +    .global test_\func_name
>> +    test_\func_name:
>> +.endm
>> +
>> +.macro gen_test_inst inst
>> +    .option push
>> +    gen_func_header \inst, -c
>> +    __gen_test_inst \inst, a0
>> +    .option pop
>> +    ret
>> +.endm
>> +
>> +.macro __gen_test_inst_c name, src_reg
>> +    .option push
>> +    gen_func_header c_\name, +c
>> +     __gen_test_inst c.\name, \src_reg
>> +    .option pop
>> +    ret
>> +.endm
>> +
>> +.macro gen_test_inst_c name
>> +     __gen_test_inst_c \name, a0
>> +.endm
>> +
>> +
>> +.macro gen_test_inst_load_c_sp name
>> +    .option push
>> +    gen_func_header c_\name\()sp, +c
>> +    sp_stack_prologue a1
>> +    copy_long_to t0, a0, sp
>> +    c.ldsp a0, 0(sp)
>> +    sp_stack_epilogue a1
>> +    .option pop
>> +    ret
>> +.endm
>> +
>> +.macro lb_sp_sb_a0 reg, offset
>> +    lb_sb \reg, \offset, sp, a0
>> +.endm
>> +
>> +.macro gen_test_inst_store_c_sp inst_name
>> +    .option push
>> +    gen_func_header c_\inst_name\()sp, +c
>> +    /* Misalign stack pointer */
>> +    sp_stack_prologue a1
>> +    /* Misalign access */
>> +    c.sdsp a2, 0(sp)
>> +    copy_long_to t0, sp, a0
>> +    sp_stack_epilogue a1
>> +    .option pop
>> +    ret
>> +.endm
>> +
>> +
>> + /*
>> + * a0 = addr to load from
>> + * a1 = address offset
>> + * a2 = value to be loaded
>> + */
>> +gen_test_inst lh
>> +gen_test_inst lhu
>> +gen_test_inst lw
>> +gen_test_inst lwu
>> +gen_test_inst ld
>> +#ifdef __riscv_compressed
>> +gen_test_inst_c lw
>> +gen_test_inst_c ld
>> +gen_test_inst_load_c_sp ld
>> +#endif
>> +
>> +/*
>> + * a0 = addr where to store value
>> + * a1 = address offset
>> + * a2 = value to be stored
>> + */
>> +gen_test_inst sh
>> +gen_test_inst sw
>> +gen_test_inst sd
>> +#ifdef __riscv_compressed
>> +gen_test_inst_c sw
>> +gen_test_inst_c sd
>> +gen_test_inst_store_c_sp sd
>> +#endif
>> +
>> diff --git a/tools/testing/selftests/riscv/misaligned/misaligned.c b/
>> tools/testing/selftests/riscv/misaligned/misaligned.c
>> new file mode 100644
>> index 000000000000..c66aa87ec03e
>> --- /dev/null
>> +++ b/tools/testing/selftests/riscv/misaligned/misaligned.c
>> @@ -0,0 +1,254 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2025 Rivos Inc.
>> + *
>> + * Authors:
>> + *     Clément Léger <cleger@rivosinc.com>
>> + */
>> +#include <signal.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <linux/ptrace.h>
>> +#include "../../kselftest_harness.h"
>> +
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <stdint.h>
>> +#include <float.h>
>> +#include <errno.h>
>> +#include <math.h>
>> +#include <string.h>
>> +#include <signal.h>
>> +#include <stdbool.h>
>> +#include <unistd.h>
>> +#include <inttypes.h>
>> +#include <ucontext.h>
>> +
>> +#include <sys/prctl.h>
>> +
>> +#define stringify(s) __stringify(s)
>> +#define __stringify(s) #s
>> +
>> +#define VAL16    0x1234
>> +#define VAL32    0xDEADBEEF
>> +#define VAL64    0x45674321D00DF789
>> +
>> +#define VAL_float    78951.234375
>> +#define VAL_double    567890.512396965789589290
>> +
>> +static bool float_equal(float a, float b)
>> +{
>> +    float scaled_epsilon;
>> +    float difference = fabsf(a - b);
>> +
>> +    // Scale to the largest value.
>> +    a = fabsf(a);
>> +    b = fabsf(b);
>> +    if (a > b)
>> +        scaled_epsilon = FLT_EPSILON * a;
>> +    else
>> +        scaled_epsilon = FLT_EPSILON * b;
>> +
>> +    return difference <= scaled_epsilon;
>> +}
>> +
>> +static bool double_equal(double a, double b)
>> +{
>> +    double scaled_epsilon;
>> +    double difference = fabsf(a - b);
>> +
>> +    // Scale to the largest value.
>> +    a = fabs(a);
>> +    b = fabs(b);
>> +    if (a > b)
>> +        scaled_epsilon = DBL_EPSILON * a;
>> +    else
>> +        scaled_epsilon = DBL_EPSILON * b;
>> +
>> +    return difference <= scaled_epsilon;
>> +}
>> +
>> +#define fpu_load_proto(__inst, __type) \
>> +extern __type test_ ## __inst(unsigned long fp_reg, void *addr,
>> unsigned long offset, __type value)
>> +
>> +fpu_load_proto(flw, float);
>> +fpu_load_proto(fld, double);
>> +fpu_load_proto(c_flw, float);
>> +fpu_load_proto(c_fld, double);
>> +fpu_load_proto(c_fldsp, double);
>> +
>> +#define fpu_store_proto(__inst, __type) \
>> +extern void test_ ## __inst(unsigned long fp_reg, void *addr,
>> unsigned long offset, __type value)
>> +
>> +fpu_store_proto(fsw, float);
>> +fpu_store_proto(fsd, double);
>> +fpu_store_proto(c_fsw, float);
>> +fpu_store_proto(c_fsd, double);
>> +fpu_store_proto(c_fsdsp, double);
>> +
>> +#define gp_load_proto(__inst, __type) \
>> +extern __type test_ ## __inst(void *addr, unsigned long offset,
>> __type value)
>> +
>> +gp_load_proto(lh, uint16_t);
>> +gp_load_proto(lhu, uint16_t);
>> +gp_load_proto(lw, uint32_t);
>> +gp_load_proto(lwu, uint32_t);
>> +gp_load_proto(ld, uint64_t);
>> +gp_load_proto(c_lw, uint32_t);
>> +gp_load_proto(c_ld, uint64_t);
>> +gp_load_proto(c_ldsp, uint64_t);
>> +
>> +#define gp_store_proto(__inst, __type) \
>> +extern void test_ ## __inst(void *addr, unsigned long offset, __type
>> value)
>> +
>> +gp_store_proto(sh, uint16_t);
>> +gp_store_proto(sw, uint32_t);
>> +gp_store_proto(sd, uint64_t);
>> +gp_store_proto(c_sw, uint32_t);
>> +gp_store_proto(c_sd, uint64_t);
>> +gp_store_proto(c_sdsp, uint64_t);
>> +
>> +#define TEST_GP_LOAD(__inst, __type_size)                    \
>> +TEST(gp_load_ ## __inst)                            \
>> +{                                        \
>> +    int offset, ret;                            \
>> +    uint8_t buf[16] __attribute__((aligned(16)));                \
>> +                                        \
>> +    ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);            \
>> +    ASSERT_EQ(ret, 0);                            \
>> +                                        \
>> +    for (offset = 1; offset < __type_size / 8; offset++) {            \
>> +        uint ## __type_size ## _t val = VAL ## __type_size;        \
>> +        uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t
>> *) (buf + offset); \
>> +        memcpy(ptr, &val, sizeof(val));                    \
>> +        val = test_ ## __inst(ptr, offset, val);            \
>> +        EXPECT_EQ(VAL ## __type_size, val);                \
>> +    }                                    \
>> +}
>> +
>> +TEST_GP_LOAD(lh, 16);
>> +TEST_GP_LOAD(lhu, 16);
>> +TEST_GP_LOAD(lw, 32);
>> +TEST_GP_LOAD(lwu, 32);
>> +TEST_GP_LOAD(ld, 64);
>> +#ifdef __riscv_compressed
>> +TEST_GP_LOAD(c_lw, 32);
>> +TEST_GP_LOAD(c_ld, 64);
>> +TEST_GP_LOAD(c_ldsp, 64);
>> +#endif
>> +
>> +#define TEST_GP_STORE(__inst, __type_size)                    \
>> +TEST(gp_load_ ## __inst)                            \
>> +{                                        \
>> +    int offset, ret;                            \
>> +    uint8_t buf[16] __attribute__((aligned(16)));                \
>> +                                        \
>> +    ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);            \
>> +    ASSERT_EQ(ret, 0);                            \
>> +                                        \
>> +    for (offset = 1; offset < __type_size / 8; offset++) {            \
>> +        uint ## __type_size ## _t val = VAL ## __type_size;        \
>> +        uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t
>> *) (buf + offset); \
>> +        memset(ptr, 0, sizeof(val));                    \
>> +        test_ ## __inst(ptr, offset, val);                \
>> +        memcpy(&val, ptr, sizeof(val));                    \
>> +        EXPECT_EQ(VAL ## __type_size, val);                \
>> +    }                                    \
>> +}
>> +TEST_GP_STORE(sh, 16);
>> +TEST_GP_STORE(sw, 32);
>> +TEST_GP_STORE(sd, 64);
>> +#ifdef __riscv_compressed
>> +TEST_GP_STORE(c_sw, 32);
>> +TEST_GP_STORE(c_sd, 64);
>> +TEST_GP_STORE(c_sdsp, 64);
>> +#endif
>> +
>> +#define __TEST_FPU_LOAD(__type, __inst, __reg_start,
>> __reg_end)            \
>> +TEST(fpu_load_ ## __inst)                            \
>> +{                                        \
>> +    int i, ret, offset, fp_reg;                        \
>> +    uint8_t buf[16] __attribute__((aligned(16)));                \
>> +                                        \
>> +    ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);            \
>> +    ASSERT_EQ(ret, 0);                            \
>> +                                        \
>> +    for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) {        \
>> +        for (offset = 1; offset < 4; offset++) {            \
>> +            void *load_addr = (buf + offset);            \
>> +            __type val = VAL_ ## __type ;                \
>> +                                        \
>> +            memcpy(load_addr, &val, sizeof(val));            \
>> +            val = test_ ## __inst(fp_reg, load_addr, offset, val);    \
>> +            EXPECT_TRUE(__type ##_equal(val, VAL_## __type));    \
>> +        }                                \
>> +    }                                    \
>> +}
>> +#define TEST_FPU_LOAD(__type, __inst) \
>> +    __TEST_FPU_LOAD(__type, __inst, 0, 32)
>> +#define TEST_FPU_LOAD_COMPRESSED(__type, __inst) \
>> +    __TEST_FPU_LOAD(__type, __inst, 8, 16)
>> +
>> +TEST_FPU_LOAD(float, flw)
>> +TEST_FPU_LOAD(double, fld)
>> +#ifdef __riscv_compressed
>> +TEST_FPU_LOAD_COMPRESSED(double, c_fld)
>> +TEST_FPU_LOAD_COMPRESSED(double, c_fldsp)
>> +#endif
>> +
>> +#define __TEST_FPU_STORE(__type, __inst, __reg_start,
>> __reg_end)        \
>> +TEST(fpu_store_ ## __inst)                            \
>> +{                                        \
>> +    int i, ret, offset, fp_reg;                        \
>> +    uint8_t buf[16] __attribute__((aligned(16)));                \
>> +                                        \
>> +    ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);            \
>> +    ASSERT_EQ(ret, 0);                            \
>> +                                        \
>> +    for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) {        \
>> +        for (offset = 1; offset < 4; offset++) {            \
>> +                                        \
>> +            void *store_addr = (buf + offset);            \
>> +            __type val = VAL_ ## __type ;                \
>> +                                        \
>> +            test_ ## __inst(fp_reg, store_addr, offset, val);    \
>> +            memcpy(&val, store_addr, sizeof(val));            \
>> +            EXPECT_TRUE(__type ## _equal(val, VAL_## __type));    \
>> +        }                                \
>> +    }                                    \
>> +}
>> +#define TEST_FPU_STORE(__type, __inst) \
>> +    __TEST_FPU_STORE(__type, __inst, 0, 32)
>> +#define TEST_FPU_STORE_COMPRESSED(__type, __inst) \
>> +    __TEST_FPU_STORE(__type, __inst, 8, 16)
>> +
>> +TEST_FPU_STORE(float, fsw)
>> +TEST_FPU_STORE(double, fsd)
>> +#ifdef __riscv_compressed
>> +TEST_FPU_STORE_COMPRESSED(double, c_fsd)
>> +TEST_FPU_STORE_COMPRESSED(double, c_fsdsp)
>> +#endif
>> +
>> +TEST_SIGNAL(gen_sigbus, SIGBUS)
>> +{
>> +    uint32_t *ptr;
>> +    uint8_t buf[16] __attribute__((aligned(16)));
>> +    int ret;
>> +
>> +    ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_SIGBUS);
>> +    ASSERT_EQ(ret, 0);
>> +
>> +    ptr = (uint32_t *)(buf + 1);
>> +    *ptr = 0xDEADBEEFULL;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +    int ret, val;
>> +
>> +    ret = prctl(PR_GET_UNALIGN, &val);
>> +    if (ret == -1 && errno == EINVAL)
>> +        ksft_exit_skip("SKIP GET_UNALIGN_CTL not supported\n");
>> +
>> +    exit(test_harness_run(argc, argv));
>> +}
> 
> 
> So I had to add the following to actually compile this selftest along
> with other riscv tests and fix some warnings:
> 
> diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/
> selftests/riscv/Makefile
> index 099b8c1f46f89..95a98ceeb3b3a 100644
> --- a/tools/testing/selftests/riscv/Makefile
> +++ b/tools/testing/selftests/riscv/Makefile
> @@ -5,7 +5,7 @@
>  ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> 
>  ifneq (,$(filter $(ARCH),riscv))
> -RISCV_SUBTARGETS ?= abi hwprobe mm sigreturn vector
> +RISCV_SUBTARGETS ?= abi hwprobe mm sigreturn vector misaligned
>  else
>  RISCV_SUBTARGETS :=
>  endif
> diff --git a/tools/testing/selftests/riscv/misaligned/misaligned.c b/
> tools/testing/selftests/riscv/misaligned/misaligned.c
> index c66aa87ec03ec..8fa5ad1a93d17 100644
> --- a/tools/testing/selftests/riscv/misaligned/misaligned.c
> +++ b/tools/testing/selftests/riscv/misaligned/misaligned.c
> @@ -167,7 +167,7 @@ TEST_GP_STORE(c_sdsp, 64);
>  #define __TEST_FPU_LOAD(__type, __inst, __reg_start,
> __reg_end)                             \
>  TEST(fpu_load_ ##
> __inst)                                                      \
>  { \
> -       int i, ret, offset,
> fp_reg;                                             \
> +       int ret, offset,
> fp_reg;                                                \
>         uint8_t buf[16]
> __attribute__((aligned(16)));                           \
> \
>         ret = prctl(PR_SET_UNALIGN,
> PR_UNALIGN_NOPRINT);                        \
> @@ -199,7 +199,7 @@ TEST_FPU_LOAD_COMPRESSED(double, c_fldsp)
>  #define __TEST_FPU_STORE(__type, __inst, __reg_start,
> __reg_end)               \
>  TEST(fpu_store_ ##
> __inst)                                                     \
>  { \
> -       int i, ret, offset,
> fp_reg;                                             \
> +       int ret, offset,
> fp_reg;                                                \
>         uint8_t buf[16]
> __attribute__((aligned(16)));                           \
> \
>         ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT);
> 
> 
> I already merged the first 3 commits of this patchset in fixes, so can
> you resend only the last 2 patches with the fixes above?

Yes sure,

Thanks,

Clément

> 
> Thanks,
> 
> Alex
> 
> 


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

end of thread, other threads:[~2025-05-12  7:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22 16:23 [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests Clément Léger
2025-04-22 16:23 ` [PATCH v2 1/5] riscv: misaligned: factorize trap handling Clément Léger
2025-05-06 10:58   ` Alexandre Ghiti
2025-04-22 16:23 ` [PATCH v2 2/5] riscv: misaligned: enable IRQs while handling misaligned accesses Clément Léger
2025-05-06 11:07   ` Alexandre Ghiti
2025-04-22 16:23 ` [PATCH v2 3/5] riscv: misaligned: use get_user() instead of __get_user() Clément Léger
2025-04-22 16:23 ` [PATCH v2 4/5] Documentation/sysctl: add riscv to unaligned-trap supported archs Clément Léger
2025-04-22 16:23 ` [PATCH v2 5/5] selftests: riscv: add misaligned access testing Clément Léger
2025-05-09  8:22   ` Alexandre Ghiti
2025-05-12  7:18     ` Clément Léger
2025-05-08 16:52 ` [PATCH v2 0/5] riscv: misaligned: fix interruptible context and add tests 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).