1. Patchew
  2. QEMU
  3. View series

[PATCH v2] target/riscv: stop translating after WFI

Zephyr Li posted 1 patch 2 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260910025324.54990-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>
[PATCH v2] target/riscv: stop translating after WFI
Posted by Zephyr Li 2 weeks, 2 days ago 
helper_wfi() never returns to translated code: it either raises an
exception or exits the CPU loop after marking the CPU as halted. However,
trans_wfi() leaves the translation state as DISAS_NEXT, so instructions
following WFI can be included in the same TB.

With icount enabled, this makes the following instruction part of the TB's
instruction count when WFI exits, causing minstret to be incremented once
too many. Set DISAS_NORETURN after emitting the helper.

Add a TCG test for both reported cases: WFI waking for a locally enabled
pending interrupt with mstatus.MIE clear, and WFI taking a timer interrupt.

Fixes: 55c2a12cbcd3 ("RISC-V TCG Code Generation")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4230
Signed-off-by: Zephyr Li <fritchleybohrer@gmail.com>

---
Changes in v2:
- Register test-minstret-wfi.S with the Meson TCG test framework so
  check-tcg executes it.

diff --git a/target/riscv/tcg/insn_trans/trans_privileged.c.inc b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
index a8eaccef67..ebbbb01179 100644
--- a/target/riscv/tcg/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
@@ -145,6 +145,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
     decode_save_opc(ctx, 0);
     gen_update_pc(ctx, ctx->cur_insn_len);
     gen_helper_wfi(tcg_env);
+    ctx->base.is_jmp = DISAS_NORETURN;
     return true;
 #else
     return false;
diff --git a/tests/tcg/riscv64/system/meson.build b/tests/tcg/riscv64/system/meson.build
index 8604c2a45a..2aca68e078 100644
--- a/tests/tcg/riscv64/system/meson.build
+++ b/tests/tcg/riscv64/system/meson.build
@@ -23,6 +23,7 @@ tests += {
   'test-mepc-masking.S': setup,
   'test-crc32.S': setup + {'qemu_args': ['-cpu', 'rv64,xlrbr=true', qemu_args]},
   'test-minstret-ecall.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]},
+  'test-minstret-wfi.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]},
   'test-misa-w.S': setup + {'qemu_args': ['-cpu', 'rv64,x-misa-w=true,c=true,v=true', qemu_args]},
 }
 
diff --git a/tests/tcg/riscv64/test-minstret-wfi.S b/tests/tcg/riscv64/test-minstret-wfi.S
new file mode 100644
index 0000000000..0fd0158d5b
--- /dev/null
+++ b/tests/tcg/riscv64/test-minstret-wfi.S
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+	.option	norvc
+
+	.text
+	.global	_start
+_start:
+	lla	t0, trap
+	csrw	mtvec, t0
+	li	s3, 0
+
+	/*
+	 * A pending, locally enabled interrupt wakes WFI even with MIE
+	 * clear.
+	 */
+	li	t0, 0x8		/* MIE_MSIE */
+	csrw	mie, t0
+	csrci	mstatus, 0x8	/* MSTATUS_MIE */
+	li	t0, 0x2000000	/* MSIP0 */
+	li	t1, 1
+	sw	t1, 0(t0)
+
+	/*
+	 * The first CSR read and WFI retire before the second read obtains s1,
+	 * so the expected difference is two.
+	 */
+	csrr	s0, minstret
+	wfi
+	csrr	s1, minstret
+	sub	s1, s1, s0
+	li	t1, 2
+	beq	s1, t1, 1f
+	ori	s3, s3, 1
+1:
+
+	/* Clear the software interrupt before testing the trap path. */
+	sw	zero, 0(t0)
+	csrw	mie, zero
+
+	/* Schedule a timer interrupt far enough ahead to reach WFI first. */
+	li	t0, 0x200bff8	/* MTIME */
+	ld	t1, 0(t0)
+	addi	t1, t1, 100
+	li	t0, 0x2004000	/* MTIMECMP0 */
+	sd	t1, 0(t0)
+	li	t0, 0x80		/* MIE_MTIE */
+	csrw	mie, t0
+	csrsi	mstatus, 0x8	/* MSTATUS_MIE */
+
+	/*
+	 * The first CSR read and WFI retire before the trap handler obtains s1,
+	 * so the expected difference is again two.
+	 */
+	li	s2, 1
+	csrr	s0, minstret
+	wfi
+	beqz	s2, 1f
+	ori	s3, s3, 2
+1:
+	bnez	s3, fail
+	li	a0, 0
+	j	_exit
+
+fail:
+	mv	a0, s3
+
+_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	.
+
+	.balign	4
+trap:
+	/* Read minstret before retiring an instruction in the handler. */
+	csrr	s1, minstret
+	sub	s1, s1, s0
+	addi	s1, s1, -2
+	snez	s2, s1
+
+	/* Disable the timer interrupt before returning past WFI. */
+	li	t0, 0x2004000	/* MTIMECMP0 */
+	li	t1, -1
+	sd	t1, 0(t0)
+	li	t0, 0x80		/* MIE_MTIE */
+	csrc	mie, t0
+	mret
+
+	.data
+	.balign	16
+semiargs:
+	.space	16
-- 
2.43.0
Re: [PATCH v2] target/riscv: stop translating after WFI
Posted by Alistair Francis 4 days, 18 hours ago 
On Thu, 2026-09-10 at 10:53 +0800, Zephyr Li wrote:
> helper_wfi() never returns to translated code: it either raises an
> exception or exits the CPU loop after marking the CPU as halted.
> However,
> trans_wfi() leaves the translation state as DISAS_NEXT, so
> instructions
> following WFI can be included in the same TB.
> 
> With icount enabled, this makes the following instruction part of the
> TB's
> instruction count when WFI exits, causing minstret to be incremented
> once
> too many. Set DISAS_NORETURN after emitting the helper.
> 
> Add a TCG test for both reported cases: WFI waking for a locally
> enabled
> pending interrupt with mstatus.MIE clear, and WFI taking a timer
> interrupt.
> 
> Fixes: 55c2a12cbcd3 ("RISC-V TCG Code Generation")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4230
> Signed-off-by: Zephyr Li <fritchleybohrer@gmail.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> 
> ---
> Changes in v2:
> - Register test-minstret-wfi.S with the Meson TCG test framework so
>   check-tcg executes it.
> 
> diff --git a/target/riscv/tcg/insn_trans/trans_privileged.c.inc
> b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
> index a8eaccef67..ebbbb01179 100644
> --- a/target/riscv/tcg/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
> @@ -145,6 +145,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi
> *a)
>      decode_save_opc(ctx, 0);
>      gen_update_pc(ctx, ctx->cur_insn_len);
>      gen_helper_wfi(tcg_env);
> +    ctx->base.is_jmp = DISAS_NORETURN;
>      return true;
>  #else
>      return false;
> diff --git a/tests/tcg/riscv64/system/meson.build
> b/tests/tcg/riscv64/system/meson.build
> index 8604c2a45a..2aca68e078 100644
> --- a/tests/tcg/riscv64/system/meson.build
> +++ b/tests/tcg/riscv64/system/meson.build
> @@ -23,6 +23,7 @@ tests += {
>    'test-mepc-masking.S': setup,
>    'test-crc32.S': setup + {'qemu_args': ['-cpu', 'rv64,xlrbr=true',
> qemu_args]},
>    'test-minstret-ecall.S': setup + {'qemu_args': ['-icount',
> 'shift=1', qemu_args]},
> +  'test-minstret-wfi.S': setup + {'qemu_args': ['-icount',
> 'shift=1', qemu_args]},
>    'test-misa-w.S': setup + {'qemu_args': ['-cpu', 'rv64,x-misa-
> w=true,c=true,v=true', qemu_args]},
>  }
>  
> diff --git a/tests/tcg/riscv64/test-minstret-wfi.S
> b/tests/tcg/riscv64/test-minstret-wfi.S
> new file mode 100644
> index 0000000000..0fd0158d5b
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-minstret-wfi.S
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +	.option	norvc
> +
> +	.text
> +	.global	_start
> +_start:
> +	lla	t0, trap
> +	csrw	mtvec, t0
> +	li	s3, 0
> +
> +	/*
> +	 * A pending, locally enabled interrupt wakes WFI even with
> MIE
> +	 * clear.
> +	 */
> +	li	t0, 0x8		/* MIE_MSIE */
> +	csrw	mie, t0
> +	csrci	mstatus, 0x8	/* MSTATUS_MIE */
> +	li	t0, 0x2000000	/* MSIP0 */
> +	li	t1, 1
> +	sw	t1, 0(t0)
> +
> +	/*
> +	 * The first CSR read and WFI retire before the second read
> obtains s1,
> +	 * so the expected difference is two.
> +	 */
> +	csrr	s0, minstret
> +	wfi
> +	csrr	s1, minstret
> +	sub	s1, s1, s0
> +	li	t1, 2
> +	beq	s1, t1, 1f
> +	ori	s3, s3, 1
> +1:
> +
> +	/* Clear the software interrupt before testing the trap
> path. */
> +	sw	zero, 0(t0)
> +	csrw	mie, zero
> +
> +	/* Schedule a timer interrupt far enough ahead to reach WFI
> first. */
> +	li	t0, 0x200bff8	/* MTIME */
> +	ld	t1, 0(t0)
> +	addi	t1, t1, 100
> +	li	t0, 0x2004000	/* MTIMECMP0 */
> +	sd	t1, 0(t0)
> +	li	t0, 0x80		/* MIE_MTIE */
> +	csrw	mie, t0
> +	csrsi	mstatus, 0x8	/* MSTATUS_MIE */
> +
> +	/*
> +	 * The first CSR read and WFI retire before the trap handler
> obtains s1,
> +	 * so the expected difference is again two.
> +	 */
> +	li	s2, 1
> +	csrr	s0, minstret
> +	wfi
> +	beqz	s2, 1f
> +	ori	s3, s3, 2
> +1:
> +	bnez	s3, fail
> +	li	a0, 0
> +	j	_exit
> +
> +fail:
> +	mv	a0, s3
> +
> +_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	.
> +
> +	.balign	4
> +trap:
> +	/* Read minstret before retiring an instruction in the
> handler. */
> +	csrr	s1, minstret
> +	sub	s1, s1, s0
> +	addi	s1, s1, -2
> +	snez	s2, s1
> +
> +	/* Disable the timer interrupt before returning past WFI. */
> +	li	t0, 0x2004000	/* MTIMECMP0 */
> +	li	t1, -1
> +	sd	t1, 0(t0)
> +	li	t0, 0x80		/* MIE_MTIE */
> +	csrc	mie, t0
> +	mret
> +
> +	.data
> +	.balign	16
> +semiargs:
> +	.space	16
Re: [PATCH v2] target/riscv: stop translating after WFI
Posted by Daniel Henrique Barboza 2 weeks, 2 days ago 

On 9/9/2026 11:53 PM, Zephyr Li wrote:
> helper_wfi() never returns to translated code: it either raises an
> exception or exits the CPU loop after marking the CPU as halted. However,
> trans_wfi() leaves the translation state as DISAS_NEXT, so instructions
> following WFI can be included in the same TB.
> 
> With icount enabled, this makes the following instruction part of the TB's
> instruction count when WFI exits, causing minstret to be incremented once
> too many. Set DISAS_NORETURN after emitting the helper.
> 
> Add a TCG test for both reported cases: WFI waking for a locally enabled
> pending interrupt with mstatus.MIE clear, and WFI taking a timer interrupt.
> 
> Fixes: 55c2a12cbcd3 ("RISC-V TCG Code Generation")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4230
> Signed-off-by: Zephyr Li <fritchleybohrer@gmail.com>
> 
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

> Changes in v2:
> - Register test-minstret-wfi.S with the Meson TCG test framework so
>    check-tcg executes it.
> 
> diff --git a/target/riscv/tcg/insn_trans/trans_privileged.c.inc b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
> index a8eaccef67..ebbbb01179 100644
> --- a/target/riscv/tcg/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
> @@ -145,6 +145,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
>       decode_save_opc(ctx, 0);
>       gen_update_pc(ctx, ctx->cur_insn_len);
>       gen_helper_wfi(tcg_env);
> +    ctx->base.is_jmp = DISAS_NORETURN;
>       return true;
>   #else
>       return false;
> diff --git a/tests/tcg/riscv64/system/meson.build b/tests/tcg/riscv64/system/meson.build
> index 8604c2a45a..2aca68e078 100644
> --- a/tests/tcg/riscv64/system/meson.build
> +++ b/tests/tcg/riscv64/system/meson.build
> @@ -23,6 +23,7 @@ tests += {
>     'test-mepc-masking.S': setup,
>     'test-crc32.S': setup + {'qemu_args': ['-cpu', 'rv64,xlrbr=true', qemu_args]},
>     'test-minstret-ecall.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]},
> +  'test-minstret-wfi.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]},
>     'test-misa-w.S': setup + {'qemu_args': ['-cpu', 'rv64,x-misa-w=true,c=true,v=true', qemu_args]},
>   }
>   
> diff --git a/tests/tcg/riscv64/test-minstret-wfi.S b/tests/tcg/riscv64/test-minstret-wfi.S
> new file mode 100644
> index 0000000000..0fd0158d5b
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-minstret-wfi.S
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +	.option	norvc
> +
> +	.text
> +	.global	_start
> +_start:
> +	lla	t0, trap
> +	csrw	mtvec, t0
> +	li	s3, 0
> +
> +	/*
> +	 * A pending, locally enabled interrupt wakes WFI even with MIE
> +	 * clear.
> +	 */
> +	li	t0, 0x8		/* MIE_MSIE */
> +	csrw	mie, t0
> +	csrci	mstatus, 0x8	/* MSTATUS_MIE */
> +	li	t0, 0x2000000	/* MSIP0 */
> +	li	t1, 1
> +	sw	t1, 0(t0)
> +
> +	/*
> +	 * The first CSR read and WFI retire before the second read obtains s1,
> +	 * so the expected difference is two.
> +	 */
> +	csrr	s0, minstret
> +	wfi
> +	csrr	s1, minstret
> +	sub	s1, s1, s0
> +	li	t1, 2
> +	beq	s1, t1, 1f
> +	ori	s3, s3, 1
> +1:
> +
> +	/* Clear the software interrupt before testing the trap path. */
> +	sw	zero, 0(t0)
> +	csrw	mie, zero
> +
> +	/* Schedule a timer interrupt far enough ahead to reach WFI first. */
> +	li	t0, 0x200bff8	/* MTIME */
> +	ld	t1, 0(t0)
> +	addi	t1, t1, 100
> +	li	t0, 0x2004000	/* MTIMECMP0 */
> +	sd	t1, 0(t0)
> +	li	t0, 0x80		/* MIE_MTIE */
> +	csrw	mie, t0
> +	csrsi	mstatus, 0x8	/* MSTATUS_MIE */
> +
> +	/*
> +	 * The first CSR read and WFI retire before the trap handler obtains s1,
> +	 * so the expected difference is again two.
> +	 */
> +	li	s2, 1
> +	csrr	s0, minstret
> +	wfi
> +	beqz	s2, 1f
> +	ori	s3, s3, 2
> +1:
> +	bnez	s3, fail
> +	li	a0, 0
> +	j	_exit
> +
> +fail:
> +	mv	a0, s3
> +
> +_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	.
> +
> +	.balign	4
> +trap:
> +	/* Read minstret before retiring an instruction in the handler. */
> +	csrr	s1, minstret
> +	sub	s1, s1, s0
> +	addi	s1, s1, -2
> +	snez	s2, s1
> +
> +	/* Disable the timer interrupt before returning past WFI. */
> +	li	t0, 0x2004000	/* MTIMECMP0 */
> +	li	t1, -1
> +	sd	t1, 0(t0)
> +	li	t0, 0x80		/* MIE_MTIE */
> +	csrc	mie, t0
> +	mret
> +
> +	.data
> +	.balign	16
> +semiargs:
> +	.space	16

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.