All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
To: TANG Tiancheng <lyndra@linux.alibaba.com>, qemu-devel@nongnu.org
Cc: "Zephyr Li" <fritchleybohrer@gmail.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Weiwei Li" <liwei1518@gmail.com>,
	"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
	"Chao Liu" <chao.liu@processmission.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 12/14] target/riscv: Migrate fixed PMU counter state
Date: Fri, 25 Sep 2026 13:40:17 -0300	[thread overview]
Message-ID: <3da698b9-8b0f-40a9-bc70-87c18363e033@oss.qualcomm.com> (raw)
In-Reply-To: <20260910-riscv-pmu-correctness-v2-12-5da5159a0c64@linux.alibaba.com>



On 9/10/2026 11:39 AM, TANG Tiancheng wrote:
> Migration saves fixed-source baselines but not the per-mode totals they
> refer to, nor mcyclecfg/minstretcfg. TCG can therefore subtract an
> unrelated baseline or apply the wrong privilege filter after loading.
> 
> Before saving, add pending increments allowed by the current filters to
> each enabled fixed-source counter, including HPM cycle/instruction
> counters. The per-mode totals then need not migrate.
> 
> On load, clear those totals and establish destination-local baselines.
> Inhibited counters get a new baseline when enabled; other event counters
> get one when switched to a fixed source. This excludes migration downtime
> and avoids using the source QEMU's saved baselines.
> 
> Rebuild the event map and overflow timer. Recompute interrupt requests
> because pre-save can set LCOFIP after cpu_common saved CPU_INTERRUPT_HARD.
> 
> Keep the existing main-section fields and add cpu/pmu-fixed to carry
> mcyclecfg/minstretcfg and identify values that include pending increments.
> All TCG CPUs send and require it because mcycle/minstret exist even
> without Zicntr, Zihpm or HPM counters. Reject older TCG streams, whose
> counter values cannot be reconstructed reliably. KVM PMU migration is
> unchanged.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4422
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4425
> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
> ---

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

>   target/riscv/cpu.h     |  1 +
>   target/riscv/machine.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++----
>   target/riscv/tcg/pmu.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
>   target/riscv/tcg/pmu.h |  2 ++
>   4 files changed, 163 insertions(+), 7 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a4d33f55c4e5cb6052cea6bee4c0afa46372b5c5..788d5a3ced45b32be05e29eaeb9ea3af0c0ccd6d 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -584,6 +584,7 @@ struct ArchCPU {
>       uint64_t pmu_timer_instret_snapshot;
>       bool pmu_timer_stalled;
>       bool pmu_timer_work_pending;
> +    bool pmu_fixed_subsection_present;
>       /* A bitmask of Available programmable counters */
>       uint32_t pmu_avail_ctrs;
>       /* Mapping of events to counters */
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index b0ff2fc7f2ac10fab1f2ff845a953649091e1f43..7aa38b739cfd4d9274fe249eb914411e8a65b91f 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -267,6 +267,30 @@ static const VMStateDescription vmstate_kvm_mp_state = {
>   };
>   #endif
>   
> +static int riscv_cpu_pre_load(void *opaque)
> +{
> +    RISCVCPU *cpu = opaque;
> +
> +    cpu->pmu_fixed_subsection_present = false;
> +#ifdef CONFIG_KVM
> +    return riscv_cpu_kvm_pre_load(opaque);
> +#else
> +    return 0;
> +#endif
> +}
> +
> +static int riscv_cpu_pre_save(void *opaque)
> +{
> +#ifdef CONFIG_TCG
> +    RISCVCPU *cpu = opaque;
> +
> +    if (tcg_enabled()) {
> +        riscv_pmu_prepare_save(&cpu->env);
> +    }
> +#endif
> +    return 0;
> +}
> +
>   static bool debug_needed(void *opaque)
>   {
>       RISCVCPU *cpu = opaque;
> @@ -308,16 +332,25 @@ static const VMStateDescription vmstate_debug = {
>       }
>   };
>   
> -static int riscv_cpu_post_load(void *opaque, int version_id)
> +static bool riscv_cpu_post_load(void *opaque, int version_id, Error **errp)
>   {
>       RISCVCPU *cpu = opaque;
>       CPURISCVState *env = &cpu->env;
>   
>       env->xl = cpu_recompute_xl(env);
>   #ifdef CONFIG_TCG
> -    riscv_pmu_rebuild_event_map(env);
> +    if (tcg_enabled()) {
> +        if (!cpu->pmu_fixed_subsection_present) {
> +            error_setg(errp,
> +                       "missing RISC-V fixed-counter PMU migration state");
> +            return false;
> +        }
> +        riscv_pmu_complete_load(env);
> +        /* PMU pre-save can raise an interrupt after cpu_common was saved. */
> +        riscv_cpu_interrupt(env);
> +    }
>   #endif
> -    return 0;
> +    return true;
>   }
>   
>   static bool smstateen_needed(void *opaque)
> @@ -404,6 +437,42 @@ static const VMStateDescription vmstate_pmu_ctr_state = {
>       }
>   };
>   
> +static int pmu_fixed_post_load(void *opaque, int version_id)
> +{
> +    RISCVCPU *cpu = opaque;
> +
> +    /* Let the outer post-load distinguish this format from a legacy stream. */
> +    cpu->pmu_fixed_subsection_present = true;
> +    return 0;
> +}
> +
> +static bool pmu_fixed_needed(void *opaque)
> +{
> +    /*
> +     * KVM keeps PMU state in the kernel, not in the TCG counter model.
> +     * TCG implements mcycle/minstret even without Zicntr, Zihpm or
> +     * programmable counters.
> +     */
> +    return tcg_enabled();
> +}
> +
> +/*
> + * This subsection identifies TCG streams whose fixed-source counter values
> + * include pending deltas. It also carries mcyclecfg and minstretcfg.
> + */
> +static const VMStateDescription vmstate_pmu_fixed = {
> +    .name = "cpu/pmu-fixed",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = pmu_fixed_needed,
> +    .post_load = pmu_fixed_post_load,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_UINT64(env.mcyclecfg, RISCVCPU),
> +        VMSTATE_UINT64(env.minstretcfg, RISCVCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>   static bool jvt_needed(void *opaque)
>   {
>       RISCVCPU *cpu = opaque;
> @@ -505,10 +574,9 @@ const VMStateDescription vmstate_riscv_cpu = {
>       .name = "cpu",
>       .version_id = 12,
>       .minimum_version_id = 12,
> -#ifdef CONFIG_KVM
> -    .pre_load = riscv_cpu_kvm_pre_load,
> -#endif
> -    .post_load = riscv_cpu_post_load,
> +    .pre_load = riscv_cpu_pre_load,
> +    .pre_save = riscv_cpu_pre_save,
> +    .post_load_errp = riscv_cpu_post_load,
>       .fields = (const VMStateField[]) {
>           VMSTATE_UINT64_ARRAY(env.gpr, RISCVCPU, 32),
>           VMSTATE_UINT64_ARRAY(env.fpr, RISCVCPU, 32),
> @@ -554,6 +622,11 @@ const VMStateDescription vmstate_riscv_cpu = {
>           VMSTATE_UINT32(env.mcounteren, RISCVCPU),
>           VMSTATE_UINT32(env.scountinhibit, RISCVCPU),
>           VMSTATE_UINT32(env.mcountinhibit, RISCVCPU),
> +        /*
> +         * TCG includes pending fixed-source deltas in mhpmcounter_val
> +         * before saving. After loading, it ignores mhpmcounter_prev and
> +         * rebuilds the baseline from the destination source.
> +         */
>           VMSTATE_STRUCT_ARRAY(env.pmu_ctrs, RISCVCPU, RV_MAX_MHPMCOUNTERS, 0,
>                                vmstate_pmu_ctr_state, PMUCTRState),
>           VMSTATE_UINT64_ARRAY(env.mhpmevent_val, RISCVCPU, RV_MAX_MHPMEVENTS),
> @@ -582,6 +655,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>           &vmstate_ctr,
>           &vmstate_sstc,
>           &vmstate_mseccfg,
> +        &vmstate_pmu_fixed,
>           NULL
>       }
>   };
> diff --git a/target/riscv/tcg/pmu.c b/target/riscv/tcg/pmu.c
> index 6286552a4ebf614df0252f84ddfadbc25d8d2258..df99b572a4c16cb1ac65c2f7cde35c6f8349e681 100644
> --- a/target/riscv/tcg/pmu.c
> +++ b/target/riscv/tcg/pmu.c
> @@ -663,6 +663,85 @@ void riscv_pmu_rebuild_timer(CPURISCVState *env)
>       riscv_pmu_rebuild_timer_internal(env, false);
>   }
>   
> +static uint32_t riscv_pmu_fixed_source_counter_mask(CPURISCVState *env)
> +{
> +    RISCVCPU *cpu = env_archcpu(env);
> +    uint32_t mask = COUNTEREN_CY | COUNTEREN_IR;
> +
> +    mask |= riscv_pmu_event_counter_mask(
> +        cpu, RISCV_PMU_EVENT_HW_CPU_CYCLES);
> +    mask |= riscv_pmu_event_counter_mask(
> +        cpu, RISCV_PMU_EVENT_HW_INSTRUCTIONS);
> +    return mask;
> +}
> +
> +static void riscv_pmu_accumulate_fixed_source_counters(
> +    CPURISCVState *env, const RISCVPMUFixedSnapshot *snapshot)
> +{
> +    uint32_t mask = riscv_pmu_fixed_source_counter_mask(env);
> +
> +    while (mask) {
> +        uint32_t ctr_idx = ctz32(mask);
> +
> +        mask &= ~BIT(ctr_idx);
> +        if (riscv_pmu_fixed_ctr_running(env, ctr_idx)) {
> +            riscv_pmu_accumulate_fixed_delta(env, ctr_idx, snapshot);
> +        }
> +    }
> +}
> +
> +void riscv_pmu_prepare_save(CPURISCVState *env)
> +{
> +    RISCVPMUFixedSnapshot snapshot;
> +
> +    riscv_pmu_take_fixed_snapshot(env, &snapshot);
> +    riscv_pmu_accumulate_fixed_source_counters(env, &snapshot);
> +}
> +
> +static void riscv_pmu_rebase_fixed_source_counters(
> +    CPURISCVState *env, const RISCVPMUFixedSnapshot *snapshot)
> +{
> +    uint32_t mask;
> +
> +    memset(env->pmu_fixed_ctrs, 0, sizeof(env->pmu_fixed_ctrs));
> +    if (env->virt_enabled) {
> +        env->pmu_fixed_ctrs[RISCV_PMU_FIXED_DOMAIN_CYCLE]
> +            .counter_virt_prev[env->priv] = snapshot->cycle;
> +        env->pmu_fixed_ctrs[RISCV_PMU_FIXED_DOMAIN_INSTRET]
> +            .counter_virt_prev[env->priv] = snapshot->instret;
> +    } else {
> +        env->pmu_fixed_ctrs[RISCV_PMU_FIXED_DOMAIN_CYCLE]
> +            .counter_prev[env->priv] = snapshot->cycle;
> +        env->pmu_fixed_ctrs[RISCV_PMU_FIXED_DOMAIN_INSTRET]
> +            .counter_prev[env->priv] = snapshot->instret;
> +    }
> +
> +    mask = riscv_pmu_fixed_source_counter_mask(env);
> +    while (mask) {
> +        uint32_t ctr_idx = ctz32(mask);
> +
> +        mask &= ~BIT(ctr_idx);
> +        if (riscv_pmu_fixed_ctr_enabled(env, ctr_idx)) {
> +            riscv_pmu_set_fixed_baseline(env, ctr_idx, snapshot);
> +        }
> +    }
> +}
> +
> +void riscv_pmu_complete_load(CPURISCVState *env)
> +{
> +    RISCVCPU *cpu = env_archcpu(env);
> +    RISCVPMUFixedSnapshot snapshot;
> +
> +    riscv_pmu_rebuild_event_map(env);
> +    riscv_pmu_take_fixed_snapshot(env, &snapshot);
> +    riscv_pmu_rebase_fixed_source_counters(env, &snapshot);
> +
> +    qatomic_set(&cpu->pmu_timer_work_pending, false);
> +    cpu->pmu_timer_stalled = false;
> +    cpu->pmu_timer_instret_snapshot = snapshot.instret;
> +    riscv_pmu_rebuild_timer(env);
> +}
> +
>   static void riscv_pmu_timer_work(CPUState *cs, run_on_cpu_data data)
>   {
>       RISCVCPU *cpu = RISCV_CPU(cs);
> diff --git a/target/riscv/tcg/pmu.h b/target/riscv/tcg/pmu.h
> index d9238ae680f5e67031511db4f9afc2884212c5c2..1cfe6acf55b5468f5c00c4a136ece88981384341 100644
> --- a/target/riscv/tcg/pmu.h
> +++ b/target/riscv/tcg/pmu.h
> @@ -45,6 +45,8 @@ void riscv_pmu_write_counter(CPURISCVState *env, uint32_t ctr_idx,
>   void riscv_pmu_write_inhibit(CPURISCVState *env, uint32_t value);
>   void riscv_pmu_timer_cb(void *priv);
>   void riscv_pmu_rebuild_timer(CPURISCVState *env);
> +void riscv_pmu_prepare_save(CPURISCVState *env);
> +void riscv_pmu_complete_load(CPURISCVState *env);
>   void riscv_pmu_init(RISCVCPU *cpu, Error **errp);
>   void riscv_pmu_rebuild_event_map(CPURISCVState *env);
>   int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx);
> 



  reply	other threads:[~2026-09-25 16:41 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
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 [this message]
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=3da698b9-8b0f-40a9-bc70-87c18363e033@oss.qualcomm.com \
    --to=daniel.barboza@oss.qualcomm.com \
    --cc=alistair.francis@wdc.com \
    --cc=chao.liu@processmission.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.