Re: [PATCH V3 04/13] target/i386: Adjust maximum number of PMU counters

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 3/5/2026 2:07 AM, Zide Chen wrote:
> Changing either MAX_GP_COUNTERS or MAX_FIXED_COUNTERS affects the
> VMState layout and therefore requires bumping the migration version
> IDs.  Adjust both limits together to avoid repeated VMState version
> bumps in follow-up patches.
>
> To support full-width writes, QEMU needs to handle the alias MSRs
> starting at 0x4c1.  With the current limits, the alias range can
> extend into MSR_MCG_EXT_CTL (0x4d0).  Reducing MAX_GP_COUNTERS from 18
> to 15 avoids the overlap while still leaving room for future expansion
> beyond current hardware (which supports at most 10 GP counters).
>
> Increase MAX_FIXED_COUNTERS to 7 to support additional fixed counters
> (e.g. Topdown metric events).
>
> With these changes, bump version_id to prevent migration to older
> QEMU, and bump minimum_version_id to prevent migration from older
> QEMU, which could otherwise result in VMState overflows.
>
> Signed-off-by: Zide Chen <zide.chen@intel.com>
> ---
>  target/i386/cpu.h     | 8 ++------
>  target/i386/machine.c | 4 ++--
>  2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 6d3e70395dbd..23d4ee13abfa 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1749,12 +1749,8 @@ typedef struct {
>  #define CPU_NB_REGS CPU_NB_REGS32
>  #endif
>  
> -#define MAX_FIXED_COUNTERS 3
> -/*
> - * This formula is based on Intel's MSR. The current size also meets AMD's
> - * needs.
> - */
> -#define MAX_GP_COUNTERS    (MSR_IA32_PERF_STATUS - MSR_P6_EVNTSEL0)
> +#define MAX_FIXED_COUNTERS 7
> +#define MAX_GP_COUNTERS    15

I suppose it's good enough to reduce MAX_GP_COUNTERS to 10. I don't think
there would be 10+ GP counters for Intel platforms in near future. But need
AMD guys to confirm if it's enough for AMD platforms.

Of course, shrinking MAX_GP_COUNTERS to 15 is fine for me as well.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>


>  
>  #define NB_OPMASK_REGS 8
>  
> diff --git a/target/i386/machine.c b/target/i386/machine.c
> index 1125c8a64ec5..7d08a05835fc 100644
> --- a/target/i386/machine.c
> +++ b/target/i386/machine.c
> @@ -685,8 +685,8 @@ static bool pmu_enable_needed(void *opaque)
>  
>  static const VMStateDescription vmstate_msr_architectural_pmu = {
>      .name = "cpu/msr_architectural_pmu",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .needed = pmu_enable_needed,
>      .fields = (const VMStateField[]) {
>          VMSTATE_UINT64(env.msr_fixed_ctr_ctrl, X86CPU),