diff mbox series

[linux-next] x86/xen/time: prefer tsc as clocksource when it is invariant

Message ID 20221208163650.GA3225@templeofstupid.com (mailing list archive)
State Superseded
Headers show
Series [linux-next] x86/xen/time: prefer tsc as clocksource when it is invariant | expand

Commit Message

Krister Johansen Dec. 8, 2022, 4:36 p.m. UTC
Kvm elects to use tsc instead of kvm-clock when it can detect that the
TSC is invariant.

(As of commit 7539b174aef4 ("x86: kvmguest: use TSC clocksource if
invariant TSC is exposed")).

Notable cloud vendors[1] and performance engineers[2] recommend that Xen
users preferentially select tsc over xen-clocksource due the performance
penalty incurred by the latter.  These articles are persuasive and
tailored to specific use cases.  In order to understand the tradeoffs
around this choice more fully, this author had to reference the
documented[3] complexities around the Xen configuration, as well as the
kernel's clocksource selection algorithm.  Many users may not attempt
this to correctly configure the right clock source in their guest.

The approach taken in the kvm-clock module spares users this confusion,
where possible.

Both the Intel SDM[4] and the Xen tsc documentation explain that marking
a tsc as invariant means that it should be considered stable by the OS
and is elibile to be used as a wall clock source.  The Xen documentation
further clarifies that this is only reliable on HVM and PVH because PV
cannot intercept a cpuid instruction.

In order to obtain better out-of-the-box performance, and reduce the
need for user tuning, follow kvm's approach and decrease the xen clock
rating so that tsc is preferable, if it is invariant, stable, and the
guest is a HVM or PVH domain.

[1] https://aws.amazon.com/premiumsupport/knowledge-center/manage-ec2-linux-clock-source/
[2] https://www.brendangregg.com/blog/2021-09-26/the-speed-of-time.html
[3] https://xenbits.xen.org/docs/unstable/man/xen-tscmode.7.html
[4] Intel 64 and IA-32 Architectures Sofware Developer's Manual Volume
    3b: System Programming Guide, Part 2, Section 17.17.1, Invariant TSC

Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
Code-reviewed-by: David Reaver <me@davidreaver.com>
---
 arch/x86/xen/time.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Boris Ostrovsky Dec. 9, 2022, 7:32 p.m. UTC | #1
On 12/8/22 11:36 AM, Krister Johansen wrote:
> +	/*
> +	 * As Dom0 is never moved, no penalty on using TSC there.
> +	 *
> +	 * If the guest has invariant tsc, then set xen_clocksource rating
> +	 * below that of the tsc so that the system prefers tsc instead.  This
> +	 * check excludes PV domains, because PV is unable to guarantee that the
> +	 * guest's cpuid call has been intercepted by the hypervisor.
> +	 */
> +	if (xen_initial_domain()) {
>   		xen_clocksource.rating = 275;
> +	} else if ((xen_hvm_domain() || xen_pvh_domain()) &&
> +	    boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> +	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
> +	    !check_tsc_unstable()) {
> +		xen_clocksource.rating = 299;
> +	}


What if RDTSC is intercepted?


-boris
Krister Johansen Dec. 12, 2022, 3:57 p.m. UTC | #2
On Fri, Dec 09, 2022 at 02:32:15PM -0500, Boris Ostrovsky wrote:
> 
> On 12/8/22 11:36 AM, Krister Johansen wrote:
> > +	/*
> > +	 * As Dom0 is never moved, no penalty on using TSC there.
> > +	 *
> > +	 * If the guest has invariant tsc, then set xen_clocksource rating
> > +	 * below that of the tsc so that the system prefers tsc instead.  This
> > +	 * check excludes PV domains, because PV is unable to guarantee that the
> > +	 * guest's cpuid call has been intercepted by the hypervisor.
> > +	 */
> > +	if (xen_initial_domain()) {
> >   		xen_clocksource.rating = 275;
> > +	} else if ((xen_hvm_domain() || xen_pvh_domain()) &&
> > +	    boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> > +	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
> > +	    !check_tsc_unstable()) {
> > +		xen_clocksource.rating = 299;
> > +	}
> 
> 
> What if RDTSC is intercepted?

Right, thanks.  I'll send out an updated patch here shortly that
attempts to address this by examining the cpuid information to determine
if the tsc is being emulated.

-K
diff mbox series

Patch

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 9ef0a5cca96e..ca78581e4221 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -480,9 +480,22 @@  static void __init xen_time_init(void)
 	int cpu = smp_processor_id();
 	struct timespec64 tp;
 
-	/* As Dom0 is never moved, no penalty on using TSC there */
-	if (xen_initial_domain())
+	/*
+	 * As Dom0 is never moved, no penalty on using TSC there.
+	 *
+	 * If the guest has invariant tsc, then set xen_clocksource rating
+	 * below that of the tsc so that the system prefers tsc instead.  This
+	 * check excludes PV domains, because PV is unable to guarantee that the
+	 * guest's cpuid call has been intercepted by the hypervisor.
+	 */
+	if (xen_initial_domain()) {
 		xen_clocksource.rating = 275;
+	} else if ((xen_hvm_domain() || xen_pvh_domain()) &&
+	    boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
+	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
+	    !check_tsc_unstable()) {
+		xen_clocksource.rating = 299;
+	}
 
 	clocksource_register_hz(&xen_clocksource, NSEC_PER_SEC);