Reading the platform timer isn't cheap, so we'd better avoid it when the
resulting value is of no interest to anyone.
The consumer of master_stime, obtained by
time_calibration_{std,tsc}_rendezvous() and propagated through
this_cpu(cpu_calibration), is local_time_calibration(). With
CONSTANT_TSC the latter function uses an early exit path, which doesn't
explicitly use the field. While this_cpu(cpu_calibration) (including the
master_stime field) gets propagated to this_cpu(cpu_time).stamp on that
path, both structures' fields get consumed only by the !CONSTANT_TSC
logic of the function.
Signed-off-by: Jan Beulich <[email protected]>
---
v4: New.
---
I realize there's some risk associated with potential new uses of the
field down the road. What would people think about compiling time.c a
2nd time into a dummy object file, with a conditional enabled to force
assuming CONSTANT_TSC, and with that conditional used to suppress
presence of the field as well as all audited used of it (i.e. in
particular that large part of local_time_calibration())? Unexpected new
users of the field would then cause build time errors.
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -52,6 +52,7 @@ unsigned long pit0_ticks;
struct cpu_time_stamp {
u64 local_tsc;
s_time_t local_stime;
+ /* Next field unconditionally valid only when !CONSTANT_TSC. */
s_time_t master_stime;
};
@@ -1702,7 +1703,7 @@ static void time_calibration_tsc_rendezv
* iteration.
*/
r->master_tsc_stamp = r->max_tsc_stamp;
- else if ( i == 0 )
+ else if ( !boot_cpu_has(X86_FEATURE_CONSTANT_TSC) && i == 0 )
r->master_stime = read_platform_stime(NULL);
atomic_inc(&r->semaphore);
@@ -1776,8 +1777,11 @@ static void time_calibration_std_rendezv
{
while ( atomic_read(&r->semaphore) != (total_cpus - 1) )
cpu_relax();
- r->master_stime = read_platform_stime(NULL);
- smp_wmb(); /* write r->master_stime /then/ signal */
+ if ( !boot_cpu_has(X86_FEATURE_CONSTANT_TSC) )
+ {
+ r->master_stime = read_platform_stime(NULL);
+ smp_wmb(); /* write r->master_stime /then/ signal */
+ }
atomic_inc(&r->semaphore);
}
else