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:05:28 +0800 Message-ID: <201012301705.28771.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]:32427 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753193Ab0L3JER (ORCPT ); Thu, 30 Dec 2010 04:04:17 -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? Yes, seems something happened on paging to nonpaging switch process. > > 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. OK. > > > 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. -- regards Yang, Sheng > > > } > > > > if (!(cr0& X86_CR0_WP))