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

Reply via email to