linux-kernel - Re: [PATCH v2] x86: Trust initial offset in architectural TSC-adjust MSRs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c4aa31c3-8008-4b56-aa78-96fdc23f618d@redhat.com>
Date: Thu, 25 Apr 2024 17:51:01 -0400
From: Prarit Bhargava <prarit@...hat.com>
To: Daniel J Blueman <daniel@...ra.org>, Thomas Gleixner
 <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>
Cc: x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
 Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org,
 Steffen Persvold <sp@...ascale.com>,
 James Cleverdon <james.cleverdon.external@...den.com>,
 Dimitri Sivanich <sivanich@....com>, Steve Wahl <steve.wahl@....com>,
 Frank Ramsay <frank.ramsay@....com>, Russ Anderson <rja@....com>
Subject: Re: [PATCH v2] x86: Trust initial offset in architectural TSC-adjust
 MSRs

On 4/19/24 04:51, Daniel J Blueman wrote:
> When the BIOS configures the architectural TSC-adjust MSRs on secondary
> sockets to correct a constant inter-chassis offset, after Linux brings
> the cores online, the TSC sync check later resets the core-local MSR to
> 0, triggering HPET fallback and leading to performance loss.
> 
> Fix this by unconditionally using the initial adjust values read from the
> MSRs. Trusting the initial offsets in this architectural mechanism is a
> better approach than special-casing workarounds for specific platforms.
> 
> Signed-off-by: Daniel J Blueman <daniel@...ra.org>
> Reviewed-by: Steffen Persvold <sp@...ascale.com>
> Reviewed-by: James Cleverdon <james.cleverdon.external@...den.com>
> Reviewed-by: Dimitri Sivanich <sivanich@....com>
> Cc: Steve Wahl <steve.wahl@....com>
> Cc: Prarit Bhargava <prarit@...hat.com>
> Cc: Frank Ramsay <frank.ramsay@....com>
> Cc: Russ Anderson <rja@....com>
> ---
> Changes in v2:
> - Maintain comment based on feedback
> - Rebase against v6.9-rc4
> 
>   arch/x86/kernel/tsc_sync.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/tsc_sync.c b/arch/x86/kernel/tsc_sync.c
> index 1123ef3ccf90..4334033658ed 100644
> --- a/arch/x86/kernel/tsc_sync.c
> +++ b/arch/x86/kernel/tsc_sync.c
> @@ -193,11 +193,9 @@ bool tsc_store_and_check_tsc_adjust(bool bootcpu)
>   	cur->warned = false;
>   
>   	/*
> -	 * If a non-zero TSC value for socket 0 may be valid then the default
> -	 * adjusted value cannot assumed to be zero either.
> +	 * The default adjust value cannot be assumed to be zero on any socket.
>   	 */
> -	if (tsc_async_resets)
> -		cur->adjusted = bootval;
> +	cur->adjusted = bootval;
>   
>   	/*
>   	 * Check whether this CPU is the first in a package to come up. In

This covers the concerns raise by HPE (cc'd) on this patch.

Reviewed-by: Prarit Bhargava <prarit@...hat.com>

P.