Messages in this thread | | | Subject | Re: [PATCH v2] KVM: SVM: Delay restoration of host MSR_TSC_AUX until return to userspace | From | Paolo Bonzini <> | Date | Fri, 23 Apr 2021 16:18:49 +0200 |
| |
On 23/04/21 16:12, Sean Christopherson wrote: > On Fri, Apr 23, 2021, Paolo Bonzini wrote: >> On 22/04/21 22:12, Sean Christopherson wrote: >>> case MSR_TSC_AUX: >>> if (!boot_cpu_has(X86_FEATURE_RDTSCP)) >>> return 1; >>> >>> if (!msr_info->host_initiated && >>> !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP)) >>> return 1; >>> >>> /* >>> * TSC_AUX is usually changed only during boot and never read >>> * directly. Intercept TSC_AUX instead of exposing it to the >>> * guest via direct_access_msrs, and switch it via user return. >>> */ >>> preempt_disable(); >>> r = kvm_set_user_return_msr(TSC_AUX_URET_SLOT, data, -1ull); >>> preempt_enable(); >>> if (r) >>> return 1; >>> >>> /* >>> * Bits 63:32 are dropped by AMD CPUs, but are reserved on >>> * Intel CPUs. AMD's APM has incomplete and conflicting info >>> * on the architectural behavior; emulate current hardware as >>> * doing so ensures migrating from AMD to Intel won't explode. >>> */ >>> svm->tsc_aux = (u32)data; >>> break; >>> >> >> Ok, squashed in the following: > > Too fast! The below won't compile (s/msr_info/msr and 'r' needs to be defined), > and the get_msr() path needs the guest_cpuid_has() check.
Oops I missed the get_msr(). (I modify my local tree very aggressively, often without even compiling, so that I can use "git range-diff kvm/next..kvm/queue kvm/next.." as a reminder of things that are pending).
Paolo
I'll spin a v3. > >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >> index 14ff7f0963e9..00e9680969a2 100644 >> --- a/arch/x86/kvm/svm/svm.c >> +++ b/arch/x86/kvm/svm/svm.c >> @@ -2875,16 +2875,28 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) >> if (!boot_cpu_has(X86_FEATURE_RDTSCP)) >> return 1; >> + if (!msr_info->host_initiated && >> + !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP)) >> + return 1; >> + >> /* >> * TSC_AUX is usually changed only during boot and never read >> * directly. Intercept TSC_AUX instead of exposing it to the >> * guest via direct_access_msrs, and switch it via user return. >> */ >> - svm->tsc_aux = data; >> - >> preempt_disable(); >> - kvm_set_user_return_msr(TSC_AUX_URET_SLOT, data, -1ull); >> + r = kvm_set_user_return_msr(TSC_AUX_URET_SLOT, data, -1ull); >> preempt_enable(); >> + if (r) >> + return 1; >> + >> + /* >> + * Bits 63:32 are dropped by AMD CPUs, but are reserved on >> + * Intel CPUs. AMD's APM has incomplete and conflicting info >> + * on the architectural behavior; emulate current hardware as >> + * doing so ensures migrating from AMD to Intel won't explode. >> + */ >> + svm->tsc_aux = (u32)data; >> break; >> case MSR_IA32_DEBUGCTLMSR: >> if (!boot_cpu_has(X86_FEATURE_LBRV)) { >> >> Paolo >> >
| |