Re: [PATCH] KVM: VMX: Fix 32bit Windows blue screen with EPT

[Avi Kivity <avi@redhat.com>]

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