Re: [PATCH 2/4] perf/x86/intel: Don't context switch DS_AREA (and PEBS config) if PEBS is unused

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Critical] Omitting MSRs from the returned array fails to clear them from KVM's VMCS autoload list, defeating the optimization and causing host PMU state corruption and cross-CPU memory corruption.
--

commit 96e2a3492ccedcf6d37eda9e864908154fe181d9
Author: Sean Christopherson <seanjc@google.com>

perf/x86/intel: Don't context switch DS_AREA (and PEBS config) if PEBS is unused

This commit conditionally omits DS_AREA and MSR_PEBS_DATA_CFG from the list
of MSRs to be loaded by KVM on VMX transitions when PEBS is not active in
the guest. The goal is to save MSR writes and eliminate pointless writes
on roundtrips when the guest isn't using PEBS.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
[ ... ]
> +	/*
> +	 * Context switch DS_AREA and PEBS_DATA_CFG if and only if PEBS will be
> +	 * active in the guest; if no records will be generated while the guest
> +	 * is running, then running with host values is safe (see above).
> +	 */
> +	if (!guest_pebs_mask)
> +		return arr;

Will KVM actually stop context switching these MSRs if they are simply omitted
from the array?

To instruct KVM to stop context switching an MSR, the perf subsystem
needs to explicitly return the MSR in the array with .host == .guest,
which triggers KVM to call clear_atomic_switch_msr().

By dynamically omitting MSR_IA32_DS_AREA and MSR_PEBS_DATA_CFG when
guest_pebs_mask is 0, KVM never processes these MSRs and permanently leaves
them in its msr_autoload lists with stale .host and .guest values.

Does this regression cause cross-CPU memory corruption?

If a vCPU migrates to a different physical CPU while guest PEBS is disabled,
the VMCS retains the old physical CPU's MSR_IA32_DS_AREA host pointer.
On VM-Exit, the new physical CPU will load the old CPU's DS_AREA pointer.

Any subsequent host PEBS records generated on the new CPU would then be
erroneously written to the old CPU's memory.

> +
>  	arr[(*nr)++] = (struct perf_guest_switch_msr){
>  		.msr = MSR_IA32_DS_AREA,
>  		.host = (unsigned long)cpuc->ds,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260414191425.2697918-1-seanjc@google.com?part=2