[PATCH] KVM: SVM: Set HWCR[TscFreqSel] to host's value

[PATCH] KVM: SVM: Set HWCR[TscFreqSel] to host's value

[Shannon Zhao]

KVM sets CPUID.80000007H:EDX[8] to 1, but not set HWCR[TscFreqSel].
This will cause guest kernel printing below log on AMD platform even
though the hardware TSC exactly counts with P0 frequency.
"[Firmware Bug]: TSC doesn't count with P0 frequency!"

Fix it by setting HWCR[TscFreqSel] to host's value to indicate whether
the TSC increments at the P0 frequency.

Signed-off-by: Shannon Zhao <shannon.zhao@linux.alibaba.com>
---
 arch/x86/kvm/svm/svm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7e45d03..fb4bb51 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1139,6 +1139,11 @@ static void __svm_vcpu_reset(struct kvm_vcpu *vcpu)
 	svm_init_osvw(vcpu);
 	vcpu->arch.microcode_version = 0x01000065;
 	svm->tsc_ratio_msr = kvm_default_tsc_scaling_ratio;
+	/* 
+	 * TSC frequency select is HWCR[24], set it to host's value to indicate
+	 * whether the TSC increments at the P0 frequency. 
+	 */
+	vcpu->arch.msr_hwcr = native_read_msr(MSR_K7_HWCR) & BIT_ULL(24); 
 
 	if (sev_es_guest(vcpu->kvm))
 		sev_es_vcpu_reset(svm);
-- 
1.8.3.1

Re: [PATCH] KVM: SVM: Set HWCR[TscFreqSel] to host's value

[Sean Christopherson]

On Tue, May 10, 2022, Shannon Zhao wrote:
> KVM sets CPUID.80000007H:EDX[8] to 1, but not set HWCR[TscFreqSel].
> This will cause guest kernel printing below log on AMD platform even
> though the hardware TSC exactly counts with P0 frequency.
> "[Firmware Bug]: TSC doesn't count with P0 frequency!"
> 
> Fix it by setting HWCR[TscFreqSel] to host's value to indicate whether
> the TSC increments at the P0 frequency.

I don't think this is safe.  The APM says

  Some HWCR bits are implementation specific, and are described in the BIOS and
  Kernel Developer’s Guide (BKDG) or Processor Programming Reference Manual
  applicable to your product. Implementation specific HWCR bits are not listed below.

and then omits bit 24.  One thought to handle this would be to let userspace
write all the non-architectural bits, then userspace can set the magic,
non-architectural bits based on CPUID and vCPU FMS.

> Signed-off-by: Shannon Zhao <shannon.zhao@linux.alibaba.com>
> ---
>  arch/x86/kvm/svm/svm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 7e45d03..fb4bb51 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1139,6 +1139,11 @@ static void __svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  	svm_init_osvw(vcpu);
>  	vcpu->arch.microcode_version = 0x01000065;
>  	svm->tsc_ratio_msr = kvm_default_tsc_scaling_ratio;
> +	/* 
> +	 * TSC frequency select is HWCR[24], set it to host's value to indicate

This really should get a #define in msr-index.h, but maybe that's "impossible"
because it's not an architectural bit.

> +	 * whether the TSC increments at the P0 frequency. 
> +	 */
> +	vcpu->arch.msr_hwcr = native_read_msr(MSR_K7_HWCR) & BIT_ULL(24);

This will break live save/restore, a.k.a. live migration.  KVM doesn't allow
writes to MSR_K7_HWCR to set anything other than bit 18, even if the write comes
from userspace.

>  
>  	if (sev_es_guest(vcpu->kvm))
>  		sev_es_vcpu_reset(svm);
> -- 
> 1.8.3.1
>