Received-SPF: pass (google.com: domain of linux-kernel+bounces-159242-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <c4aa31c3-8008-4b56-aa78-96fdc23f618d@redhat.com>
Date: Thu, 25 Apr 2024 17:51:01 -0400
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] x86: Trust initial offset in architectural TSC-adjust
 MSRs
To: Daniel J Blueman <daniel@quora.org>, Thomas Gleixner
 <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
 Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
 Peter Zijlstra <peterz@infradead.org>, linux-kernel@vger.kernel.org,
 Steffen Persvold <sp@numascale.com>,
 James Cleverdon <james.cleverdon.external@eviden.com>,
 Dimitri Sivanich <sivanich@hpe.com>, Steve Wahl <steve.wahl@hpe.com>,
 Frank Ramsay <frank.ramsay@hpe.com>, Russ Anderson <rja@hpe.com>
References: <20240419085146.175665-1-daniel@quora.org>
Content-Language: en-US
From: Prarit Bhargava <prarit@redhat.com>
In-Reply-To: <20240419085146.175665-1-daniel@quora.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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@quora.org>
> Reviewed-by: Steffen Persvold <sp@numascale.com>
> Reviewed-by: James Cleverdon <james.cleverdon.external@eviden.com>
> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
> Cc: Steve Wahl <steve.wahl@hpe.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: Frank Ramsay <frank.ramsay@hpe.com>
> Cc: Russ Anderson <rja@hpe.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@redhat.com>

P.