Re: [PATCH v2 11/16] KVM: x86/pmu: Emulate the GLOBAL_STATUS_SET and GLOBAL_INUSE MSRs

["Chen, Zide" <zide.chen@intel.com>]

On 8/27/2026 6:05 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Incorrect IN_USE bit calculation for fixed counters in IA32_PERF_GLOBAL_INUSE omits the INTEL_FIXED_0_ENABLE_PMI condition.
> - [Medium] Emulated writes to MSR_CORE_PERF_GLOBAL_STATUS_SET fail to trigger a PMI when the corresponding global_ctrl bit is set.
> --
> 
> commit a29f967a55c3b6e45cb682091b4b9a33c0b976dc
> Author: Zide Chen <zide.chen@intel.com>
> 
> KVM: x86/pmu: Emulate the GLOBAL_STATUS_SET and GLOBAL_INUSE MSRs
> 
> Intel PerfMon v4 introduces IA32_PERF_GLOBAL_STATUS_SET (0x391) and
> IA32_PERF_GLOBAL_INUSE (0x392). This patch adds emulation for both MSRs
> in KVM, but there appear to be a few issues with the implementation.
> 
>> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
>> index 437a7bc49bf84..7c05cf5157bfc 100644
>> --- a/arch/x86/kvm/pmu.c
>> +++ b/arch/x86/kvm/pmu.c
> [ ... ]
>> @@ -931,6 +934,12 @@ 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:
>> +		if (data & pmu->global_status_rsvd)
>> +			return 1;
>> +		if (!msr_info->host_initiated)
>> +			pmu->global_status |= data;
>> +		break;
> 
> [Severity: Medium]
> Will emulated guest writes to MSR_CORE_PERF_GLOBAL_STATUS_SET fail to trigger
> a PMI when the corresponding global_ctrl bit is set?
> 
> This handler correctly updates the global status, but it appears to omit the
> required secondary side-effect of generating a performance monitoring interrupt.
> 
> If a guest writes a 1 to a bit in MSR_CORE_PERF_GLOBAL_STATUS_SET while the
> corresponding bit in IA32_PERF_GLOBAL_CTRL is enabled, it seems the interrupt
> will silently fail to generate because the code doesn't check global_ctrl or
> set KVM_REQ_PMI.

According to the Intel SDM, IA32_PERF_GLOBAL_STATUS_SET only sets the
corresponding bits in IA32_PERF_GLOBAL_STATUS, but not generate a PMI.

Generating a PMI / KVM_REQ_PMI on writes to this MSR would deviate from
hardware behavior and cause spurious PMIs in the guest, e.g., during
live migration state restoration, or during KVM pmu context switch.

> 
>>  	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 4df55a3e21da9..3070fba2687f4 100644
>> --- a/arch/x86/kvm/vmx/pmu_intel.c
>> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> [ ... ]
>> @@ -341,6 +343,38 @@ static bool intel_pmu_handle_lbr_msrs_access(struct kvm_vcpu *vcpu,
>>  	return true;
>>  }
>>  
>> +static u64 intel_pmu_get_global_inuse(struct kvm_vcpu *vcpu)
>> +{
>> +	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>> +	unsigned long fixed_mask = kvm_fixed_pmc_mask(pmu);
>> +	unsigned long gp_mask = kvm_gp_pmc_mask(pmu);
>> +	bool pmi_inuse = false;
>> +	u64 eventsel, data = 0;
>> +	u32 fixed_ctrl;
>> +	int i;
>> +
>> +	kvm_for_each_gp_counter(i, gp_mask) {
>> +		eventsel = pmu->gp_counters[i].eventsel;
>> +
>> +		if (eventsel & ARCH_PERFMON_EVENTSEL_EVENT)
>> +			data |= BIT_ULL(i);
>> +		pmi_inuse |= eventsel & ARCH_PERFMON_EVENTSEL_INT;
>> +	}
>> +	kvm_for_each_fixed_counter(i, fixed_mask) {
>> +		fixed_ctrl = fixed_ctrl_field(pmu->fixed_ctr_ctrl, i);
>> +
>> +		if (fixed_ctrl & (INTEL_FIXED_0_KERNEL | INTEL_FIXED_0_USER))
>> +			data |= BIT_ULL(KVM_FIXED_PMC_BASE_IDX + i);
> 
> [Severity: Medium]
> Does this code improperly omit INTEL_FIXED_0_ENABLE_PMI when calculating the
> IN_USE bit for fixed counters?
> 
> If a fixed counter is configured only for PMI (without OS or User bits), this
> code seems to falsely report it as unused, which violates the Intel SDM
> specification for FCn_InUse. 
> 
> Should INTEL_FIXED_0_ENABLE_PMI be included in this condition to ensure the
> availability check remains accurate?

False positive. The implementation strictly follows the SDM:

IA32_PERF_GLOBAL_INUSE.FC0_InUse[bit 32]: This bit reflects the logical
state of (IA32_FIXED_CTR_CTRL[1:0] != 0).

Also I verified that the behavior is same between the host and the guest
in this regard.

Note that in practice, reading this MSR on the host may be inconsistent.
The Linux perf driver clear the EN bit but not the whole EvtSel MSRs
after a perf event finishes, so the corresponding bits in INUSE may
remain set. In contrast, kvm_pmu_put_guest_pmcs() explicitly clears
these MSRs, which means the in-use bits will be cleared on physical CPUs
running a vCPU.



> 
>> +		pmi_inuse |= fixed_ctrl & INTEL_FIXED_0_ENABLE_PMI;
>> +	}
>> +	pmi_inuse |= pmu->pebs_enable;
>> +
>> +	if (pmi_inuse)
>> +		data |= PERF_GLOBAL_INUSE_PMI_INUSE;
>> +
>> +	return data;
>> +}
>