From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v2 5/8] KVM: nVMX: Fix guest CR3 read-back on VM-exit Date: Wed, 07 Aug 2013 15:59:46 +0200 Message-ID: <520252D2.4070501@siemens.com> References: <20130806140248.GB8218@redhat.com> <20130806144117.GD8218@redhat.com> <52011AE6.2010006@siemens.com> <20130806155344.GE8218@redhat.com> <52011CCE.4000109@siemens.com> <20130807123958.GA30470@redhat.com> <520241A7.5060301@siemens.com> <52024C75.9000304@redhat.com> <20130807133819.GA3672@redhat.com> <52025195.6020808@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Gleb Natapov , "Zhang, Yang Z" , kvm , Xiao Guangrong , "Nakajima, Jun" , Arthur Chunqi Li To: Paolo Bonzini Return-path: Received: from goliath.siemens.de ([192.35.17.28]:23023 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752893Ab3HGOAC (ORCPT ); Wed, 7 Aug 2013 10:00:02 -0400 In-Reply-To: <52025195.6020808@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2013-08-07 15:54, Paolo Bonzini wrote: > On 08/07/2013 03:38 PM, Gleb Natapov wrote: >> On Wed, Aug 07, 2013 at 03:32:37PM +0200, Paolo Bonzini wrote: >>>>>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>>>>>>> index 44494ed..60a3644 100644 >>>>>>>> --- a/arch/x86/kvm/vmx.c >>>>>>>> +++ b/arch/x86/kvm/vmx.c >>>>>>>> @@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu >>>>>>>> *vcpu, unsigned long cr3) >>>>>>>> if (enable_ept) { >>>>>>>> eptp = construct_eptp(cr3); >>>>>>>> vmcs_write64(EPT_POINTER, eptp); >>>>>>>> - guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) : >>>>>>>> - vcpu->kvm->arch.ept_identity_map_addr; >>>>>>>> + if (is_paging(vcpu) || is_guest_mode(vcpu)) >>>>>>>> + guest_cr3 = kvm_read_cr3(vcpu) : >>>>>>>> + else >>>>>>>> + guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr; >>>>>>>> ept_load_pdptrs(vcpu); >>>>>>>> } >>>>>>>> >>>>>>> That what I am thinking, will think about it some more tomorrow. >>>>>> >>>>>> OK, and I'll feed it into a local test. >>>>>> >>>>> Thought about is some more. So without nested unrestricted guest (nUG) >>>>> is_paging() will always be true (since without nUG guest entry is not >>>>> possible otherwise) and guest's cr3 will be used, but with nUG >>>>> identity >>>>> map is not used (that is why L2 still works even though wrong identity >>>>> map pointer is assigned to cr3), so the code here just corrupts nested >>>>> guest's cr3 for no reason and that is why you had to use >>>>> kvm_read_cr3() >>>>> in prepare_vmcs12() to get correct cr3 value. The patch above >>>>> should be >>>>> used instead of original one IMO. How is testing going? >>>> >>>> Yes, testing worked fine. I've queued above patch and will send it out >>>> within the next round. >>> >>> Just reply here with the commit message you desire and >>> Signed-off-by, so I can queue it for people who wish to play with >>> nEPT. >> >> I would love to have a comment there too :) > > Ok, then it can wait since it is only needed with nested unrestricted > guest. Yes, it's related to that feature. > On the other hand, it should come before patch 4 on the next > submission. I'll reorder the whole series, moving the feature enabling to the end. The ordering still reflects more the history than the dependencies. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux