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: 2012 bytes --]

On 2013-02-24 10:21, Avi Kivity wrote:
> On Sun, Feb 24, 2013 at 11:01 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2013-02-24 09:56, Avi Kivity wrote:
>>> On Sat, Feb 23, 2013 at 11:57 PM, Jan Kiszka <jan.kiszka@web.de> 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.
>>>
>>> We have the same problem in KVM_SET_SREGS.
>>
>> I don't see the problem. kvm_arch_vcpu_ioctl_set_sregs open-codes the
>> state update, not applying any transition checks.
> 
> That's the problem.  We have this open coding in three different
> places (KVM_SET_SREGS, nvmx, nsvm).
> 
> It's not as if vmx_set_cr0() is defined as "kvm_set_cr0() without the
> transition checks".

...and without mmu updates. The latter is done via or after the closing
cr3 update. Interestingly, nsvm does not perform kvm_set_cr3 on vmexit
when in npt mode. Seems things aren't that regular.

Jan



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