Re: [PATCH v7 11/17] KVM: x86/pmu: Track mediated PMU counters with mode-specific enables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Non-atomic modification of `pmu->global_status` in NMI context can cause lost updates.
--

commit 0456def2f622ab0d0efee8781cb983557b090999
Author: Yosry Ahmed <yosry@kernel.org>

KVM: x86/pmu: Track mediated PMU counters with mode-specific enables

This commit tracks mediated PMU counters with mode-specific enables by adding
a new bitmap to the PMU state. This avoids having to constantly check if a
counter needs to be disabled for mode-specific reasons, such as Host-Only or
Guest-Only bits in EVENTSEL on SVM.

> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 0f3cbf9fc1ae1..32a413182a751 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -615,6 +615,8 @@ struct kvm_pmu {
>  	DECLARE_BITMAP(pmc_counting_instructions, X86_PMC_IDX_MAX);
>  	DECLARE_BITMAP(pmc_counting_branches, X86_PMC_IDX_MAX);
>  
> +	DECLARE_BITMAP(pmc_has_mode_specific_enables, X86_PMC_IDX_MAX);

[Severity: High]
This is a pre-existing issue, but can the non-atomic modification of
pmu->global_status in NMI context cause lost updates?

Looking at kvm_perf_overflow(), it runs in NMI context on the host when
a performance counter overflows. It calls __kvm_perf_overflow(), which
uses the non-atomic __set_bit() and __test_and_set_bit() to update
pmu->global_status.

Concurrently, the vCPU thread modifies pmu->global_status during MSR
emulation (e.g., handling MSR_CORE_PERF_GLOBAL_OVF_CTRL or
MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR) using non-atomic read-modify-write
operations like pmu->global_status &= ~data.

If an NMI fires between the vCPU thread's load and store of global_status:

vCPU thread:
    u64 status = pmu->global_status;

NMI fires here:
    __kvm_perf_overflow()
        __set_bit(..., (unsigned long *)&pmu->global_status);

vCPU thread resumes:
    pmu->global_status = status & ~data;

Will the vCPU thread completely overwrite the NMI's update? Does this cause
KVM to silently drop performance counter overflows, leading to lost PMIs
in the guest?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527234711.4175166-1-yosry@kernel.org?part=11