Re: [PATCH v3 11/18] KVM: x86/pmu: Emulate IA32_PERF_GLOBAL_STATUS_SET MSR

[Yosry Ahmed <yosry@kernel.org>]

On Thu, Nov 05, 2026 at 06:39:20PM -0700, Zide Chen wrote:
> Intel PerfMon v4 introduces IA32_PERF_GLOBAL_STATUS_SET (0x391) to
> allow software to set individual bits in the global status MSR. Reads
> of IA32_PERF_GLOBAL_STATUS_SET always return zero.
> 
> IA32_PERF_GLOBAL_STATUS_SET is not part of the VM state, so it doesn't
> need to be advertised to userspace, nor saved and restored during live
> migration.
> 
> Always intercept IA32_PERF_GLOBAL_STATUS_SET to prevent the guest from
> modifying status bits that are not virtualized by KVM.
> 
> Bit 63 (CondChgd) stays unreserved in pmu->global_status_rsvd to
> architecturally allow the guest to write this bit on STATUS_RESET (0x390)
> to clear it, and allow userspace to set it on GLOBAL_STATUS (0x38E).
> 
> However, bit 63 is architecturally reserved in STATUS_SET per the SDM.
> Reject writes to bit 63, injecting #GP for guest writes. While older
> processors had silicon errata that accepted writes without #GP (e.g.
> SKX95), strictly enforce the architectural specification for
> deterministic behavior across migrations.
> 
> Platform-specific EDS clarifies that CondChgd indicates a change to
> IA32_MISC_ENABLE[7], which KVM intercepts and keeps strictly read-only
> for guests. This guarantees hardware will never assert CondChgd during
> guest execution.
> 
> Thus, the architectural asymmetry among these three MSRs is safe; being
> unable to restore bit 63 to hardware via STATUS_SET is benign because
> the guest will never legitimately observe CondChgd. Filter it out
> defensively before writing to hardware in intel_mediated_pmu_load().
> 
> Originally-by: Yang Weijiang <weijiang.yang@intel.com>
> Signed-off-by: Zide Chen <zide.chen@intel.com>
> ---
> v3:
> - Split from combined v2 patch into standalone GLOBAL_STATUS_SET patch.
> - Always intercept IA32_PERF_GLOBAL_STATUS_SET to prevent passthrough of
>   unvirtualized status bits. (Jim)
> - Reject writing bit 63 (CondChgd) to IA32_PERF_GLOBAL_STATUS_SET. (Jim)
> - Filter out bit 63 before writing to hardware in intel_mediated_pmu_load().
> - Move validation for MSR_CORE_PERF_GLOBAL_STATUS_SET to pmu_intel.c rather
>   than common pmu.c to avoid exposing the Intel MSR on AMD.
> 
> v2:
> - Add the missing vmcs02 updates for these two MSRs.
> - Change "> 3" to ">= 4" to make the "v4-gated" more obvious and match
>   the existing code style.
> ---
>  arch/x86/kvm/pmu.c           | 13 +++++++++++++
>  arch/x86/kvm/vmx/pmu_intel.c | 14 ++++++++++++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 27d6a9169821..2cb5bccc7044 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -855,6 +855,7 @@ int kvm_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	case MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR:
>  	case MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_SET:
>  	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> +	case MSR_CORE_PERF_GLOBAL_STATUS_SET:
>  		msr_info->data = 0;
>  		break;
>  	default:
> @@ -921,6 +922,18 @@ int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		if (!msr_info->host_initiated)
>  			pmu->global_status &= ~data;
>  		break;
> +	case MSR_CORE_PERF_GLOBAL_STATUS_SET:
> +		/*
> +		 * Bit 63 (CondChgd) is architecturally reserved in STATUS_SET.
> +		 * Older CPUs errantly accepted writes without #GP (e.g. SKX95),
> +		 * but KVM strictly enforces the architectural spec to ensure
> +		 * consistent behavior across migrations.
> +		 */
> +		if (data & (pmu->global_status_rsvd | GLOBAL_STATUS_COND_CHG))
> +			return 1;

Is it intentional to disallow userspace from writing
GLOBAL_STATUS_COND_CHG through MSR_CORE_PERF_GLOBAL_STATUS_SET, even tho
it's allowed through MSR_CORE_PERF_GLOBAL_STATUS?

> +		if (!msr_info->host_initiated)
> +			pmu->global_status |= data;
> +		break;
>  	case MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_SET:
>  		if (!msr_info->host_initiated)
>  			pmu->global_status |= data & ~pmu->global_status_rsvd;
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index ce668ea08da5..e81a3f8b9b02 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -197,6 +197,8 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>  	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
>  	case MSR_CORE_PERF_FIXED_CTR_CTRL:
>  		return kvm_pmu_has_perf_global_ctrl(pmu);
> +	case MSR_CORE_PERF_GLOBAL_STATUS_SET:
> +		return pmu->version >= 4;
>  	case MSR_IA32_PEBS_ENABLE:
>  		ret = vcpu_get_perf_capabilities(vcpu) & PERF_CAP_PEBS_FORMAT;
>  		break;
> @@ -827,8 +829,16 @@ static void intel_mediated_pmu_load(struct kvm_vcpu *vcpu)
>  	toggle = pmu->global_status ^ global_status;
>  	if (global_status & toggle)
>  		wrmsrq(MSR_CORE_PERF_GLOBAL_OVF_CTRL, global_status & toggle);
> -	if (pmu->global_status & toggle)
> -		wrmsrq(MSR_CORE_PERF_GLOBAL_STATUS_SET, pmu->global_status & toggle);
> +
> +	/*
> +	 * CondChgd is architecturally reserved in GLOBAL_STATUS_SET. Unless
> +	 * set by malicious userspace, this bit should never be set in
> +	 * pmu->global_status; filter it out defensively to protect against
> +	 * a host #GP.

IIUC, this will ignore bit 63 completely when written by userspace,
should KVM be doing this?

> +	 */
> +	if ((pmu->global_status & toggle) & ~GLOBAL_STATUS_COND_CHG)

Maybe store this in a variable as it's used in wrmsrq() as well?

> +		wrmsrq(MSR_CORE_PERF_GLOBAL_STATUS_SET,
> +		       (pmu->global_status & toggle) & ~GLOBAL_STATUS_COND_CHG);
>  
>  	wrmsrq(MSR_CORE_PERF_FIXED_CTR_CTRL, pmu->fixed_ctr_ctrl_hw);
>  }
> -- 
> 2.56.0
>