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

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

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

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/


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