Re: [PATCH] KVM: x86/pmu: Move Intel PMU global MSRs to intel_is_valid_msr()

[Like Xu <like.xu.linux@gmail.com>]

On 9/3/26 2:47 AM, Jim Mattson wrote:
> Commit c85cdc1cc1ea ("KVM: x86/pmu: Move handling PERF_GLOBAL_CTRL and
> friends to common x86") moved the existence check for the following Intel
> PMU MSRs to kvm_pmu_is_valid_msr():
>   - MSR_CORE_PERF_GLOBAL_STATUS
>   - MSR_CORE_PERF_GLOBAL_CTRL
>   - MSR_CORE_PERF_GLOBAL_OVF_CTRL
> 
> That commit deemed these MSRs valid whenever pmu->version > 1. It intended
> to share the check with AMD PerfMonV2 because both vendor implementations
> require version 2 or greater for global PMU controls.  However, as noted in
> the commit message, AMD uses different MSR indices for its global PMU
> registers.
> 
> Commit 4a2771895ca6 ("KVM: x86/svm/pmu: Add AMD PerfMonV2 support")
> subsequently added AMD PerfMonV2 support and set pmu->version = 2.  Because
> kvm_pmu_is_valid_msr() validated the Intel MSRs whenever pmu->version > 1,
> KVM incorrectly permitted AMD guests with PerfMonV2 to access these Intel
> MSRs without a #GP.
> 
> Move the validation of these Intel MSRs to intel_is_valid_msr() and remove
> the common switch statement from kvm_pmu_is_valid_msr(). AMD already
> validates its own global PMU MSRs in amd_is_valid_msr().
> 
> Fixes: 4a2771895ca6 ("KVM: x86/svm/pmu: Add AMD PerfMonV2 support")
> Signed-off-by: Jim Mattson <jmattson@google.com>

Reviewed-by: Like Xu <likexu@tencent.com>

> ---
>   arch/x86/kvm/pmu.c           | 8 --------
>   arch/x86/kvm/vmx/pmu_intel.c | 3 +++
>   2 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index a7d60c8785cd..d2fd47ee5ec8 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -823,14 +823,6 @@ void kvm_pmu_deliver_pmi(struct kvm_vcpu *vcpu)
>   
>   bool kvm_pmu_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>   {
> -	switch (msr) {
> -	case MSR_CORE_PERF_GLOBAL_STATUS:
> -	case MSR_CORE_PERF_GLOBAL_CTRL:
> -	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> -		return kvm_pmu_has_perf_global_ctrl(vcpu_to_pmu(vcpu));
> -	default:
> -		break;
> -	}
>   	return kvm_pmu_call(msr_idx_to_pmc)(vcpu, msr) ||
>   	       kvm_pmu_call(is_valid_msr)(vcpu, msr);
>   }
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index bfa8612fb450..70a8c4816135 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -187,6 +187,9 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>   	int ret;
>   
>   	switch (msr) {
> +	case MSR_CORE_PERF_GLOBAL_STATUS:
> +	case MSR_CORE_PERF_GLOBAL_CTRL:
> +	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
>   	case MSR_CORE_PERF_FIXED_CTR_CTRL:
>   		return kvm_pmu_has_perf_global_ctrl(pmu);
>   	case MSR_IA32_PEBS_ENABLE: