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
| ||
|
Date: Thu, 19 Aug 2010 22:29:15 +0200 From: Borislav Petkov <bp@...64.org> To: john stultz <johnstul@...ibm.com> Cc: "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>, Alok Kataria <akataria@...are.com>, the arch/x86 maintainers <x86@...nel.org>, Greg KH <gregkh@...e.de>, "greg@...ah.com" <greg@...ah.com>, "ksrinivasan@...ell.com" <ksrinivasan@...ell.com>, LKML <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] x86, tsc: Limit CPU frequency calibration on AMD From: john stultz <johnstul@...ibm.com> Date: Thu, Aug 19, 2010 at 02:47:35PM -0400 Hi John, > On Wed, Aug 18, 2010 at 9:16 AM, Borislav Petkov <bp@...64.org> wrote: > > 6b37f5a20c0e5c334c010a587058354215433e92 introduced the CPU frequency > > calibration code for AMD CPUs whose TSCs didn't increment with the > > core's P0 frequency. From F10h, revB onward, the TSC increment rate is > > denoted by MSRC001_0015[24] and when this bit is set (which is normally > > done by the BIOS,) the TSC increments with the P0 frequency so the > > calibration is not needed and booting can be a couple of mcecs faster on > > those machines. > > Very cool. This was brought up on an earlier thread and is really nice > because it also avoids the 50+ppm variability easily seen in the > calibrate results boot to boot. That variance causes difficulty > keeping close NTP sync across reboots, as the persistent drift value > is invalid after a reboot. > > I recently proposed a timer based solution that doesn't block bootup, > and allows for very accurate calibration. This might help fill the > gaps on legacy systems that do not provide TSC freq information. I'd > be interested if you had any comments. > > https://kerneltrap.org/mailarchive/linux-kernel/2010/7/28/4598868 > > Notes on the code below. > > > Signed-off-by: Borislav Petkov <borislav.petkov@....com> > > --- > > arch/x86/kernel/tsc.c | 14 ++++++++++++-- > > 1 files changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > > index ce8e502..41b2b8b 100644 > > --- a/arch/x86/kernel/tsc.c > > +++ b/arch/x86/kernel/tsc.c > > @@ -927,8 +927,18 @@ void __init tsc_init(void) > > } > > > > if (cpu_has(&boot_cpu_data, X86_FEATURE_CONSTANT_TSC) && > > - (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)) > > - cpu_khz = calibrate_cpu(); > > + (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)) { > > + > > + if (boot_cpu_data.x86 > 0x10 || > > + (boot_cpu_data.x86 == 0x10 && > > + boot_cpu_data.x86_model >= 0x2)) { > > + u64 val; > > + > > + rdmsrl(MSR_K7_HWCR, val); > > + if (!(val & BIT(24))) > > + cpu_khz = calibrate_cpu(); > > + } > > + } > > Am I just missing the point in the code where you actually use the msr > value to assign cpu_khz? Or is the idea that the default tsc > calibration already is correct, and we don't need to further refine > it? Yes. Actually Alok brought the code to my attention originally, and what puzzled me was the fact that we do calibrate_cpu() on all F10h and later AMD machines needlessly (well, almost all, maybe 99%). This is because, on F10h, revB machines and later we support "TSC increments with P0 frequency" so what native_calibrate_tsc() comes up with in terms of tsc_khz should be the same as cpu_khz. So the MSR bit check above is to see whether the TSC increments with P0 frequency and call calibrate_cpu only if _not_. As a result, this patch basically drops the additional cpu_khz calibration when it isn't needed. And as such it doesn't have a whole lot to do with the actual TSC calibration - this is up to you guys :). The original reason for the calibrate_cpu() is that there's the possibility for the TSC to count with the northbridge clockrate and there we need to recalibrate obviously. Hope that makes it more clear. > And yea, the calibrate_cpu function needs to be renamed. Done, the whole code is moved to cpu/amd.c anyway. I'm currently testing the new version and will be sending out maybe tomorrow or so. Thanks. -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists