[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] x86: reduce redundancy in tsc_[gs]et_info()
On 30/04/14 14:53, Jan Beulich wrote:
- some of the case statements are effectively or mostly special cases
of others, so there's no good reason not to share the code
- in the "get" function, a variable can be made case-wide instead of
having multiple instance of it (and those even with a pointless
initializer)
- minor formatting adjustments
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1786,26 +1786,22 @@ void tsc_get_info(struct domain *d, uint
switch ( *tsc_mode )
{
+ uint64_t tsc;
+
case TSC_MODE_NEVER_EMULATE:
- *elapsed_nsec = *gtsc_khz = 0;
+ *elapsed_nsec = *gtsc_khz = 0;
break;
- case TSC_MODE_ALWAYS_EMULATE:
- *elapsed_nsec = get_s_time() - d->arch.vtsc_offset;
- *gtsc_khz = d->arch.tsc_khz;
- break;
case TSC_MODE_DEFAULT:
if ( d->arch.vtsc )
{
+ case TSC_MODE_ALWAYS_EMULATE:
*elapsed_nsec = get_s_time() - d->arch.vtsc_offset;
*gtsc_khz = d->arch.tsc_khz;
double space after =
+ break;
}
- else
- {
- uint64_t tsc = 0;
- rdtscll(tsc);
- *elapsed_nsec = scale_delta(tsc,&d->arch.vtsc_to_ns);
- *gtsc_khz = cpu_khz;
- }
+ rdtscll(tsc);
+ *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns);
+ *gtsc_khz = cpu_khz;
break;
case TSC_MODE_PVRDTSCP:
if ( d->arch.vtsc )
@@ -1815,10 +1811,9 @@ void tsc_get_info(struct domain *d, uint
}
else
{
- uint64_t tsc = 0;
rdtscll(tsc);
- *elapsed_nsec = (scale_delta(tsc,&d->arch.vtsc_to_ns) -
- d->arch.vtsc_offset);
+ *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
+ d->arch.vtsc_offset;
*gtsc_khz = 0; /* ignored by tsc_set_info */
}
break;
There is a coverity complaint that following this switch statement,
there is a read of *elapsed_nsec which might be uninitialised if
tsc_mode missed on of the case statements. While I believe this is
currently impossible, it might we wise to have a "default: BUG();"
in case half a new tsc_mode appears.
@@ -1875,28 +1870,24 @@ void tsc_set_info(struct domain *d,
switch ( d->arch.tsc_mode = tsc_mode )
{
- case TSC_MODE_NEVER_EMULATE:
- d->arch.vtsc = 0;
- break;
- case TSC_MODE_ALWAYS_EMULATE:
- d->arch.vtsc = 1;
- d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
- 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;
+ case TSC_MODE_ALWAYS_EMULATE:
d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
- 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, has not migrated
- * yet and tsc_khz == cpu_khz */
- if ( host_tsc_is_safe() && incarnation == 0 &&
- d->arch.tsc_khz == cpu_khz )
This wont apply cleanly on top of staging. Boris made some changed
in this area with his HVM guest TSC series.
~Andrew
+ d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
+ set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
+ /*
+ * In default mode use native TSC if the host has safe TSC,
+ * the VM has not migrated yet, and tsc_khz == cpu_khz.
+ */
+ if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
+ incarnation == 0 && d->arch.tsc_khz == cpu_khz )
+ {
+ case TSC_MODE_NEVER_EMULATE:
d->arch.vtsc = 0;
- else
- d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
+ break;
+ }
+ d->arch.vtsc = 1;
+ d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
break;
case TSC_MODE_PVRDTSCP:
d->arch.vtsc = boot_cpu_has(X86_FEATURE_RDTSCP) &&
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|