On 28/06/2017 03:29, Wanpeng Li wrote:
> u64 tscdeadline = apic->lapic_timer.tscdeadline;
> + int ret = 0;
>
> if ((atomic_read(&apic->lapic_timer.pending) &&
> !apic_lvtt_period(apic)) ||
> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
> + (ret = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline))) {
> if (apic->lapic_timer.hv_timer_in_use)
> cancel_hv_timer(apic);
> + if (ret == 1) {
> + apic_timer_expired(apic);
> + return true;
> + }
The preemption timer can also be used for modes other than TSC deadline.
In periodic mode, your patch would miss a call to
advance_periodic_target_expiration, which is only called by
kvm_lapic_expired_hv_timer.
You could use something like this:
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d24c8742d9b0..15b751aa7625 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
static bool start_hv_timer(struct kvm_lapic *apic)
{
u64 tscdeadline = apic->lapic_timer.tscdeadline;
+ bool need_cancel = apic->lapic_timer.hv_timer_in_use;
+ if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
+ int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
+ if (r >= 0) {
+ need_cancel = false;
+ apic->lapic_timer.hv_timer_in_use = true;
+ hrtimer_cancel(&apic->lapic_timer.timer);
- if ((atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic)) ||
- kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
- if (apic->lapic_timer.hv_timer_in_use)
- cancel_hv_timer(apic);
- } else {
- apic->lapic_timer.hv_timer_in_use = true;
- hrtimer_cancel(&apic->lapic_timer.timer);
-
- /* In case the sw timer triggered in the window */
- if (atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic))
- cancel_hv_timer(apic);
+ /* In case the sw timer triggered in the window */
+ if (atomic_read(&apic->lapic_timer.pending) &&
+ !apic_lvtt_period(apic))
+ need_cancel = true;
+ else if (r)
+ kvm_lapic_expired_hv_timer(vcpu);
+ }
}
+
+ if (need_cancel)
+ cancel_hv_timer(apic);
+
trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
apic->lapic_timer.hv_timer_in_use);
return apic->lapic_timer.hv_timer_in_use;
but I'm afraid of introducing a mutual recursion between
start_hv_timer and kvm_lapic_expired_hv_timer.
Paolo
