[PATCH v2] target/riscv: save ELP in MPELP for NMIE=0 exceptions

Zephyr Li posted 1 patch 3 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260903013627.1105-1-fritchleybohrer@gmail.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu@processmission.com>
target/riscv/tcg/cpu_helper.c             |  18 +---
tests/tcg/riscv64/Makefile.softmmu-target |   8 ++
tests/tcg/riscv64/test-zicfilp-smrnmi.S   | 117 ++++++++++++++++++++++
3 files changed, 127 insertions(+), 16 deletions(-)
create mode 100644 tests/tcg/riscv64/test-zicfilp-smrnmi.S
[PATCH v2] target/riscv: save ELP in MPELP for NMIE=0 exceptions
Posted by Zephyr Li 3 weeks, 2 days ago
The Smrnmi specification, section 8.5, "RNMI Operation", states:

  If the hart encounters an exception while executing in M-mode with the
  mnstatus.NMIE bit clear, the exception is an RNMI exception. Trap state
  is still saved in the regular M-mode CSRs and the handler returns with
  MRET.

riscv_cpu_do_interrupt() instead saves ELP in mnstatus.MNPELP on this
path. MRET restores ELP from mstatus.MPELP, so the expected landing-pad
state is lost.

Always save ELP in mstatus.MPELP for M-mode exceptions. Keep MNPELP for
the actual RNMI interrupt path, which returns with MNRET.

Add a TCG test that checks ELP preservation across an NMIE=0 M-mode
exception and MRET.

Fixes: 0266fd8b56a4 ("target/riscv: Add Zicfilp support for Smrnmi")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4223
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Signed-off-by: Zephyr Li <fritchleybohrer@gmail.com>
---
Changes in v2:
- Quote the relevant Smrnmi specification text in the commit message.

---
 target/riscv/tcg/cpu_helper.c             |  18 +---
 tests/tcg/riscv64/Makefile.softmmu-target |   8 ++
 tests/tcg/riscv64/test-zicfilp-smrnmi.S   | 117 ++++++++++++++++++++++
 3 files changed, 127 insertions(+), 16 deletions(-)
 create mode 100644 tests/tcg/riscv64/test-zicfilp-smrnmi.S

diff --git a/target/riscv/tcg/cpu_helper.c b/target/riscv/tcg/cpu_helper.c
index 07d9222652..a0d79b33a5 100644
--- a/target/riscv/tcg/cpu_helper.c
+++ b/target/riscv/tcg/cpu_helper.c
@@ -2264,23 +2264,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 
         src = env->sepc;
     } else {
-        /*
-         * If the hart encounters an exception while executing in M-mode
-         * with the mnstatus.NMIE bit clear, the exception is an RNMI exception.
-         */
-        nnmi_excep = cpu->cfg.ext_smrnmi &&
-                     !get_field(env->mnstatus, MNSTATUS_NMIE) &&
-                     !async;
-
-        /* handle the trap in M-mode */
-        /* save elp status */
+        /* Save ELP for MRET. */
         if (cpu_get_fcfien(env)) {
-            if (nnmi_excep) {
-                env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPELP,
-                                          env->elp);
-            } else {
-                env->mstatus = set_field(env->mstatus, MSTATUS_MPELP, env->elp);
-            }
+            env->mstatus = set_field(env->mstatus, MSTATUS_MPELP, env->elp);
         }
 
         if (riscv_has_ext(env, RVH)) {
diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
index 6a219c306c..8522c3fe6f 100644
--- a/tests/tcg/riscv64/Makefile.softmmu-target
+++ b/tests/tcg/riscv64/Makefile.softmmu-target
@@ -45,6 +45,14 @@ comma:= ,
 run-test-crc32: test-crc32
 	$(call run-test, $<, $(QEMU) -cpu rv64$(comma)xlrbr=true $(QEMU_OPTS)$<)
 
+EXTRA_RUNS += run-test-zicfilp-smrnmi
+run-test-zicfilp-smrnmi: test-zicfilp-smrnmi
+	$(call run-test, $<, \
+	  $(QEMU) \
+	  -cpu rv64$(comma)zicsr=true$(comma)zicfilp=true$(comma)smrnmi=true \
+	  -global rv64-riscv-cpu.rnmi-exception-vector=0x80000100 \
+	  $(QEMU_OPTS)$<)
+
 # Zicclsm: misaligned load/store support. Assemble one source twice: the
 # default build expects every misaligned access to succeed (zicclsm=true),
 # the -DZICCLSM_DISABLED build expects every one to trap (zicclsm=false).
diff --git a/tests/tcg/riscv64/test-zicfilp-smrnmi.S b/tests/tcg/riscv64/test-zicfilp-smrnmi.S
new file mode 100644
index 0000000000..b5965bf705
--- /dev/null
+++ b/tests/tcg/riscv64/test-zicfilp-smrnmi.S
@@ -0,0 +1,117 @@
+/*
+ * Test that an M-mode exception taken with mnstatus.NMIE clear saves ELP in
+ * mstatus.MPELP, so that it is restored by MRET.  MNPELP is reserved for the
+ * actual RNMI path, which returns with MNRET.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+	.option	norvc
+
+	.equ	CSR_MNSTATUS, 0x744
+	.equ	CSR_MSECCFG, 0x747
+	.equ	MNSTATUS_NMIE, 1 << 3
+	.equ	MNSTATUS_MNPELP, 1 << 9
+	.equ	MSECCFG_MLPE, 1 << 10
+	.equ	MSTATUS_MPELP, 1 << 41
+	.equ	EXCP_INST_ACCESS_FAULT, 1
+	.equ	EXCP_SW_CHECK, 18
+	.equ	SW_CHECK_FCFI_TVAL, 2
+
+	.text
+	.global	_start
+_start:
+	/* Verify that NMIE is clear before exercising the exception path. */
+	csrr	t0, CSR_MNSTATUS
+	andi	t0, t0, MNSTATUS_NMIE
+	bnez	t0, fail_nmie
+
+	/* Enable landing-pad checks in M-mode, then start a new TB. */
+	li	t0, MSECCFG_MLPE
+	csrs	CSR_MSECCFG, t0
+	j	tracked_jump
+
+tracked_jump:
+	/* Make a Zicfilp-tracked indirect jump to an address that faults. */
+	li	t1, 0
+	jalr	zero, 0(t1)
+
+	/*
+	 * The instruction access fault should have redirected to the handler.
+	 */
+	li	a0, 1
+	j	_exit
+
+fail_nmie:
+	li	a0, 6
+	j	_exit
+
+	/* Must match rnmi-exception-vector in Makefile.softmmu-target. */
+	.org	0x100
+rnmi_exception:
+	csrr	t0, mcause
+	li	t1, EXCP_INST_ACCESS_FAULT
+	beq	t0, t1, handle_fetch_fault
+	li	t1, EXCP_SW_CHECK
+	beq	t0, t1, handle_sw_check
+
+	li	a0, 4
+	j	_exit
+
+handle_fetch_fault:
+	/* This is an M-mode exception and will return with MRET. */
+	csrr	t0, mstatus
+	li	t1, MSTATUS_MPELP
+	and	t0, t0, t1
+	beqz	t0, fail_mpelp
+
+	csrr	t0, CSR_MNSTATUS
+	andi	t0, t0, MNSTATUS_MNPELP
+	bnez	t0, fail_mnpelp
+
+	lla	t0, resume_non_lpad
+	csrw	mepc, t0
+	mret
+
+resume_non_lpad:
+	/* MRET must restore ELP, so this instruction must not retire. */
+	li	a0, 1
+	j	_exit
+
+handle_sw_check:
+	csrr	t0, mtval
+	li	t1, SW_CHECK_FCFI_TVAL
+	bne	t0, t1, fail_tval
+
+	li	a0, 0
+	j	_exit
+
+fail_mpelp:
+	li	a0, 2
+	j	_exit
+
+fail_mnpelp:
+	li	a0, 3
+	j	_exit
+
+fail_tval:
+	li	a0, 5
+
+_exit:
+	lla	a1, semiargs
+	li	t0, 0x20026	/* ADP_Stopped_ApplicationExit */
+	sd	t0, 0(a1)
+	sd	a0, 8(a1)
+	li	a0, 0x20	/* TARGET_SYS_EXIT_EXTENDED */
+
+	/* Semihosting call sequence. */
+	.balign	16
+	slli	zero, zero, 0x1f
+	ebreak
+	srai	zero, zero, 0x7
+	j	.
+
+	.data
+	.balign	16
+semiargs:
+	.space	16
-- 
2.43.0
Re: [PATCH v2] target/riscv: save ELP in MPELP for NMIE=0 exceptions
Posted by Alistair Francis 1 day, 15 hours ago
On Thu, 2026-09-03 at 09:36 +0800, Zephyr Li wrote:
> The Smrnmi specification, section 8.5, "RNMI Operation", states:
> 
>   If the hart encounters an exception while executing in M-mode with
> the
>   mnstatus.NMIE bit clear, the exception is an RNMI exception. Trap
> state
>   is still saved in the regular M-mode CSRs and the handler returns
> with
>   MRET.
> 
> riscv_cpu_do_interrupt() instead saves ELP in mnstatus.MNPELP on this
> path. MRET restores ELP from mstatus.MPELP, so the expected landing-
> pad
> state is lost.
> 
> Always save ELP in mstatus.MPELP for M-mode exceptions. Keep MNPELP
> for
> the actual RNMI interrupt path, which returns with MNRET.
> 
> Add a TCG test that checks ELP preservation across an NMIE=0 M-mode
> exception and MRET.
> 
> Fixes: 0266fd8b56a4 ("target/riscv: Add Zicfilp support for Smrnmi")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4223
> Reviewed-by: Daniel Henrique Barboza
> <daniel.barboza@oss.qualcomm.com>
> Signed-off-by: Zephyr Li <fritchleybohrer@gmail.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> Changes in v2:
> - Quote the relevant Smrnmi specification text in the commit message.
> 
> ---
>  target/riscv/tcg/cpu_helper.c             |  18 +---
>  tests/tcg/riscv64/Makefile.softmmu-target |   8 ++
>  tests/tcg/riscv64/test-zicfilp-smrnmi.S   | 117
> ++++++++++++++++++++++
>  3 files changed, 127 insertions(+), 16 deletions(-)
>  create mode 100644 tests/tcg/riscv64/test-zicfilp-smrnmi.S
> 
> diff --git a/target/riscv/tcg/cpu_helper.c
> b/target/riscv/tcg/cpu_helper.c
> index 07d9222652..a0d79b33a5 100644
> --- a/target/riscv/tcg/cpu_helper.c
> +++ b/target/riscv/tcg/cpu_helper.c
> @@ -2264,23 +2264,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>  
>          src = env->sepc;
>      } else {
> -        /*
> -         * If the hart encounters an exception while executing in M-
> mode
> -         * with the mnstatus.NMIE bit clear, the exception is an
> RNMI exception.
> -         */
> -        nnmi_excep = cpu->cfg.ext_smrnmi &&
> -                     !get_field(env->mnstatus, MNSTATUS_NMIE) &&
> -                     !async;
> -
> -        /* handle the trap in M-mode */
> -        /* save elp status */
> +        /* Save ELP for MRET. */
>          if (cpu_get_fcfien(env)) {
> -            if (nnmi_excep) {
> -                env->mnstatus = set_field(env->mnstatus,
> MNSTATUS_MNPELP,
> -                                          env->elp);
> -            } else {
> -                env->mstatus = set_field(env->mstatus,
> MSTATUS_MPELP, env->elp);
> -            }
> +            env->mstatus = set_field(env->mstatus, MSTATUS_MPELP,
> env->elp);
>          }
>  
>          if (riscv_has_ext(env, RVH)) {
> diff --git a/tests/tcg/riscv64/Makefile.softmmu-target
> b/tests/tcg/riscv64/Makefile.softmmu-target
> index 6a219c306c..8522c3fe6f 100644
> --- a/tests/tcg/riscv64/Makefile.softmmu-target
> +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> @@ -45,6 +45,14 @@ comma:= ,
>  run-test-crc32: test-crc32
>  	$(call run-test, $<, $(QEMU) -cpu rv64$(comma)xlrbr=true
> $(QEMU_OPTS)$<)
>  
> +EXTRA_RUNS += run-test-zicfilp-smrnmi
> +run-test-zicfilp-smrnmi: test-zicfilp-smrnmi
> +	$(call run-test, $<, \
> +	  $(QEMU) \
> +	  -cpu
> rv64$(comma)zicsr=true$(comma)zicfilp=true$(comma)smrnmi=true \
> +	  -global rv64-riscv-cpu.rnmi-exception-vector=0x80000100 \
> +	  $(QEMU_OPTS)$<)
> +
>  # Zicclsm: misaligned load/store support. Assemble one source twice:
> the
>  # default build expects every misaligned access to succeed
> (zicclsm=true),
>  # the -DZICCLSM_DISABLED build expects every one to trap
> (zicclsm=false).
> diff --git a/tests/tcg/riscv64/test-zicfilp-smrnmi.S
> b/tests/tcg/riscv64/test-zicfilp-smrnmi.S
> new file mode 100644
> index 0000000000..b5965bf705
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-zicfilp-smrnmi.S
> @@ -0,0 +1,117 @@
> +/*
> + * Test that an M-mode exception taken with mnstatus.NMIE clear
> saves ELP in
> + * mstatus.MPELP, so that it is restored by MRET.  MNPELP is
> reserved for the
> + * actual RNMI path, which returns with MNRET.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +	.option	norvc
> +
> +	.equ	CSR_MNSTATUS, 0x744
> +	.equ	CSR_MSECCFG, 0x747
> +	.equ	MNSTATUS_NMIE, 1 << 3
> +	.equ	MNSTATUS_MNPELP, 1 << 9
> +	.equ	MSECCFG_MLPE, 1 << 10
> +	.equ	MSTATUS_MPELP, 1 << 41
> +	.equ	EXCP_INST_ACCESS_FAULT, 1
> +	.equ	EXCP_SW_CHECK, 18
> +	.equ	SW_CHECK_FCFI_TVAL, 2
> +
> +	.text
> +	.global	_start
> +_start:
> +	/* Verify that NMIE is clear before exercising the exception
> path. */
> +	csrr	t0, CSR_MNSTATUS
> +	andi	t0, t0, MNSTATUS_NMIE
> +	bnez	t0, fail_nmie
> +
> +	/* Enable landing-pad checks in M-mode, then start a new TB.
> */
> +	li	t0, MSECCFG_MLPE
> +	csrs	CSR_MSECCFG, t0
> +	j	tracked_jump
> +
> +tracked_jump:
> +	/* Make a Zicfilp-tracked indirect jump to an address that
> faults. */
> +	li	t1, 0
> +	jalr	zero, 0(t1)
> +
> +	/*
> +	 * The instruction access fault should have redirected to
> the handler.
> +	 */
> +	li	a0, 1
> +	j	_exit
> +
> +fail_nmie:
> +	li	a0, 6
> +	j	_exit
> +
> +	/* Must match rnmi-exception-vector in Makefile.softmmu-
> target. */
> +	.org	0x100
> +rnmi_exception:
> +	csrr	t0, mcause
> +	li	t1, EXCP_INST_ACCESS_FAULT
> +	beq	t0, t1, handle_fetch_fault
> +	li	t1, EXCP_SW_CHECK
> +	beq	t0, t1, handle_sw_check
> +
> +	li	a0, 4
> +	j	_exit
> +
> +handle_fetch_fault:
> +	/* This is an M-mode exception and will return with MRET. */
> +	csrr	t0, mstatus
> +	li	t1, MSTATUS_MPELP
> +	and	t0, t0, t1
> +	beqz	t0, fail_mpelp
> +
> +	csrr	t0, CSR_MNSTATUS
> +	andi	t0, t0, MNSTATUS_MNPELP
> +	bnez	t0, fail_mnpelp
> +
> +	lla	t0, resume_non_lpad
> +	csrw	mepc, t0
> +	mret
> +
> +resume_non_lpad:
> +	/* MRET must restore ELP, so this instruction must not
> retire. */
> +	li	a0, 1
> +	j	_exit
> +
> +handle_sw_check:
> +	csrr	t0, mtval
> +	li	t1, SW_CHECK_FCFI_TVAL
> +	bne	t0, t1, fail_tval
> +
> +	li	a0, 0
> +	j	_exit
> +
> +fail_mpelp:
> +	li	a0, 2
> +	j	_exit
> +
> +fail_mnpelp:
> +	li	a0, 3
> +	j	_exit
> +
> +fail_tval:
> +	li	a0, 5
> +
> +_exit:
> +	lla	a1, semiargs
> +	li	t0, 0x20026	/* ADP_Stopped_ApplicationExit */
> +	sd	t0, 0(a1)
> +	sd	a0, 8(a1)
> +	li	a0, 0x20	/* TARGET_SYS_EXIT_EXTENDED */
> +
> +	/* Semihosting call sequence. */
> +	.balign	16
> +	slli	zero, zero, 0x1f
> +	ebreak
> +	srai	zero, zero, 0x7
> +	j	.
> +
> +	.data
> +	.balign	16
> +semiargs:
> +	.space	16