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.
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 | 57 ++-- 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, 614 insertions(+), 32 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
misaligned accesses traps are not nmi and should be treated as normal one using irqentry_enter()/exit(). 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 | 49 ++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 29 deletions(-)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 8ff8e8b36524..55d9f3450398 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -198,47 +198,38 @@ 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 void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type) { - if (user_mode(regs)) { - irqentry_enter_from_user_mode(regs); + irqentry_state_t state = irqentry_enter(regs);
+ if (type == MISALIGNED_LOAD) { if (handle_misaligned_load(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - load address misaligned"); - - irqentry_exit_to_user_mode(regs); + "Oops - load address misaligned"); } else { - irqentry_state_t state = irqentry_nmi_enter(regs); - - if (handle_misaligned_load(regs)) + if (handle_misaligned_store(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - load address misaligned"); - - irqentry_nmi_exit(regs, state); + "Oops - store (or AMO) address misaligned"); } + + irqentry_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,
Hi Clément,
On 14/04/2025 14:34, Clément Léger wrote:
misaligned accesses traps are not nmi and should be treated as normal one using irqentry_enter()/exit().
All the traps that come from kernel mode are treated as nmi as it was suggested by Peter here: https://lore.kernel.org/linux-riscv/Yyhv4UUXuSfvMOw+@hirez.programming.kicks...
I don't know the differences between irq_nmi_entry/exit() and irq_entry/exit(), so is that still correct to now treat the kernel traps as non-nmi?
Thanks,
Alex
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égercleger@rivosinc.com
arch/riscv/kernel/traps.c | 49 ++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 29 deletions(-)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 8ff8e8b36524..55d9f3450398 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -198,47 +198,38 @@ 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 void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type) {
- if (user_mode(regs)) {
irqentry_enter_from_user_mode(regs);
- irqentry_state_t state = irqentry_enter(regs);
- if (type == MISALIGNED_LOAD) { if (handle_misaligned_load(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
"Oops - load address misaligned");
irqentry_exit_to_user_mode(regs);
} else {"Oops - load address misaligned");
irqentry_state_t state = irqentry_nmi_enter(regs);
if (handle_misaligned_load(regs))
if (handle_misaligned_store(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
"Oops - load address misaligned");
irqentry_nmi_exit(regs, state);
}"Oops - store (or AMO) address misaligned");
- irqentry_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,
On 21/04/2025 09:06, Alexandre Ghiti wrote:
Hi Clément,
On 14/04/2025 14:34, Clément Léger wrote:
misaligned accesses traps are not nmi and should be treated as normal one using irqentry_enter()/exit().
All the traps that come from kernel mode are treated as nmi as it was suggested by Peter here: https://lore.kernel.org/linux-riscv/ Yyhv4UUXuSfvMOw+@hirez.programming.kicks-ass.net/
I don't know the differences between irq_nmi_entry/exit() and irq_entry/ exit(), so is that still correct to now treat the kernel traps as non-nmi?
Hi Alex,
Actually, this discussion was raised on a previous series [1] by Maciej which replied that we should actually reenable interrupt depending on the state that was interrupted. Looking at other architecture/code, it seems like treating misaligned accesses as NMI is probably not the right way. For instance, loongarch treats them as normal IRQ using a irqentry_enter()/exit() and reenabling IRQS if possible.
Moreover, it looks like to me that misaligned access traps are similar (in the way they can be handled) to how the page fault are handled in RISC-V. It uses irqentry_enter()/exit() and reenables interrupts if needed. Additionally, the misaligned access handling does will not take any locks if handling a misaligned access taken while in kernel so even if we interrupt a kernel critical section that does not have interrupts disable, we are good to go. But my understanding might be wrong as well and I might be missing specific cases :)
Thanks,
Clément
https://lore.kernel.org/lkml/alpine.DEB.2.21.2501070143250.18889@angie.orcam... [1]
Thanks,
Alex
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égercleger@rivosinc.com
arch/riscv/kernel/traps.c | 49 ++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 29 deletions(-)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 8ff8e8b36524..55d9f3450398 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -198,47 +198,38 @@ 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 void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type) { - if (user_mode(regs)) { - irqentry_enter_from_user_mode(regs); + irqentry_state_t state = irqentry_enter(regs); + if (type == MISALIGNED_LOAD) { if (handle_misaligned_load(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - load address misaligned");
- irqentry_exit_to_user_mode(regs); + "Oops - load address misaligned"); } else { - irqentry_state_t state = irqentry_nmi_enter(regs);
- if (handle_misaligned_load(regs)) + if (handle_misaligned_store(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - load address misaligned");
- irqentry_nmi_exit(regs, state); + "Oops - store (or AMO) address misaligned"); }
+ irqentry_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,
On Tue, Apr 22, 2025 at 09:57:12AM +0200, Clément Léger wrote:
On 21/04/2025 09:06, Alexandre Ghiti wrote:
Hi Clément,
On 14/04/2025 14:34, Clément Léger wrote:
misaligned accesses traps are not nmi and should be treated as normal one using irqentry_enter()/exit().
All the traps that come from kernel mode are treated as nmi as it was suggested by Peter here: https://lore.kernel.org/linux-riscv/ Yyhv4UUXuSfvMOw+@hirez.programming.kicks-ass.net/
I don't know the differences between irq_nmi_entry/exit() and irq_entry/ exit(), so is that still correct to now treat the kernel traps as non-nmi?
Hi Alex,
Actually, this discussion was raised on a previous series [1] by Maciej which replied that we should actually reenable interrupt depending on the state that was interrupted. Looking at other architecture/code, it seems like treating misaligned accesses as NMI is probably not the right way. For instance, loongarch treats them as normal IRQ using a irqentry_enter()/exit() and reenabling IRQS if possible.
So, a trap that happens in kernel space while IRQs are disabled, SHOULD really be NMI-like.
You then have a choice, make all such traps from kernel space NMI-like; this makes it easy on the trap handler, since the context is always the same. Mistakes are 'easy' to find.
Or,.. do funny stuff and only make it NMI like if IRQs were disabled. Which gives inconsistent context for the handler and you'll find yourself scratching your head at some point in the future wondering why this one rare occasion goes BOOM.
x86 mostly does the first, any trap that can happen with IRQs disabled is treated unconditionally as NMI like. The obvious exception is page-fault, but that already has a from-non-preemptible-context branch that is 'careful'.
As to unaligned traps from kernel space, I would imagine they mostly BUG the kernel, except when there's an exception entry for that location, in which case it might do a fixup?
Anyway, the reason these exceptions should be NMI like, is because interrupts are not allowed to nest. Notably something like:
raw_spin_lock_irqsave(&foo); <IRQ> raw_spin_lock_irqsave(&foo); ...
Is an obvious problem. Exceptions that can run while IRQs are disabled, must not use locks -- treating them as NMI-like (they are non-maskable after all), ensures this.
On 22/04/2025 11:44, Peter Zijlstra wrote:
On Tue, Apr 22, 2025 at 09:57:12AM +0200, Clément Léger wrote:
On 21/04/2025 09:06, Alexandre Ghiti wrote:
Hi Clément,
On 14/04/2025 14:34, Clément Léger wrote:
misaligned accesses traps are not nmi and should be treated as normal one using irqentry_enter()/exit().
All the traps that come from kernel mode are treated as nmi as it was suggested by Peter here: https://lore.kernel.org/linux-riscv/ Yyhv4UUXuSfvMOw+@hirez.programming.kicks-ass.net/
I don't know the differences between irq_nmi_entry/exit() and irq_entry/ exit(), so is that still correct to now treat the kernel traps as non-nmi?
Hi Alex,
Actually, this discussion was raised on a previous series [1] by Maciej which replied that we should actually reenable interrupt depending on the state that was interrupted. Looking at other architecture/code, it seems like treating misaligned accesses as NMI is probably not the right way. For instance, loongarch treats them as normal IRQ using a irqentry_enter()/exit() and reenabling IRQS if possible.
So, a trap that happens in kernel space while IRQs are disabled, SHOULD really be NMI-like.
You then have a choice, make all such traps from kernel space NMI-like; this makes it easy on the trap handler, since the context is always the same. Mistakes are 'easy' to find.
Or,.. do funny stuff and only make it NMI like if IRQs were disabled. Which gives inconsistent context for the handler and you'll find yourself scratching your head at some point in the future wondering why this one rare occasion goes BOOM.
Hi Peter,
Yeah agreed, that might be hazardous.
x86 mostly does the first, any trap that can happen with IRQs disabled is treated unconditionally as NMI like. The obvious exception is page-fault, but that already has a from-non-preemptible-context branch that is 'careful'.
As to unaligned traps from kernel space, I would imagine they mostly BUG the kernel, except when there's an exception entry for that location, in which case it might do a fixup?
The misaligned access exception handling currently handles misaligned access for the kernel as well (except if explicitly disabled).
Anyway, the reason these exceptions should be NMI like, is because interrupts are not allowed to nest. Notably something like:
raw_spin_lock_irqsave(&foo);
<IRQ> raw_spin_lock_irqsave(&foo); ...
Is an obvious problem. Exceptions that can run while IRQs are disabled, must not use locks -- treating them as NMI-like (they are non-maskable after all), ensures this.
As said in my previous reply, the misaligned handling code does not currently access any locks (when handling kernel misaligned accesses) and is self contained. That being said, that assumption might not be true in future so that might be better to take your approach and treat the misaligned access like an NMI.
Thanks,
Clément
We can safely reenable IRQs if they were enabled in the previous context. 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 | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 55d9f3450398..3eecc2addc41 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -206,6 +206,11 @@ enum misaligned_access_type { static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type) { irqentry_state_t state = irqentry_enter(regs); + bool enable_irqs = !regs_irqs_disabled(regs); + + /* Enable interrupts if they were enabled in the interrupted context. */ + if (enable_irqs) + local_irq_enable();
if (type == MISALIGNED_LOAD) { if (handle_misaligned_load(regs)) @@ -217,6 +222,9 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type "Oops - store (or AMO) address misaligned"); }
+ if (enable_irqs) + local_irq_disable(); + irqentry_exit(regs, state); }
On 14/04/2025 14:34, Clément Léger wrote:
We can safely reenable IRQs if they were enabled in the previous context. 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 | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 55d9f3450398..3eecc2addc41 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -206,6 +206,11 @@ enum misaligned_access_type { static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type) { irqentry_state_t state = irqentry_enter(regs);
- bool enable_irqs = !regs_irqs_disabled(regs);
- /* Enable interrupts if they were enabled in the interrupted context. */
- if (enable_irqs)
local_irq_enable();
if (type == MISALIGNED_LOAD) { if (handle_misaligned_load(regs)) @@ -217,6 +222,9 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type "Oops - store (or AMO) address misaligned"); }
- if (enable_irqs)
local_irq_disable();
- irqentry_exit(regs, state); }
Reviewed-by: Alexandre Ghiti alexghiti@rivosinc.com
Thanks,
Alex
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 --- 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; \
On 14/04/2025 14:34, Clément Léger wrote:
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
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); \
} else { \ insn = *(type *)insn_addr; \ __ret = 0; \__ret = get_user(insn, (type __user *) insn_addr); \
Reviewed-by: Alexandre Ghiti alexghiti@rivosinc.com
Thanks,
Alex
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 --- 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.
On 14/04/2025 14:34, Clément Léger wrote:
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
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.
Reviewed-by: Alexandre Ghiti alexghiti@rivosinc.com
Thanks,
Alex
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)); +}
linux-kselftest-mirror@lists.linaro.org