Re: [kvm-unit-tests PATCH v3 03/13] x86/pmu: Reset the expected count of the fixed counter 0 when i386

[Sean Christopherson <seanjc@google.com>]

On Fri, Aug 19, 2022, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
> 
> The pmu test check_counter_overflow() always fails with the "./configure
> --arch=i386".

Explicitly state that the failures are with 32-bit binaries.  E.g. I can and do
run KUT in 32-bit VMs, which doesn't require the explicit --arch=i386.

> The cnt.count obtained from the latter run of measure()
> (based on fixed counter 0) is not equal to the expected value (based
> on gp counter 0) and there is a positive error with a value of 2.
> 
> The two extra instructions come from inline wrmsr() and inline rdmsr()
> inside the global_disable() binary code block. Specifically, for each msr
> access, the i386 code will have two assembly mov instructions before
> rdmsr/wrmsr (mark it for fixed counter 0, bit 32), but only one assembly
> mov is needed for x86_64 and gp counter 0 on i386.
> 
> Fix the expected init cnt.count for fixed counter 0 overflow based on
> the same fixed counter 0, not always using gp counter 0.

You lost me here.  I totally understand the problem, but I don't understand the
fix.

> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
>  x86/pmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/x86/pmu.c b/x86/pmu.c
> index 45ca2c6..057fd4a 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -315,6 +315,9 @@ static void check_counter_overflow(void)
>  
>  		if (i == nr_gp_counters) {
>  			cnt.ctr = fixed_events[0].unit_sel;
> +			__measure(&cnt, 0);
> +			count = cnt.count;
> +			cnt.count = 1 - count;

This definitely needs a comment.

Dumb question time: if the count is off by 2, why can't we just subtract 2?

#ifndef __x86_64__
			/* comment about extra MOV insns for RDMSR/WRMSR */
			cnt.count -= 2;
#endif

>  			cnt.count &= (1ull << pmu_fixed_counter_width()) - 1;
>  		}
>  
> -- 
> 2.37.2
>