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

[Like Xu <like.xu.linux@gmail.com>]

On 1/8/2022 11:43 pm, Sean Christopherson wrote:
> 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?
> 

None object to this idea, after all, there is obvious redundancy here.

> 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);

Now we have to fix the last call to measure() in check_counter_overflow(), since 
it will
also call start_event() after it has been modified and in that case, the 
requested high count
has to be passed in from another function parameter.

Also, the naming of start_event() does not imply that the counter will be set to 
zero implicitly,
it just lets a counter continue to run, not caring about the current value of 
the counter,
which is more flexible.

I may try to do that on the test-cases of AMD vPMU, to help verify the gain of 
your idea.

>       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().