linux-kernel - Re: [PATCH] KVM: SVM: Emulate PERF_CNTR_GLOBAL_STATUS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ab9bcf19-3804-4678-99c2-d2ac8b04a343@amd.com>
Date: Tue, 15 Jul 2025 20:08:33 +0530
From: Sandipan Das <sandipan.das@....com>
To: Sean Christopherson <seanjc@...gle.com>,
 Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: SVM: Emulate PERF_CNTR_GLOBAL_STATUS_SET for
 PerfMonV2

On 7/11/2025 11:08 PM, Sean Christopherson wrote:
> On Fri, Jul 11, 2025, Sean Christopherson wrote:
>> Emulate PERF_CNTR_GLOBAL_STATUS_SET when PerfMonV2 is enumerated to the
>> guest, as the MSR is supposed to exist in all AMD v2 PMUs.
>>
>> Fixes: 4a2771895ca6 ("KVM: x86/svm/pmu: Add AMD PerfMonV2 support")
>> Cc: stable@...r.kernel.org
>> Cc: Sandipan Das <sandipan.das@....com>
>> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
>> ---
> 
> ...
> 
>> @@ -711,6 +712,10 @@ 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_AMD64_PERF_CNTR_GLOBAL_STATUS_SET:
>> +		if (!msr_info->host_initiated)
>> +			pmu->global_status |= data & ~pmu->global_status_rsvd;
>> +		break;
>>  	default:
>>  		kvm_pmu_mark_pmc_in_use(vcpu, msr_info->index);
>>  		return kvm_pmu_call(set_msr)(vcpu, msr_info);
> 
> Tested with a hacky KUT test to verify I got the semantics correct.  I think I did?
> 
>   static void test_pmu_msrs(void)
>   {
> 	const unsigned long long rsvd = GENMASK_ULL(63, 6);
> 
> 	wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, -1ull);
> 	report(!rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS),
> 	       "Wanted '0', got 0x%" PRIx64, rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS));
> 
> 	wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_SET, -1ull);
> 	report(rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS) == ~rsvd,
> 	       "Wanted '0x%llx', got 0x%" PRIx64,
> 	       ~rsvd, rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS));
> 
> 	wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, -1ull);
> 	report(!rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS),
> 	       "Wanted '0', got 0x%" PRIx64, rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS));
> 
> 	wrmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_SET, 0);
> 	report(!rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS),
> 	       "Wanted '0', got 0x%" PRIx64, rdmsr(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS));
>   }
> 
> One oddity is that the test fails when run on the mediated PMU on Turin, i.e. when
> the guest can write MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_SET directly.
> 
>   FAIL: Wanted '0x3f', got 0xc000000000000ff
> 
> Bits 59:58 failing is expected, because lack of KVM support for DebugCtl[FPCI]
> and DebugCtl[FLBRI] doesn't remove them from hardware.  Disabling interception
> of MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_SET creates a virtualization hole on that
> front, but I don't know that it's worth closing.  Letting the guest manually
> freeze its counters doesn't seem terribly interesting.
> 
> Bits 7:6 being set is _much_ more interesting, at least to me.  They're allegedly
> reserved per the APM, and CPUID 0x80000022 says there are only 6 counters, so...

Some of these reserved bits correspond to internal functionality :)
While it is not ideal for software to be able to toggle them, my understanding is
that these bits can be ignored. Otherwise, KVM has to intercept writes to both
MSR_AMD64_PERF_CNTR_GLOBAL_CTRL and MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_SET.