| Message ID | 1473874670-4986-6-git-send-email-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 14.09.16 at 19:37, <joao.m.martins@oracle.com> wrote: > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -270,7 +270,9 @@ If set, override Xen's default choice for the platform timer. > Having TSC as platform timer requires being explicitly set. This is because > TSC can only be safely used if CPU hotplug isn't performed on the system. In > some platforms, "maxcpus" parameter may require further adjustment to the > -number of online cpus. > +number of online cpus. When running under platforms that can guarantee a ... running on platforms ... > +monotonic TSC across sockets you require adjusting "tsc" command line parameter ... you may want to adjust the ... > +parameter to "stable:sockets". Redundant "parameter" (I guess the one on the earlier line was meant to get moved due to line length). Also you say "sockets" here but ... > @@ -1508,7 +1510,7 @@ pages) must also be specified via the tbuf\_size parameter. > > `= <integer>` > > ### tsc > -> `= unstable | skewed` > +> `= unstable | skewed | stable:socket` "socket" here - the two really should match. > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -477,6 +477,10 @@ uint64_t ns_to_acpi_pm_tick(uint64_t ns) > /************************************************************ > * PLATFORM TIMER 4: TSC > */ > +static unsigned int __read_mostly tsc_flags; __initdata instead of __read_mostly Jan
On 09/19/2016 11:29 AM, Jan Beulich wrote: >>>> On 14.09.16 at 19:37, <joao.m.martins@oracle.com> wrote: >> --- a/docs/misc/xen-command-line.markdown >> +++ b/docs/misc/xen-command-line.markdown >> @@ -270,7 +270,9 @@ If set, override Xen's default choice for the platform timer. >> Having TSC as platform timer requires being explicitly set. This is because >> TSC can only be safely used if CPU hotplug isn't performed on the system. In >> some platforms, "maxcpus" parameter may require further adjustment to the >> -number of online cpus. >> +number of online cpus. When running under platforms that can guarantee a > > ... running on platforms ... > >> +monotonic TSC across sockets you require adjusting "tsc" command line parameter > > ... you may want to adjust the ... > >> +parameter to "stable:sockets". > > Redundant "parameter" (I guess the one on the earlier line was meant > to get moved due to line length). Also you say "sockets" here but ... > >> @@ -1508,7 +1510,7 @@ pages) must also be specified via the tbuf\_size parameter. >> > `= <integer>` >> >> ### tsc >> -> `= unstable | skewed` >> +> `= unstable | skewed | stable:socket` > > "socket" here - the two really should match. Correct, I will fix this parameter name mismatch, and the spelling mistakes you pointed out above. >> --- a/xen/arch/x86/time.c >> +++ b/xen/arch/x86/time.c >> @@ -477,6 +477,10 @@ uint64_t ns_to_acpi_pm_tick(uint64_t ns) >> /************************************************************ >> * PLATFORM TIMER 4: TSC >> */ >> +static unsigned int __read_mostly tsc_flags; > > __initdata instead of __read_mostly OK. Joao
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index f92fb3f..7161788 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -270,7 +270,9 @@ If set, override Xen's default choice for the platform timer. Having TSC as platform timer requires being explicitly set. This is because TSC can only be safely used if CPU hotplug isn't performed on the system. In some platforms, "maxcpus" parameter may require further adjustment to the -number of online cpus. +number of online cpus. When running under platforms that can guarantee a +monotonic TSC across sockets you require adjusting "tsc" command line parameter +parameter to "stable:sockets". ### cmci-threshold > `= <integer>` @@ -1508,7 +1510,7 @@ pages) must also be specified via the tbuf\_size parameter. > `= <integer>` ### tsc -> `= unstable | skewed` +> `= unstable | skewed | stable:socket` ### ucode > `= [<integer> | scan]` diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 0c1badc..c1255db 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -477,6 +477,10 @@ uint64_t ns_to_acpi_pm_tick(uint64_t ns) /************************************************************ * PLATFORM TIMER 4: TSC */ +static unsigned int __read_mostly tsc_flags; + +/* TSC is reliable across sockets */ +#define TSC_RELIABLE_SOCKET (1 << 0) /* * Called in verify_tsc_reliability() under reliable TSC conditions @@ -492,7 +496,7 @@ static s64 __init init_tsc(struct platform_timesource *pts) ret = 0; } - if ( nr_sockets > 1 ) + if ( nr_sockets > 1 && !(tsc_flags & TSC_RELIABLE_SOCKET) ) { printk(XENLOG_INFO "TSC: Not invariant across sockets\n"); ret = 0; @@ -1851,6 +1855,7 @@ int hwdom_pit_access(struct ioreq *ioreq) /* * tsc=unstable: Override all tests; assume TSC is unreliable. * tsc=skewed: Assume TSCs are individually reliable, but skewed across CPUs. + * tsc=stable:socket: Assume TSCs are reliable across sockets. */ static void __init tsc_parse(const char *s) { @@ -1861,9 +1866,9 @@ static void __init tsc_parse(const char *s) setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE); } else if ( !strcmp(s, "skewed") ) - { setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE); - } + else if ( !strcmp(s, "stable:socket") ) + tsc_flags |= TSC_RELIABLE_SOCKET; } custom_param("tsc", tsc_parse);
Extend the "tsc" boot parameter is to further relax TSC restrictions and allow it to be used on machines that guarantee reliable TSC across sockets. This is up to board manufacturers and there's no way for the OS to probe this property, therefore user needs to explicitly set this option. Also make one style adjustment that is to remove the unnecessary parenthesis around clearing TSC_RELIABLE. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Alternatively to having tsc_flags, I could instead introduce a new X86_FEATURE in the Xen defined mapping. This would mean introducing a "set_caps" similar to "cleared_caps" in order to set the feature bit after x86_capability get's zeroed out in common identify_cpu(). It was unclear to me what would maintainers prefer so I went for the simplest for starters. If you prefer the other way I can redo it. NB: Didn't add "stable:node" (and consequently tsc=stable as it would refer to both) because I don't have a host with multiple nodes that I can test with. --- docs/misc/xen-command-line.markdown | 6 ++++-- xen/arch/x86/time.c | 11 ++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-)