Re: [PATCH v2 5/8] KVM: nVMX: Fix guest CR3 read-back on VM-exit

[Paolo Bonzini <pbonzini@redhat.com>]

On 08/07/2013 03:38 PM, Gleb Natapov wrote:
> On Wed, Aug 07, 2013 at 03:32:37PM +0200, Paolo Bonzini wrote:
>>>>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>>>>> index 44494ed..60a3644 100644
>>>>>>> --- a/arch/x86/kvm/vmx.c
>>>>>>> +++ b/arch/x86/kvm/vmx.c
>>>>>>> @@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
>>>>>>>   	if (enable_ept) {
>>>>>>>   		eptp = construct_eptp(cr3);
>>>>>>>   		vmcs_write64(EPT_POINTER, eptp);
>>>>>>> -		guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
>>>>>>> -			vcpu->kvm->arch.ept_identity_map_addr;
>>>>>>> +		if (is_paging(vcpu) || is_guest_mode(vcpu))
>>>>>>> +			guest_cr3 = kvm_read_cr3(vcpu) :
>>>>>>> +		else
>>>>>>> +			guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
>>>>>>>   		ept_load_pdptrs(vcpu);
>>>>>>>   	}
>>>>>>>
>>>>>> That what I am thinking, will think about it some more tomorrow.
>>>>>
>>>>> OK, and I'll feed it into a local test.
>>>>>
>>>> Thought about is some more. So without nested unrestricted guest (nUG)
>>>> is_paging() will always be true (since without nUG guest entry is not
>>>> possible otherwise) and guest's cr3 will be used, but with nUG identity
>>>> map is not used (that is why L2 still works even though wrong identity
>>>> map pointer is assigned to cr3), so the code here just corrupts nested
>>>> guest's cr3 for no reason and that is why you had to use kvm_read_cr3()
>>>> in prepare_vmcs12() to get correct cr3 value. The patch above should be
>>>> used instead of original one IMO. How is testing going?
>>>
>>> Yes, testing worked fine. I've queued above patch and will send it out
>>> within the next round.
>>
>> Just reply here with the commit message you desire and
>> Signed-off-by, so I can queue it for people who wish to play with
>> nEPT.
>
> I would love to have a comment there too :)

Ok, then it can wait since it is only needed with nested unrestricted 
guest.  On the other hand, it should come before patch 4 on the next 
submission.

Paolo