Re: [PATCH v3] x86/tsc: Disable clocksource watchdog for TSC on recent UV

[Dimitri Sivanich <sivanich@hpe.com>]

On Tue, Oct 07, 2025 at 09:48:58AM -0700, Dave Hansen wrote:
> On 10/7/25 09:32, Dimitri Sivanich wrote:
> > +static inline int is_uvx_hub(void) { return 0; }
> > +static inline int is_uvy_hub(void) { return 0; }
> > +static inline int is_uv_hub(void) { return 0; }
> >  
> >  #endif	/* X86_UV */
> >  
> > diff --git a/arch/x86/include/asm/uv/uv_hub.h b/arch/x86/include/asm/uv/uv_hub.h
> > index ea877fd83114..6e085ce8fc02 100644
> > --- a/arch/x86/include/asm/uv/uv_hub.h
> > +++ b/arch/x86/include/asm/uv/uv_hub.h
> > @@ -246,6 +246,7 @@ static inline int is_uv5_hub(void) { return is_uv(UV5); }
> >   * then test if is UV4.
> >   */
> >  
> > +#ifdef CONFIG_X86_UV
> >  /* UVX class: UV2,3,4 */
> >  static inline int is_uvx_hub(void) { return is_uv(UVX); }
> >  
> > @@ -254,6 +255,7 @@ static inline int is_uvy_hub(void) { return is_uv(UVY); }
> >  
> >  /* Any UV Hubbed System */
> >  static inline int is_uv_hub(void) { return is_uv(UV_ANY); }
> > +#endif
> 
> Defining those helpers across two different headers seems like a recipe
> for pain.
> 
> I suspect a big chunk of those stubs (and their #ifdefs could completely
> go away if you _just_ did:
> 
> #ifdef CONFIG_X86_UV
> static inline int uv_hub_type(void)
> {
>         return uv_hub_info->hub_type;
> }
> #else
> static inline int uv_hub_type(void)
> {
> 	return 0;
> }
> #endif
> 
> In any case, this is precisely the kind of patch that would be best
> refactored into two piece: one to expose the is_uv...() function and
> another to actually use it.
> 
> Also, at this point, this:
> 
> >  	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> >  	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
> >  	    boot_cpu_has(X86_FEATURE_TSC_ADJUST) &&
> > -	    topology_max_packages() <= 4)
> > +	    (topology_max_packages() <= 4 || is_uvy_hub()))
> >  		tsc_disable_clocksource_watchdog();
> 
> has IMNHO gotten out of hand.
> 
> It should probably be:
> 
>  	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
>  	    boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
>  	    boot_cpu_has(X86_FEATURE_TSC_ADJUST) &&
> 	    platform_is_exempt_from_watchdog())
>  		tsc_disable_clocksource_watchdog();
> 
> In addition, 233756a640be talked quite a bit about *why* the 4-socket
> line was chosen. This needs to have a similar explanation for UV systems.

I will add the following to the description:

"HPE UV hardware and firmware is designed to ensure a reliable and synchronized
 TSC mechanism.  Comparing the TSC against secondary clocksources can result in
 false positives due to variable access latency caused by system traffic.  The
 best course of action against these false positives has been found to simply
 disable watchdog checking of the TSC.  Currently we recommend that customers
 apply 'tsc=nowatchdog' to the kernel command line."

The requested code changes will be forthcoming.