diff mbox

[10/12] KVM: nVMX: Update guest activity state field on L2 exits

Message ID eea7589c322fd14a29817c389815abcda4398b38.1388857646.git.jan.kiszka@web.de (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka Jan. 4, 2014, 5:47 p.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

Set guest activity state in L1's VMCS according to the VCPUs mp_state.
This ensures we report the correct state in case we L2 executed HLT or
if we put L2 into HLT state and it was now woken up by an event.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kvm/vmx.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Paolo Bonzini Jan. 5, 2014, 8:01 p.m. UTC | #1
Il 04/01/2014 18:47, Jan Kiszka ha scritto:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Set guest activity state in L1's VMCS according to the VCPUs mp_state.
> This ensures we report the correct state in case we L2 executed HLT or
> if we put L2 into HLT state and it was now woken up by an event.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  arch/x86/kvm/vmx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index bde8ddd..1245ff1 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8223,6 +8223,10 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>  		vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
>  	vmcs12->guest_pending_dbg_exceptions =
>  		vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS);
> +	if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED)
> +		vmcs12->guest_activity_state = GUEST_ACTIVITY_HLT;
> +	else
> +		vmcs12->guest_activity_state = GUEST_ACTIVITY_ACTIVE;
>  
>  	if ((vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIMER) &&
>  	    (vmcs12->vm_exit_controls & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER))
> 

So, without this patch, CPU_BASED_HLT_EXITING should have been set in
nested_vmx_procbased_ctls_low (because disabling HLT exits means L2 can
go "spontaneously" to KVM_MP_STATE_HALTED and without this patch it
would exit the HLT on a subsequent unrelated vmexit).

I cannot think of any other control that we do not support disabling,
but perhaps I'm missing something.

Paolo
--
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
Jan Kiszka Jan. 5, 2014, 8:16 p.m. UTC | #2
On 2014-01-05 21:01, Paolo Bonzini wrote:
> Il 04/01/2014 18:47, Jan Kiszka ha scritto:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Set guest activity state in L1's VMCS according to the VCPUs mp_state.
>> This ensures we report the correct state in case we L2 executed HLT or
>> if we put L2 into HLT state and it was now woken up by an event.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  arch/x86/kvm/vmx.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index bde8ddd..1245ff1 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -8223,6 +8223,10 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>>  		vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
>>  	vmcs12->guest_pending_dbg_exceptions =
>>  		vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS);
>> +	if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED)
>> +		vmcs12->guest_activity_state = GUEST_ACTIVITY_HLT;
>> +	else
>> +		vmcs12->guest_activity_state = GUEST_ACTIVITY_ACTIVE;
>>  
>>  	if ((vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIMER) &&
>>  	    (vmcs12->vm_exit_controls & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER))
>>
> 
> So, without this patch, CPU_BASED_HLT_EXITING should have been set in
> nested_vmx_procbased_ctls_low (because disabling HLT exits means L2 can
> go "spontaneously" to KVM_MP_STATE_HALTED and without this patch it
> would exit the HLT on a subsequent unrelated vmexit).

I guess so. I've no hardware around that does not support
GUEST_ACTIVITY_HLT, but it makes most sense that those machines enforced
HLT_EXITING that way - so should have we. But that inconsistency is
fixed now, hopefully.

> 
> I cannot think of any other control that we do not support disabling,
> but perhaps I'm missing something.

Yeah, I wouldn't be surprised to find a few more broken corner cases. As
the number of hypervisor continue to increase, they may help to find
them. Reminds me that I still want to run [1] over my queue. They claim
to use the preemption timer extensively.

Jan

[1] http://muen.codelabs.ch/
diff mbox

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index bde8ddd..1245ff1 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8223,6 +8223,10 @@  static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 		vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
 	vmcs12->guest_pending_dbg_exceptions =
 		vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS);
+	if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED)
+		vmcs12->guest_activity_state = GUEST_ACTIVITY_HLT;
+	else
+		vmcs12->guest_activity_state = GUEST_ACTIVITY_ACTIVE;
 
 	if ((vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIMER) &&
 	    (vmcs12->vm_exit_controls & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER))