[Xen-devel] [PATCH v3 2/3] x86/vlapic: Keep timer running when switching between one-shot and periodic mode

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

If we take TSC-deadline mode timer out of the picture, the Intel SDM
does not say that the timer is disable when the timer mode is change,
either from one-shot to periodic or vice versa.

After this patch, the timer is no longer disarmed on change of mode, so
the counter (TMCCT) keeps counting down.

So what does a write to LVTT changes ? On baremetal, the change of mode
is probably taken into account only when the counter reach 0. When this
happen, LVTT is use to figure out if the counter should restard counting
down from TMICT (so periodic mode) or stop counting (if one-shot mode).

This also mean that if the counter reach 0 and the mode is one-shot, a
change to periodic would not restart the timer. This is achieve by
setting vlapic->timer_last_update=0.

This patch is based on observation of the behavior of the APIC timer on
baremetal as well as check that they does not go against the description
written in the Intel SDM.

Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
---
Changes in V3:
- new argument for vlapic_update_timer: tmict_updated.
  To avoid setting timer_last_update twice when TMICT is been updated.
- use values of period and delta to calculate timer_last_update only
  when register other than TMICT are been updated
---
 xen/arch/x86/hvm/vlapic.c | 52 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 587ef8defe..7a5fbb40cd 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -520,7 +520,8 @@ static uint32_t vlapic_get_tmcct(struct vlapic *vlapic)
     counter_passed = ((hvm_get_guest_time(v) - vlapic->timer_last_update)
                       / (APIC_BUS_CYCLE_NS * vlapic->hw.timer_divisor));
 
-    if ( tmict != 0 )
+    /* If timer_last_update is 0, then TMCCT should return 0 as well.  */
+    if ( tmict && vlapic->timer_last_update )
     {
         if ( vlapic_lvtt_period(vlapic) )
             counter_passed %= tmict;
@@ -675,28 +676,47 @@ static void vlapic_tdt_pt_cb(struct vcpu *v, void *data)
  * It expect the new value of LVTT to be set *after* being called, with this
  * new values passed as parameter (only APIC_TIMER_MODE_MASK bits matter).
  */
-static void vlapic_update_timer(struct vlapic *vlapic, uint32_t lvtt)
+static void vlapic_update_timer(struct vlapic *vlapic, uint32_t lvtt,
+                                bool tmict_updated)
 {
-    uint64_t period;
-    bool is_periodic;
+    uint64_t period, delta = 0;
+    bool is_oneshot, is_periodic;
 
     is_periodic = (lvtt & APIC_TIMER_MODE_MASK) == APIC_TIMER_MODE_PERIODIC;
+    is_oneshot = (lvtt & APIC_TIMER_MODE_MASK) == APIC_TIMER_MODE_ONESHOT;
 
     period = (uint64_t)vlapic_get_reg(vlapic, APIC_TMICT)
         * APIC_BUS_CYCLE_NS * vlapic->hw.timer_divisor;
 
-    if ( period )
+    /* Calculate the next time the timer should trigger an interrupt. */
+    if ( tmict_updated )
+        delta = period;
+    else if ( period && vlapic->timer_last_update )
     {
-        TRACE_2_LONG_3D(TRC_HVM_EMUL_LAPIC_START_TIMER, TRC_PAR_LONG(period),
+        uint64_t time_passed = hvm_get_guest_time(current)
+            - vlapic->timer_last_update;
+
+        /* This depends of the previous mode, if a new mode is being set */
+        if ( vlapic_lvtt_period(vlapic) )
+            time_passed %= period;
+        if ( time_passed < period )
+            delta = period - time_passed;
+    }
+
+    if ( delta && (is_oneshot || is_periodic) )
+    {
+        TRACE_2_LONG_3D(TRC_HVM_EMUL_LAPIC_START_TIMER, TRC_PAR_LONG(delta),
                         TRC_PAR_LONG(is_periodic ? period : 0),
                         vlapic->pt.irq);
 
-        create_periodic_time(current, &vlapic->pt, period,
+        create_periodic_time(current, &vlapic->pt, delta,
                              is_periodic ? period : 0, vlapic->pt.irq,
                              is_periodic ? vlapic_pt_cb : NULL,
                              &vlapic->timer_last_update);
 
         vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+        if ( !tmict_updated )
+            vlapic->timer_last_update -= period - delta;
 
         HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                     "bus cycle is %uns, "
@@ -709,6 +729,12 @@ static void vlapic_update_timer(struct vlapic *vlapic, 
uint32_t lvtt)
     {
         TRACE_0D(TRC_HVM_EMUL_LAPIC_STOP_TIMER);
         destroy_periodic_time(&vlapic->pt);
+        /*
+         * From now, TMCCT should return 0 until TMICT is set again.
+         * This is because the timer mode was one-shot when the counter reach 0
+         * or just because the timer is disable.
+         */
+        vlapic->timer_last_update = 0;
     }
 }
 
@@ -776,16 +802,16 @@ static void vlapic_reg_write(struct vcpu *v,
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
-        if ( (vlapic_get_reg(vlapic, offset) & APIC_TIMER_MODE_MASK) !=
-             (val & APIC_TIMER_MODE_MASK) )
+        if ( vlapic_lvtt_tdt(vlapic) !=
+             ((val & APIC_TIMER_MODE_MASK) == APIC_TIMER_MODE_TSC_DEADLINE))
         {
-            TRACE_0D(TRC_HVM_EMUL_LAPIC_STOP_TIMER);
-            destroy_periodic_time(&vlapic->pt);
             vlapic_set_reg(vlapic, APIC_TMICT, 0);
-            vlapic_set_reg(vlapic, APIC_TMCCT, 0);
             vlapic->hw.tdt_msr = 0;
         }
         vlapic->pt.irq = val & APIC_VECTOR_MASK;
+
+        vlapic_update_timer(vlapic, val, false);
+
         /* fallthrough */
     case APIC_LVTTHMR:      /* LVT Thermal Monitor */
     case APIC_LVTPC:        /* LVT Performance Counter */
@@ -813,7 +839,7 @@ static void vlapic_reg_write(struct vcpu *v,
 
         vlapic_set_reg(vlapic, APIC_TMICT, val);
 
-        vlapic_update_timer(vlapic, vlapic_get_reg(vlapic, APIC_LVTT));
+        vlapic_update_timer(vlapic, vlapic_get_reg(vlapic, APIC_LVTT), true);
         break;
 
     case APIC_TDCR:
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel