[PATCH v2] KVM: SVM: emulate MSR_IA32_PERF_CAPABILITIES

[PATCH v2] KVM: SVM: emulate MSR_IA32_PERF_CAPABILITIES

[Vitaly Kuznetsov]

state_test/smm_test selftests are failing on AMD with:
"Unexpected result from KVM_GET_MSRS, r: 51 (failed MSR was 0x345)"

MSR_IA32_PERF_CAPABILITIES is an emulated MSR on Intel but it is not
known to AMD code, emulate it there too (by returning 0 and allowing
userspace to write 0). This way the code is better prepared to the
eventual appearance of the feature in AMD hardware.

Fixes: 27461da31089 ("KVM: x86/pmu: Support full width counting")
Suggested-by: Jim Mattson <jmattson@google.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/svm/pmu.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index 035da07500e8..f13ee3cd6d0f 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -200,7 +200,13 @@ static struct kvm_pmc *amd_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu,
 
 static bool amd_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
 {
-	/* All MSRs refer to exactly one PMC, so msr_idx_to_pmc is enough.  */
+	switch (msr) {
+	case MSR_IA32_PERF_CAPABILITIES:
+		return true;
+	default:
+		break;
+	}
+
 	return false;
 }
 
@@ -221,6 +227,14 @@ static int amd_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	struct kvm_pmc *pmc;
 	u32 msr = msr_info->index;
 
+	if (msr == MSR_IA32_PERF_CAPABILITIES) {
+		if (!msr_info->host_initiated &&
+		    !guest_cpuid_has(vcpu, X86_FEATURE_PDCM))
+			return 1;
+		msr_info->data = vcpu->arch.perf_capabilities;
+		return 0;
+	}
+
 	/* MSR_PERFCTRn */
 	pmc = get_gp_pmc_amd(pmu, msr, PMU_TYPE_COUNTER);
 	if (pmc) {
@@ -244,6 +258,18 @@ static int amd_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	u32 msr = msr_info->index;
 	u64 data = msr_info->data;
 
+	if (msr == MSR_IA32_PERF_CAPABILITIES) {
+		if (!msr_info->host_initiated)
+			return 1;
+
+		/* No feature bits are currently supported */
+		if (data)
+			return 1;
+
+		vcpu->arch.perf_capabilities = data;
+		return 0;
+	}
+
 	/* MSR_PERFCTRn */
 	pmc = get_gp_pmc_amd(pmu, msr, PMU_TYPE_COUNTER);
 	if (pmc) {
@@ -281,6 +307,7 @@ static void amd_pmu_refresh(struct kvm_vcpu *vcpu)
 	pmu->nr_arch_fixed_counters = 0;
 	pmu->global_status = 0;
 	bitmap_set(pmu->all_valid_pmc_idx, 0, pmu->nr_arch_gp_counters);
+	vcpu->arch.perf_capabilities = 0;
 }
 
 static void amd_pmu_init(struct kvm_vcpu *vcpu)
-- 
2.25.4

Re: [PATCH v2] KVM: SVM: emulate MSR_IA32_PERF_CAPABILITIES

[Paolo Bonzini]

On 18/06/20 13:13, Vitaly Kuznetsov wrote:
> state_test/smm_test selftests are failing on AMD with:
> "Unexpected result from KVM_GET_MSRS, r: 51 (failed MSR was 0x345)"
> 
> MSR_IA32_PERF_CAPABILITIES is an emulated MSR on Intel but it is not
> known to AMD code, emulate it there too (by returning 0 and allowing
> userspace to write 0). This way the code is better prepared to the
> eventual appearance of the feature in AMD hardware.
> 
> Fixes: 27461da31089 ("KVM: x86/pmu: Support full width counting")
> Suggested-by: Jim Mattson <jmattson@google.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  arch/x86/kvm/svm/pmu.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)

This is okay and I'll apply it, but it would be even better to move the
whole handling of the MSR to common x86 code.

Paolo

Re: [PATCH v2] KVM: SVM: emulate MSR_IA32_PERF_CAPABILITIES

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/06/20 13:13, Vitaly Kuznetsov wrote:
>> state_test/smm_test selftests are failing on AMD with:
>> "Unexpected result from KVM_GET_MSRS, r: 51 (failed MSR was 0x345)"
>> 
>> MSR_IA32_PERF_CAPABILITIES is an emulated MSR on Intel but it is not
>> known to AMD code, emulate it there too (by returning 0 and allowing
>> userspace to write 0). This way the code is better prepared to the
>> eventual appearance of the feature in AMD hardware.
>> 
>> Fixes: 27461da31089 ("KVM: x86/pmu: Support full width counting")
>> Suggested-by: Jim Mattson <jmattson@google.com>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  arch/x86/kvm/svm/pmu.c | 29 ++++++++++++++++++++++++++++-
>>  1 file changed, 28 insertions(+), 1 deletion(-)
>
> This is okay and I'll apply it, but it would be even better to move the
> whole handling of the MSR to common x86 code.

I thought about that but intel_pmu_set_msr() looks at
vmx_get_perf_capabilities(), we'll need to abstract this somehow.

-- 
Vitaly

Re: [PATCH v2] KVM: SVM: emulate MSR_IA32_PERF_CAPABILITIES

[Paolo Bonzini]

On 18/06/20 14:54, Vitaly Kuznetsov wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 18/06/20 13:13, Vitaly Kuznetsov wrote:
>>> state_test/smm_test selftests are failing on AMD with:
>>> "Unexpected result from KVM_GET_MSRS, r: 51 (failed MSR was 0x345)"
>>>
>>> MSR_IA32_PERF_CAPABILITIES is an emulated MSR on Intel but it is not
>>> known to AMD code, emulate it there too (by returning 0 and allowing
>>> userspace to write 0). This way the code is better prepared to the
>>> eventual appearance of the feature in AMD hardware.
>>>
>>> Fixes: 27461da31089 ("KVM: x86/pmu: Support full width counting")
>>> Suggested-by: Jim Mattson <jmattson@google.com>
>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>> ---
>>>  arch/x86/kvm/svm/pmu.c | 29 ++++++++++++++++++++++++++++-
>>>  1 file changed, 28 insertions(+), 1 deletion(-)
>> This is okay and I'll apply it, but it would be even better to move the
>> whole handling of the MSR to common x86 code.
> I thought about that but intel_pmu_set_msr() looks at
> vmx_get_perf_capabilities(), we'll need to abstract this somehow.

Indeed, you could use kvm_get_msr_feature for that.

Paolo

Re: [PATCH v2] KVM: SVM: emulate MSR_IA32_PERF_CAPABILITIES

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/06/20 14:54, Vitaly Kuznetsov wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> On 18/06/20 13:13, Vitaly Kuznetsov wrote:
>>>> state_test/smm_test selftests are failing on AMD with:
>>>> "Unexpected result from KVM_GET_MSRS, r: 51 (failed MSR was 0x345)"
>>>>
>>>> MSR_IA32_PERF_CAPABILITIES is an emulated MSR on Intel but it is not
>>>> known to AMD code, emulate it there too (by returning 0 and allowing
>>>> userspace to write 0). This way the code is better prepared to the
>>>> eventual appearance of the feature in AMD hardware.
>>>>
>>>> Fixes: 27461da31089 ("KVM: x86/pmu: Support full width counting")
>>>> Suggested-by: Jim Mattson <jmattson@google.com>
>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>>> ---
>>>>  arch/x86/kvm/svm/pmu.c | 29 ++++++++++++++++++++++++++++-
>>>>  1 file changed, 28 insertions(+), 1 deletion(-)
>>> This is okay and I'll apply it, but it would be even better to move the
>>> whole handling of the MSR to common x86 code.
>> I thought about that but intel_pmu_set_msr() looks at
>> vmx_get_perf_capabilities(), we'll need to abstract this somehow.
>
> Indeed, you could use kvm_get_msr_feature for that.
>

Turns out I completely forgot about this patch and just stumbled about
the same issue again. The suggestion to move this to common x86 code
makes perfect sense, I'll be sending v3 shortly.

Thanks!

-- 
Vitaly