linux-kernel - Re: Re: [PATCH] x86: tsc: fix L2 guest always in tsc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Tue, 24 Apr 2018 10:06:09 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     peng.hao2@....com.cn
Cc:     tglx@...utronix.de, pbonzini@...hat.com, mingo@...hat.com,
        x86@...nel.org, len.brown@...el.com, linux-kernel@...r.kernel.org
Subject: Re: Re: [PATCH] x86: tsc: fix L2 guest always in tsc_early
 clocksource

On Tue, Apr 24, 2018 at 09:56:52AM +0800, peng.hao2@....com.cn wrote:
> >On Tue, Apr 24, 2018 at 09:08:54AM +0800, Peng Hao wrote:
> >> In L2 guest tsc_read_refs always return ULLONG_MAX, and that will
> >> call tsc_refine_calibration_work periodly. So L2 guest will read
> >> acpi timer port 0x608 periodly.
> >> The patch will let it out of "if(tsc_start == -1){}" infinite loop.
> 
> >Help me out a little. What's an L2 guest?

> kvm nested virtual machine.

So why not say nested guest? I though someone did a new hypervisor based
on L4 or so and called it L2 for funnies.

> >So if the whole TSC sync has no way of ever working; what is your patch
> >doing? How is the actual TSC calibrated?

> It exists a period loop in tsc_refine_calibration_work: tsc_start = tsc_read_refs() = -1,so it call schedule_delayed_work(&tsc_irqwork,HZ),
> then tsc_start = tsc_read_refs() = -1 ....
> during the process it accesses 0x608 periodly and it is unnecessary and just influencees performance for L2 guest.
> The patch let it out of the period loop and changes from early-tsc to tsc clocksource.So it will not access ioport 0x608
> periodly.

Can you please teach your mailer to wrap at 78 chars?

> The kvmclock give a interface kvm_get_tsc_khz  to get the tsc_khz.

But why are you getting to tsc_refine_calibration_work() in the first
place? If kvm_get_tsc_khz() returns a known good value, it should set
X86_FEATURE_TSC_KNOWN_FREQ such that we don't attempt to 'refine'
things.

Something like the below perhaps?

---
 arch/x86/kernel/kvmclock.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 8b26c9e01cc4..918948eaa232 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -138,6 +138,13 @@ static unsigned long kvm_get_tsc_khz(void)
 	src = &hv_clock[cpu].pvti;
 	tsc_khz = pvclock_tsc_khz(src);
 	put_cpu();
+
+	/*
+	 * TSC frequency is reported by the host; calibration against (virtual)
+	 * HPET/PM-timer in a guest is dodgy and will not be accurate.
+	 */
+	setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
+
 	return tsc_khz;
 }