Re: [PATCH] KVM: nVMX: Replace kvm_set_cr0 with vmx_set_cr0 in load_vmcs12_host_state

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

On 2013-02-23 22:57, Jan Kiszka wrote:
> On 2013-02-23 22:45, Nadav Har'El wrote:
>> On Sat, Feb 23, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Replace kvm_set_cr0 with vmx_set_cr0 in load_vmcs12_host_state":
>>> -	kvm_set_cr0(vcpu, vmcs12->host_cr0);
>>> +	vmx_set_cr0(vcpu, vmcs12->host_cr0);
>>
>> I don't remember now why I did this (and I'm not looking at the code),
>> but this you'll need to really test carefully, including
>> shadow-on-shadow mode (ept=0 in L0), to verify you're not missing any
>> important side-effect of kvm_set_cr0.
>>
>> Also, if I remember correctly, during nVMX's review, Avi Kivity asked
>> in several places that when I called vmx_set_cr0, I should instead call
>> kvm_set_cr0(), because it does some extra stuff and does some extra
>> checks. Hmm, see, see this:
>> 	http://markmail.org/message/hhidqyhbo2mrgxxc
>>
>> where Avi asked for the reverse patch you're attempting now.
> 
> At least, kvm_set_cr0 can't be used as it assumes an otherwise
> consistent guest state and an explicitly initiated transition - which is
> naturally not the case while emulating a vmexit.
> 
> However, there are some side effects that need a closer look, likely
> also in other places where vmx_set_cr0 is used directly. E.g.
> load_pdptrs. The lack of calling them on host->guest switch may explain
> the PAE issues you once saw with your nEPT implementation.
> 
> OK, more work...

Checked again, and the patch should be fine: load_pdptrs is executed by
kvm_set_cr3 later in this functions (same for prepare_vmcs02, so no
problem for PAE here), kvm_mmu_reset_context is even run explicitly and
clearing the async-pf queue makes no sense when leaving the guest (the
guest can't use it unless L1 is broken and passes this through).
However, we should clear the queue when leaving the host - separate patch.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]