[v1] target/riscv: stop translating after WFI

[PATCH] target/riscv: stop translating after WFI

Posted by Zephyr Li 2 weeks, 4 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>
---
 .../tcg/insn_trans/trans_privileged.c.inc     |   1 +
 tests/tcg/riscv64/Makefile.softmmu-target     |   4 +
 tests/tcg/riscv64/test-minstret-wfi.S         | 100 ++++++++++++++++++
 3 files changed, 105 insertions(+)
 create mode 100644 tests/tcg/riscv64/test-minstret-wfi.S

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/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
index 6a219c306c..96a3ab32c9 100644
--- a/tests/tcg/riscv64/Makefile.softmmu-target
+++ b/tests/tcg/riscv64/Makefile.softmmu-target
@@ -28,6 +28,10 @@ EXTRA_RUNS += run-test-minstret-ecall
 run-test-minstret-ecall: test-minstret-ecall
 	$(call run-test, $<, $(QEMU) -icount shift=1 $(QEMU_OPTS)$<)
 
+EXTRA_RUNS += run-test-minstret-wfi
+run-test-minstret-wfi: test-minstret-wfi
+	$(call run-test, $<, $(QEMU) -icount shift=1 $(QEMU_OPTS)$<)
+
 EXTRA_RUNS += run-plugin-doubletrap
 run-plugin-doubletrap: doubletrap
 	$(call run-test, $<, \
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] target/riscv: stop translating after WFI

Posted by Daniel Henrique Barboza 2 weeks, 2 days ago


On 9/7/2026 11:27 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>
> ---

Code LGTM.  We just need to use the meson stuff for the new test-minstret-wfi.S
test.  Otherwise 'check-tcg' won't execute it.


Thanks,
Daniel

>   .../tcg/insn_trans/trans_privileged.c.inc     |   1 +
>   tests/tcg/riscv64/Makefile.softmmu-target     |   4 +
>   tests/tcg/riscv64/test-minstret-wfi.S         | 100 ++++++++++++++++++
>   3 files changed, 105 insertions(+)
>   create mode 100644 tests/tcg/riscv64/test-minstret-wfi.S
> 
> 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/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
> index 6a219c306c..96a3ab32c9 100644
> --- a/tests/tcg/riscv64/Makefile.softmmu-target
> +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> @@ -28,6 +28,10 @@ EXTRA_RUNS += run-test-minstret-ecall
>   run-test-minstret-ecall: test-minstret-ecall
>   	$(call run-test, $<, $(QEMU) -icount shift=1 $(QEMU_OPTS)$<)
>   
> +EXTRA_RUNS += run-test-minstret-wfi
> +run-test-minstret-wfi: test-minstret-wfi
> +	$(call run-test, $<, $(QEMU) -icount shift=1 $(QEMU_OPTS)$<)
> +
>   EXTRA_RUNS += run-plugin-doubletrap
>   run-plugin-doubletrap: doubletrap
>   	$(call run-test, $<, \
> 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] target/riscv: stop translating after WFI

Posted by Zephyr Li 2 weeks, 2 days ago

Got it, thanks!

On Thu, Sep 10, 2026 at 4:00 AM Daniel Henrique Barboza <
daniel.barboza@oss.qualcomm.com> wrote:

>
>
> On 9/7/2026 11:27 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>
> > ---
>
> Code LGTM.  We just need to use the meson stuff for the new
> test-minstret-wfi.S
> test.  Otherwise 'check-tcg' won't execute it.
>
>
> Thanks,
> Daniel
>
> >   .../tcg/insn_trans/trans_privileged.c.inc     |   1 +
> >   tests/tcg/riscv64/Makefile.softmmu-target     |   4 +
> >   tests/tcg/riscv64/test-minstret-wfi.S         | 100 ++++++++++++++++++
> >   3 files changed, 105 insertions(+)
> >   create mode 100644 tests/tcg/riscv64/test-minstret-wfi.S
> >
> > 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/Makefile.softmmu-target
> b/tests/tcg/riscv64/Makefile.softmmu-target
> > index 6a219c306c..96a3ab32c9 100644
> > --- a/tests/tcg/riscv64/Makefile.softmmu-target
> > +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> > @@ -28,6 +28,10 @@ EXTRA_RUNS += run-test-minstret-ecall
> >   run-test-minstret-ecall: test-minstret-ecall
> >       $(call run-test, $<, $(QEMU) -icount shift=1 $(QEMU_OPTS)$<)
> >
> > +EXTRA_RUNS += run-test-minstret-wfi
> > +run-test-minstret-wfi: test-minstret-wfi
> > +     $(call run-test, $<, $(QEMU) -icount shift=1 $(QEMU_OPTS)$<)
> > +
> >   EXTRA_RUNS += run-plugin-doubletrap
> >   run-plugin-doubletrap: doubletrap
> >       $(call run-test, $<, \
> > 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
>
>