Re: [PATCH v4 03/13] nEPT: Fix wrong test in kvm_set_cr3

[Gleb Natapov <gleb@redhat.com>]

On Mon, Jul 29, 2013 at 10:36:05AM +0200, Paolo Bonzini wrote:
> Il 25/07/2013 12:59, Gleb Natapov ha scritto:
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 89b15df..56d0066 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -7596,8 +7596,7 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> >  	kvm_mmu_reset_context(vcpu);
> >  
> >  	/*
> > -	 * Additionally, except when L0 is using shadow page tables, L1 or
> > -	 * L2 control guest_cr3 for L2, so they may also have saved PDPTEs
> > +	 * L1 may access the L2's PDPTR, so save them to construct vmcs12
> >  	 */
> >  	if (enable_ept) {
> >  		vmcs_write64(GUEST_PDPTR0, vmcs12->guest_pdptr0);
> > @@ -7933,14 +7932,11 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> >  	 * own CR3 without exiting. If it has changed it, we must keep it.
> >  	 * Of course, if L0 is using shadow page tables, GUEST_CR3 was defined
> >  	 * by L0, not L1 or L2, so we mustn't unconditionally copy it to vmcs12.
> > -	 */
> > -	if (enable_ept)
> > -		vmcs12->guest_cr3 = vmcs_read64(GUEST_CR3);
> > -	/*
> > -	 * Additionally, except when L0 is using shadow page tables, L1 or
> > -	 * L2 control guest_cr3 for L2, so save their PDPTEs
> > +	 *
> > +	 * Additionally, restore L2's PDPTR to vmcs12.
> >  	 */
> >  	if (enable_ept) {
> > +		vmcs12->guest_cr3 = vmcs_read64(GUEST_CR3);
> >  		vmcs12->guest_pdptr0 = vmcs_read64(GUEST_PDPTR0);
> >  		vmcs12->guest_pdptr1 = vmcs_read64(GUEST_PDPTR1);
> >  		vmcs12->guest_pdptr2 = vmcs_read64(GUEST_PDPTR2);
> 
> This part addresses your review comments for v3 patch 6, and should be
> squashed in patch 2 of this series.
> 
Yeah, I noticed it in the wrong patch, but forget to move.

--
			Gleb.