Re: [RFC PATCH] x86/tsc: use topology_max_packages() in tsc watchdog check

[Feng Tang <feng.tang@intel.com>]

On Mon, Oct 17, 2022 at 04:15:24PM -0700, Dave Hansen wrote:
> On 10/17/22 06:29, Feng Tang wrote:
> > +	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() <= 2)
> > +		clocksource_tsc.flags &= ~CLOCK_SOURCE_MUST_VERIFY;
> 
> I couldn't help but notice the comment in here:
> 
> > void __init calculate_max_logical_packages(void)
> > {
> >         int ncpus;
> > 
> >         /*
> >          * Today neither Intel nor AMD support heterogeneous systems so
> >          * extrapolate the boot cpu's data to all packages.
> >          */
> >         ncpus = cpu_data(0).booted_cores * topology_max_smt_threads();
> >         __max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
> >         pr_info("Max logical packages: %u\n", __max_logical_packages);
> > }
> 
> Could you double check for me that the Alder Lake combination Core/Atom
> CPUs don't count as "heterogeneous systems" in this case?

Thanks for the great catch! This API seems to be obsolete and shouldn't
be used, even if the number happens to equal to 'logical_packages'  

I found a Alder Lake platform which has 6 P-cores and 8 E-cores (Thanks
Rui for sharing it), the log shows "smpboot: Max logical packages: 1",
and 'cat /proc/cpuinfo | grep "physical id"' shows all its CPU's package
ID is '0'

When writing the RFC patch, I followed the smp_init() discussed by Peter
and Rui, where 'logical_packages' is the appropriate number, which is
updated in topology_update_package_map() after each CPU gets initialized.

Possible fix for this could be:
* export 'logical_packages' for tsc use
* update calculate_max_logical_packages() to return 'logical_packages'

Thoughts?

Thanks,
Feng