On Fri, 19 Apr 2019 10:57:10 +0200 (CEST) Thomas Gleixner <t...@linutronix.de> wrote:
> On Fri, 19 Apr 2019, Daniel Drake wrote: > > On Fri, Apr 19, 2019 at 6:30 AM Thomas Gleixner > > <t...@linutronix.de> wrote: > > > Time Stamp Counter/Core Crystal Clock Information (0x15): > > > TSC/clock ratio = 168/2 > > > nominal core crystal clock = 0 Hz > > > > > > Processor Frequency Information (0x16): > > > Core Base Frequency (MHz) = 0x834 (2100) > > > Core Maximum Frequency (MHz) = 0xed8 (3800) > > > Bus (Reference) Frequency (MHz) = 0x64 (100) > > > > > > Assuming that TSC and local APIC timer run from the same > > > frequency on these modern machines. > > > > > > 2100MHz * 2 / 168 = 25MHz > > > > > > and disabling the tsc deadline timer tells me: > > > > > > ..... calibration result: 24999 > > > > > > Close enough. > > > > I tested all the Intel SoC generations we have on hand. The > > assumption that the core crystal clock feeds the APIC seems to be > > consistently true. > > > > (Please note that all the following results are done with > > CONFIG_HZ=250, which is why the "calibration result" is 4x higher > > than HZ=1000 as used in previous mails) > > > > In the easy case, the low cost platforms do not support CPUID.0x16 > > (so no CPU frequency reporting), but they do tell us the core > > crystal clock, which is consistent with the APIC calibration > > result: > > ... > > > And the 4 higher-end SoCs that we have available for testing all > > report crystal clock 0Hz from CPUID 0x15, but by combining the > > CPUID.0x16 base frequency with the CPUID.0x15 TSC/clock ratio, the > > crystal frequency can be calculated as you describe, and it > > consistently matches the APIC timer calibration result. > > ... > > > Is this data convincing enough or should we additionally wait for > > some comments from Intel? > > For me it's pretty convincing, but having some confirmation from Intel > wouldn't be a bad thing. > > > I came up with the patch below. However, upon testing, I realised > > that, at least for the platforms I have in hand, only the first > > hunk is really needed. We don't need to use your magic calculation > > to find the crystal frequency because Intel already told us! > > native_calibrate_tsc() already hardcodes the crystal frequency for > > Kabylake, and Amber/Whiskey/Coffee also report the 0x8e/0x9e > > Kabylake model codes. > > I'd rather replace these model checks with math. These tables are > horrible to maintain. > > > Plus ApolloLake/GeminiLake do report the crystal frequency in > > CPUID.0x15 so that is covered too. > > > While looking around this code I also spotted something curious. > > In calibrate_APIC_clock() for the case where lapic_timer_frequency > > has been externally provided, we have: > > lapic_clockevent.max_delta_ns = > > clockevent_delta2ns(0x7FFFFF, > > &lapic_clockevent); lapic_clockevent.max_delta_ticks = 0x7FFFFF; > > > > But in the case where we calibrate, we have: > > lapic_clockevent.max_delta_ns = > > clockevent_delta2ns(0x7FFFFFFF, &lapic_clockevent); > > lapic_clockevent.max_delta_ticks = 0x7FFFFFFF; > > > > 0x7FFFFF vs 0x7FFFFFFF, is that intentional? > > I don't think so. Looks like a failed copy and paste. Cc'ed Jacob, he > might know. > At the time of v2.6.35 both places use 0x7FFFFF. But later this patch increased the latter to 0x7FFFFFFF but forgot the first part. So I guess it is not exactly a failed copy and paste. commit 4aed89d6b515b9185351706ca95cd712c9d8d6a3 Author: Pierre Tardy <pierre.ta...@intel.com> Date: Thu Jan 6 16:23:29 2011 +0100 x86, lapic-timer: Increase the max_delta to 31 bits > Thanks, > > tglx [Jacob Pan]