From: Chao Liu <chao.liu@processmission.com>
To: TANG Tiancheng <lyndra@linux.alibaba.com>
Cc: qemu-devel@nongnu.org, "Zephyr Li" <fritchleybohrer@gmail.com>,
"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>,
qemu-riscv@nongnu.org,
"Richard Henderson" <richard.henderson@linaro.org>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>
Subject: Re: [PATCH v2 01/14] target/riscv: Preserve PMU state across event selector writes
Date: Fri, 11 Sep 2026 11:47:07 +0800 [thread overview]
Message-ID: <aqN3Q-7jqKPyt9Nn@MacBook-Pro-4.local> (raw)
In-Reply-To: <20260910-riscv-pmu-correctness-v2-1-5da5159a0c64@linux.alibaba.com>
On Thu, Sep 10, 2026 at 10:39:38PM +0800, TANG Tiancheng wrote:
> Changing mhpmevent can lose pending cycle/instruction counts or leave a
> new fixed source without a baseline and overflow timer.
>
> Account for the old source before replacing the selector, then establish
> the enabled counter's new baseline and timer. Apply this to direct and
> indirect writes.
>
> Test overflow after initializing a counter with event zero and then
> selecting instructions.
>
> Fixes: 14664483457b ("target/riscv: Add sscofpmf extension support")
> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Reviewed-by: Chao Liu <chao.liu@processmission.com>
Thanks,
Chao
> ---
> target/riscv/tcg/csr.c | 73 +++++++++++++++++++++++++----------
> tests/tcg/riscv64/sscofpmf-overflow.S | 60 ++++++++++++++++++++++++++++
> tests/tcg/riscv64/system/meson.build | 7 ++++
> 3 files changed, 119 insertions(+), 21 deletions(-)
>
> diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c
> index 65985efb220c80023cfd9d08e1878a19342aa879..52664a26f5a97a5dc8ff37abf99b4d10927fb120 100644
> --- a/target/riscv/tcg/csr.c
> +++ b/target/riscv/tcg/csr.c
> @@ -1209,23 +1209,58 @@ static RISCVException write_minstretcfgh(CPURISCVState *env, int csrno,
> static RISCVException read_mhpmevent(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - int evt_index = csrno - CSR_MCOUNTINHIBIT;
> + int ctr_idx = csrno - CSR_MCOUNTINHIBIT;
> bool rv32 = riscv_cpu_mxl(env) == MXL_RV32;
>
> - *val = extract64(env->mhpmevent_val[evt_index], 0, rv32 ? 32 : 64);
> + *val = extract64(env->mhpmevent_val[ctr_idx], 0, rv32 ? 32 : 64);
>
> return RISCV_EXCP_NONE;
> }
>
> +static uint64_t riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env,
> + int counter_idx);
> +
> +static void riscv_pmu_write_mhpmevent(CPURISCVState *env,
> + uint32_t ctr_idx, uint64_t value)
> +{
> + PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
> + bool enabled = !get_field(env->mcountinhibit, BIT(ctr_idx));
> +
> + /*
> + * A programmable counter backed by a fixed source uses mhpmcounter_val
> + * as its base and mhpmcounter_prev as the source snapshot. Preserve the
> + * visible value before changing the source or its privilege filters.
> + */
> + if (enabled &&
> + (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
> + riscv_pmu_ctr_monitor_instructions(env, ctr_idx))) {
> + uint64_t source = riscv_pmu_ctr_get_fixed_counters_val(env,
> + ctr_idx);
> +
> + counter->mhpmcounter_val += source - counter->mhpmcounter_prev;
> + }
> +
> + env->mhpmevent_val[ctr_idx] = value;
> + riscv_pmu_update_event_map(env, value, ctr_idx);
> +
> + if (enabled &&
> + (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
> + riscv_pmu_ctr_monitor_instructions(env, ctr_idx))) {
> + counter->mhpmcounter_prev =
> + riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx);
> + riscv_pmu_setup_timer(env, counter->mhpmcounter_val, ctr_idx);
> + }
> +}
> +
> static RISCVException write_mhpmevent(CPURISCVState *env, int csrno,
> target_ulong val, uintptr_t ra)
> {
> - int evt_index = csrno - CSR_MCOUNTINHIBIT;
> + int ctr_idx = csrno - CSR_MCOUNTINHIBIT;
> uint64_t mhpmevt_val;
> uint64_t inh_avail_mask;
>
> if (riscv_cpu_mxl(env) == MXL_RV32) {
> - mhpmevt_val = deposit64(env->mhpmevent_val[evt_index], 0, 32, val);
> + mhpmevt_val = deposit64(env->mhpmevent_val[ctr_idx], 0, 32, val);
> } else {
> inh_avail_mask = ~MHPMEVENT_FILTER_MASK | MHPMEVENT_BIT_MINH;
> inh_avail_mask |= riscv_has_ext(env, RVU) ? MHPMEVENT_BIT_UINH : 0;
> @@ -1237,8 +1272,7 @@ static RISCVException write_mhpmevent(CPURISCVState *env, int csrno,
> mhpmevt_val = val & inh_avail_mask;
> }
>
> - env->mhpmevent_val[evt_index] = mhpmevt_val;
> - riscv_pmu_update_event_map(env, mhpmevt_val, evt_index);
> + riscv_pmu_write_mhpmevent(env, ctr_idx, mhpmevt_val);
>
> return RISCV_EXCP_NONE;
> }
> @@ -1246,9 +1280,9 @@ static RISCVException write_mhpmevent(CPURISCVState *env, int csrno,
> static RISCVException read_mhpmeventh(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - int evt_index = csrno - CSR_MHPMEVENT3H + 3;
> + int ctr_idx = csrno - CSR_MHPMEVENT3H + 3;
>
> - *val = extract64(env->mhpmevent_val[evt_index], 32, 32);
> + *val = extract64(env->mhpmevent_val[ctr_idx], 32, 32);
>
> return RISCV_EXCP_NONE;
> }
> @@ -1256,7 +1290,7 @@ static RISCVException read_mhpmeventh(CPURISCVState *env, int csrno,
> static RISCVException write_mhpmeventh(CPURISCVState *env, int csrno,
> target_ulong val, uintptr_t ra)
> {
> - int evt_index = csrno - CSR_MHPMEVENT3H + 3;
> + int ctr_idx = csrno - CSR_MHPMEVENT3H + 3;
> target_ulong inh_avail_mask = (target_ulong)(~MHPMEVENTH_FILTER_MASK |
> MHPMEVENTH_BIT_MINH);
>
> @@ -1267,10 +1301,9 @@ static RISCVException write_mhpmeventh(CPURISCVState *env, int csrno,
> inh_avail_mask |= (riscv_has_ext(env, RVH) &&
> riscv_has_ext(env, RVS)) ? MHPMEVENTH_BIT_VSINH : 0;
>
> - env->mhpmevent_val[evt_index] = deposit64(env->mhpmevent_val[evt_index],
> - 32, 32, val & inh_avail_mask);
> -
> - riscv_pmu_update_event_map(env, env->mhpmevent_val[evt_index], evt_index);
> + riscv_pmu_write_mhpmevent(env, ctr_idx,
> + deposit64(env->mhpmevent_val[ctr_idx], 32, 32,
> + val & inh_avail_mask));
>
> return RISCV_EXCP_NONE;
> }
> @@ -1512,11 +1545,11 @@ static int rmw_cd_mhpmcounterh(CPURISCVState *env, int ctr_idx,
> return 0;
> }
>
> -static int rmw_cd_mhpmevent(CPURISCVState *env, int evt_index,
> +static int rmw_cd_mhpmevent(CPURISCVState *env, int ctr_idx,
> target_ulong *val, target_ulong new_val,
> uint64_t wr_mask)
> {
> - uint64_t mhpmevt_val = env->mhpmevent_val[evt_index];
> + uint64_t mhpmevt_val = env->mhpmevent_val[ctr_idx];
>
> if (wr_mask != 0 && wr_mask != -1) {
> return -EINVAL;
> @@ -1531,8 +1564,7 @@ static int rmw_cd_mhpmevent(CPURISCVState *env, int evt_index,
> wr_mask &= ~MHPMEVENT_BIT_MINH;
> /* wr_mask is 64-bit so upper 32 bits of mhpmevt_val are retained */
> mhpmevt_val = (new_val & wr_mask) | (mhpmevt_val & ~wr_mask);
> - env->mhpmevent_val[evt_index] = mhpmevt_val;
> - riscv_pmu_update_event_map(env, mhpmevt_val, evt_index);
> + riscv_pmu_write_mhpmevent(env, ctr_idx, mhpmevt_val);
> } else {
> return -EINVAL;
> }
> @@ -1540,11 +1572,11 @@ static int rmw_cd_mhpmevent(CPURISCVState *env, int evt_index,
> return 0;
> }
>
> -static int rmw_cd_mhpmeventh(CPURISCVState *env, int evt_index,
> +static int rmw_cd_mhpmeventh(CPURISCVState *env, int ctr_idx,
> target_ulong *val, target_ulong new_val,
> target_ulong wr_mask)
> {
> - uint64_t mhpmevt_val = env->mhpmevent_val[evt_index];
> + uint64_t mhpmevt_val = env->mhpmevent_val[ctr_idx];
> uint32_t mhpmevth_val = extract64(mhpmevt_val, 32, 32);
>
> if (wr_mask != 0 && wr_mask != -1) {
> @@ -1560,8 +1592,7 @@ static int rmw_cd_mhpmeventh(CPURISCVState *env, int evt_index,
> wr_mask &= ~MHPMEVENTH_BIT_MINH;
> mhpmevth_val = (new_val & wr_mask) | (mhpmevth_val & ~wr_mask);
> mhpmevt_val = deposit64(mhpmevt_val, 32, 32, mhpmevth_val);
> - env->mhpmevent_val[evt_index] = mhpmevt_val;
> - riscv_pmu_update_event_map(env, mhpmevt_val, evt_index);
> + riscv_pmu_write_mhpmevent(env, ctr_idx, mhpmevt_val);
> } else {
> return -EINVAL;
> }
> diff --git a/tests/tcg/riscv64/sscofpmf-overflow.S b/tests/tcg/riscv64/sscofpmf-overflow.S
> new file mode 100644
> index 0000000000000000000000000000000000000000..97f03037bbfdd44f2288b257afb91499e4534f13
> --- /dev/null
> +++ b/tests/tcg/riscv64/sscofpmf-overflow.S
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> + .option norvc
> + .option norelax
> +
> + .text
> + .global _start
> +_start:
> + /* Program hpmcounter3 while no event is selected. */
> + csrw mhpmevent3, zero
> + li t0, -256
> + csrw mhpmcounter3, t0
> +
> + /* Start counting retired instructions with overflow enabled. */
> + li t0, 2
> + csrw mhpmevent3, t0
> +
> + /* Cross the 64-bit unsigned overflow boundary. */
> + .rept 1024
> + nop
> + .endr
> +
> + /* OF must be sticky and LCOFIP must pend even with LCOFIE clear. */
> + li t4, 0
> + csrr t0, mhpmevent3
> + srli t1, t0, 63
> + xori t1, t1, 1
> + or t4, t4, t1
> +
> + csrr t0, mip
> + li t1, 1 << 13
> + and t0, t0, t1
> + sltu t0, zero, t0
> + xori t0, t0, 1
> + or t4, t4, t0
> +
> + /* The counter wraps and continues counting after overflow. */
> + csrr t0, mhpmcounter3
> + li t1, -256
> + sltu t0, t0, t1
> + xori t0, t0, 1
> + or t4, t4, t0
> +
> + lla a1, semiargs
> + li t0, 0x20026 /* ADP_Stopped_ApplicationExit */
> + sd t0, 0(a1)
> + sd t4, 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
> diff --git a/tests/tcg/riscv64/system/meson.build b/tests/tcg/riscv64/system/meson.build
> index 8604c2a45a9ad6bf8589f90d5b0d8fd1b2736db4..ebe78200fd551b42d3c99ae19ca03803797f803d 100644
> --- a/tests/tcg/riscv64/system/meson.build
> +++ b/tests/tcg/riscv64/system/meson.build
> @@ -61,6 +61,13 @@ tests += {
> }
> }
>
> +tests += {
> + 'sscofpmf-overflow.S': {
> + 'cflags': cflags,
> + 'qemu_args': ['-cpu', 'max', '-icount', 'shift=0', qemu_args],
> + },
> +}
> +
> if 'qemu-system-riscv64' in emulators
> tcg_tests += {
> 'riscv64-softmmu': {
>
> --
> 2.43.0
>
next prev parent reply other threads:[~2026-09-11 3:47 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-10 14:39 [PATCH v2 00/14] RISC-V TCG PMU correctness fixes TANG Tiancheng
2026-09-10 14:39 ` [PATCH v2 01/14] target/riscv: Preserve PMU state across event selector writes TANG Tiancheng
2026-09-11 3:47 ` Chao Liu [this message]
2026-09-10 14:39 ` [PATCH v2 02/14] target/riscv: Support multiple counters per PMU event TANG Tiancheng
2026-09-11 3:54 ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 03/14] target/riscv: Use VM-elapsed sources for fixed PMU events TANG Tiancheng
2026-09-11 3:55 ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 04/14] target/riscv: Preserve MINH on delegated config reads TANG Tiancheng
2026-09-11 3:56 ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 05/14] target/riscv: Preserve minstretcfgh on RV32 minstretcfg writes TANG Tiancheng
2026-09-11 4:43 ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 06/14] target/riscv: Fix RV32 accesses to delegated PMU registers TANG Tiancheng
2026-09-25 15:44 ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 07/14] target/riscv: Preserve fixed counters across PMU state changes TANG Tiancheng
2026-09-25 15:47 ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 08/14] target/riscv: Require Sscofpmf for non-fixed event overflow TANG Tiancheng
2026-09-25 16:08 ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 09/14] target/riscv: Rebuild fixed-event PMU overflow deadlines TANG Tiancheng
2026-09-25 16:21 ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 10/14] target/riscv: Apply minstret exception accounting to HPM counters TANG Tiancheng
2026-09-25 16:31 ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 11/14] target/riscv: Process PMU timer expiry on the owner vCPU TANG Tiancheng
2026-09-25 16:34 ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 12/14] target/riscv: Migrate fixed PMU counter state TANG Tiancheng
2026-09-25 16:40 ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 13/14] target/riscv: Clear virtualization mode on reset TANG Tiancheng
2026-09-25 16:45 ` Daniel Henrique Barboza
2026-09-28 1:31 ` TianCheng TANG
2026-09-10 14:39 ` [PATCH v2 14/14] target/riscv: Preserve fixed PMU state across reset TANG Tiancheng
2026-09-25 16:45 ` Daniel Henrique Barboza
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aqN3Q-7jqKPyt9Nn@MacBook-Pro-4.local \
--to=chao.liu@processmission.com \
--cc=alistair.francis@wdc.com \
--cc=daniel.barboza@oss.qualcomm.com \
--cc=fritchleybohrer@gmail.com \
--cc=liwei1518@gmail.com \
--cc=lyndra@linux.alibaba.com \
--cc=palmer@dabbelt.com \
--cc=pbonzini@redhat.com \
--cc=philmd@oss.qualcomm.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.