From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH] KVM: VMX: Fix 32bit Windows blue screen with EPT Date: Thu, 30 Dec 2010 17:09:46 +0800 Message-ID: <201012301709.46648.sheng@linux.intel.com> References: <1293698159-30452-1-git-send-email-sheng@linux.intel.com> <4D1C4970.9000409@redhat.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mga02.intel.com ([134.134.136.20]:17424 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753409Ab0L3JIL (ORCPT ); Thu, 30 Dec 2010 04:08:11 -0500 In-Reply-To: <4D1C4970.9000409@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thursday 30 December 2010 16:57:20 Avi Kivity wrote: > On 12/30/2010 10:35 AM, Sheng Yang wrote: > > After CR0 is changed during VMExit, the result of kvm_read_cr3() may be > > different. Commit d95bfcdd7cda4dfdac9588e684bc7c75794a075e "KVM: Fetch > > guest cr3 from hardware on demand" caused 32bit Windows guest blue > > screen when using with EPT. This patch fixes it by decache CR3 before > > CR0 change, for both paging to nonpaging, and nonpaging to paging > > switch. > > > > Signed-off-by: Sheng Yang > > --- > > > > But I haven't found the exactly point affected by this, any clue? > > Can't see it either. > > > @@ -1921,8 +1921,7 @@ static void ept_update_paging_mode_cr0(unsigned > > long *hw_cr0, > > > > unsigned long cr0, > > struct kvm_vcpu *vcpu) > > > > { > > > > - ulong cr3; > > - > > + kvm_read_cr3(vcpu); > > Without this line, it fails? > > I think it's better to call vmx_decache_cr3() explicitly, since it > explains what we're doing. vmx_decache_cr3 depends on arch.cr0, and > we're changing that here. > > > 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? BTW: What did you find when you added this two lines? -- regards Yang, Sheng > > > } > > > > if (!(cr0& X86_CR0_WP))