* [PATCH] target/riscv: stop translating after WFI
@ 2026-09-08 2:27 Zephyr Li
2026-09-09 20:00 ` Daniel Henrique Barboza
0 siblings, 1 reply; 3+ messages in thread
From: Zephyr Li @ 2026-09-08 2:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, Chao Liu, Zephyr Li
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] target/riscv: stop translating after WFI
2026-09-08 2:27 [PATCH] target/riscv: stop translating after WFI Zephyr Li
@ 2026-09-09 20:00 ` Daniel Henrique Barboza
2026-09-10 0:54 ` Zephyr Li
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Henrique Barboza @ 2026-09-09 20:00 UTC (permalink / raw)
To: Zephyr Li, qemu-devel
Cc: qemu-riscv, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Liu Zhiwei, Chao Liu
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] target/riscv: stop translating after WFI
2026-09-09 20:00 ` Daniel Henrique Barboza
@ 2026-09-10 0:54 ` Zephyr Li
0 siblings, 0 replies; 3+ messages in thread
From: Zephyr Li @ 2026-09-10 0:54 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis,
Weiwei Li, Liu Zhiwei, Chao Liu
[-- Attachment #1: Type: text/plain, Size: 6104 bytes --]
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
>
>
[-- Attachment #2: Type: text/html, Size: 7896 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-10 0:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 2:27 [PATCH] target/riscv: stop translating after WFI Zephyr Li
2026-09-09 20:00 ` Daniel Henrique Barboza
2026-09-10 0:54 ` Zephyr Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.