Building guests on monotonic Xen system time

RFC PATCH In review Dan Magenheimer 2 participants 12 messages May 16, 2008 – Jun 3, 2008

Share & tools

Dan Magenheimer

author

2008-05-16 17:31 UTC #0 lore raw

RFC PATCH 1/1 Building guests on monotonic Xen system time

Currently, hvm guest platform timers are built on top of the tsc. Even though the guest believes it is utilizing a monotonic timesource (such as pit, hpet, or pmtimer), all of these plumb down to an rdtsc instruction.

Since on many SMP platforms tsc's in different processors are not synchronized, VMs re-scheduled from a "fast tsc" processor to a "slow tsc" processor may experience "time going backwards". This is discussed in the following thread:

http://lists.xensource.com/archives/html/xen-devel/2008-04/msg00277.html

The fix proposed by Keir is that "The logic in vpt.c should be fixed to use Xen's concept of system time and everything, guest TSC included, should be derived from that."

The attached patch is a first attempt to derive all guest timers from Xen's system time... and also to ensure that system time is non-decreasing. I don't believe the patch is complete... and possibly not even correct. For example, I'm concerned that Xen's system time uses different units than a guest tsc and I don't recall if all guest tsc accesses are trapped.

Dan

===================================
Thanks... for the memory
I really could use more / My throughput's on the floor The balloon is flat / My swap disk's fat / I've OOM's in store Overcommitted so much
(with apologies to the late great Bob Hope)

xen/arch/x86/hvm/hvm.c +3 -5

diff -r e3b13e1ecf6c xen/arch/x86/hvm/hvm.c
@@ -31,6 +31,7 @@
 #include <xen/hypercall.h>
 #include <xen/guest_access.h>
 #include <xen/event.h>
+#include <xen/time.h>
 #include <asm/current.h>
 #include <asm/e820.h>
 #include <asm/io.h>

@@ -136,9 +137,7 @@ uint8_t hvm_combine_hw_exceptions(uint8_
 
 void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc)
 {
-    u64 host_tsc;
-
-    rdtscll(host_tsc);
+    u64 host_tsc = (u64)get_s_time_mono();
 
     v->arch.hvm_vcpu.cache_tsc_offset = guest_tsc - host_tsc;
     hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset);

@@ -146,9 +145,8 @@ void hvm_set_guest_tsc(struct vcpu *v, u
 
 u64 hvm_get_guest_tsc(struct vcpu *v)
 {
-    u64 host_tsc;
+    u64 host_tsc = (u64)get_s_time_mono();
 
-    rdtscll(host_tsc);
     return host_tsc + v->arch.hvm_vcpu.cache_tsc_offset;
 }

xen/arch/x86/time.c +26 -0

diff -r e3b13e1ecf6c xen/arch/x86/time.c
@@ -535,6 +535,8 @@ static s_time_t read_platform_stime(void
     return stime;
 }
 
+extern void get_s_time_mono_init(void);
+
 static void platform_time_calibration(void)
 {
     u64 count;

@@ -593,6 +595,7 @@ static void init_platform_timer(void)
     plt_overflow(NULL);
 
     platform_timer_stamp = plt_stamp64;
+    get_s_time_mono_init();
 
     printk("Platform timer is %s %s\n",
            freq_string(pts->frequency), pts->name);

@@ -728,6 +731,29 @@ s_time_t get_s_time(void)
     rdtscll(tsc);
     delta = tsc - t->local_tsc_stamp;
     now = t->stime_local_stamp + scale_delta(delta, &t->tsc_scale);
+
+    return now;
+}
+
+spinlock_t get_s_time_mono_lock;
+
+void get_s_time_mono_init(void)
+{
+    spin_lock_init(&get_s_time_mono_lock);
+}
+
+s_time_t get_s_time_mono(void)
+{
+    s_time_t now;
+    static s_time_t last = (s_time_t)0;
+
+    spin_lock(&get_s_time_mono_lock);
+    now = get_s_time();
+    if (now > last)
+        last = now;
+    else
+        now = last;
+    spin_unlock(&get_s_time_mono_lock);
 
     return now;
 }

xen/include/xen/time.h +3 -0

diff -r e3b13e1ecf6c xen/include/xen/time.h
@@ -24,6 +24,8 @@ struct domain;
  * System Time
  * 64 bit value containing the nanoseconds elapsed since boot time.
  * This value is adjusted by frequency drift.
+ * mono version guarantees that consecutive reads are non-decreasing,
+ * (e.g. when first read is on a different processor than second read)
  * NOW() returns the current time.
  * The other macros are for convenience to approximate short intervals
  * of real time into system time

@@ -32,6 +34,7 @@ typedef s64 s_time_t;
 typedef s64 s_time_t;
 
 s_time_t get_s_time(void);
+s_time_t get_s_time_mono(void);
 unsigned long get_localtime(struct domain *d);
 
 struct tm {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Dan Magenheimer

author

2008-05-19 18:27 UTC #1 lore raw

As I look at this more, I'm convincing myself that it may not make sense to build the guest TSC on top of Xen system time and only the guest virtual platform timer(s) should be built on Xen system time (and should be monotonically non-decreasing).

Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:

rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */

Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.

Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.

Unfortunately, hvm_get/set_system_time() are #defined to be hvm_get/set_system_tsc() so the usage is pretty muddled. I think the attached patch has teased the two apart though.

Note this is still RFC, not a finished patch.

Quoted 15 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]]On Behalf Of Dan Magenheimer
Sent: Friday, May 16, 2008 11:31 AM
To: Xen-Devel (E-mail)
Subject: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen system time
Currently, hvm guest platform timers are built on top of the tsc. Even though the guest believes it is utilizing a monotonic timesource (such as pit, hpet, or pmtimer), all of these plumb down to an rdtsc instruction.

Quoted 6 lines

Since on many SMP platforms tsc's in different processors are not synchronized, VMs re-scheduled from a "fast tsc" processor to a "slow tsc" processor may experience "time going backwards". This is discussed in the following thread:
http://lists.xensource.com/archives/html/xen-devel/2008-04/msg

00277.html

The fix proposed by Keir is that "The logic in vpt.c should be fixed to use Xen's concept of system time and everything, guest TSC included, should be derived from that."

Dan

xen/arch/x86/hvm/hpet.c +8 -8

diff -r e3b13e1ecf6c xen/arch/x86/hvm/hpet.c
@@ -29,9 +29,9 @@
 #define S_TO_NS  1000000000ULL           /* 1s  = 10^9  ns */
 #define S_TO_FS  1000000000000000ULL     /* 1s  = 10^15 fs */
 
-/* Frequency_of_TSC / frequency_of_HPET = 32 */
-#define TSC_PER_HPET_TICK 32
-#define guest_time_hpet(v) (hvm_get_guest_time(v) / TSC_PER_HPET_TICK)
+/* Frequency_of_Xen_systeme_time / frequency_of_HPET = 16 */
+#define STIME_PER_HPET_TICK 16
+#define guest_time_hpet(v) (hvm_get_guest_time_mono(v) / STIME_PER_HPET_TICK)
 
 #define HPET_ID         0x000
 #define HPET_PERIOD     0x004

@@ -192,7 +192,7 @@ static void hpet_stop_timer(HPETState *h
 
 /* the number of HPET tick that stands for
  * 1/(2^10) second, namely, 0.9765625 milliseconds */
-#define  HPET_TINY_TIME_SPAN  ((h->tsc_freq >> 10) / TSC_PER_HPET_TICK)
+#define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
 
 static void hpet_set_timer(HPETState *h, unsigned int tn)
 {

@@ -558,17 +558,17 @@ void hpet_init(struct vcpu *v)
     spin_lock_init(&h->lock);
 
     h->vcpu = v;
-    h->tsc_freq = ticks_per_sec(v);
+    h->stime_freq = S_TO_NS;
 
-    h->hpet_to_ns_scale = ((S_TO_NS * TSC_PER_HPET_TICK) << 10) / h->tsc_freq;
+    h->hpet_to_ns_scale = ((S_TO_NS * STIME_PER_HPET_TICK) << 10) / h->stime_freq;
     h->hpet_to_ns_limit = ~0ULL / h->hpet_to_ns_scale;
 
     /* 64-bit main counter; 3 timers supported; LegacyReplacementRoute. */
     h->hpet.capability = 0x8086A201ULL;
 
     /* This is the number of femptoseconds per HPET tick. */
-    /* Here we define HPET's frequency to be 1/32 of the TSC's */
-    h->hpet.capability |= ((S_TO_FS*TSC_PER_HPET_TICK/h->tsc_freq) << 32);
+    /* Here we define HPET's frequency to be 1/16 of Xen system time */
+    h->hpet.capability |= ((S_TO_FS*STIME_PER_HPET_TICK/h->stime_freq) << 32);
 
     for ( i = 0; i < HPET_TIMER_NUM; i++ )
     {

xen/arch/x86/hvm/hvm.c +25 -3

diff -r e3b13e1ecf6c xen/arch/x86/hvm/hvm.c
@@ -31,6 +31,7 @@
 #include <xen/hypercall.h>
 #include <xen/guest_access.h>
 #include <xen/event.h>
+#include <xen/time.h>
 #include <asm/current.h>
 #include <asm/e820.h>
 #include <asm/io.h>

@@ -139,7 +140,6 @@ void hvm_set_guest_tsc(struct vcpu *v, u
     u64 host_tsc;
 
     rdtscll(host_tsc);
-
     v->arch.hvm_vcpu.cache_tsc_offset = guest_tsc - host_tsc;
     hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset);
 }

@@ -150,6 +150,28 @@ u64 hvm_get_guest_tsc(struct vcpu *v)
 
     rdtscll(host_tsc);
     return host_tsc + v->arch.hvm_vcpu.cache_tsc_offset;
+}
+
+void hvm_set_guest_time(struct vcpu *v, u64 guest_time)
+{
+    u64 host_time = (u64)get_s_time_mono();
+
+    v->arch.hvm_vcpu.cache_time_offset = guest_time - host_time;
+}
+
+u64 hvm_get_guest_time(struct vcpu *v)
+{
+    u64 host_time = (u64)get_s_time();
+
+    return host_time + v->arch.hvm_vcpu.cache_time_offset;
+}
+
+
+u64 hvm_get_guest_time_mono(struct vcpu *v)
+{
+    u64 host_time = (u64)get_s_time_mono();
+
+    return host_time + v->arch.hvm_vcpu.cache_time_offset;
 }
 
 void hvm_migrate_timers(struct vcpu *v)

@@ -1632,7 +1654,7 @@ int hvm_msr_read_intercept(struct cpu_us
     switch ( ecx )
     {
     case MSR_IA32_TSC:
-        msr_content = hvm_get_guest_time(v);
+        msr_content = hvm_get_guest_tsc(v);
         break;
 
     case MSR_IA32_APICBASE:

@@ -1725,7 +1747,7 @@ int hvm_msr_write_intercept(struct cpu_u
     switch ( ecx )
     {
      case MSR_IA32_TSC:
-        hvm_set_guest_time(v, msr_content);
+        hvm_set_guest_tsc(v, msr_content);
         pt_reset(v);
         break;

xen/arch/x86/hvm/i8254.c +10 -6

diff -r e3b13e1ecf6c xen/arch/x86/hvm/i8254.c
@@ -86,7 +86,7 @@ static int pit_get_count(PITState *pit, 
 
     ASSERT(spin_is_locked(&pit->lock));
 
-    d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel],
+    d = muldiv64(hvm_get_guest_time_mono(v) - pit->count_load_time[channel],
                  PIT_FREQ, ticks_per_sec(v));
 
     switch ( c->mode )

@@ -117,7 +117,7 @@ static int pit_get_out(PITState *pit, in
 
     ASSERT(spin_is_locked(&pit->lock));
 
-    d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel], 
+    d = muldiv64(hvm_get_guest_time_mono(v) - pit->count_load_time[channel], 
                  PIT_FREQ, ticks_per_sec(v));
 
     switch ( s->mode )

@@ -164,7 +164,7 @@ static void pit_set_gate(PITState *pit, 
     case 3:
         /* Restart counting on rising edge. */
         if ( s->gate < val )
-            pit->count_load_time[channel] = hvm_get_guest_time(v);
+            pit->count_load_time[channel] = hvm_get_guest_time_mono(v);
         break;
     }

@@ -180,7 +180,7 @@ static void pit_time_fired(struct vcpu *
 static void pit_time_fired(struct vcpu *v, void *priv)
 {
     uint64_t *count_load_time = priv;
-    *count_load_time = hvm_get_guest_time(v);
+    *count_load_time = hvm_get_guest_time_mono(v);
 }
 
 static void pit_load_count(PITState *pit, int channel, int val)

@@ -195,9 +195,13 @@ static void pit_load_count(PITState *pit
         val = 0x10000;
 
     if ( v == NULL )
+        gdprintk(XENLOG_ERR, "NULL vcpu in pit_load_count.\n");
+    /*
         rdtscll(pit->count_load_time[channel]);
     else
-        pit->count_load_time[channel] = hvm_get_guest_time(v);
+*/
+/*    ASSERT(v); */
+        pit->count_load_time[channel] = hvm_get_guest_time_mono(v);
     s->count = val;
     period = DIV_ROUND((val * 1000000000ULL), PIT_FREQ);

@@ -435,7 +439,7 @@ static int pit_load(struct domain *d, hv
      * time jitter here, but the wall-clock will have jumped massively, so 
      * we hope the guest can handle it.
      */
-    pit->pt0.last_plt_gtime = hvm_get_guest_time(d->vcpu[0]);
+    pit->pt0.last_plt_gtime = hvm_get_guest_time_mono(d->vcpu[0]);
     for ( i = 0; i < 3; i++ )
         pit_load_count(pit, i, pit->hw.channels[i].count);

xen/arch/x86/hvm/pmtimer.c +2 -2

diff -r e3b13e1ecf6c xen/arch/x86/hvm/pmtimer.c
@@ -71,7 +71,7 @@ static void pmt_update_time(PMTState *s)
     ASSERT(spin_is_locked(&s->lock));
 
     /* Update the timer */
-    curr_gtime = hvm_get_guest_time(s->vcpu);
+    curr_gtime = hvm_get_guest_time_mono(s->vcpu);
     s->pm.tmr_val += ((curr_gtime - s->last_gtime) * s->scale) >> 32;
     s->pm.tmr_val &= TMR_VAL_MASK;
     s->last_gtime = curr_gtime;

@@ -238,7 +238,7 @@ static int pmtimer_load(struct domain *d
     }
 
     /* Calculate future counter values from now. */
-    s->last_gtime = hvm_get_guest_time(s->vcpu);
+    s->last_gtime = hvm_get_guest_time_mono(s->vcpu);
 
     /* Set the SCI state from the registers */ 
     pmt_update_sci(s);

xen/arch/x86/hvm/svm/svm.c +9 -2

diff -r e3b13e1ecf6c xen/arch/x86/hvm/svm/svm.c
@@ -299,7 +299,7 @@ static void svm_save_cpu_state(struct vc
     data->msr_efer         = v->arch.hvm_vcpu.guest_efer;
     data->msr_flags        = -1ULL;
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }

@@ -315,7 +315,7 @@ static void svm_load_cpu_state(struct vc
     v->arch.hvm_vcpu.guest_efer = data->msr_efer;
     svm_update_guest_efer(v);
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }
 
 static void svm_save_vmcb_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)

@@ -583,6 +583,12 @@ static void svm_set_tsc_offset(struct vc
     v->arch.hvm_svm.vmcb->tsc_offset = offset;
 }
 
+
+static void svm_set_time_offset(struct vcpu *v, u64 offset)
+{
+    v->arch.hvm_vcpu.cache_time_offset = offset;
+}
+
 static void svm_init_hypercall_page(struct domain *d, void *hypercall_page)
 {
     char *p;

@@ -790,6 +796,7 @@ static struct hvm_function_table svm_fun
     .update_guest_efer    = svm_update_guest_efer,
     .flush_guest_tlbs     = svm_flush_guest_tlbs,
     .set_tsc_offset       = svm_set_tsc_offset,
+    .set_time_offset      = svm_set_time_offset,
     .inject_exception     = svm_inject_exception,
     .init_hypercall_page  = svm_init_hypercall_page,
     .event_pending        = svm_event_pending,

xen/arch/x86/hvm/vlapic.c +3 -3

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vlapic.c
@@ -535,8 +535,8 @@ static uint32_t vlapic_get_tmcct(struct 
     uint32_t tmcct, tmict = vlapic_get_reg(vlapic, APIC_TMICT);
     uint64_t counter_passed;
 
-    counter_passed = ((hvm_get_guest_time(v) - vlapic->timer_last_update)
-                      * 1000000000ULL / ticks_per_sec(v)
+    counter_passed = ((hvm_get_guest_time_mono(v) - vlapic->timer_last_update)
+                      /* * 1000000000ULL / ticks_per_sec(v) */
                       / APIC_BUS_CYCLE_NS / vlapic->hw.timer_divisor);
     tmcct = tmict - counter_passed;

@@ -638,7 +638,7 @@ static int vlapic_read(
 
 void vlapic_pt_cb(struct vcpu *v, void *data)
 {
-    *(s_time_t *)data = hvm_get_guest_time(v);
+    *(s_time_t *)data = hvm_get_guest_time_mono(v);
 }
 
 static int vlapic_write(struct vcpu *v, unsigned long address,

xen/arch/x86/hvm/vmx/vmx.c +8 -2

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vmx/vmx.c
@@ -607,7 +607,7 @@ static void vmx_save_cpu_state(struct vc
     data->msr_syscall_mask = guest_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK];
 #endif
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }
 
 static void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)

@@ -625,7 +625,7 @@ static void vmx_load_cpu_state(struct vc
     v->arch.hvm_vmx.shadow_gs = data->shadow_gs;
 #endif
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }

@@ -866,6 +866,11 @@ static void vmx_set_tsc_offset(struct vc
     __vmwrite(TSC_OFFSET_HIGH, offset >> 32);
 #endif
     vmx_vmcs_exit(v);
+}
+
+static void vmx_set_time_offset(struct vcpu *v, u64 offset)
+{
+    v->arch.hvm_vcpu.cache_time_offset = offset;
 }
 
 void do_nmi(struct cpu_user_regs *);

@@ -1186,6 +1191,7 @@ static struct hvm_function_table vmx_fun
     .update_guest_efer    = vmx_update_guest_efer,
     .flush_guest_tlbs     = vmx_flush_guest_tlbs,
     .set_tsc_offset       = vmx_set_tsc_offset,
+    .set_time_offset       = vmx_set_time_offset,
     .inject_exception     = vmx_inject_exception,
     .init_hypercall_page  = vmx_init_hypercall_page,
     .event_pending        = vmx_event_pending,

xen/arch/x86/hvm/vpt.c +6 -6

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vpt.c
@@ -108,7 +108,7 @@ static void pt_freeze_time(struct vcpu *
     if ( !mode_is(v->domain, delay_for_missed_ticks) )
         return;
 
-    v->arch.hvm_vcpu.guest_time = hvm_get_guest_time(v);
+    v->arch.hvm_vcpu.guest_time = hvm_get_guest_time_mono(v);
 }
 
 static void pt_thaw_time(struct vcpu *v)

@@ -267,7 +267,7 @@ void pt_intr_post(struct vcpu *v, struct
         if ( mode_is(v->domain, one_missed_tick_pending) ||
              mode_is(v->domain, no_missed_ticks_pending) )
         {
-            pt->last_plt_gtime = hvm_get_guest_time(v);
+            pt->last_plt_gtime = hvm_get_guest_time_mono(v);
             pt->pending_intr_nr = 0; /* 'collapse' all missed ticks */
         }
         else

@@ -278,7 +278,7 @@ void pt_intr_post(struct vcpu *v, struct
     }
 
     if ( mode_is(v->domain, delay_for_missed_ticks) &&
-         (hvm_get_guest_time(v) < pt->last_plt_gtime) )
+         (hvm_get_guest_time_mono(v) < pt->last_plt_gtime) )
         hvm_set_guest_time(v, pt->last_plt_gtime);
 
     cb = pt->cb;

@@ -300,7 +300,7 @@ void pt_reset(struct vcpu *v)
     list_for_each_entry ( pt, head, list )
     {
         pt->pending_intr_nr = 0;
-        pt->last_plt_gtime = hvm_get_guest_time(pt->vcpu);
+        pt->last_plt_gtime = hvm_get_guest_time_mono(pt->vcpu);
         pt->scheduled = NOW() + pt->period;
         set_timer(&pt->timer, pt->scheduled);
     }

@@ -346,9 +346,9 @@ void create_periodic_time(
 
     pt->period = period;
     pt->vcpu = v;
-    pt->last_plt_gtime = hvm_get_guest_time(pt->vcpu);
+    pt->last_plt_gtime = hvm_get_guest_time_mono(pt->vcpu);
     pt->irq = irq;
-    pt->period_cycles = (u64)period * cpu_khz / 1000000L;
+    pt->period_cycles = (u64)period /* * cpu_khz / 1000000L*/;
     pt->one_shot = one_shot;
     pt->scheduled = NOW() + period;
     /*

xen/arch/x86/time.c +26 -0

diff -r e3b13e1ecf6c xen/arch/x86/time.c
@@ -535,6 +535,8 @@ static s_time_t read_platform_stime(void
     return stime;
 }
 
+extern void get_s_time_mono_init(void);
+
 static void platform_time_calibration(void)
 {
     u64 count;

@@ -593,6 +595,7 @@ static void init_platform_timer(void)
     plt_overflow(NULL);
 
     platform_timer_stamp = plt_stamp64;
+    get_s_time_mono_init();
 
     printk("Platform timer is %s %s\n",
            freq_string(pts->frequency), pts->name);

@@ -728,6 +731,29 @@ s_time_t get_s_time(void)
     rdtscll(tsc);
     delta = tsc - t->local_tsc_stamp;
     now = t->stime_local_stamp + scale_delta(delta, &t->tsc_scale);
+
+    return now;
+}
+
+spinlock_t get_s_time_mono_lock;
+
+void get_s_time_mono_init(void)
+{
+    spin_lock_init(&get_s_time_mono_lock);
+}
+
+s_time_t get_s_time_mono(void)
+{
+    s_time_t now;
+    static s_time_t last = (s_time_t)0;
+
+    spin_lock(&get_s_time_mono_lock);
+    now = get_s_time();
+    if (now > last)
+        last = now;
+    else
+        now = last;
+    spin_unlock(&get_s_time_mono_lock);
 
     return now;
 }

xen/include/asm-x86/hvm/hvm.h +7 -0

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/hvm.h
@@ -107,6 +107,8 @@ struct hvm_function_table {
 
     void (*set_tsc_offset)(struct vcpu *v, u64 offset);
 
+    void (*set_time_offset)(struct vcpu *v, u64 offset);
+
     void (*inject_exception)(unsigned int trapnr, int errcode,
                              unsigned long cr2);

@@ -146,8 +148,13 @@ void hvm_send_assist_req(struct vcpu *v)
 
 void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc);
 u64 hvm_get_guest_tsc(struct vcpu *v);
+void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
+u64 hvm_get_guest_time(struct vcpu *v);
+u64 hvm_get_guest_time_mono(struct vcpu *v);
+/*
 #define hvm_set_guest_time(vcpu, gtime) hvm_set_guest_tsc(vcpu, gtime)
 #define hvm_get_guest_time(vcpu)        hvm_get_guest_tsc(vcpu)
+*/
 
 #define hvm_paging_enabled(v) \
     (!!((v)->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PG))

xen/include/asm-x86/hvm/vcpu.h +1 -0

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vcpu.h
@@ -50,6 +50,7 @@ struct hvm_vcpu {
     struct vlapic       vlapic;
     s64                 cache_tsc_offset;
     u64                 guest_time;
+    s64                 cache_time_offset;
 
     /* Lock and list for virtual platform timers. */
     spinlock_t          tm_lock;

xen/include/asm-x86/hvm/vmx/vmx.h +0 -1

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vmx/vmx.h
@@ -49,7 +49,6 @@ void vmx_asm_do_vmentry(void);
 void vmx_asm_do_vmentry(void);
 void vmx_intr_assist(void);
 void vmx_do_resume(struct vcpu *);
-void set_guest_time(struct vcpu *v, u64 gtime);
 void vmx_vlapic_msr_changed(struct vcpu *v);
 void vmx_realmode(struct cpu_user_regs *regs);

xen/include/asm-x86/hvm/vpt.h +1 -1

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vpt.h
@@ -57,7 +57,7 @@ typedef struct HPETState {
 typedef struct HPETState {
     struct hpet_registers hpet;
     struct vcpu *vcpu;
-    uint64_t tsc_freq;
+    uint64_t stime_freq;
     uint64_t hpet_to_ns_scale; /* hpet ticks to ns (multiplied by 2^10) */
     uint64_t hpet_to_ns_limit; /* max hpet ticks convertable to ns      */
     uint64_t mc_offset;

xen/include/xen/time.h +3 -0

diff -r e3b13e1ecf6c xen/include/xen/time.h
@@ -24,6 +24,8 @@ struct domain;
  * System Time
  * 64 bit value containing the nanoseconds elapsed since boot time.
  * This value is adjusted by frequency drift.
+ * mono version guarantees that consecutive reads are non-decreasing,
+ * (e.g. when first read is on a different processor than second read)
  * NOW() returns the current time.
  * The other macros are for convenience to approximate short intervals
  * of real time into system time

@@ -32,6 +34,7 @@ typedef s64 s_time_t;
 typedef s64 s_time_t;
 
 s_time_t get_s_time(void);
+s_time_t get_s_time_mono(void);
 unsigned long get_localtime(struct domain *d);
 
 struct tm {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Dan Magenheimer

author

2008-05-20 00:56 UTC #2 lore raw

Next version of guest-virtual-platform-time-on-xen-system-time attached. This version seems to work but could use more testing.

Quoted 76 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]]On Behalf Of Dan Magenheimer
Sent: Monday, May 19, 2008 12:27 PM
To: Xen-Devel (E-mail)
Subject: RE: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
As I look at this more, I'm convincing myself that it may not make sense to build the guest TSC on top of Xen system time and only the guest virtual platform timer(s) should be built on Xen system time (and should be monotonically non-decreasing).
Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:
rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */
Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.
Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.
Unfortunately, hvm_get/set_system_time() are #defined to be hvm_get/set_system_tsc() so the usage is pretty muddled. I think the attached patch has teased the two apart though.
Note this is still RFC, not a finished patch.
Quoted 10 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]]On Behalf Of Dan Magenheimer
Sent: Friday, May 16, 2008 11:31 AM
To: Xen-Devel (E-mail)
Subject: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen system time
Quoted 2 lines
Currently, hvm guest platform timers are built on top of the tsc. Even though the guest believes it is utilizing a monotonic
timesource
Quoted 9 lines
(such as pit, hpet, or pmtimer), all of these plumb down to an rdtsc instruction.
Since on many SMP platforms tsc's in different processors are not synchronized, VMs re-scheduled from a "fast tsc" processor to a "slow tsc" processor may experience "time going backwards". This is discussed in the following thread:
http://lists.xensource.com/archives/html/xen-devel/2008-04/msg
00277.html
The fix proposed by Keir is that "The logic in vpt.c should be fixed to use Xen's concept of system time and everything, guest TSC included, should be derived from that."
The attached patch is a first attempt to derive all guest timers from Xen's system time... and also to ensure that system time is non-decreasing. I don't believe the patch is complete... and possibly not even correct. For example, I'm concerned that Xen's system time uses different units than a guest tsc and I don't recall if all guest tsc accesses are trapped.

Quoted 9 lines

Dan
===================================
Thanks... for the memory
I really could use more / My throughput's on the floor The balloon is flat / My swap disk's fat / I've OOM's in store Overcommitted so much
(with apologies to the late great Bob Hope)

xen/arch/x86/hvm/hpet.c +8 -8

diff -r e3b13e1ecf6c xen/arch/x86/hvm/hpet.c
@@ -29,9 +29,9 @@
 #define S_TO_NS  1000000000ULL           /* 1s  = 10^9  ns */
 #define S_TO_FS  1000000000000000ULL     /* 1s  = 10^15 fs */
 
-/* Frequency_of_TSC / frequency_of_HPET = 32 */
-#define TSC_PER_HPET_TICK 32
-#define guest_time_hpet(v) (hvm_get_guest_time(v) / TSC_PER_HPET_TICK)
+/* Frequency_of_Xen_systeme_time / frequency_of_HPET = 16 */
+#define STIME_PER_HPET_TICK 16
+#define guest_time_hpet(v) (hvm_get_guest_time_mono(v) / STIME_PER_HPET_TICK)
 
 #define HPET_ID         0x000
 #define HPET_PERIOD     0x004

@@ -192,7 +192,7 @@ static void hpet_stop_timer(HPETState *h
 
 /* the number of HPET tick that stands for
  * 1/(2^10) second, namely, 0.9765625 milliseconds */
-#define  HPET_TINY_TIME_SPAN  ((h->tsc_freq >> 10) / TSC_PER_HPET_TICK)
+#define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
 
 static void hpet_set_timer(HPETState *h, unsigned int tn)
 {

@@ -558,17 +558,17 @@ void hpet_init(struct vcpu *v)
     spin_lock_init(&h->lock);
 
     h->vcpu = v;
-    h->tsc_freq = ticks_per_sec(v);
+    h->stime_freq = S_TO_NS;
 
-    h->hpet_to_ns_scale = ((S_TO_NS * TSC_PER_HPET_TICK) << 10) / h->tsc_freq;
+    h->hpet_to_ns_scale = ((S_TO_NS * STIME_PER_HPET_TICK) << 10) / h->stime_freq;
     h->hpet_to_ns_limit = ~0ULL / h->hpet_to_ns_scale;
 
     /* 64-bit main counter; 3 timers supported; LegacyReplacementRoute. */
     h->hpet.capability = 0x8086A201ULL;
 
     /* This is the number of femptoseconds per HPET tick. */
-    /* Here we define HPET's frequency to be 1/32 of the TSC's */
-    h->hpet.capability |= ((S_TO_FS*TSC_PER_HPET_TICK/h->tsc_freq) << 32);
+    /* Here we define HPET's frequency to be 1/16 of Xen system time */
+    h->hpet.capability |= ((S_TO_FS*STIME_PER_HPET_TICK/h->stime_freq) << 32);
 
     for ( i = 0; i < HPET_TIMER_NUM; i++ )
     {

xen/arch/x86/hvm/hvm.c +25 -3

diff -r e3b13e1ecf6c xen/arch/x86/hvm/hvm.c
@@ -31,6 +31,7 @@
 #include <xen/hypercall.h>
 #include <xen/guest_access.h>
 #include <xen/event.h>
+#include <xen/time.h>
 #include <asm/current.h>
 #include <asm/e820.h>
 #include <asm/io.h>

@@ -139,7 +140,6 @@ void hvm_set_guest_tsc(struct vcpu *v, u
     u64 host_tsc;
 
     rdtscll(host_tsc);
-
     v->arch.hvm_vcpu.cache_tsc_offset = guest_tsc - host_tsc;
     hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset);
 }

@@ -150,6 +150,28 @@ u64 hvm_get_guest_tsc(struct vcpu *v)
 
     rdtscll(host_tsc);
     return host_tsc + v->arch.hvm_vcpu.cache_tsc_offset;
+}
+
+void hvm_set_guest_time(struct vcpu *v, u64 guest_time)
+{
+    u64 host_time = (u64)get_s_time_mono();
+
+    v->arch.hvm_vcpu.cache_time_offset = guest_time - host_time;
+}
+
+u64 hvm_get_guest_time(struct vcpu *v)
+{
+    u64 host_time = (u64)get_s_time();
+
+    return host_time + v->arch.hvm_vcpu.cache_time_offset;
+}
+
+
+u64 hvm_get_guest_time_mono(struct vcpu *v)
+{
+    u64 host_time = (u64)get_s_time_mono();
+
+    return host_time + v->arch.hvm_vcpu.cache_time_offset;
 }
 
 void hvm_migrate_timers(struct vcpu *v)

@@ -1632,7 +1654,7 @@ int hvm_msr_read_intercept(struct cpu_us
     switch ( ecx )
     {
     case MSR_IA32_TSC:
-        msr_content = hvm_get_guest_time(v);
+        msr_content = hvm_get_guest_tsc(v);
         break;
 
     case MSR_IA32_APICBASE:

@@ -1725,7 +1747,7 @@ int hvm_msr_write_intercept(struct cpu_u
     switch ( ecx )
     {
      case MSR_IA32_TSC:
-        hvm_set_guest_time(v, msr_content);
+        hvm_set_guest_tsc(v, msr_content);
         pt_reset(v);
         break;

xen/arch/x86/hvm/i8254.c +11 ~~-10~~

diff -r e3b13e1ecf6c xen/arch/x86/hvm/i8254.c
@@ -31,6 +31,7 @@
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <xen/sched.h>
+#include <asm/time.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/io.h>
 #include <asm/hvm/support.h>

@@ -86,8 +87,8 @@ static int pit_get_count(PITState *pit, 
 
     ASSERT(spin_is_locked(&pit->lock));
 
-    d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel],
-                 PIT_FREQ, ticks_per_sec(v));
+    d = muldiv64(hvm_get_guest_time_mono(v) - pit->count_load_time[channel],
+                 PIT_FREQ, s_ticks_per_sec(v));
 
     switch ( c->mode )
     {

@@ -117,8 +118,8 @@ static int pit_get_out(PITState *pit, in
 
     ASSERT(spin_is_locked(&pit->lock));
 
-    d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel], 
-                 PIT_FREQ, ticks_per_sec(v));
+    d = muldiv64(hvm_get_guest_time_mono(v) - pit->count_load_time[channel], 
+                 PIT_FREQ, s_ticks_per_sec(v));
 
     switch ( s->mode )
     {

@@ -164,7 +165,7 @@ static void pit_set_gate(PITState *pit, 
     case 3:
         /* Restart counting on rising edge. */
         if ( s->gate < val )
-            pit->count_load_time[channel] = hvm_get_guest_time(v);
+            pit->count_load_time[channel] = hvm_get_guest_time_mono(v);
         break;
     }

@@ -180,7 +181,7 @@ static void pit_time_fired(struct vcpu *
 static void pit_time_fired(struct vcpu *v, void *priv)
 {
     uint64_t *count_load_time = priv;
-    *count_load_time = hvm_get_guest_time(v);
+    *count_load_time = hvm_get_guest_time_mono(v);
 }
 
 static void pit_load_count(PITState *pit, int channel, int val)

@@ -195,11 +196,11 @@ static void pit_load_count(PITState *pit
         val = 0x10000;
 
     if ( v == NULL )
-        rdtscll(pit->count_load_time[channel]);
+        pit->count_load_time[channel] = 0; /* FIXME? */
     else
-        pit->count_load_time[channel] = hvm_get_guest_time(v);
+        pit->count_load_time[channel] = hvm_get_guest_time_mono(v);
     s->count = val;
-    period = DIV_ROUND((val * 1000000000ULL), PIT_FREQ);
+    period = DIV_ROUND((val * s_ticks_per_sec(v)), PIT_FREQ);
 
     if ( (v == NULL) || !is_hvm_vcpu(v) || (channel != 0) )
         return;

@@ -435,7 +436,7 @@ static int pit_load(struct domain *d, hv
      * time jitter here, but the wall-clock will have jumped massively, so 
      * we hope the guest can handle it.
      */
-    pit->pt0.last_plt_gtime = hvm_get_guest_time(d->vcpu[0]);
+    pit->pt0.last_plt_gtime = hvm_get_guest_time_mono(d->vcpu[0]);
     for ( i = 0; i < 3; i++ )
         pit_load_count(pit, i, pit->hw.channels[i].count);

xen/arch/x86/hvm/pmtimer.c +3 -3

diff -r e3b13e1ecf6c xen/arch/x86/hvm/pmtimer.c
@@ -71,7 +71,7 @@ static void pmt_update_time(PMTState *s)
     ASSERT(spin_is_locked(&s->lock));
 
     /* Update the timer */
-    curr_gtime = hvm_get_guest_time(s->vcpu);
+    curr_gtime = hvm_get_guest_time_mono(s->vcpu);
     s->pm.tmr_val += ((curr_gtime - s->last_gtime) * s->scale) >> 32;
     s->pm.tmr_val &= TMR_VAL_MASK;
     s->last_gtime = curr_gtime;

@@ -238,7 +238,7 @@ static int pmtimer_load(struct domain *d
     }
 
     /* Calculate future counter values from now. */
-    s->last_gtime = hvm_get_guest_time(s->vcpu);
+    s->last_gtime = hvm_get_guest_time_mono(s->vcpu);
 
     /* Set the SCI state from the registers */ 
     pmt_update_sci(s);

@@ -257,7 +257,7 @@ void pmtimer_init(struct vcpu *v)
 
     spin_lock_init(&s->lock);
 
-    s->scale = ((uint64_t)FREQUENCE_PMTIMER << 32) / ticks_per_sec(v);
+    s->scale = ((uint64_t)FREQUENCE_PMTIMER << 32) / s_ticks_per_sec(v);
     s->vcpu = v;
 
     /* Intercept port I/O (need two handlers because PM1a_CNT is between

xen/arch/x86/hvm/svm/svm.c +9 -2

diff -r e3b13e1ecf6c xen/arch/x86/hvm/svm/svm.c
@@ -299,7 +299,7 @@ static void svm_save_cpu_state(struct vc
     data->msr_efer         = v->arch.hvm_vcpu.guest_efer;
     data->msr_flags        = -1ULL;
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }

@@ -315,7 +315,7 @@ static void svm_load_cpu_state(struct vc
     v->arch.hvm_vcpu.guest_efer = data->msr_efer;
     svm_update_guest_efer(v);
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }
 
 static void svm_save_vmcb_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)

@@ -583,6 +583,12 @@ static void svm_set_tsc_offset(struct vc
     v->arch.hvm_svm.vmcb->tsc_offset = offset;
 }
 
+
+static void svm_set_time_offset(struct vcpu *v, u64 offset)
+{
+    v->arch.hvm_vcpu.cache_time_offset = offset;
+}
+
 static void svm_init_hypercall_page(struct domain *d, void *hypercall_page)
 {
     char *p;

@@ -790,6 +796,7 @@ static struct hvm_function_table svm_fun
     .update_guest_efer    = svm_update_guest_efer,
     .flush_guest_tlbs     = svm_flush_guest_tlbs,
     .set_tsc_offset       = svm_set_tsc_offset,
+    .set_time_offset      = svm_set_time_offset,
     .inject_exception     = svm_inject_exception,
     .init_hypercall_page  = svm_init_hypercall_page,
     .event_pending        = svm_event_pending,

xen/arch/x86/hvm/vlapic.c +2 -3

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vlapic.c
@@ -535,8 +535,7 @@ static uint32_t vlapic_get_tmcct(struct 
     uint32_t tmcct, tmict = vlapic_get_reg(vlapic, APIC_TMICT);
     uint64_t counter_passed;
 
-    counter_passed = ((hvm_get_guest_time(v) - vlapic->timer_last_update)
-                      * 1000000000ULL / ticks_per_sec(v)
+    counter_passed = ((hvm_get_guest_time_mono(v) - vlapic->timer_last_update)
                       / APIC_BUS_CYCLE_NS / vlapic->hw.timer_divisor);
     tmcct = tmict - counter_passed;

@@ -638,7 +637,7 @@ static int vlapic_read(
 
 void vlapic_pt_cb(struct vcpu *v, void *data)
 {
-    *(s_time_t *)data = hvm_get_guest_time(v);
+    *(s_time_t *)data = hvm_get_guest_time_mono(v);
 }
 
 static int vlapic_write(struct vcpu *v, unsigned long address,

xen/arch/x86/hvm/vmx/vmx.c +8 -2

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vmx/vmx.c
@@ -607,7 +607,7 @@ static void vmx_save_cpu_state(struct vc
     data->msr_syscall_mask = guest_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK];
 #endif
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }
 
 static void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)

@@ -625,7 +625,7 @@ static void vmx_load_cpu_state(struct vc
     v->arch.hvm_vmx.shadow_gs = data->shadow_gs;
 #endif
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }

@@ -866,6 +866,11 @@ static void vmx_set_tsc_offset(struct vc
     __vmwrite(TSC_OFFSET_HIGH, offset >> 32);
 #endif
     vmx_vmcs_exit(v);
+}
+
+static void vmx_set_time_offset(struct vcpu *v, u64 offset)
+{
+    v->arch.hvm_vcpu.cache_time_offset = offset;
 }
 
 void do_nmi(struct cpu_user_regs *);

@@ -1186,6 +1191,7 @@ static struct hvm_function_table vmx_fun
     .update_guest_efer    = vmx_update_guest_efer,
     .flush_guest_tlbs     = vmx_flush_guest_tlbs,
     .set_tsc_offset       = vmx_set_tsc_offset,
+    .set_time_offset       = vmx_set_time_offset,
     .inject_exception     = vmx_inject_exception,
     .init_hypercall_page  = vmx_init_hypercall_page,
     .event_pending        = vmx_event_pending,

xen/arch/x86/hvm/vpt.c +6 -6

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vpt.c
@@ -108,7 +108,7 @@ static void pt_freeze_time(struct vcpu *
     if ( !mode_is(v->domain, delay_for_missed_ticks) )
         return;
 
-    v->arch.hvm_vcpu.guest_time = hvm_get_guest_time(v);
+    v->arch.hvm_vcpu.guest_time = hvm_get_guest_time_mono(v);
 }
 
 static void pt_thaw_time(struct vcpu *v)

@@ -267,7 +267,7 @@ void pt_intr_post(struct vcpu *v, struct
         if ( mode_is(v->domain, one_missed_tick_pending) ||
              mode_is(v->domain, no_missed_ticks_pending) )
         {
-            pt->last_plt_gtime = hvm_get_guest_time(v);
+            pt->last_plt_gtime = hvm_get_guest_time_mono(v);
             pt->pending_intr_nr = 0; /* 'collapse' all missed ticks */
         }
         else

@@ -278,7 +278,7 @@ void pt_intr_post(struct vcpu *v, struct
     }
 
     if ( mode_is(v->domain, delay_for_missed_ticks) &&
-         (hvm_get_guest_time(v) < pt->last_plt_gtime) )
+         (hvm_get_guest_time_mono(v) < pt->last_plt_gtime) )
         hvm_set_guest_time(v, pt->last_plt_gtime);
 
     cb = pt->cb;

@@ -300,7 +300,7 @@ void pt_reset(struct vcpu *v)
     list_for_each_entry ( pt, head, list )
     {
         pt->pending_intr_nr = 0;
-        pt->last_plt_gtime = hvm_get_guest_time(pt->vcpu);
+        pt->last_plt_gtime = hvm_get_guest_time_mono(pt->vcpu);
         pt->scheduled = NOW() + pt->period;
         set_timer(&pt->timer, pt->scheduled);
     }

@@ -346,9 +346,9 @@ void create_periodic_time(
 
     pt->period = period;
     pt->vcpu = v;
-    pt->last_plt_gtime = hvm_get_guest_time(pt->vcpu);
+    pt->last_plt_gtime = hvm_get_guest_time_mono(pt->vcpu);
     pt->irq = irq;
-    pt->period_cycles = (u64)period * cpu_khz / 1000000L;
+    pt->period_cycles = (u64)period;
     pt->one_shot = one_shot;
     pt->scheduled = NOW() + period;
     /*

xen/arch/x86/time.c +26 -0

diff -r e3b13e1ecf6c xen/arch/x86/time.c
@@ -535,6 +535,8 @@ static s_time_t read_platform_stime(void
     return stime;
 }
 
+extern void get_s_time_mono_init(void);
+
 static void platform_time_calibration(void)
 {
     u64 count;

@@ -593,6 +595,7 @@ static void init_platform_timer(void)
     plt_overflow(NULL);
 
     platform_timer_stamp = plt_stamp64;
+    get_s_time_mono_init();
 
     printk("Platform timer is %s %s\n",
            freq_string(pts->frequency), pts->name);

@@ -728,6 +731,29 @@ s_time_t get_s_time(void)
     rdtscll(tsc);
     delta = tsc - t->local_tsc_stamp;
     now = t->stime_local_stamp + scale_delta(delta, &t->tsc_scale);
+
+    return now;
+}
+
+spinlock_t get_s_time_mono_lock;
+
+void get_s_time_mono_init(void)
+{
+    spin_lock_init(&get_s_time_mono_lock);
+}
+
+s_time_t get_s_time_mono(void)
+{
+    s_time_t now;
+    static s_time_t last = (s_time_t)0;
+
+    spin_lock(&get_s_time_mono_lock);
+    now = get_s_time();
+    if (now > last)
+        last = now;
+    else
+        now = last;
+    spin_unlock(&get_s_time_mono_lock);
 
     return now;
 }

xen/include/asm-x86/hvm/hvm.h +7 -0

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/hvm.h
@@ -107,6 +107,8 @@ struct hvm_function_table {
 
     void (*set_tsc_offset)(struct vcpu *v, u64 offset);
 
+    void (*set_time_offset)(struct vcpu *v, u64 offset);
+
     void (*inject_exception)(unsigned int trapnr, int errcode,
                              unsigned long cr2);

@@ -146,8 +148,13 @@ void hvm_send_assist_req(struct vcpu *v)
 
 void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc);
 u64 hvm_get_guest_tsc(struct vcpu *v);
+void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
+u64 hvm_get_guest_time(struct vcpu *v);
+u64 hvm_get_guest_time_mono(struct vcpu *v);
+/*
 #define hvm_set_guest_time(vcpu, gtime) hvm_set_guest_tsc(vcpu, gtime)
 #define hvm_get_guest_time(vcpu)        hvm_get_guest_tsc(vcpu)
+*/
 
 #define hvm_paging_enabled(v) \
     (!!((v)->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PG))

xen/include/asm-x86/hvm/vcpu.h +1 -0

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vcpu.h
@@ -50,6 +50,7 @@ struct hvm_vcpu {
     struct vlapic       vlapic;
     s64                 cache_tsc_offset;
     u64                 guest_time;
+    s64                 cache_time_offset;
 
     /* Lock and list for virtual platform timers. */
     spinlock_t          tm_lock;

xen/include/asm-x86/hvm/vmx/vmx.h +0 -1

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vmx/vmx.h
@@ -49,7 +49,6 @@ void vmx_asm_do_vmentry(void);
 void vmx_asm_do_vmentry(void);
 void vmx_intr_assist(void);
 void vmx_do_resume(struct vcpu *);
-void set_guest_time(struct vcpu *v, u64 gtime);
 void vmx_vlapic_msr_changed(struct vcpu *v);
 void vmx_realmode(struct cpu_user_regs *regs);

xen/include/asm-x86/hvm/vpt.h +1 -1

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vpt.h
@@ -57,7 +57,7 @@ typedef struct HPETState {
 typedef struct HPETState {
     struct hpet_registers hpet;
     struct vcpu *vcpu;
-    uint64_t tsc_freq;
+    uint64_t stime_freq;
     uint64_t hpet_to_ns_scale; /* hpet ticks to ns (multiplied by 2^10) */
     uint64_t hpet_to_ns_limit; /* max hpet ticks convertable to ns      */
     uint64_t mc_offset;

xen/include/asm-x86/time.h +2 -0

diff -r e3b13e1ecf6c xen/include/asm-x86/time.h
@@ -34,4 +34,6 @@ struct tm;
 struct tm;
 struct tm wallclock_time(void);
 
+#define s_ticks_per_sec(v)	1000000000ULL
+
 #endif /* __X86_TIME_H__ */

xen/include/xen/time.h +3 -0

diff -r e3b13e1ecf6c xen/include/xen/time.h
@@ -24,6 +24,8 @@ struct domain;
  * System Time
  * 64 bit value containing the nanoseconds elapsed since boot time.
  * This value is adjusted by frequency drift.
+ * mono version guarantees that consecutive reads are non-decreasing,
+ * (e.g. when first read is on a different processor than second read)
  * NOW() returns the current time.
  * The other macros are for convenience to approximate short intervals
  * of real time into system time

@@ -32,6 +34,7 @@ typedef s64 s_time_t;
 typedef s64 s_time_t;
 
 s_time_t get_s_time(void);
+s_time_t get_s_time_mono(void);
 unsigned long get_localtime(struct domain *d);
 
 struct tm {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2008-05-20 07:36 UTC #3 lore raw

In reply to Dan Magenheimer

Quoted 9 lines · Dan Magenheimer

Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:
rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */

Quoted 7 lines

Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.
Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.

I am inclined to say we should have a guest-TSC-on-system-time mode where *all* RDTSC executions get trapped. This would at least be useful as a baseline for tracking down guest time problems, and also provide a guaranteed stable TSC timesource for those who care about that more than pure performance.

*However* I would agree that, with TSC virtualisation as it currently is, there actually isn't really a way to build guest TSC on Xen system time without periodically warping the TSC back or forth. The guest TSC runs at the host TSC rate and that is that!

I think my original point was that at least we should not build all our other virtual time sources on this dodgy 'guest TSC'. :-)

-- Keir

Dan Magenheimer

author

2008-05-21 19:01 UTC #4 lore raw

OK, I think this version is ready for prime-time.

Keir, please check-in to unstable.

[PATCH] Build hvm guest platform timers on monotonic Xen system time

This patch moves hvm platform timers from underlying physical CPU TSC to Xen system time and ensures monotonicity. TSC on many systems may skew between processors, thus hvm platform timer reads were not monotonic which led to "Time going backwards" problems.

Signed-off-by Dan Magenheimer <[email protected]>

Quoted 45 lines

-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Tuesday, May 20, 2008 1:37 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Quoted 9 lines · Dan Magenheimer
Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:
rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */
Quoted 7 lines
Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.
Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.
I am inclined to say we should have a guest-TSC-on-system-time mode where
*all* RDTSC executions get trapped. This would at least be useful as a baseline for tracking down guest time problems, and also provide a guaranteed stable TSC timesource for those who care about that more than
pure performance.
*However* I would agree that, with TSC virtualisation as it currently is,
there actually isn't really a way to build guest TSC on Xen system time
without periodically warping the TSC back or forth. The guest TSC runs at
the host TSC rate and that is that!

Quoted 8 lines

I think my original point was that at least we should not build all our
other virtual time sources on this dodgy 'guest TSC'. :-)
-- Keir

xen/arch/x86/hvm/hpet.c +8 -8

diff -r e3b13e1ecf6c xen/arch/x86/hvm/hpet.c
@@ -29,9 +29,9 @@
 #define S_TO_NS  1000000000ULL           /* 1s  = 10^9  ns */
 #define S_TO_FS  1000000000000000ULL     /* 1s  = 10^15 fs */
 
-/* Frequency_of_TSC / frequency_of_HPET = 32 */
-#define TSC_PER_HPET_TICK 32
-#define guest_time_hpet(v) (hvm_get_guest_time(v) / TSC_PER_HPET_TICK)
+/* Frequency_of_Xen_systeme_time / frequency_of_HPET = 16 */
+#define STIME_PER_HPET_TICK 16
+#define guest_time_hpet(v) (hvm_get_guest_time_mono(v) / STIME_PER_HPET_TICK)
 
 #define HPET_ID         0x000
 #define HPET_PERIOD     0x004

@@ -192,7 +192,7 @@ static void hpet_stop_timer(HPETState *h
 
 /* the number of HPET tick that stands for
  * 1/(2^10) second, namely, 0.9765625 milliseconds */
-#define  HPET_TINY_TIME_SPAN  ((h->tsc_freq >> 10) / TSC_PER_HPET_TICK)
+#define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
 
 static void hpet_set_timer(HPETState *h, unsigned int tn)
 {

@@ -558,17 +558,17 @@ void hpet_init(struct vcpu *v)
     spin_lock_init(&h->lock);
 
     h->vcpu = v;
-    h->tsc_freq = ticks_per_sec(v);
+    h->stime_freq = S_TO_NS;
 
-    h->hpet_to_ns_scale = ((S_TO_NS * TSC_PER_HPET_TICK) << 10) / h->tsc_freq;
+    h->hpet_to_ns_scale = ((S_TO_NS * STIME_PER_HPET_TICK) << 10) / h->stime_freq;
     h->hpet_to_ns_limit = ~0ULL / h->hpet_to_ns_scale;
 
     /* 64-bit main counter; 3 timers supported; LegacyReplacementRoute. */
     h->hpet.capability = 0x8086A201ULL;
 
     /* This is the number of femptoseconds per HPET tick. */
-    /* Here we define HPET's frequency to be 1/32 of the TSC's */
-    h->hpet.capability |= ((S_TO_FS*TSC_PER_HPET_TICK/h->tsc_freq) << 32);
+    /* Here we define HPET's frequency to be 1/16 of Xen system time */
+    h->hpet.capability |= ((S_TO_FS*STIME_PER_HPET_TICK/h->stime_freq) << 32);
 
     for ( i = 0; i < HPET_TIMER_NUM; i++ )
     {

xen/arch/x86/hvm/hvm.c +48 -3

81 lines not on this page · raw message

xen/arch/x86/hvm/i8254.c +11 ~~-10~~

diff -r e3b13e1ecf6c xen/arch/x86/hvm/i8254.c
@@ -31,6 +31,7 @@
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <xen/sched.h>
+#include <asm/time.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/io.h>
 #include <asm/hvm/support.h>

@@ -86,8 +87,8 @@ static int pit_get_count(PITState *pit, 
 
     ASSERT(spin_is_locked(&pit->lock));
 
-    d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel],
-                 PIT_FREQ, ticks_per_sec(v));
+    d = muldiv64(hvm_get_guest_time_mono(v) - pit->count_load_time[channel],
+                 PIT_FREQ, s_ticks_per_sec(v));
 
     switch ( c->mode )
     {

@@ -117,8 +118,8 @@ static int pit_get_out(PITState *pit, in
 
     ASSERT(spin_is_locked(&pit->lock));
 
-    d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel], 
-                 PIT_FREQ, ticks_per_sec(v));
+    d = muldiv64(hvm_get_guest_time_mono(v) - pit->count_load_time[channel], 
+                 PIT_FREQ, s_ticks_per_sec(v));
 
     switch ( s->mode )
     {

@@ -164,7 +165,7 @@ static void pit_set_gate(PITState *pit, 
     case 3:
         /* Restart counting on rising edge. */
         if ( s->gate < val )
-            pit->count_load_time[channel] = hvm_get_guest_time(v);
+            pit->count_load_time[channel] = hvm_get_guest_time_mono(v);
         break;
     }

@@ -180,7 +181,7 @@ static void pit_time_fired(struct vcpu *
 static void pit_time_fired(struct vcpu *v, void *priv)
 {
     uint64_t *count_load_time = priv;
-    *count_load_time = hvm_get_guest_time(v);
+    *count_load_time = hvm_get_guest_time_mono(v);
 }
 
 static void pit_load_count(PITState *pit, int channel, int val)

@@ -195,11 +196,11 @@ static void pit_load_count(PITState *pit
         val = 0x10000;
 
     if ( v == NULL )
-        rdtscll(pit->count_load_time[channel]);
+        pit->count_load_time[channel] = 0;
     else
-        pit->count_load_time[channel] = hvm_get_guest_time(v);
+        pit->count_load_time[channel] = hvm_get_guest_time_mono(v);
     s->count = val;
-    period = DIV_ROUND((val * 1000000000ULL), PIT_FREQ);
+    period = DIV_ROUND((val * s_ticks_per_sec(v)), PIT_FREQ);
 
     if ( (v == NULL) || !is_hvm_vcpu(v) || (channel != 0) )
         return;

@@ -435,7 +436,7 @@ static int pit_load(struct domain *d, hv
      * time jitter here, but the wall-clock will have jumped massively, so 
      * we hope the guest can handle it.
      */
-    pit->pt0.last_plt_gtime = hvm_get_guest_time(d->vcpu[0]);
+    pit->pt0.last_plt_gtime = hvm_get_guest_time_mono(d->vcpu[0]);
     for ( i = 0; i < 3; i++ )
         pit_load_count(pit, i, pit->hw.channels[i].count);

xen/arch/x86/hvm/pmtimer.c +3 -3

diff -r e3b13e1ecf6c xen/arch/x86/hvm/pmtimer.c
@@ -71,7 +71,7 @@ static void pmt_update_time(PMTState *s)
     ASSERT(spin_is_locked(&s->lock));
 
     /* Update the timer */
-    curr_gtime = hvm_get_guest_time(s->vcpu);
+    curr_gtime = hvm_get_guest_time_mono(s->vcpu);
     s->pm.tmr_val += ((curr_gtime - s->last_gtime) * s->scale) >> 32;
     s->pm.tmr_val &= TMR_VAL_MASK;
     s->last_gtime = curr_gtime;

@@ -238,7 +238,7 @@ static int pmtimer_load(struct domain *d
     }
 
     /* Calculate future counter values from now. */
-    s->last_gtime = hvm_get_guest_time(s->vcpu);
+    s->last_gtime = hvm_get_guest_time_mono(s->vcpu);
 
     /* Set the SCI state from the registers */ 
     pmt_update_sci(s);

@@ -257,7 +257,7 @@ void pmtimer_init(struct vcpu *v)
 
     spin_lock_init(&s->lock);
 
-    s->scale = ((uint64_t)FREQUENCE_PMTIMER << 32) / ticks_per_sec(v);
+    s->scale = ((uint64_t)FREQUENCE_PMTIMER << 32) / s_ticks_per_sec(v);
     s->vcpu = v;
 
     /* Intercept port I/O (need two handlers because PM1a_CNT is between

xen/arch/x86/hvm/svm/svm.c +2 -2

diff -r e3b13e1ecf6c xen/arch/x86/hvm/svm/svm.c
@@ -299,7 +299,7 @@ static void svm_save_cpu_state(struct vc
     data->msr_efer         = v->arch.hvm_vcpu.guest_efer;
     data->msr_flags        = -1ULL;
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }

@@ -315,7 +315,7 @@ static void svm_load_cpu_state(struct vc
     v->arch.hvm_vcpu.guest_efer = data->msr_efer;
     svm_update_guest_efer(v);
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }
 
 static void svm_save_vmcb_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)

xen/arch/x86/hvm/vlapic.c +2 -3

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vlapic.c
@@ -535,8 +535,7 @@ static uint32_t vlapic_get_tmcct(struct 
     uint32_t tmcct, tmict = vlapic_get_reg(vlapic, APIC_TMICT);
     uint64_t counter_passed;
 
-    counter_passed = ((hvm_get_guest_time(v) - vlapic->timer_last_update)
-                      * 1000000000ULL / ticks_per_sec(v)
+    counter_passed = ((hvm_get_guest_time_mono(v) - vlapic->timer_last_update)
                       / APIC_BUS_CYCLE_NS / vlapic->hw.timer_divisor);
     tmcct = tmict - counter_passed;

@@ -638,7 +637,7 @@ static int vlapic_read(
 
 void vlapic_pt_cb(struct vcpu *v, void *data)
 {
-    *(s_time_t *)data = hvm_get_guest_time(v);
+    *(s_time_t *)data = hvm_get_guest_time_mono(v);
 }
 
 static int vlapic_write(struct vcpu *v, unsigned long address,

xen/arch/x86/hvm/vmx/vmx.c +2 -2

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vmx/vmx.c
@@ -607,7 +607,7 @@ static void vmx_save_cpu_state(struct vc
     data->msr_syscall_mask = guest_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK];
 #endif
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }
 
 static void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)

@@ -625,7 +625,7 @@ static void vmx_load_cpu_state(struct vc
     v->arch.hvm_vmx.shadow_gs = data->shadow_gs;
 #endif
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }

xen/arch/x86/hvm/vpt.c +6 -6

diff -r e3b13e1ecf6c xen/arch/x86/hvm/vpt.c
@@ -108,7 +108,7 @@ static void pt_freeze_time(struct vcpu *
     if ( !mode_is(v->domain, delay_for_missed_ticks) )
         return;
 
-    v->arch.hvm_vcpu.guest_time = hvm_get_guest_time(v);
+    v->arch.hvm_vcpu.guest_time = hvm_get_guest_time_mono(v);
 }
 
 static void pt_thaw_time(struct vcpu *v)

@@ -267,7 +267,7 @@ void pt_intr_post(struct vcpu *v, struct
         if ( mode_is(v->domain, one_missed_tick_pending) ||
              mode_is(v->domain, no_missed_ticks_pending) )
         {
-            pt->last_plt_gtime = hvm_get_guest_time(v);
+            pt->last_plt_gtime = hvm_get_guest_time_mono(v);
             pt->pending_intr_nr = 0; /* 'collapse' all missed ticks */
         }
         else

@@ -278,7 +278,7 @@ void pt_intr_post(struct vcpu *v, struct
     }
 
     if ( mode_is(v->domain, delay_for_missed_ticks) &&
-         (hvm_get_guest_time(v) < pt->last_plt_gtime) )
+         (hvm_get_guest_time_mono(v) < pt->last_plt_gtime) )
         hvm_set_guest_time(v, pt->last_plt_gtime);
 
     cb = pt->cb;

@@ -300,7 +300,7 @@ void pt_reset(struct vcpu *v)
     list_for_each_entry ( pt, head, list )
     {
         pt->pending_intr_nr = 0;
-        pt->last_plt_gtime = hvm_get_guest_time(pt->vcpu);
+        pt->last_plt_gtime = hvm_get_guest_time_mono(pt->vcpu);
         pt->scheduled = NOW() + pt->period;
         set_timer(&pt->timer, pt->scheduled);
     }

@@ -346,9 +346,9 @@ void create_periodic_time(
 
     pt->period = period;
     pt->vcpu = v;
-    pt->last_plt_gtime = hvm_get_guest_time(pt->vcpu);
+    pt->last_plt_gtime = hvm_get_guest_time_mono(pt->vcpu);
     pt->irq = irq;
-    pt->period_cycles = (u64)period * cpu_khz / 1000000L;
+    pt->period_cycles = (u64)period;
     pt->one_shot = one_shot;
     pt->scheduled = NOW() + period;
     /*

xen/arch/x86/time.c +26 -0

diff -r e3b13e1ecf6c xen/arch/x86/time.c
@@ -535,6 +535,8 @@ static s_time_t read_platform_stime(void
     return stime;
 }
 
+extern void get_s_time_mono_init(void);
+
 static void platform_time_calibration(void)
 {
     u64 count;

@@ -593,6 +595,7 @@ static void init_platform_timer(void)
     plt_overflow(NULL);
 
     platform_timer_stamp = plt_stamp64;
+    get_s_time_mono_init();
 
     printk("Platform timer is %s %s\n",
            freq_string(pts->frequency), pts->name);

@@ -728,6 +731,29 @@ s_time_t get_s_time(void)
     rdtscll(tsc);
     delta = tsc - t->local_tsc_stamp;
     now = t->stime_local_stamp + scale_delta(delta, &t->tsc_scale);
+
+    return now;
+}
+
+spinlock_t get_s_time_mono_lock;
+
+void get_s_time_mono_init(void)
+{
+    spin_lock_init(&get_s_time_mono_lock);
+}
+
+s_time_t get_s_time_mono(void)
+{
+    s_time_t now;
+    static s_time_t last = (s_time_t)0;
+
+    spin_lock(&get_s_time_mono_lock);
+    now = get_s_time();
+    if (now > last)
+        last = now;
+    else
+        now = last;
+    spin_unlock(&get_s_time_mono_lock);
 
     return now;
 }

xen/include/asm-x86/hvm/hvm.h +3 -2

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/hvm.h
@@ -146,8 +146,9 @@ void hvm_send_assist_req(struct vcpu *v)
 
 void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc);
 u64 hvm_get_guest_tsc(struct vcpu *v);
-#define hvm_set_guest_time(vcpu, gtime) hvm_set_guest_tsc(vcpu, gtime)
-#define hvm_get_guest_time(vcpu)        hvm_get_guest_tsc(vcpu)
+void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
+u64 hvm_get_guest_time(struct vcpu *v);
+u64 hvm_get_guest_time_mono(struct vcpu *v);
 
 #define hvm_paging_enabled(v) \
     (!!((v)->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PG))

xen/include/asm-x86/hvm/vmx/vmx.h +0 -1

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vmx/vmx.h
@@ -49,7 +49,6 @@ void vmx_asm_do_vmentry(void);
 void vmx_asm_do_vmentry(void);
 void vmx_intr_assist(void);
 void vmx_do_resume(struct vcpu *);
-void set_guest_time(struct vcpu *v, u64 gtime);
 void vmx_vlapic_msr_changed(struct vcpu *v);
 void vmx_realmode(struct cpu_user_regs *regs);

xen/include/asm-x86/hvm/vpt.h +4 -1

diff -r e3b13e1ecf6c xen/include/asm-x86/hvm/vpt.h
@@ -57,7 +57,7 @@ typedef struct HPETState {
 typedef struct HPETState {
     struct hpet_registers hpet;
     struct vcpu *vcpu;
-    uint64_t tsc_freq;
+    uint64_t stime_freq;
     uint64_t hpet_to_ns_scale; /* hpet ticks to ns (multiplied by 2^10) */
     uint64_t hpet_to_ns_limit; /* max hpet ticks convertable to ns      */
     uint64_t mc_offset;

@@ -137,6 +137,9 @@ struct pl_time {    /* platform time */
     struct RTCState  vrtc;
     struct HPETState vhpet;
     struct PMTState  vpmt;
+    int64_t stime_offset;       /* guest_time = Xen sys time + stime_offset */
+    uint64_t last_guest_time;   /* ensure monotonicity */
+    spinlock_t pl_time_lock;
 };
 
 #define ticks_per_sec(v) (v->domain->arch.hvm_domain.tsc_frequency)

xen/include/asm-x86/time.h +2 -0

diff -r e3b13e1ecf6c xen/include/asm-x86/time.h
@@ -34,4 +34,6 @@ struct tm;
 struct tm;
 struct tm wallclock_time(void);
 
+#define s_ticks_per_sec(v)	1000000000ULL
+
 #endif /* __X86_TIME_H__ */

xen/include/xen/time.h +3 -0

diff -r e3b13e1ecf6c xen/include/xen/time.h
@@ -24,6 +24,8 @@ struct domain;
  * System Time
  * 64 bit value containing the nanoseconds elapsed since boot time.
  * This value is adjusted by frequency drift.
+ * mono version guarantees that consecutive reads are non-decreasing,
+ * (e.g. when first read is on a different processor than second read)
  * NOW() returns the current time.
  * The other macros are for convenience to approximate short intervals
  * of real time into system time

@@ -32,6 +34,7 @@ typedef s64 s_time_t;
 typedef s64 s_time_t;
 
 s_time_t get_s_time(void);
+s_time_t get_s_time_mono(void);
 unsigned long get_localtime(struct domain *d);
 
 struct tm {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2008-05-22 08:46 UTC #5 lore raw

Attached is a modified version (really just renames). However the new hvm_set_guest_time() interface doesn't work for vpt.c timer modes which allow progress of time to temporarily deviate across VCPUs. I guess you don't use those modes so might not notice this issue. I think hvm guest time handling may need to be integrated into vpt.c so that correct handling can be applied for the various different types of timer mode. Having a blanket per-domain stime offset and enforcing monotonicity regardless of timer mode doesn't fly.

-- Keir

Quoted 4 lines · Dan Magenheimer

OK, I think this version is ready for prime-time.
Keir, please check-in to unstable.

Quoted 62 lines

[PATCH] Build hvm guest platform timers on monotonic Xen system time
This patch moves hvm platform timers from underlying physical CPU TSC to Xen system time and ensures monotonicity. TSC on many systems may skew between processors, thus hvm platform timer reads were not monotonic which led to "Time going backwards" problems.
Signed-off-by Dan Magenheimer <[email protected]>
Quoted 53 lines
-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Tuesday, May 20, 2008 1:37 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Quoted 9 lines · Dan Magenheimer
Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:
rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */
Quoted 7 lines
Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.
Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.
I am inclined to say we should have a guest-TSC-on-system-time mode where
*all* RDTSC executions get trapped. This would at least be useful as a baseline for tracking down guest time problems, and also provide a guaranteed stable TSC timesource for those who care about that more than
pure performance.
*However* I would agree that, with TSC virtualisation as it currently is,
there actually isn't really a way to build guest TSC on Xen system time
without periodically warping the TSC back or forth. The guest TSC runs at
the host TSC rate and that is that!
I think my original point was that at least we should not build all our
other virtual time sources on this dodgy 'guest TSC'. :-)
-- Keir

xen/arch/x86/hvm/hpet.c +9 -9

diff -r e64c3a8c60e1 xen/arch/x86/hvm/hpet.c
@@ -29,9 +29,9 @@
 #define S_TO_NS  1000000000ULL           /* 1s  = 10^9  ns */
 #define S_TO_FS  1000000000000000ULL     /* 1s  = 10^15 fs */
 
-/* Frequency_of_TSC / frequency_of_HPET = 32 */
-#define TSC_PER_HPET_TICK 32
-#define guest_time_hpet(v) (hvm_get_guest_time(v) / TSC_PER_HPET_TICK)
+/* Frequency_of_Xen_systeme_time / frequency_of_HPET = 16 */
+#define STIME_PER_HPET_TICK 16
+#define guest_time_hpet(v) (hvm_get_guest_time(v) / STIME_PER_HPET_TICK)
 
 #define HPET_ID         0x000
 #define HPET_PERIOD     0x004

@@ -192,7 +192,7 @@ static void hpet_stop_timer(HPETState *h
 
 /* the number of HPET tick that stands for
  * 1/(2^10) second, namely, 0.9765625 milliseconds */
-#define  HPET_TINY_TIME_SPAN  ((h->tsc_freq >> 10) / TSC_PER_HPET_TICK)
+#define  HPET_TINY_TIME_SPAN  ((h->stime_freq >> 10) / STIME_PER_HPET_TICK)
 
 static void hpet_set_timer(HPETState *h, unsigned int tn)
 {

@@ -558,17 +558,17 @@ void hpet_init(struct vcpu *v)
     spin_lock_init(&h->lock);
 
     h->vcpu = v;
-    h->tsc_freq = ticks_per_sec(v);
-
-    h->hpet_to_ns_scale = ((S_TO_NS * TSC_PER_HPET_TICK) << 10) / h->tsc_freq;
+    h->stime_freq = S_TO_NS;
+
+    h->hpet_to_ns_scale = ((S_TO_NS * STIME_PER_HPET_TICK) << 10) / h->stime_freq;
     h->hpet_to_ns_limit = ~0ULL / h->hpet_to_ns_scale;
 
     /* 64-bit main counter; 3 timers supported; LegacyReplacementRoute. */
     h->hpet.capability = 0x8086A201ULL;
 
     /* This is the number of femptoseconds per HPET tick. */
-    /* Here we define HPET's frequency to be 1/32 of the TSC's */
-    h->hpet.capability |= ((S_TO_FS*TSC_PER_HPET_TICK/h->tsc_freq) << 32);
+    /* Here we define HPET's frequency to be 1/16 of Xen system time */
+    h->hpet.capability |= ((S_TO_FS*STIME_PER_HPET_TICK/h->stime_freq) << 32);
 
     for ( i = 0; i < HPET_TIMER_NUM; i++ )
     {

xen/arch/x86/hvm/hvm.c +48 -3

81 lines not on this page · raw message

xen/arch/x86/hvm/i8254.c +5 -4

diff -r e64c3a8c60e1 xen/arch/x86/hvm/i8254.c
@@ -31,6 +31,7 @@
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <xen/sched.h>
+#include <asm/time.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/io.h>
 #include <asm/hvm/support.h>

@@ -87,7 +88,7 @@ static int pit_get_count(PITState *pit, 
     ASSERT(spin_is_locked(&pit->lock));
 
     d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel],
-                 PIT_FREQ, ticks_per_sec(v));
+                 PIT_FREQ, SYSTEM_TIME_HZ);
 
     switch ( c->mode )
     {

@@ -118,7 +119,7 @@ static int pit_get_out(PITState *pit, in
     ASSERT(spin_is_locked(&pit->lock));
 
     d = muldiv64(hvm_get_guest_time(v) - pit->count_load_time[channel], 
-                 PIT_FREQ, ticks_per_sec(v));
+                 PIT_FREQ, SYSTEM_TIME_HZ);
 
     switch ( s->mode )
     {

@@ -195,11 +196,11 @@ static void pit_load_count(PITState *pit
         val = 0x10000;
 
     if ( v == NULL )
-        rdtscll(pit->count_load_time[channel]);
+        pit->count_load_time[channel] = 0;
     else
         pit->count_load_time[channel] = hvm_get_guest_time(v);
     s->count = val;
-    period = DIV_ROUND((val * 1000000000ULL), PIT_FREQ);
+    period = DIV_ROUND(val * SYSTEM_TIME_HZ, PIT_FREQ);
 
     if ( (v == NULL) || !is_hvm_vcpu(v) || (channel != 0) )
         return;

xen/arch/x86/hvm/pmtimer.c +1 -1

diff -r e64c3a8c60e1 xen/arch/x86/hvm/pmtimer.c
@@ -257,7 +257,7 @@ void pmtimer_init(struct vcpu *v)
 
     spin_lock_init(&s->lock);
 
-    s->scale = ((uint64_t)FREQUENCE_PMTIMER << 32) / ticks_per_sec(v);
+    s->scale = ((uint64_t)FREQUENCE_PMTIMER << 32) / SYSTEM_TIME_HZ;
     s->vcpu = v;
 
     /* Intercept port I/O (need two handlers because PM1a_CNT is between

xen/arch/x86/hvm/svm/svm.c +2 -2

diff -r e64c3a8c60e1 xen/arch/x86/hvm/svm/svm.c
@@ -299,7 +299,7 @@ static void svm_save_cpu_state(struct vc
     data->msr_efer         = v->arch.hvm_vcpu.guest_efer;
     data->msr_flags        = -1ULL;
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }

@@ -315,7 +315,7 @@ static void svm_load_cpu_state(struct vc
     v->arch.hvm_vcpu.guest_efer = data->msr_efer;
     svm_update_guest_efer(v);
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }
 
 static void svm_save_vmcb_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)

xen/arch/x86/hvm/vlapic.c +0 -1

diff -r e64c3a8c60e1 xen/arch/x86/hvm/vlapic.c
@@ -474,7 +474,6 @@ static uint32_t vlapic_get_tmcct(struct 
     uint64_t counter_passed;
 
     counter_passed = ((hvm_get_guest_time(v) - vlapic->timer_last_update)
-                      * 1000000000ULL / ticks_per_sec(v)
                       / APIC_BUS_CYCLE_NS / vlapic->hw.timer_divisor);
     tmcct = tmict - counter_passed;

xen/arch/x86/hvm/vmx/vmx.c +2 -2

diff -r e64c3a8c60e1 xen/arch/x86/hvm/vmx/vmx.c
@@ -607,7 +607,7 @@ static void vmx_save_cpu_state(struct vc
     data->msr_syscall_mask = guest_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK];
 #endif
 
-    data->tsc = hvm_get_guest_time(v);
+    data->tsc = hvm_get_guest_tsc(v);
 }
 
 static void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)

@@ -625,7 +625,7 @@ static void vmx_load_cpu_state(struct vc
     v->arch.hvm_vmx.shadow_gs = data->shadow_gs;
 #endif
 
-    hvm_set_guest_time(v, data->tsc);
+    hvm_set_guest_tsc(v, data->tsc);
 }

xen/arch/x86/hvm/vpt.c +1 -1

diff -r e64c3a8c60e1 xen/arch/x86/hvm/vpt.c
@@ -348,7 +348,7 @@ void create_periodic_time(
     pt->vcpu = v;
     pt->last_plt_gtime = hvm_get_guest_time(pt->vcpu);
     pt->irq = irq;
-    pt->period_cycles = (u64)period * cpu_khz / 1000000L;
+    pt->period_cycles = (u64)period;
     pt->one_shot = one_shot;
     pt->scheduled = NOW() + period;
     /*

xen/arch/x86/time.c +26 -0

diff -r e64c3a8c60e1 xen/arch/x86/time.c
@@ -501,6 +501,8 @@ static s_time_t read_platform_stime(void
     return stime;
 }
 
+extern void get_s_time_mono_init(void);
+
 static void platform_time_calibration(void)
 {
     u64 count;

@@ -559,6 +561,7 @@ static void init_platform_timer(void)
     plt_overflow(NULL);
 
     platform_timer_stamp = plt_stamp64;
+    get_s_time_mono_init();
 
     printk("Platform timer is %s %s\n",
            freq_string(pts->frequency), pts->name);

@@ -694,6 +697,29 @@ s_time_t get_s_time(void)
     rdtscll(tsc);
     delta = tsc - t->local_tsc_stamp;
     now = t->stime_local_stamp + scale_delta(delta, &t->tsc_scale);
+
+    return now;
+}
+
+spinlock_t get_s_time_mono_lock;
+
+void get_s_time_mono_init(void)
+{
+    spin_lock_init(&get_s_time_mono_lock);
+}
+
+s_time_t get_s_time_mono(void)
+{
+    s_time_t now;
+    static s_time_t last = (s_time_t)0;
+
+    spin_lock(&get_s_time_mono_lock);
+    now = get_s_time();
+    if (now > last)
+        last = now;
+    else
+        now = last;
+    spin_unlock(&get_s_time_mono_lock);
 
     return now;
 }

xen/include/asm-x86/hvm/hvm.h +2 -2

diff -r e64c3a8c60e1 xen/include/asm-x86/hvm/hvm.h
@@ -147,8 +147,8 @@ void hvm_send_assist_req(struct vcpu *v)
 
 void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc);
 u64 hvm_get_guest_tsc(struct vcpu *v);
-#define hvm_set_guest_time(vcpu, gtime) hvm_set_guest_tsc(vcpu, gtime)
-#define hvm_get_guest_time(vcpu)        hvm_get_guest_tsc(vcpu)
+void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
+u64 hvm_get_guest_time(struct vcpu *v);
 
 #define hvm_paging_enabled(v) \
     (!!((v)->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PG))

xen/include/asm-x86/hvm/vmx/vmx.h +0 -1

diff -r e64c3a8c60e1 xen/include/asm-x86/hvm/vmx/vmx.h
@@ -49,7 +49,6 @@ void vmx_asm_do_vmentry(void);
 void vmx_asm_do_vmentry(void);
 void vmx_intr_assist(void);
 void vmx_do_resume(struct vcpu *);
-void set_guest_time(struct vcpu *v, u64 gtime);
 void vmx_vlapic_msr_changed(struct vcpu *v);
 void vmx_realmode(struct cpu_user_regs *regs);

xen/include/asm-x86/hvm/vpt.h +4 -1

diff -r e64c3a8c60e1 xen/include/asm-x86/hvm/vpt.h
@@ -57,7 +57,7 @@ typedef struct HPETState {
 typedef struct HPETState {
     struct hpet_registers hpet;
     struct vcpu *vcpu;
-    uint64_t tsc_freq;
+    uint64_t stime_freq;
     uint64_t hpet_to_ns_scale; /* hpet ticks to ns (multiplied by 2^10) */
     uint64_t hpet_to_ns_limit; /* max hpet ticks convertable to ns      */
     uint64_t mc_offset;

@@ -137,6 +137,9 @@ struct pl_time {    /* platform time */
     struct RTCState  vrtc;
     struct HPETState vhpet;
     struct PMTState  vpmt;
+    int64_t stime_offset;       /* guest_time = Xen sys time + stime_offset */
+    uint64_t last_guest_time;   /* ensure monotonicity */
+    spinlock_t pl_time_lock;
 };
 
 #define ticks_per_sec(v) (v->domain->arch.hvm_domain.tsc_frequency)

xen/include/xen/time.h +4 -0

diff -r e64c3a8c60e1 xen/include/xen/time.h
@@ -24,6 +24,8 @@ struct domain;
  * System Time
  * 64 bit value containing the nanoseconds elapsed since boot time.
  * This value is adjusted by frequency drift.
+ * mono version guarantees that consecutive reads are non-decreasing,
+ * (e.g. when first read is on a different processor than second read)
  * NOW() returns the current time.
  * The other macros are for convenience to approximate short intervals
  * of real time into system time

@@ -32,6 +34,7 @@ typedef s64 s_time_t;
 typedef s64 s_time_t;
 
 s_time_t get_s_time(void);
+s_time_t get_s_time_mono(void);
 unsigned long get_localtime(struct domain *d);
 
 struct tm {

@@ -47,6 +50,7 @@ struct tm {
 };
 struct tm gmtime(unsigned long t);
 
+#define SYSTEM_TIME_HZ  1000000000ULL
 #define NOW()           ((s_time_t)get_s_time())
 #define SECONDS(_s)     ((s_time_t)((_s)  * 1000000000ULL))
 #define MILLISECS(_ms)  ((s_time_t)((_ms) * 1000000ULL))

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Dan Magenheimer

author

2008-05-22 16:05 UTC #6 lore raw

Thanks for the review and the modifications!

It does allow time to temporarily deviate across VCPUs, but doesn't allow them to "apparently" deviate; that is a VCPU can never observe or set a time which is older than the time that another VCPU previously observed or set.

Unless you can guarantee this, there's no way to avoid "Time going backwards" errors, is there?

Admittedly, I don't fully understand the inner workings of all the timer modes, but if this doesn't fly for one or more of the modes, doesn't this mean it is impossible for those timer modes to guarantee monotonicity -- at least without also requiring gang scheduling?

Thanks,
Dan

P.S. I had left the naming as hvm_get_guest_time_mono() because I wasn't sure there would never be a version needed that was non-mono, so thought it would be better to be explicit in the name. (I was especially sensitized to this after having just completed teasing apart the hvm_get_guest_tsc() from hvm_get_guest_time() ;-)

Quoted 102 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]]On Behalf Of Keir Fraser
Sent: Thursday, May 22, 2008 2:46 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Attached is a modified version (really just renames). However the new hvm_set_guest_time() interface doesn't work for vpt.c timer modes which
allow progress of time to temporarily deviate across VCPUs. I guess you
don't use those modes so might not notice this issue. I think hvm guest time
handling may need to be integrated into vpt.c so that correct handling can
be applied for the various different types of timer mode. Having a blanket
per-domain stime offset and enforcing monotonicity regardless of timer mode
doesn't fly.
-- Keir
Quoted 7 lines · Dan Magenheimer
OK, I think this version is ready for prime-time.
Keir, please check-in to unstable.
[PATCH] Build hvm guest platform timers on monotonic Xen system time
This patch moves hvm platform timers from underlying
physical CPU TSC
Quoted 1 line
to Xen system time and ensures monotonicity. TSC on many
systems may
Quoted 37 lines
skew between processors, thus hvm platform timer reads were not monotonic which led to "Time going backwards" problems.
Signed-off-by Dan Magenheimer <[email protected]>
Quoted 32 lines
-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Tuesday, May 20, 2008 1:37 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Quoted 9 lines · Dan Magenheimer
Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:
rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */
Quoted 7 lines
Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.
Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.
I am inclined to say we should have a guest-TSC-on-system-time mode where
*all* RDTSC executions get trapped. This would at least be
useful as a
Quoted 21 lines
Quoted 21 lines
baseline for tracking down guest time problems, and also provide a guaranteed stable TSC timesource for those who care about that more than
pure performance.
*However* I would agree that, with TSC virtualisation as it currently is,
there actually isn't really a way to build guest TSC on Xen system time
without periodically warping the TSC back or forth. The guest TSC runs at
the host TSC rate and that is that!
I think my original point was that at least we should not build all our
other virtual time sources on this dodgy 'guest TSC'. :-)
-- Keir

Keir Fraser

2008-05-22 16:11 UTC #7 lore raw

Well, really it's baked into the design of some of the timer modes that time can differ across VCPUs: it's their reason for existence. We could say we don't support them any more -- I'm not sure anyone enables them for productised releases of Xen -- but we should at least do that explicitly rather than subtly breaking them.

What I'll do is append doing some more work to your patch to my work queue. I would say I'm moderately confident that your patch actually will work fine for the timer modes that you guys are using in your packaging of Xen, but I think some more architectural cleanup in this area would be good for xen-unstable. Whether that is remove some of the older timer modes now, or clean up hvm_[sg]et_guest_time() to work nicely with those modes, is open to debate.

-- Keir

Quoted 129 lines · Dan Magenheimer

Thanks for the review and the modifications!
It does allow time to temporarily deviate across VCPUs, but doesn't allow them to "apparently" deviate; that is a VCPU can never observe or set a time which is older than the time that another VCPU previously observed or set.
Unless you can guarantee this, there's no way to avoid "Time going backwards" errors, is there?
Admittedly, I don't fully understand the inner workings of all the timer modes, but if this doesn't fly for one or more of the modes, doesn't this mean it is impossible for those timer modes to guarantee monotonicity -- at least without also requiring gang scheduling?
Thanks,
Dan
P.S. I had left the naming as hvm_get_guest_time_mono() because I wasn't sure there would never be a version needed that was non-mono, so thought it would be better to be explicit in the name. (I was especially sensitized to this after having just completed teasing apart the hvm_get_guest_tsc() from hvm_get_guest_time() ;-)
Quoted 102 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]]On Behalf Of Keir Fraser
Sent: Thursday, May 22, 2008 2:46 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Attached is a modified version (really just renames). However the new hvm_set_guest_time() interface doesn't work for vpt.c timer modes which
allow progress of time to temporarily deviate across VCPUs. I guess you
don't use those modes so might not notice this issue. I think hvm guest time
handling may need to be integrated into vpt.c so that correct handling can
be applied for the various different types of timer mode. Having a blanket
per-domain stime offset and enforcing monotonicity regardless of timer mode
doesn't fly.
-- Keir
Quoted 7 lines · Dan Magenheimer
OK, I think this version is ready for prime-time.
Keir, please check-in to unstable.
[PATCH] Build hvm guest platform timers on monotonic Xen system time
This patch moves hvm platform timers from underlying
physical CPU TSC
Quoted 1 line
to Xen system time and ensures monotonicity. TSC on many
systems may
Quoted 37 lines
skew between processors, thus hvm platform timer reads were not monotonic which led to "Time going backwards" problems.
Signed-off-by Dan Magenheimer <[email protected]>
Quoted 32 lines
-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Tuesday, May 20, 2008 1:37 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Quoted 9 lines · Dan Magenheimer
Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:
rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */
Quoted 7 lines
Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.
Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.
I am inclined to say we should have a guest-TSC-on-system-time mode where
*all* RDTSC executions get trapped. This would at least be
useful as a
Quoted 21 lines
Quoted 21 lines
baseline for tracking down guest time problems, and also provide a guaranteed stable TSC timesource for those who care about that more than
pure performance.
*However* I would agree that, with TSC virtualisation as it currently is,
there actually isn't really a way to build guest TSC on Xen system time
without periodically warping the TSC back or forth. The guest TSC runs at
the host TSC rate and that is that!
I think my original point was that at least we should not build all our
other virtual time sources on this dodgy 'guest TSC'. :-)
-- Keir

Dan Magenheimer

author

2008-05-23 22:44 UTC #8 lore raw

Quoted 1 line

Whether that is remove some of the older timer modes now,

Looking at the various timer modes, it appears that:

0==delay_for_missed_ticks is required for non-resilient Linuxes
(e.g. 32-bit RHEL4-based systems)
1==no_delay_for_missed_ticks is required for Windows(??) 2==no_missed_ticks_pending is required for resilient Linuxes
(e.g. 64-bit RHEL5-based systems)
3==one_missed_tick_pending DON'T KNOW WHAT IS USED FOR

So timer_mode=3=one_missed_tick_pending might possibly be retired, but the others couldn't.

Here's another option: I think delay_for_missed_ticks is the only mode where time must be set backwards. All of the other modes track wallclock. It looks possible that we could enforce monotonicity for the other three modes and not enforce it for delay_for_missed_ticks (which is, unfortunately, the default).

If this is acceptable to you (and you don't disagree with my "only mode...set backwards" conjecture), I'll try to code a patch for this.

Thanks,
Dan

P.S. I think the answer is no, but is there any way to specify gang-scheduling for a domain? If so, we could say if your OS is non-resilient to preemption and your application requires time monotonicity, you must specify gang scheduling.

Quoted 173 lines

-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Thursday, May 22, 2008 10:12 AM
To: [email protected]; Xen-Devel (E-mail)
Cc: Dave Winchell
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Well, really it's baked into the design of some of the timer modes that time
can differ across VCPUs: it's their reason for existence. We could say we
don't support them any more -- I'm not sure anyone enables them for productised releases of Xen -- but we should at least do that explicitly
rather than subtly breaking them.
What I'll do is append doing some more work to your patch to my work queue.
I would say I'm moderately confident that your patch actually will work fine
for the timer modes that you guys are using in your packaging of Xen, but I
think some more architectural cleanup in this area would be good for xen-unstable. Whether that is remove some of the older timer modes now, or
clean up hvm_[sg]et_guest_time() to work nicely with those modes, is open to
debate.
-- Keir
Quoted 19 lines · Dan Magenheimer
Thanks for the review and the modifications!
It does allow time to temporarily deviate across VCPUs, but doesn't allow them to "apparently" deviate; that is a VCPU can never observe or set a time which is older than the time that another VCPU previously observed or set.
Unless you can guarantee this, there's no way to avoid "Time going backwards" errors, is there?
Admittedly, I don't fully understand the inner workings of all the timer modes, but if this doesn't fly for one or more of the modes, doesn't this mean it is impossible for those timer modes to guarantee monotonicity -- at least without also requiring gang scheduling?
Thanks,
Dan
Quoted 10 lines
P.S. I had left the naming as hvm_get_guest_time_mono() because I wasn't sure there would never be a version needed that was non-mono, so thought it would be better to be explicit in the name. (I was especially sensitized to this after having just completed teasing apart the hvm_get_guest_tsc() from hvm_get_guest_time() ;-)
Quoted 3 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]]On Behalf Of
Keir Fraser
Quoted 8 lines
Quoted 8 lines
Sent: Thursday, May 22, 2008 2:46 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Attached is a modified version (really just renames).
However the new
Quoted 16 lines
Quoted 16 lines
hvm_set_guest_time() interface doesn't work for vpt.c timer modes which
allow progress of time to temporarily deviate across VCPUs. I guess you
don't use those modes so might not notice this issue. I think hvm guest time
handling may need to be integrated into vpt.c so that correct handling can
be applied for the various different types of timer mode. Having a blanket
per-domain stime offset and enforcing monotonicity regardless of timer mode
doesn't fly.
-- Keir
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines · Dan Magenheimer
OK, I think this version is ready for prime-time.
Keir, please check-in to unstable.
[PATCH] Build hvm guest platform timers on monotonic Xen
system time
Quoted 36 lines
Quoted 36 lines
Quoted 2 lines
This patch moves hvm platform timers from underlying
physical CPU TSC
Quoted 1 line
to Xen system time and ensures monotonicity. TSC on many
systems may
Quoted 31 lines
skew between processors, thus hvm platform timer reads were not monotonic which led to "Time going backwards" problems.
Signed-off-by Dan Magenheimer <[email protected]>
Quoted 26 lines
-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Tuesday, May 20, 2008 1:37 AM
To: [email protected]; Xen-Devel (E-mail)
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Quoted 14 lines · Dan Magenheimer
Why? Scaling and adjusting of xen-time-based-tsc will be very difficult to coordinate with processor-based-tsc. We need to always ensure that A < B < C for a guest executing:
rdtsc(A) /* untrapped */
emulated_rdtsc(B)
rdtsc(C) /* untrapped */
Further, OS's use TSC as a highest-resolution time source with knowledge that TSCs on different processors may not be synchronized, whereas they assume that a platform timer is one-per-system and monotonically increasing.
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
Keir, if you disagree and see guest-TSC-on-Xen-system-time as an absolute must, please let me know.
I am inclined to say we should have a guest-TSC-on-system-time mode where
*all* RDTSC executions get trapped. This would at least be
useful as a
Quoted 1 line
Quoted 1 line
baseline for tracking down guest time problems, and also
provide a
Quoted 24 lines
Quoted 23 lines
Quoted 20 lines
Quoted 20 lines
guaranteed stable TSC timesource for those who care about that more than
pure performance.
*However* I would agree that, with TSC virtualisation as it currently is,
there actually isn't really a way to build guest TSC on Xen system time
without periodically warping the TSC back or forth. The guest TSC runs at
the host TSC rate and that is that!
I think my original point was that at least we should not build all our
other virtual time sources on this dodgy 'guest TSC'. :-)
-- Keir

Keir Fraser

2008-05-24 07:34 UTC #9 lore raw

Quoted 10 lines · Dan Magenheimer

Here's another option: I think delay_for_missed_ticks is the only mode where time must be set backwards. All of the other modes track wallclock. It looks possible that we could enforce monotonicity for the other three modes and not enforce it for delay_for_missed_ticks (which is, unfortunately, the default).
If this is acceptable to you (and you don't disagree with my "only mode...set backwards" conjecture), I'll try to code a patch for this.

I agree with your analysis. I couldn't remember if any modes other than 0 would require non-monotonic time across VCPUs, but it does appear that this property is exclusive to mode 0.

Quoted 4 lines

P.S. I think the answer is no, but is there any way to specify gang-scheduling for a domain? If so, we could say if your OS is non-resilient to preemption and your application requires time monotonicity, you must specify gang scheduling.

There isn't currently a way to request gang scheduling. I think that is a second step anyway. It sounds like you could simply turn off your monotonicity checks for timer mode 0 and I'll be happy.

I have a pretty clear vision of what I want now, so I will make the necessary adjustments to the patch. Then we can go from there if you think further modifications are required.

-- Keir

Dan Magenheimer

author

2008-06-03 19:47 UTC #10 lore raw

Quoted 2 lines

I have a pretty clear vision of what I want now, so I will make the necessary adjustments to the patch. Then we can go from there

I noticed you fixed a related bug (cset 17760), but haven't seen anything that changes timer_mode==0 back to non-monotonic. Is that still on your list, or do you want me to submit a patch for that?

Related, I'm wondering if we should change the defaults for timer_mode and hpet now. I'll start a new thread on that to avoid topic drift.

Dan

Quoted 49 lines

-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Saturday, May 24, 2008 1:34 AM
To: [email protected]; Xen-Devel (E-mail)
Cc: Dave Winchell
Subject: Re: [Xen-devel] [PATCH] [RFC] Building guests on monotonic Xen
system time
Quoted 10 lines · Dan Magenheimer
Here's another option: I think delay_for_missed_ticks is the only mode where time must be set backwards. All of the other modes track wallclock. It looks possible that we could enforce monotonicity for the other three modes and not enforce it for delay_for_missed_ticks (which is, unfortunately, the default).
If this is acceptable to you (and you don't disagree with my "only mode...set backwards" conjecture), I'll try to code a patch for this.
I agree with your analysis. I couldn't remember if any modes other than 0
would require non-monotonic time across VCPUs, but it does appear that this
property is exclusive to mode 0.
Quoted 4 lines
P.S. I think the answer is no, but is there any way to specify gang-scheduling for a domain? If so, we could say if your OS is non-resilient to preemption and your application requires time monotonicity, you must specify gang scheduling.
There isn't currently a way to request gang scheduling. I think that is a
second step anyway. It sounds like you could simply turn off your monotonicity checks for timer mode 0 and I'll be happy.
I have a pretty clear vision of what I want now, so I will make the necessary adjustments to the patch. Then we can go from there if you think
further modifications are required.
-- Keir

Keir Fraser

2008-06-03 21:10 UTC #11 lore raw

Quoted 7 lines · Dan Magenheimer

Quoted 2 lines
I have a pretty clear vision of what I want now, so I will make the necessary adjustments to the patch. Then we can go from there
I noticed you fixed a related bug (cset 17760), but haven't seen anything that changes timer_mode==0 back to non-monotonic. Is that still on your list, or do you want me to submit a patch for that?

I think it is correctly handled by my patch. The per-vcpu time offset is only adjusted when timer_mode==0 and that is added into the result returned by hvm_get_guest_time() outside of the domain lock or monotinicity checks.

-- Keir