From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752948AbeDLREK (ORCPT ); Thu, 12 Apr 2018 13:04:10 -0400 Received: from smtp-fw-6001.amazon.com ([52.95.48.154]:42278 "EHLO smtp-fw-6001.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752470AbeDLREJ (ORCPT ); Thu, 12 Apr 2018 13:04:09 -0400 X-IronPort-AV: E=Sophos;i="5.48,442,1517875200"; d="scan'208";a="339546163" From: "Raslan, KarimAllah" To: "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" , "pbonzini@redhat.com" CC: "jmattson@google.com" , "rkrcmar@redhat.com" Subject: Re: [PATCH 1/2] X86/KVM: Properly restore 'tsc_offset' when running an L2 guest Thread-Topic: [PATCH 1/2] X86/KVM: Properly restore 'tsc_offset' when running an L2 guest Thread-Index: AQHT0nC4lLvkNnIRDkiE27bl+IOiw6P9U6KAgAAIAIA= Date: Thu, 12 Apr 2018 17:04:04 +0000 Message-ID: <1523552643.32594.18.camel@amazon.de> References: <1523545958-28059-1-git-send-email-karahmed@amazon.de> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.43.165.101] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id w3CH4Ec4015264 On Thu, 2018-04-12 at 18:35 +0200, Paolo Bonzini wrote: > On 12/04/2018 17:12, KarimAllah Ahmed wrote: > > > > When the TSC MSR is captured while an L2 guest is running then restored, > > the 'tsc_offset' ends up capturing the L02 TSC_OFFSET instead of the L01 > > TSC_OFFSET. So ensure that this is compensated for when storing the value. > > > > Cc: Jim Mattson > > Cc: Paolo Bonzini > > Cc: Radim Krčmář > > Cc: kvm@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: KarimAllah Ahmed > > --- > > arch/x86/kvm/vmx.c | 12 +++++++++--- > > arch/x86/kvm/x86.c | 1 - > > 2 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index cff2f50..2f57571 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -2900,6 +2900,8 @@ static u64 guest_read_tsc(struct kvm_vcpu *vcpu) > > */ > > static void vmx_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset) > > { > > + u64 l1_tsc_offset = 0; > > + > > if (is_guest_mode(vcpu)) { > > /* > > * We're here if L1 chose not to trap WRMSR to TSC. According > > @@ -2908,16 +2910,20 @@ static void vmx_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset) > > * to the newly set TSC to get L2's TSC. > > */ > > struct vmcs12 *vmcs12; > > + > > /* recalculate vmcs02.TSC_OFFSET: */ > > vmcs12 = get_vmcs12(vcpu); > > - vmcs_write64(TSC_OFFSET, offset + > > - (nested_cpu_has(vmcs12, CPU_BASED_USE_TSC_OFFSETING) ? > > - vmcs12->tsc_offset : 0)); > > + > > + l1_tsc_offset = nested_cpu_has(vmcs12, CPU_BASED_USE_TSC_OFFSETING) ? > > + vmcs12->tsc_offset : 0; > > + vmcs_write64(TSC_OFFSET, offset + l1_tsc_offset); > > } else { > > trace_kvm_write_tsc_offset(vcpu->vcpu_id, > > vmcs_read64(TSC_OFFSET), offset); > > vmcs_write64(TSC_OFFSET, offset); > > } > > + > > + vcpu->arch.tsc_offset = offset - l1_tsc_offset; > > Using both "offset + l1_tsc_offset" and "offset - l1_tsc_offset" in this > function seems wrong to me: if vcpu->arch.tsc_offset must be "offset - > l1_tsc_offset", then "offset" must be written to TSC_OFFSET. Ooops! I forgot to remove the + l1_tsc_offset :D > > I think the bug was introduced by commit 3e3f50262. Before, > vmx_read_tsc_offset returned the L02 offset; now it always contains the > L01 offset. So the right fix is to adjust vcpu->arch.tsc_offset on > nested vmentry/vmexit. If is_guest_mode(vcpu), kvm_read_l1_tsc must use > a new kvm_x86_ops callback to subtract the L12 offset from the value it > returns. ack! > > Thanks, > > Paolo > > > > > } > > > > /* > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index ac42c85..1a2ed92 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -1539,7 +1539,6 @@ EXPORT_SYMBOL_GPL(kvm_read_l1_tsc); > > static void kvm_vcpu_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset) > > { > > kvm_x86_ops->write_tsc_offset(vcpu, offset); > > - vcpu->arch.tsc_offset = offset; > > } > > > > static inline bool kvm_check_tsc_unstable(void) > > > > Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B