[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YyPm9G3xMuUVuTcs@feng-clx>
Date: Fri, 16 Sep 2022 11:01:08 +0800
From: Feng Tang <feng.tang@...el.com>
To: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...el.com>,
"H . Peter Anvin" <hpa@...or.com>,
Peter Zijlstra <peterz@...radead.org>,
Jonathan Corbet <corbet@....net>, <x86@...nel.org>,
<linux-kernel@...r.kernel.org>
CC: <paulmck@...nel.org>, <rui.zhang@...el.com>, <len.brown@...el.com>,
<tim.c.chen@...el.com>
Subject: Re: [PATCH v4] x86/tsc: Add option to force frequency recalibration
with HW timer
Hi Thomas, Peter,
You've helped reviewing the earlier version and improving the patch
description, and do you see any other places I need to improve? thanks!
- Feng
On Wed, Aug 17, 2022 at 03:40:18PM +0800, Feng Tang wrote:
> The kernel assumes that the TSC frequency which is provided by the
> hardware / firmware via MSRs or CPUID(0x15) is correct after applying
> a few basic consistency checks. This disables the TSC recalibration
> against HPET or PM timer.
>
> As a result there is no mechanism to validate that frequency in cases
> where a firmware or hardware defect is suspected. And there was case
> that some user used atomic clock to measure the TSC frequency and
> reported an inaccuracy issue, which was later fixed in firmware.
>
> Add an option 'recalibrate' for 'tsc' kernel parameter to force the
> tsc freq recalibration with HPET or PM timer, and warn if the deviation
> from previous value is more than about 500 PPM, which provides a way
> to verify the data from hardware / firmware.
>
> There is no functional change to existing work flow.
>
> [Thanks tglx for helping improving the commit log]
>
> Signed-off-by: Feng Tang <feng.tang@...el.com>
> ---
> Changelog:
>
> since v3:
> * add some real world case into commit log
> * rebase against v6.0-rc1
>
> since v2:
> * revise the option description in kernel-parameters.txt
> * rebase against v5.19-rc2
>
> since v1:
> * refine commit log to state clearly the problem and intention
> of the patch by copying Thomas' words.
>
> .../admin-guide/kernel-parameters.txt | 4 +++
> arch/x86/kernel/tsc.c | 34 ++++++++++++++++---
> 2 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index d7f30902fda0..4924256592d9 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6302,6 +6302,10 @@
> in situations with strict latency requirements (where
> interruptions from clocksource watchdog are not
> acceptable).
> + [x86] recalibrate: force to do frequency recalibration
> + with a HW timer (HPET or PM timer) for systems whose
> + TSC frequency comes from HW or FW through MSR or CPUID(0x15),
> + and warn if the difference is more than 500 ppm.
>
> tsc_early_khz= [X86] Skip early TSC calibration and use the given
> value instead. Useful when the early TSC frequency discovery
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index cafacb2e58cc..5cf62a58754a 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -48,6 +48,8 @@ static DEFINE_STATIC_KEY_FALSE(__use_tsc);
>
> int tsc_clocksource_reliable;
>
> +static int __read_mostly tsc_force_recalibrate;
> +
> static u32 art_to_tsc_numerator;
> static u32 art_to_tsc_denominator;
> static u64 art_to_tsc_offset;
> @@ -303,6 +305,8 @@ static int __init tsc_setup(char *str)
> mark_tsc_unstable("boot parameter");
> if (!strcmp(str, "nowatchdog"))
> no_tsc_watchdog = 1;
> + if (!strcmp(str, "recalibrate"))
> + tsc_force_recalibrate = 1;
> return 1;
> }
>
> @@ -1374,6 +1378,25 @@ static void tsc_refine_calibration_work(struct work_struct *work)
> else
> freq = calc_pmtimer_ref(delta, ref_start, ref_stop);
>
> + /* Will hit this only if tsc_force_recalibrate has been set */
> + if (boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ)) {
> +
> + /* Warn if the deviation exceeds 500 ppm */
> + if (abs(tsc_khz - freq) > (tsc_khz >> 11)) {
> + pr_warn("Warning: TSC freq calibrated by CPUID/MSR differs from what is calibrated by HW timer, please check with vendor!!\n");
> + pr_info("Previous calibrated TSC freq:\t %lu.%03lu MHz\n",
> + (unsigned long)tsc_khz / 1000,
> + (unsigned long)tsc_khz % 1000);
> + }
> +
> + pr_info("TSC freq recalibrated by [%s]:\t %lu.%03lu MHz\n",
> + hpet ? "HPET" : "PM_TIMER",
> + (unsigned long)freq / 1000,
> + (unsigned long)freq % 1000);
> +
> + return;
> + }
> +
> /* Make sure we're within 1% */
> if (abs(tsc_khz - freq) > tsc_khz/100)
> goto out;
> @@ -1407,8 +1430,10 @@ static int __init init_tsc_clocksource(void)
> if (!boot_cpu_has(X86_FEATURE_TSC) || !tsc_khz)
> return 0;
>
> - if (tsc_unstable)
> - goto unreg;
> + if (tsc_unstable) {
> + clocksource_unregister(&clocksource_tsc_early);
> + return 0;
> + }
>
> if (boot_cpu_has(X86_FEATURE_NONSTOP_TSC_S3))
> clocksource_tsc.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP;
> @@ -1421,9 +1446,10 @@ static int __init init_tsc_clocksource(void)
> if (boot_cpu_has(X86_FEATURE_ART))
> art_related_clocksource = &clocksource_tsc;
> clocksource_register_khz(&clocksource_tsc, tsc_khz);
> -unreg:
> clocksource_unregister(&clocksource_tsc_early);
> - return 0;
> +
> + if (!tsc_force_recalibrate)
> + return 0;
> }
>
> schedule_delayed_work(&tsc_irqwork, 0);
> --
> 2.27.0
>
Powered by blists - more mailing lists