Re: [PATCH 6/6] KVM: x86/pmu: Track emulated counter events instead of previous counter

[Sean Christopherson]

On Mon, Oct 23, 2023, Sean Christopherson wrote:
> @@ -226,13 +226,19 @@ static int pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type, u64 config,
>  
>  static void pmc_pause_counter(struct kvm_pmc *pmc)
>  {
> -	u64 counter = pmc->counter;
> +	/*
> +	 * Accumulate emulated events, even if the PMC was already paused, e.g.
> +	 * if KVM emulated an event after a WRMSR, but before reprogramming, or
> +	 * if KVM couldn't create a perf event.
> +	 */
> +	u64 counter = pmc->counter + pmc->emulated_counter;
>  
> -	if (!pmc->perf_event || pmc->is_paused)
> -		return;
> +	pmc->emulated_counter = 0;

As pointed by Mingwei, who _very_ patiently explained to me what is broken, the
snapshot used to detect overflow due to emulated_counter events needs to be taken
_after_ pausing the perf event, i.e. the count from the perf event needs to be
excluded.  If overflow happens from pmc->counter => pmc->counter + pmc->perf_event,
then hardware (via perf) will detect overflow.  I.e. KVM is only responsible for
detecting overflow solely due to emulated_counter.  Include the count from the
perf event can lead to KVM generating multiple overflow events, where architecturally
only one should occur.

>  	/* update counter, reset event value to avoid redundant accumulation */
> -	counter += perf_event_pause(pmc->perf_event, true);
> +	if (pmc->perf_event && !pmc->is_paused)
> +		counter += perf_event_pause(pmc->perf_event, true);
> +
>  	pmc->counter = counter & pmc_bitmask(pmc);
>  	pmc->is_paused = true;
>  }