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

[Sean Christopherson <seanjc@google.com>]

On Mon, Aug 01, 2022, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
> 
> The pmu test check_counter_overflow() always fails with the "./configure
> --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.
> 
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
>  x86/pmu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/x86/pmu.c b/x86/pmu.c
> index 01be1e9..4bb24e9 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -304,6 +304,10 @@ static void check_counter_overflow(void)
>  
>  		if (i == nr_gp_counters) {
>  			cnt.ctr = fixed_events[0].unit_sel;
> +			cnt.count = 0;
> +			measure(&cnt, 1);

Not directly related to this patch...

Unless I've missed something, every invocation of start_event() and measure() first
sets evt.count=0.  Rather than force every caller to ensure count is zeroed, why not
zero the count during start_event() and then drop all of the manual zeroing?

diff --git a/x86/pmu.c b/x86/pmu.c
index 01be1e90..ef804272 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -141,7 +141,7 @@ static void global_disable(pmu_counter_t *cnt)

 static void start_event(pmu_counter_t *evt)
 {
-    wrmsr(evt->ctr, evt->count);
+    wrmsr(evt->ctr, 0);
     if (is_gp(evt))
            wrmsr(MSR_P6_EVNTSEL0 + event_to_global_idx(evt),
                            evt->config | EVNTSEL_EN);


Accumulating counts can be handled by reading the current count before start_event(),
and doing something like stuffing a high count to test an edge case could be handled
by an inner helper, e.g. by adding __start_event().