@samitouri / QOSamiQemu / commits / 5a9fa8b582

target/riscv: do not count ECALL in minstret

With icount enabled, helper_raise_exception() leaves ECALL in icount_get_raw() because it exits without restoring the current TB state. This makes minstret count an instruction that does not retire. Adjust only the fixed minstret baseline so that mcycle accounting remains unchanged. Add an RV64 softmmu regression test for the issue. Fixes: 4fe8ae09062d ("target/riscv: Combine mhpmcounter and mhpmcounterh") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4087 Signed-off-by: Zephyr Li <fritchleybohrer@gmail.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-ID: <20260730032122.2564190-1-fritchleybohrer@gmail.com> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Zephyr Li committed Jul 30, 2026 at 11:21 UTC 5a9fa8b5820998a5aca02ecf3df12819f7ff2173
5 files changed +98 -12
target/riscv/tcg/op_helper.c
+6
@@ -21,6 +21,9 @@
21 #include "qemu/osdep.h"
22 #include "cpu.h"
23 #include "target/riscv/tcg/csr.h"
24 +#ifndef CONFIG_USER_ONLY
25 +#include "pmu.h"
26 +#endif
27 #include "internals.h"
28 #include "exec/cputlb.h"
29 #include "accel/tcg/cpu-ldst.h"
@@ -47,6 +50,9 @@ G_NORETURN void riscv_raise_exception(CPURISCVState *env,
50
51 void helper_raise_exception(CPURISCVState *env, uint32_t exception)
52 {
53 +#ifndef CONFIG_USER_ONLY
54 + riscv_pmu_decr_instret(env);
55 +#endif
56 riscv_raise_exception(env, exception, 0);
57 }
58
target/riscv/tcg/pmu.c
+32 -12
@@ -49,6 +49,21 @@ static bool riscv_pmu_counter_enabled(RISCVCPU *cpu, uint32_t ctr_idx)
49 }
50 }
51
52 +static bool riscv_pmu_counter_filtered(CPURISCVState *env, uint64_t cfg)
53 +{
54 + bool virt_on = env->virt_enabled;
55 +
56 + return (env->priv == PRV_M && (cfg & MHPMEVENT_BIT_MINH)) ||
57 + (env->priv == PRV_S && virt_on &&
58 + (cfg & MHPMEVENT_BIT_VSINH)) ||
59 + (env->priv == PRV_U && virt_on &&
60 + (cfg & MHPMEVENT_BIT_VUINH)) ||
61 + (env->priv == PRV_S && !virt_on &&
62 + (cfg & MHPMEVENT_BIT_SINH)) ||
63 + (env->priv == PRV_U && !virt_on &&
64 + (cfg & MHPMEVENT_BIT_UINH));
65 +}
66 +
67 /*
68 * Information needed to update counters:
69 * new_priv, new_virt: To correctly save starting snapshot for the newly
@@ -147,12 +162,27 @@ void riscv_pmu_update_fixed_ctrs(CPURISCVState *env,
162 riscv_pmu_icount_update_priv(env, newpriv, new_virt);
163 }
164
165 +void riscv_pmu_decr_instret(CPURISCVState *env)
166 +{
167 + if (!icount_enabled() ||
168 + (env->mcountinhibit & COUNTEREN_IR) ||
169 + riscv_pmu_counter_filtered(env, env->minstretcfg)) {
170 + return;
171 + }
172 +
173 + /*
174 + * minstret is derived from icount, which includes the current
175 + * instruction. Move the baseline forward to exclude an instruction
176 + * that raises an exception and therefore does not retire.
177 + */
178 + env->pmu_ctrs[2].mhpmcounter_prev++;
179 +}
180 +
181 int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx)
182 {
183 uint32_t ctr_idx;
184 CPURISCVState *env = &cpu->env;
185 uint64_t max_val = UINT64_MAX;
155 - bool virt_on = env->virt_enabled;
186 PMUCTRState *counter;
187 gpointer value;
188
@@ -170,17 +200,7 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx)
200 return -1;
201 }
202
173 - /* Privilege mode filtering */
174 - if ((env->priv == PRV_M &&
175 - (env->mhpmevent_val[ctr_idx] & MHPMEVENT_BIT_MINH)) ||
176 - (env->priv == PRV_S && virt_on &&
177 - (env->mhpmevent_val[ctr_idx] & MHPMEVENT_BIT_VSINH)) ||
178 - (env->priv == PRV_U && virt_on &&
179 - (env->mhpmevent_val[ctr_idx] & MHPMEVENT_BIT_VUINH)) ||
180 - (env->priv == PRV_S && !virt_on &&
181 - (env->mhpmevent_val[ctr_idx] & MHPMEVENT_BIT_SINH)) ||
182 - (env->priv == PRV_U && !virt_on &&
183 - (env->mhpmevent_val[ctr_idx] & MHPMEVENT_BIT_UINH))) {
203 + if (riscv_pmu_counter_filtered(env, env->mhpmevent_val[ctr_idx])) {
204 return 0;
205 }
206
target/riscv/tcg/pmu.h
+1
@@ -36,6 +36,7 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value,
36 uint32_t ctr_idx);
37 void riscv_pmu_update_fixed_ctrs(CPURISCVState *env, privilege_mode_t newpriv,
38 bool new_virt);
39 +void riscv_pmu_decr_instret(CPURISCVState *env);
40 RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
41 bool upper_half, uint32_t ctr_idx);
42
tests/tcg/riscv64/Makefile.softmmu-target
+4
@@ -24,6 +24,10 @@ EXTRA_RUNS += run-test-mepc-masking
24 run-test-mepc-masking: test-mepc-masking
25 $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
26
27 +EXTRA_RUNS += run-test-minstret-ecall
28 +run-test-minstret-ecall: test-minstret-ecall
29 + $(call run-test, $<, $(QEMU) -icount shift=1 $(QEMU_OPTS)$<)
30 +
31 EXTRA_RUNS += run-plugin-doubletrap
32 run-plugin-doubletrap: doubletrap
33 $(call run-test, $<, \
tests/tcg/riscv64/test-minstret-ecall.S new
+55
@@ -0,0 +1,55 @@
1 +/* SPDX-License-Identifier: GPL-2.0-or-later */
2 +
3 + .option norvc
4 +
5 + .text
6 + .global _start
7 +_start:
8 + lla t0, trap
9 + csrw mtvec, t0
10 +
11 + /*
12 + * The first CSR read retires after obtaining s0. The ecall does not
13 + * retire, so the trap handler must observe a difference of one.
14 + */
15 + csrr s0, minstret
16 + ecall
17 + sub t0, s1, s0
18 + li t1, 1
19 + bne t0, t1, fail
20 +
21 + li a0, 0
22 + j _exit
23 +
24 +trap:
25 + csrr s1, minstret
26 + csrr t0, mcause
27 + li t1, 11 /* Environment call from M-mode */
28 + bne t0, t1, fail
29 +
30 + csrr t0, mepc
31 + addi t0, t0, 4
32 + csrw mepc, t0
33 + mret
34 +
35 +fail:
36 + li a0, 1
37 +
38 +_exit:
39 + lla a1, semiargs
40 + li t0, 0x20026 /* ADP_Stopped_ApplicationExit */
41 + sd t0, 0(a1)
42 + sd a0, 8(a1)
43 + li a0, 0x20 /* TARGET_SYS_EXIT_EXTENDED */
44 +
45 + /* Semihosting call sequence */
46 + .balign 16
47 + slli zero, zero, 0x1f
48 + ebreak
49 + srai zero, zero, 0x7
50 + j .
51 +
52 + .data
53 + .balign 16
54 +semiargs:
55 + .space 16