Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 19 Apr 2019 10:57:10 +0200 (CEST)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Daniel Drake <drake@endlessm.com>
cc:     Len Brown <lenb@kernel.org>, x86@kernel.org,
        LKML <linux-kernel@vger.kernel.org>, linux@endlessm.com,
        rjw@rjwysocki.net, Jacob Pan <jacob.jun.pan@linux.intel.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Subject: Re: Detecting x86 LAPIC timer frequency from CPUID data
In-Reply-To: <20190419083533.32388-1-drake@endlessm.com>
Message-ID: <alpine.DEB.2.21.1904191042180.3174@nanos.tec.linutronix.de>
References:  <20190419083533.32388-1-drake@endlessm.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 19 Apr 2019, Daniel Drake wrote:
> On Fri, Apr 19, 2019 at 6:30 AM Thomas Gleixner <tglx@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.

Thanks,

	tglx