From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM: VMX: Fix 32bit Windows blue screen with EPT Date: Thu, 30 Dec 2010 11:23:45 +0200 Message-ID: <4D1C4FA1.3080503@redhat.com> References: <1293698159-30452-1-git-send-email-sheng@linux.intel.com> <201012301705.28771.sheng@linux.intel.com> <4D1C4D6F.7030707@redhat.com> <201012301720.23752.sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Sheng Yang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:23264 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751243Ab0L3JXt (ORCPT ); Thu, 30 Dec 2010 04:23:49 -0500 In-Reply-To: <201012301720.23752.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 12/30/2010 11:20 AM, Sheng Yang wrote: > On Thursday 30 December 2010 17:14:23 Avi Kivity wrote: > > On 12/30/2010 11:05 AM, Sheng Yang wrote: > > > > > if (!(cr0& X86_CR0_PG)) { > > > > > > > > > > /* From paging/starting to nonpaging */ > > > > > vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, > > > > > > > > > > @@ -1937,11 +1936,8 @@ static void > > > > > ept_update_paging_mode_cr0(unsigned long *hw_cr0, > > > > > > > > > > vmcs_read32(CPU_BASED_VM_EXEC_CONTROL)& > > > > > > > > > > ~(CPU_BASED_CR3_LOAD_EXITING | > > > > > > > > > > CPU_BASED_CR3_STORE_EXITING)); > > > > > > > > > > - /* Must fetch cr3 before updating cr0 */ > > > > > - cr3 = kvm_read_cr3(vcpu); > > > > > > > > > > vcpu->arch.cr0 = cr0; > > > > > vmx_set_cr4(vcpu, kvm_read_cr4(vcpu)); > > > > > > > > > > - vmx_set_cr3(vcpu, cr3); > > > > > > > > This is indeed bogus. But what ensures that we'll have the correct > > > > GUEST_CR3 after enabling paging? > > > > > > In fact I don't understand why we need this line. All modification is for > > > CR3 reading, why we need to set hardware CR3 again? It should be the > > > same as when we don't have CR3 accessor I think. > > > > when cr0.pg=0 then we set GUEST_CR3=identity_pagetable. We don't want > > that when we we switch to paging mode. > > I think kvm_mmu_reset_context() in kvm_set_cr0() should cover this? > I looked at it but missed it. kvm_mmu_reset_context() doesn't actually do this, but it does schedule a kvm_mmu_load() which will call vmx_set_cr3(). Thanks, I'll apply your v2 now. -- error compiling committee.c: too many arguments to function