[PATCH 0/2] Fix community TSC ADJUST breakage causing TSC failure

[PATCH 0/2] Fix community TSC ADJUST breakage causing TSC failure

[Mike Travis]

Fix a breakage caused by enabling early tsc initialization which bypasses
a check that disables the forcing of TSC ADJUST to 0 for chassis 0.
This is common on systems where all the chassis start up asynchronously
so which chassis should have a TSC ADJUST value of 0 is not predictable.

The solution is to add a check earlier than this early tsc init to
disable the potential of it incorrectly adjusting TSC ADJUST values that
are already correctly initialized.

*  Patch 1 adds an early callable function (right after efi_init) that
   will check if this system might be a UV system.

*  Patch 2 adds code into setup_arch() which disables the call to
   tsc_early_init.  This function calls the TSC adjust initialization
   functions before UV setup has checked if BIOS has set the multi-chassis
   sync flags.  Further references are in the patch.

-- 

[PATCH 1/2] x86/platform/uv: Add is_early_uv_system check

[Mike Travis]

[-- Attachment #1: add_is_early_uv_system --]
[-- Type: text/plain, Size: 1386 bytes --]

Introduce is_early_uv_system() which uses efi.uv_systab to decide early
in the boot process whether we're on a UV system.

This is needed to skip other early setup/init code that might break UV
platform if done too early such as before necessary ACPI tables parsing
takes place.

Signed-off-by: Mike Travis <mike.travis@hpe.com>
Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
Reviewed-by: Russ Anderson <rja@hpe.com>
Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
---
 arch/x86/include/asm/uv/uv.h |    6 ++++++
 1 file changed, 6 insertions(+)

--- linux.orig/arch/x86/include/asm/uv/uv.h
+++ linux/arch/x86/include/asm/uv/uv.h
@@ -10,8 +10,13 @@ struct cpumask;
 struct mm_struct;
 
 #ifdef CONFIG_X86_UV
+#include <linux/efi.h>
 
 extern enum uv_system_type get_uv_system_type(void);
+static inline int is_early_uv_system(void)
+{
+	return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab);
+}
 extern int is_uv_system(void);
 extern int is_uv_hubless(void);
 extern void uv_cpu_init(void);
@@ -23,6 +28,7 @@ extern const struct cpumask *uv_flush_tl
 #else	/* X86_UV */
 
 static inline enum uv_system_type get_uv_system_type(void) { return UV_NONE; }
+static inline int is_early_uv_system(void)	{ return 0; }
 static inline int is_uv_system(void)	{ return 0; }
 static inline int is_uv_hubless(void)	{ return 0; }
 static inline void uv_cpu_init(void)	{ }

-- 

[PATCH 2/2] x86/tsc: Fix UV TSC initialization

[Mike Travis]

[-- Attachment #1: fix_tsc_early_init --]
[-- Type: text/plain, Size: 1477 bytes --]

Fix regression introduced by

commit cf7a63ef4e02 ("x86/tsc: Calibrate tsc only once")

as it changed setup_arch() so that it now calls tsc_early_init() before
acpi_boot_table_init() which is a necessary step, in the case of UV
systems, to inform tsc_sanitize_first_cpu() that we're on a platform
with async TSC resets as documented in

commit 341102c3ef29 ("x86/tsc: Add option that TSC on Socket 0 being non-zero is valid")

Fix by skipping tsc_early_init() on UV systems and let TSC initialization
take place later in tsc_init().

Fixes: cf7a63ef4e02 ("x86/tsc: Calibrate tsc only once") 
Signed-off-by: Mike Travis <mike.travis@hpe.com>
Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
Reviewed-by: Russ Anderson <rja@hpe.com>
Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
---
 arch/x86/kernel/setup.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- linux.orig/arch/x86/kernel/setup.c
+++ linux/arch/x86/kernel/setup.c
@@ -117,6 +117,7 @@
 #include <asm/microcode.h>
 #include <asm/kaslr.h>
 #include <asm/unwind.h>
+#include <asm/uv/uv.h>
 
 /*
  * max_low_pfn_mapped: highest direct mapped pfn under 4GB
@@ -1015,7 +1016,10 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	init_hypervisor_platform();
 
-	tsc_early_init();
+	/* UV TSC multi-chassis synchronization already set, don't change it */
+	if (!is_early_uv_system())
+		tsc_early_init();
+
 	x86_init.resources.probe_roms();
 
 	/* after parse_early_param, so could debug it */

-- 

Re: [PATCH 1/2] x86/platform/uv: Add is_early_uv_system check

[Thomas Gleixner]

On Mon, 1 Oct 2018, Mike Travis wrote:

> Introduce is_early_uv_system() which uses efi.uv_systab to decide early
> in the boot process whether we're on a UV system.
> 
> This is needed to skip other early setup/init code that might break UV
> platform if done too early such as before necessary ACPI tables parsing
> takes place.
> 
> Signed-off-by: Mike Travis <mike.travis@hpe.com>
> Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>

This SOB chain is wrong. I don't see how Hedi is involved in this. If this
wants to express that the patch was developed together please see
Documentation for the Co-developed-by tag.

> Reviewed-by: Russ Anderson <rja@hpe.com>
> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
> ---
>  arch/x86/include/asm/uv/uv.h |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> --- linux.orig/arch/x86/include/asm/uv/uv.h
> +++ linux/arch/x86/include/asm/uv/uv.h
> @@ -10,8 +10,13 @@ struct cpumask;
>  struct mm_struct;
>  
>  #ifdef CONFIG_X86_UV
> +#include <linux/efi.h>
>  
>  extern enum uv_system_type get_uv_system_type(void);
> +static inline int is_early_uv_system(void)

Can you please make this bool?

> +{
> +	return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab);
> +}

Thanks,

	tglx

Re: [PATCH 2/2] x86/tsc: Fix UV TSC initialization

[Thomas Gleixner]

On Mon, 1 Oct 2018, Mike Travis wrote:

> Fix regression introduced by
> 
> commit cf7a63ef4e02 ("x86/tsc: Calibrate tsc only once")
> 
> as it changed setup_arch() so that it now calls tsc_early_init() before
> acpi_boot_table_init() which is a necessary step, in the case of UV
> systems, to inform tsc_sanitize_first_cpu() that we're on a platform
> with async TSC resets as documented in
> 
> commit 341102c3ef29 ("x86/tsc: Add option that TSC on Socket 0 being non-zero is valid")
> 
> Fix by skipping tsc_early_init() on UV systems and let TSC initialization
> take place later in tsc_init().
> 
> Fixes: cf7a63ef4e02 ("x86/tsc: Calibrate tsc only once") 
> Signed-off-by: Mike Travis <mike.travis@hpe.com>
> Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>

See previous mail.

> Reviewed-by: Russ Anderson <rja@hpe.com>
> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
> ---
>  arch/x86/kernel/setup.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --- linux.orig/arch/x86/kernel/setup.c
> +++ linux/arch/x86/kernel/setup.c
> @@ -117,6 +117,7 @@
>  #include <asm/microcode.h>
>  #include <asm/kaslr.h>
>  #include <asm/unwind.h>
> +#include <asm/uv/uv.h>
>  
>  /*
>   * max_low_pfn_mapped: highest direct mapped pfn under 4GB
> @@ -1015,7 +1016,10 @@ void __init setup_arch(char **cmdline_p)
>  	 */
>  	init_hypervisor_platform();
>  
> -	tsc_early_init();
> +	/* UV TSC multi-chassis synchronization already set, don't change it */
> +	if (!is_early_uv_system())
> +		tsc_early_init();

tsc_early_init() has already a check which makes it not invoke the
calibration code, so please put your uv check into that.

Thanks,

	tglx

Re: [PATCH 0/2] Fix community TSC ADJUST breakage causing TSC failure

[Thomas Gleixner]

On Mon, 1 Oct 2018, Mike Travis wrote:

I'm not sure what a 'community TSC adjust breakage' means.

Re: [PATCH 1/2] x86/platform/uv: Add is_early_uv_system check

[Mike Travis]

On 10/1/2018 11:20 PM, Thomas Gleixner wrote:
> On Mon, 1 Oct 2018, Mike Travis wrote:
> 
>> Introduce is_early_uv_system() which uses efi.uv_systab to decide early
>> in the boot process whether we're on a UV system.
>>
>> This is needed to skip other early setup/init code that might break UV
>> platform if done too early such as before necessary ACPI tables parsing
>> takes place.
>>
>> Signed-off-by: Mike Travis <mike.travis@hpe.com>
>> Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
> 
> This SOB chain is wrong. I don't see how Hedi is involved in this. If this
> wants to express that the patch was developed together please see
> Documentation for the Co-developed-by tag.

Hedi suggested a "Suggested-by:" tag, would that suffice?
> 
>> Reviewed-by: Russ Anderson <rja@hpe.com>
>> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
>> ---
>>   arch/x86/include/asm/uv/uv.h |    6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> --- linux.orig/arch/x86/include/asm/uv/uv.h
>> +++ linux/arch/x86/include/asm/uv/uv.h
>> @@ -10,8 +10,13 @@ struct cpumask;
>>   struct mm_struct;
>>   
>>   #ifdef CONFIG_X86_UV
>> +#include <linux/efi.h>
>>   
>>   extern enum uv_system_type get_uv_system_type(void);
>> +static inline int is_early_uv_system(void)
> 
> Can you please make this bool?
> 
>> +{
>> +	return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab);
>> +}
> 
> Thanks,
> 
> 	tglx
> 

Re: [PATCH 2/2] x86/tsc: Fix UV TSC initialization

[Mike Travis]

On 10/1/2018 11:22 PM, Thomas Gleixner wrote:
> On Mon, 1 Oct 2018, Mike Travis wrote:
> 
>> Fix regression introduced by
>>
>> commit cf7a63ef4e02 ("x86/tsc: Calibrate tsc only once")
>>
>> as it changed setup_arch() so that it now calls tsc_early_init() before
>> acpi_boot_table_init() which is a necessary step, in the case of UV
>> systems, to inform tsc_sanitize_first_cpu() that we're on a platform
>> with async TSC resets as documented in
>>
>> commit 341102c3ef29 ("x86/tsc: Add option that TSC on Socket 0 being non-zero is valid")
>>
>> Fix by skipping tsc_early_init() on UV systems and let TSC initialization
>> take place later in tsc_init().
>>
>> Fixes: cf7a63ef4e02 ("x86/tsc: Calibrate tsc only once")
>> Signed-off-by: Mike Travis <mike.travis@hpe.com>
>> Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
> 
> See previous mail.
> 
>> Reviewed-by: Russ Anderson <rja@hpe.com>
>> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
>> ---
>>   arch/x86/kernel/setup.c |    6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> --- linux.orig/arch/x86/kernel/setup.c
>> +++ linux/arch/x86/kernel/setup.c
>> @@ -117,6 +117,7 @@
>>   #include <asm/microcode.h>
>>   #include <asm/kaslr.h>
>>   #include <asm/unwind.h>
>> +#include <asm/uv/uv.h>
>>   
>>   /*
>>    * max_low_pfn_mapped: highest direct mapped pfn under 4GB
>> @@ -1015,7 +1016,10 @@ void __init setup_arch(char **cmdline_p)
>>   	 */
>>   	init_hypervisor_platform();
>>   
>> -	tsc_early_init();
>> +	/* UV TSC multi-chassis synchronization already set, don't change it */
>> +	if (!is_early_uv_system())
>> +		tsc_early_init();
> 
> tsc_early_init() has already a check which makes it not invoke the
> calibration code, so please put your uv check into that.

Will do, thanks.

> 
> Thanks,
> 
> 	tglx
> 

Re: [PATCH 1/2] x86/platform/uv: Add is_early_uv_system check

[Thomas Gleixner]

On Tue, 2 Oct 2018, Mike Travis wrote:
> On 10/1/2018 11:20 PM, Thomas Gleixner wrote:
> > On Mon, 1 Oct 2018, Mike Travis wrote:
> > 
> > > Introduce is_early_uv_system() which uses efi.uv_systab to decide early
> > > in the boot process whether we're on a UV system.
> > > 
> > > This is needed to skip other early setup/init code that might break UV
> > > platform if done too early such as before necessary ACPI tables parsing
> > > takes place.
> > > 
> > > Signed-off-by: Mike Travis <mike.travis@hpe.com>
> > > Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
> > 
> > This SOB chain is wrong. I don't see how Hedi is involved in this. If this
> > wants to express that the patch was developed together please see
> > Documentation for the Co-developed-by tag.
> 
> Hedi suggested a "Suggested-by:" tag, would that suffice?

Sure, instead of the SOB. 

Thanks,

	tglx

Re: [PATCH 0/2] Fix community TSC ADJUST breakage causing TSC failure

[Travis, Mike]

On 10/1/2018 11:24 PM, Thomas Gleixner wrote:
> On Mon, 1 Oct 2018, Mike Travis wrote:
> 
> I'm not sure what a 'community TSC adjust breakage' means.
> 

I probably should have cleaned that up.  The title is from our internal 
bug report and we target where the earliest place where the fix should 
be applied.  "Community" is shorthand for the Linux Community Kernel (as 
opposed to internal or distro specific problems).

Re: [PATCH 0/2] Fix community TSC ADJUST breakage causing TSC failure

[Thomas Gleixner]

On Tue, 2 Oct 2018, Travis, Mike wrote:
> On 10/1/2018 11:24 PM, Thomas Gleixner wrote:
> > On Mon, 1 Oct 2018, Mike Travis wrote:
> > 
> > I'm not sure what a 'community TSC adjust breakage' means.
> > 
> 
> I probably should have cleaned that up.  The title is from our internal 
> bug report and we target where the earliest place where the fix should 
> be applied.  "Community" is shorthand for the Linux Community Kernel (as 
> opposed to internal or distro specific problems).

That was my guess. I just found the subject line amusing.

Thanks,

	tglx