Re: [PATCH v4 2/2] KVM: nVMX: Fix preemption timer bit set in vmcs02 even if L1 doesn't enable it

[Wanpeng Li]

2016-07-07 22:11 GMT+08:00 Paolo Bonzini <pbonzini@xxxxxxxxxx>:
>
>
> On 07/07/2016 15:23, Wanpeng Li wrote:
>>>> >>
>>>> >>               if (kvm_lapic_hv_timer_in_use(vcpu) &&
>>>> >> +                     (is_guest_mode(vcpu) ||
>>>> >>                               kvm_x86_ops->set_hv_timer(vcpu,
>>>> >> -                                     kvm_get_lapic_tscdeadline_msr(vcpu)))
>>>> >> +                                     kvm_get_lapic_tscdeadline_msr(vcpu))))
>>>> >>                       kvm_lapic_switch_to_sw_timer(vcpu);
>>>> >>               if (check_tsc_unstable()) {
>>>> >>                       u64 offset = kvm_compute_tsc_offset(vcpu,
>>>> >>
>>> >
>>> > Thanks, this is good as a fallback.  I'll try to fix it by getting the
>>> > pin-based execution controls right but if I fail this patch is okay.
>> I believe we still need this patch even if you implement "L1 TSC
>> deadline timer to trigger while L2 is running" eventually, the codes
>> you posted before:
>>
>>   exec_control = vmcs12->pin_based_vm_exec_control;
>> +exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER;
>>   exec_control |= vmcs_config.pin_based_exec_ctrl;
>> - exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER;
>> + if (vmx->hv_deadline_tsc == -1)
>> +     exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER;
>>
>> So there is still case the preemption timer bit of vmcs02 is not set,
>> however,  the scenario I mentioned above in kvm_arch_vcpu_load() will
>> set it unnecessary.
>
> kvm_x86_ops->set_hv_timer _will_ set the preemption timer bit of vmcs02
> if vmcs02 is the loaded one.
>
> This can happen if L2 has access to L1's local APIC registers (i.e. L1
> passes the local APIC instead of emulating it, as is the case in a
> partitioning hypervisor).  While L2 runs, it writes to the TSC deadline
> MSR of L1.  This causes a call to kvm_x86_ops->set_hv_timer while the
> active VMCS is a vmcs02.

Yes, in the scenario you pointed out the call to
kvm_x86_ops->set_hv_timer while the active VMCS is vmcs02 is correct,
however, in the scenario I mentioned in the patch description is not
correct even if enable "L1 TSC deadline timer to trigger while L2 is
running".

Regards,
Wanpeng Li