Re: [Patch v2 2/2] x86/tsc: use logical_packages as a better estimation of socket numbers

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

On Fri, Jun 16, 2023 at 07:23:12PM +0800, Zhang, Rui wrote:
> On Fri, 2023-06-16 at 11:42 +0200, Peter Zijlstra wrote:
> > On Fri, Jun 16, 2023 at 09:19:18AM +0000, Zhang, Rui wrote:
> > 
> > > According to the MADT, there are indeed 40 valid CPUs. And then 80
> > > CPUs
> > > with 
> > > 
> > > APIC ID         : FF
> > > enabled         : 0
> > > Online capable  : 0
> > > 
> > > a dumb question, why are these CPUs added into the possible_mask?
> > > I can dig into this later but I just don't have a quick answer at
> > > the
> > > moment.
> > 
> > I really don't know.. I've not gotten around to reading that part of
> > the
> > x86 code yet.
> > 
> > 
> I did a double check.
> 
> The MADT is composed of
> 
> 1. 40 valid LAPIC entries.
> 2. 80 invalid LAPIC entries with
> 	APIC ID : FF
> 	Enabled : 0
> 	Online capable: 0
>    I'm mot sure why "Online capable" is decoded because this new bit is
>    introduced in ACPI 6.3. Maybe a problem in the acpica tool?
>    These entries are ignored because of the invalid APIC ID.
> 3. 120 x2APIC entries with
> 	APIC ID : valid value
> 	Enabled : 0
>    As "Online capable bit" is not supported, these 120 x2APIC entries
>    are counted as possible CPUs.
 
Nice shot!

So IIUC, this is a firmware bug, and deserves a warning or error
message? And without 'possible_cpus' or 'nr_cpus' parameter, system
will waste quite some memory due to "nr_cpu_ids == 160".

From Peter's log:

	[    2.664257] smpboot: Max logical packages: 8

it also revealed again the problem in 'calculate_max_logical_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);
	"

But 'logical_packages' should still be correct in this case.

Thanks,
Feng

> That is why we got 160 possible CPUs.
> 
> thanks,
> rui
> 
> 
>