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