Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Oct 2022 19:13:26 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Rishabh Agrawal <rishabhagr@chromium.org>
Cc:     LKML <linux-kernel@vger.kernel.org>, len.brown@intel.com,
        drake@endlessm.com, rafael.j.wysocki@intel.com, mingo@redhat.com,
        tglx@linutronix.de, vaibhav.shankar@intel.com,
        biernacki@google.com, zwisler@google.com, mattedavis@google.com,
        Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Feng Tang <feng.tang@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org
Subject: Re: [PATCH v2] Add hardcoded crystal clock for KabyLake
Message-ID: <Y1GBtjcetPpTq0V3@hirez.programming.kicks-ass.net>
References: <20221018190124.v2.1.I918ccc908c5c836c1e21e01d2cf6f59b0157bcc3@changeid>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221018190124.v2.1.I918ccc908c5c836c1e21e01d2cf6f59b0157bcc3@changeid>
Precedence: bulk

On Tue, Oct 18, 2022 at 07:01:32PM +0000, Rishabh Agrawal wrote:
> Set KabyLake crystal clock manually since the TSC calibration is off
> by 0.5%. All the tests that are based on the timer/clock will fail in
> this case.
> 
> The HPET has been disabled by upstream due to PC10 issue causing the
> 3 hatch devices, dratini, jinlon, and kohaku to not calibrate the clock

I have no idea what a hatch device is, nor what pokemon have anything to
do with things.

> precisely. These 3 devices are KabyLake devices. Intel team has verified

But no actual Intel person with a Reviewed-by tag we can go pester if
this turns out to be wrong.

> that all KBL devices have 24.0 MHz clock frequency, therefore this
> change is valid.

And yet you only change KBL_L.

And why, pray *WHY* can't Intel simply write the correct information in
CPUID leaf 15h. I mean, they defined the leaf, might as well use it, no?

> Signed-off-by: Rishabh Agrawal <rishabhagr@chromium.org>
> ---
> 
> Changes in v2:
>   - Adding Thomas Gleixner, Daniel Drake, Rafael Wysocki, Len brown and Ingo to review since you worked on this.
> 
>  arch/x86/kernel/tsc.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index cafacb2e58cc..63a06224fa48 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -644,10 +644,21 @@ unsigned long native_calibrate_tsc(void)
>  	 * Denverton SoCs don't report crystal clock, and also don't support
>  	 * CPUID.0x16 for the calculation below, so hardcode the 25MHz crystal
>  	 * clock.
> +	 *
> +	 * Intel KabyLake devices' clocks are off by 0.5% when using the below
> +	 * calculation, so hardcode 24.0 MHz crystal clock.
>  	 */
> -	if (crystal_khz == 0 &&
> -			boot_cpu_data.x86_model == INTEL_FAM6_ATOM_GOLDMONT_D)
> -		crystal_khz = 25000;
> +	if (crystal_khz == 0) {
> +		switch (boot_cpu_data.x86_model) {
> +		case INTEL_FAM6_ATOM_GOLDMONT_D:
> +			crystal_khz = 25000;
> +			break;
> +		case INTEL_FAM6_KABYLAKE_L:
> +			crystal_khz = 24000;
> +			break;
> +		}
> +
> +	}
>  
>  	/*
>  	 * TSC frequency reported directly by CPUID is a "hardware reported"
> -- 
> 2.38.0.413.g74048e4d9e-goog
>