[PATCH/RFC] report hardware tsc frequency even for emulated tsc

[PATCH/RFC] report hardware tsc frequency even for emulated tsc

[Dan Magenheimer]

[-- Attachment #1: Type: text/plain, Size: 5706 bytes --]

Report hardware tsc frequency even for emulated tsc

I was starting some documentation for tsc_mode and
realized this discussion was never resolved.  Currently
when TSC is emulated the pvclock algorithm reports
to a PV OS Xen's system clock hz rate (1GHz).  Linux
at boottime samples the TSC rate and shows it in
dmesg and the rate is also shown in the "cpu MHz"
field in /proc/cpuinfo.  So when TSC is emulated,
it appears that the processor MHz is 1000.0, which
is likely to be confusing to many Xen users.

This patch changes the reported hz rate to the
hz rate of the initial machine on which the guest
is booted and retains that reported hz rate across
save/restore/migration.

Jeremy has pointed out that reporting 1000.0 MHz is
useful because it shows that TSC is being emulated.
However, with the new tsc_mode default where
a guest may start with native TSC and switch to
emulated TSC after migration, users are likely to
get even more confused.  And "xm debug-key s"
reveals not only whether TSC is being emulated but
also the frequency so is more descriptive anyway.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>

diff -r d0b030008814 xen/arch/x86/time.c
--- a/xen/arch/x86/time.c	Fri Nov 27 08:09:26 2009 +0000
+++ b/xen/arch/x86/time.c	Sun Nov 29 16:00:06 2009 -0700
@@ -146,23 +146,7 @@ static inline u64 scale_delta(u64 delta,
     return product;
 }
 
-#define _TS_SHIFT_IDENTITY    1
 #define _TS_MUL_FRAC_IDENTITY 0x80000000UL
-#define _TS_IDENTITY { _TS_SHIFT_IDENTITY, _TS_MUL_FRAC_IDENTITY }
-static inline int time_scale_is_identity(struct time_scale *ts)
-{
-    if ( ts->shift != _TS_SHIFT_IDENTITY )
-        return 0;
-    else if ( ts->mul_frac != _TS_MUL_FRAC_IDENTITY )
-        return 0;
-    return 1;
-}
-
-static inline void set_time_scale_identity(struct time_scale *ts)
-{
-    ts->shift = _TS_SHIFT_IDENTITY;
-    ts->mul_frac = _TS_MUL_FRAC_IDENTITY;
-}
 
 /* Compute the reciprocal of the given time_scale. */
 static inline struct time_scale scale_reciprocal(struct time_scale scale)
@@ -843,11 +827,8 @@ static void __update_vcpu_system_time(st
     u = &vcpu_info(v, time);
 
     if ( d->arch.vtsc )
-    {
-        tsc_stamp = t->stime_local_stamp - d->arch.vtsc_offset;
-        if ( !time_scale_is_identity(&d->arch.ns_to_vtsc) )
-            tsc_stamp = scale_delta(tsc_stamp, &d->arch.ns_to_vtsc);
-    }
+        tsc_stamp = scale_delta(t->stime_local_stamp - d->arch.vtsc_offset,
+                                &d->arch.ns_to_vtsc);
     else
         tsc_stamp = t->local_tsc_stamp;
 
@@ -1631,9 +1612,7 @@ void pv_soft_rdtsc(struct vcpu *v, struc
 
     spin_unlock(&d->arch.vtsc_lock);
 
-    now = now - d->arch.vtsc_offset;
-    if ( !time_scale_is_identity(&d->arch.ns_to_vtsc) )
-        now = scale_delta(now, &d->arch.ns_to_vtsc);
+    now = scale_delta(now - d->arch.vtsc_offset, &d->arch.ns_to_vtsc);
 
     regs->eax = (uint32_t)now;
     regs->edx = (uint32_t)(now >> 32);
@@ -1714,7 +1693,7 @@ void tsc_get_info(struct domain *d, uint
         break;
     case TSC_MODE_ALWAYS_EMULATE:
         *elapsed_nsec = get_s_time() - d->arch.vtsc_offset;
-        *gtsc_khz = 1000000UL;
+        *gtsc_khz =  d->arch.tsc_khz;
          break;
     case TSC_MODE_DEFAULT:
         if ( d->arch.vtsc )
@@ -1760,29 +1739,20 @@ void tsc_set_info(struct domain *d,
     case TSC_MODE_ALWAYS_EMULATE:
         d->arch.vtsc = 1;
         d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
-        set_time_scale_identity(&d->arch.vtsc_to_ns);
+        d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz;
+        set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
+        d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
         break;
     case TSC_MODE_DEFAULT:
+        d->arch.vtsc = 1;
         d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
-        if ( (host_tsc_is_safe() && incarnation == 0) || !d->domain_id )
-        {
-            /* use native TSC if initial host supports it */
+        d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz;
+        set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
+        /* use native TSC if initial host has safe TSC and not migrated yet */
+        if ( host_tsc_is_safe() && incarnation == 0 )
             d->arch.vtsc = 0;
-            d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz;
-            set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
-            set_time_scale_identity(&d->arch.ns_to_vtsc);
-        } else if ( gtsc_khz != 0  && gtsc_khz != 1000000UL ) {
-            /* was native on initial host, now emulated at initial tsc hz*/
-            d->arch.vtsc = 1;
-            d->arch.tsc_khz = gtsc_khz;
-            set_time_scale(&d->arch.vtsc_to_ns, gtsc_khz * 1000 );
-            d->arch.ns_to_vtsc =
-                scale_reciprocal(d->arch.vtsc_to_ns);
-        } else {
-            d->arch.vtsc = 1;
-            set_time_scale_identity(&d->arch.vtsc_to_ns);
-            set_time_scale_identity(&d->arch.ns_to_vtsc);
-        }
+        else 
+            d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
         break;
     case TSC_MODE_PVRDTSCP:
         if ( boot_cpu_has(X86_FEATURE_RDTSCP) && gtsc_khz != 0 ) {
@@ -1791,7 +1761,7 @@ void tsc_set_info(struct domain *d,
         } else {
             d->arch.vtsc = 1;
             d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
-            set_time_scale_identity(&d->arch.vtsc_to_ns);
+            /* set_time_scale_identity(&d->arch.vtsc_to_ns, gtsc_khz * 1000 ); */
         }
         break;
     }

[-- Attachment #2: tscmode-fixedhz.patch --]
[-- Type: application/octet-stream, Size: 4431 bytes --]

diff -r d0b030008814 xen/arch/x86/time.c
--- a/xen/arch/x86/time.c	Fri Nov 27 08:09:26 2009 +0000
+++ b/xen/arch/x86/time.c	Sun Nov 29 16:00:06 2009 -0700
@@ -146,23 +146,7 @@ static inline u64 scale_delta(u64 delta,
     return product;
 }
 
-#define _TS_SHIFT_IDENTITY    1
 #define _TS_MUL_FRAC_IDENTITY 0x80000000UL
-#define _TS_IDENTITY { _TS_SHIFT_IDENTITY, _TS_MUL_FRAC_IDENTITY }
-static inline int time_scale_is_identity(struct time_scale *ts)
-{
-    if ( ts->shift != _TS_SHIFT_IDENTITY )
-        return 0;
-    else if ( ts->mul_frac != _TS_MUL_FRAC_IDENTITY )
-        return 0;
-    return 1;
-}
-
-static inline void set_time_scale_identity(struct time_scale *ts)
-{
-    ts->shift = _TS_SHIFT_IDENTITY;
-    ts->mul_frac = _TS_MUL_FRAC_IDENTITY;
-}
 
 /* Compute the reciprocal of the given time_scale. */
 static inline struct time_scale scale_reciprocal(struct time_scale scale)
@@ -843,11 +827,8 @@ static void __update_vcpu_system_time(st
     u = &vcpu_info(v, time);
 
     if ( d->arch.vtsc )
-    {
-        tsc_stamp = t->stime_local_stamp - d->arch.vtsc_offset;
-        if ( !time_scale_is_identity(&d->arch.ns_to_vtsc) )
-            tsc_stamp = scale_delta(tsc_stamp, &d->arch.ns_to_vtsc);
-    }
+        tsc_stamp = scale_delta(t->stime_local_stamp - d->arch.vtsc_offset,
+                                &d->arch.ns_to_vtsc);
     else
         tsc_stamp = t->local_tsc_stamp;
 
@@ -1631,9 +1612,7 @@ void pv_soft_rdtsc(struct vcpu *v, struc
 
     spin_unlock(&d->arch.vtsc_lock);
 
-    now = now - d->arch.vtsc_offset;
-    if ( !time_scale_is_identity(&d->arch.ns_to_vtsc) )
-        now = scale_delta(now, &d->arch.ns_to_vtsc);
+    now = scale_delta(now - d->arch.vtsc_offset, &d->arch.ns_to_vtsc);
 
     regs->eax = (uint32_t)now;
     regs->edx = (uint32_t)(now >> 32);
@@ -1714,7 +1693,7 @@ void tsc_get_info(struct domain *d, uint
         break;
     case TSC_MODE_ALWAYS_EMULATE:
         *elapsed_nsec = get_s_time() - d->arch.vtsc_offset;
-        *gtsc_khz = 1000000UL;
+        *gtsc_khz =  d->arch.tsc_khz;
          break;
     case TSC_MODE_DEFAULT:
         if ( d->arch.vtsc )
@@ -1760,29 +1739,20 @@ void tsc_set_info(struct domain *d,
     case TSC_MODE_ALWAYS_EMULATE:
         d->arch.vtsc = 1;
         d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
-        set_time_scale_identity(&d->arch.vtsc_to_ns);
+        d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz;
+        set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
+        d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
         break;
     case TSC_MODE_DEFAULT:
+        d->arch.vtsc = 1;
         d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
-        if ( (host_tsc_is_safe() && incarnation == 0) || !d->domain_id )
-        {
-            /* use native TSC if initial host supports it */
+        d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz;
+        set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
+        /* use native TSC if initial host has safe TSC and not migrated yet */
+        if ( host_tsc_is_safe() && incarnation == 0 )
             d->arch.vtsc = 0;
-            d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz;
-            set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
-            set_time_scale_identity(&d->arch.ns_to_vtsc);
-        } else if ( gtsc_khz != 0  && gtsc_khz != 1000000UL ) {
-            /* was native on initial host, now emulated at initial tsc hz*/
-            d->arch.vtsc = 1;
-            d->arch.tsc_khz = gtsc_khz;
-            set_time_scale(&d->arch.vtsc_to_ns, gtsc_khz * 1000 );
-            d->arch.ns_to_vtsc =
-                scale_reciprocal(d->arch.vtsc_to_ns);
-        } else {
-            d->arch.vtsc = 1;
-            set_time_scale_identity(&d->arch.vtsc_to_ns);
-            set_time_scale_identity(&d->arch.ns_to_vtsc);
-        }
+        else 
+            d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
         break;
     case TSC_MODE_PVRDTSCP:
         if ( boot_cpu_has(X86_FEATURE_RDTSCP) && gtsc_khz != 0 ) {
@@ -1791,7 +1761,7 @@ void tsc_set_info(struct domain *d,
         } else {
             d->arch.vtsc = 1;
             d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
-            set_time_scale_identity(&d->arch.vtsc_to_ns);
+            /* set_time_scale_identity(&d->arch.vtsc_to_ns, gtsc_khz * 1000 ); */
         }
         break;
     }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH/RFC] report hardware tsc frequency even for emulated tsc

[Jeremy Fitzhardinge]

On 11/30/09 07:55, Dan Magenheimer wrote:
> This patch changes the reported hz rate to the
> hz rate of the initial machine on which the guest
> is booted and retains that reported hz rate across
> save/restore/migration.
>   

The pvclock algorithm relies on knowing what rate the tsc advances at. 
If you return  the "real" tsc speed in the clock info, won't that break?

> Jeremy has pointed out that reporting 1000.0 MHz is
> useful because it shows that TSC is being emulated.
> However, with the new tsc_mode default where
> a guest may start with native TSC and switch to
> emulated TSC after migration, users are likely to
> get even more confused.

I don't see how the original CPU speed is any more relevant than the
emulated speed, which is at least constant.  The CPU speed is fairly
pointless info given that it has no bearing on how fast the CPU is
running from moment to moment, even ignoring migration.  (On native "cpu
MHz" updates on the fly as the CPU speed changes, but I don't think your
proposal will allow that.)

>   And "xm debug-key s"
> reveals not only whether TSC is being emulated but
> also the frequency so is more descriptive anyway.
>   

It is not available to non-privileged users.

    J

RE: [PATCH/RFC] report hardware tsc frequency even for emulated tsc

[Dan Magenheimer]

Hi Jeremy --

Thanks for the comments.

> > This patch changes the reported hz rate to the
> > hz rate of the initial machine on which the guest
> > is booted and retains that reported hz rate across
> > save/restore/migration.
>
> The pvclock algorithm relies on knowing what rate the tsc 
> advances at. 
> If you return  the "real" tsc speed in the clock info, won't 
> that break?

As long as the "illusion" provided via both rdtsc and
pvclock are consistent, it works.
 
> > Jeremy has pointed out that reporting 1000.0 MHz is
> > useful because it shows that TSC is being emulated.
> > However, with the new tsc_mode default where
> > a guest may start with native TSC and switch to
> > emulated TSC after migration, users are likely to
> > get even more confused.
> 
> I don't see how the original CPU speed is any more relevant than the
> emulated speed, which is at least constant.
> The CPU speed is fairly
> pointless info given that it has no bearing on how fast the CPU is
> running from moment to moment, even ignoring migration.

It's not more relevant, it's just less shocking.  The vast
majority of users don't know (or care) that the MHz rate
reported for the CPU is derived from the TSC rate.  They
will just think, with a 1GHz reported rate in their guest, 
that Xen has done something horribly wrong to slow down
their processor.

Our esteemed large competitor appears to report something
like the hz rate of the initial machine, even though they
always do "emulation".  So if all other things are equal,
I'd be inclined to follow suit.

> (On native "cpu
> MHz" updates on the fly as the CPU speed changes, but I don't 
> think your proposal will allow that.)

I wasn't aware of that.  My proposal could allow for that but
the specific proposed patch doesn't.  It could only be
done I think if one didn't care about the TSC hz rate
given to apps... if that's true, just turn off TSC emulation.

> >   And "xm debug-key s"
> > reveals not only whether TSC is being emulated but
> > also the frequency so is more descriptive anyway.
> 
> It is not available to non-privileged users.

Good point.  OTOH, if one cared to do something to change
tsc emulation, one would need to be privileged to relaunch
the domain.

Thanks,
Dan

Re: [PATCH/RFC] report hardware tsc frequency even for emulated tsc

[Jeremy Fitzhardinge]

On 11/30/09 12:51, Dan Magenheimer wrote:
> As long as the "illusion" provided via both rdtsc and
> pvclock are consistent, it works.
>   

OK, I see.  Rather than always emulating 1GHz, make it emulate some
other arbitrary speed depending on the initial native CPU speed.

> I wasn't aware of that.  My proposal could allow for that but
> the specific proposed patch doesn't.  It could only be
> done I think if one didn't care about the TSC hz rate
> given to apps... if that's true, just turn off TSC emulation.
>   

I don't think its worth doing.  In many ways a VCPU is like a Transmeta
CPU: they never ran at any particular speed, but with some
power/performance policy setting.  The tsc ran at a fixed rate that had
no particular relationship to how fast instructions get executed.

> Good point.  OTOH, if one cared to do something to change
> tsc emulation, one would need to be privileged to relaunch
> the domain.
>   

Well, they'd need some interface to start domains at all.  You could
imagine something like a VPS hosting service where the user can control
their own domains via a web GUI, and "tsc emulation" would be a checkbox
(buried deeping in an advanced options page, one presumes).

And you could also imagine that this patch would be a boon to them,
because it would eliminate the "you said you had 2.4GHz hosts but
/proc/cpuinfo only says 1GHz" questions/complaints...

    J