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

[Paolo Bonzini <pbonzini@redhat.com>]

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.

Paolo