[PATCH] KVM: x86: fix eflags state following processor init/reset

[PATCH] KVM: x86: fix eflags state following processor init/reset

[Wanpeng Li]

Reference SDM 3.4.3:

Following initialization of the processor (either by asserting the 
RESET pin or the INIT pin), the state of the EFLAGS register is 
00000002H.

However, the eflags fixed bit is not set and other bits are also not 
cleared during the init/reset in kvm.

This patch fix it by set eflags register to 00000002H following 
initialization of the processor.

Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b680c2e..326f6ea 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4935,6 +4935,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 	vmx_set_efer(vcpu, 0);
 	vmx_fpu_activate(vcpu);
 	update_exception_bitmap(vcpu);
+	vmx_set_rflags(vcpu, X86_EFLAGS_FIXED);
 
 	vpid_sync_context(vmx->vpid);
 }
-- 
1.9.1

Re: [PATCH] KVM: x86: fix eflags state following processor init/reset

[Wanpeng Li]

Ping, :-)
On 10/21/15 2:28 PM, Wanpeng Li wrote:
> Reference SDM 3.4.3:
>
> Following initialization of the processor (either by asserting the
> RESET pin or the INIT pin), the state of the EFLAGS register is
> 00000002H.
>
> However, the eflags fixed bit is not set and other bits are also not
> cleared during the init/reset in kvm.
>
> This patch fix it by set eflags register to 00000002H following
> initialization of the processor.
>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>   arch/x86/kvm/vmx.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index b680c2e..326f6ea 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -4935,6 +4935,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>   	vmx_set_efer(vcpu, 0);
>   	vmx_fpu_activate(vcpu);
>   	update_exception_bitmap(vcpu);
> +	vmx_set_rflags(vcpu, X86_EFLAGS_FIXED);
>   
>   	vpid_sync_context(vmx->vpid);
>   }

Re: [PATCH] KVM: x86: fix eflags state following processor init/reset

[Nadav Amit]

Here are my 5 cents. Note that vmx_vcpu_reset calls:

	vmcs_writel(GUEST_RFLAGS, 0x02);

(And the RFLAGS value is not cached by KVM, so no consistency problem should
occur.)

You may want to change the value into constant or call a wrapper function
for setting RFLAGS, but I don’t see something broken in the functionality.

Regards,
Nadav

Wanpeng Li <wanpeng.li@hotmail.com> wrote:

> Ping, :-)
> On 10/21/15 2:28 PM, Wanpeng Li wrote:
>> Reference SDM 3.4.3:
>> 
>> Following initialization of the processor (either by asserting the
>> RESET pin or the INIT pin), the state of the EFLAGS register is
>> 00000002H.
>> 
>> However, the eflags fixed bit is not set and other bits are also not
>> cleared during the init/reset in kvm.
>> 
>> This patch fix it by set eflags register to 00000002H following
>> initialization of the processor.
>> 
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  arch/x86/kvm/vmx.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index b680c2e..326f6ea 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -4935,6 +4935,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>  	vmx_set_efer(vcpu, 0);
>>  	vmx_fpu_activate(vcpu);
>>  	update_exception_bitmap(vcpu);
>> +	vmx_set_rflags(vcpu, X86_EFLAGS_FIXED);
>>    	vpid_sync_context(vmx->vpid);
>>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: x86: fix eflags state following processor init/reset

[Paolo Bonzini]

On 28/10/2015 09:10, Nadav Amit wrote:
> Here are my 5 cents. Note that vmx_vcpu_reset calls:
> 
> 	vmcs_writel(GUEST_RFLAGS, 0x02);
> 
> (And the RFLAGS value is not cached by KVM, so no consistency problem should
> occur.)
> 
> You may want to change the value into constant or call a wrapper function
> for setting RFLAGS, but I don’t see something broken in the functionality.

I agree.  Wanpeng, if this is just a cleanup, can you send v2 that
removes or modifies the existing call to vmcs_writel?  If there is a
bug, can you write a unit test for it?  It should be possible to test
for the problem using INIT+SIPI on an AP.

Thanks,

Paolo

> Regards,
> Nadav
> 
> Wanpeng Li <wanpeng.li@hotmail.com> wrote:
> 
>> Ping, :-)
>> On 10/21/15 2:28 PM, Wanpeng Li wrote:
>>> Reference SDM 3.4.3:
>>>
>>> Following initialization of the processor (either by asserting the
>>> RESET pin or the INIT pin), the state of the EFLAGS register is
>>> 00000002H.
>>>
>>> However, the eflags fixed bit is not set and other bits are also not
>>> cleared during the init/reset in kvm.
>>>
>>> This patch fix it by set eflags register to 00000002H following
>>> initialization of the processor.
>>>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>> ---
>>>  arch/x86/kvm/vmx.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index b680c2e..326f6ea 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -4935,6 +4935,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>>  	vmx_set_efer(vcpu, 0);
>>>  	vmx_fpu_activate(vcpu);
>>>  	update_exception_bitmap(vcpu);
>>> +	vmx_set_rflags(vcpu, X86_EFLAGS_FIXED);
>>>    	vpid_sync_context(vmx->vpid);
>>>  }
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH] KVM: x86: fix eflags state following processor init/reset

[Wanpeng Li]

2015-11-03 18:47 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 28/10/2015 09:10, Nadav Amit wrote:
>> Here are my 5 cents. Note that vmx_vcpu_reset calls:
>>
>>       vmcs_writel(GUEST_RFLAGS, 0x02);
>>
>> (And the RFLAGS value is not cached by KVM, so no consistency problem should
>> occur.)
>>
>> You may want to change the value into constant or call a wrapper function
>> for setting RFLAGS, but I don’t see something broken in the functionality.
>
> I agree.  Wanpeng, if this is just a cleanup, can you send v2 that
> removes or modifies the existing call to vmcs_writel?  If there is a
> bug, can you write a unit test for it?  It should be possible to test
> for the problem using INIT+SIPI on an AP.

You are right, I write a INIT+SIPI kvm-unit-test and didn't trigger
any bug. Please ignore the patch.

Regards,
Wanpeng Li