Re: [kvm-unit-tests PATCH] x86/pmu: Treat all AMD CPUs as having instruction/branch overcount errata

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 01, 2026, Sandipan Das wrote:
> On 01-10-2026 02:32, Sean Christopherson wrote:
> > diff --git a/x86/pmu.c b/x86/pmu.c
> > index b262ea59..35f8a818 100644
> > --- a/x86/pmu.c
> > +++ b/x86/pmu.c
> > @@ -222,13 +222,8 @@ static void adjust_events_range(struct pmu_event *gp_events,
> >  	 * If HW supports GLOBAL_CTRL MSR, enabling and disabling PMCs are
> >  	 * moved in __precise_loop(). Thus, instructions and branches events
> >  	 * can be verified against a precise count instead of a rough range.
> > -	 *
> > -	 * Skip the precise checks on AMD, as AMD CPUs count VMRUN as a branch
> > -	 * instruction in guest context, which* leads to intermittent failures
> > -	 * as the counts will vary depending on how many asynchronous VM-Exits
> > -	 * occur while running the measured code, e.g. if the host takes IRQs.
> >  	 */
> > -	if (pmu.is_intel && this_cpu_has_perf_global_ctrl()) {
> > +	if (this_cpu_has_perf_global_ctrl()) {
> >  		if (!pmu.errata.instructions_retired_overcount) {
> >  			gp_events[instruction_idx].min = LOOP_INSNS;
> >  			gp_events[instruction_idx].max = LOOP_INSNS;
> > 
> 
> Commit 61a7ee521d ("x86/pmu: Handle instruction overcount issue in overflow
> test") makes measure_for_overflow() always return 1 - LOOP_INSNS (-10000016)
> when pmu.errata.instructions_retired_overcount is true. For runs without
> "perfmon-v2" on a Zen 5 system, I see that the overshoot from this preset value
> is between 125 and 135,

Heh, I was going to say "woah, that's a lot of exits!", then I realized the
loop does a million runs...

> which makes the following condition in check_counter_overflow() fail.
> 
> 	report(cnt.count == 0xffffffffffff || cnt.count < 7, "cntr-%d", i);
> 
> How should we handle this?
> 
> Restrict the condition in measure_for_overflow() to return 1 - LOOP_INSNS
> if (pmu.is_intel && pmu.errata.instructions_retired_overcount)
> 
> or, accept a larger overshoot.

Doesn't it have to be the latter?  Because won't the test fail if the first run
of __measure() from measure_for_overflow() takes more exits than the second run
of __measure()?

> diff --git a/x86/pmu.c b/x86/pmu.c
> index 35f8a8183a..74e30b149d 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -584,6 +584,8 @@ static void check_counter_overflow(void)
>                         else
>                                 report(cnt.count == 1, "cntr-%d", i);
>                 }
> +               else if (pmu.errata.instructions_retired_overcount)
> +                       report(cnt.count == 0xffffffffffff || cnt.count < 150, "cntr-%d", i);

Do you have any idea where the 0xffffffffffff check comes from?  Commit b883751a
("x86/pmu: Update testcases to cover AMD PMU") doesn't explain it at all.

Depending on what's up with the 0xffffffffffff thing, my vote would be to go big
hammer, and express the overflow as a fraction of loop instructions, e.g. allow
for an extra 200 exits:

		if (pmu.errata.instructions_retired_overcount)
			report(cnt.count < N / 5000, "cntr-%d", i);
		else
			report(cnt.count == 1, "cntr-%d", i);

>                 else
>                         report(cnt.count == 0xffffffffffff || cnt.count < 7, "cntr-%d", i);
>