at 1:02 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> On Mon, 24 Sep 2018, Sean Christopherson wrote:
>
>> On Sat, 2018-09-22 at 09:56 +0800, kvm-owner@xxxxxxxxxxxxxxx wrote:
>>> vmx->vcpu.arch.cr0 will be set in vmx_set_cr0().
>>>
>>> This patch removes duplicate cr0 set in vmx_vcpu_reset().
>>>
>>> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>>> ---
>>> arch/x86/kvm/vmx.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 1519f030fd73..b1e1d63a4970 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -6734,7 +6734,6 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>> vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
>>>
>>> cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET;
>>> - vmx->vcpu.arch.cr0 = cr0;
>>
>> Initializing arch.cr0 prior to vmx_set_cr0() is necessary because it
>> might be queried by vmx_set_cr0(), e.g. via is_paging(). A stale cr0
>> could trigger side effects in vmx_set_cr0() related to toggling cr0
>> bits, which we don't want.
>
> And these side effects are completely undocumented. So if that store needs
> to happen before calling vmx_set_cr0() then this really wants a comment.
Whoever is going to deal with it - you may want to have a look at the commit
message of f24632475d4f ("KVM: x86: fix ordering of cr0 initialization code
in vmx_cpu_reset”), which fixed a bug that I caused by reversing the order
of the cr0 initialization.
Regards,
Nadav
