Re: [PATCH 1/2] x86/CPU/AMD: Present package as die instead of socket

[Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>]

On 6/27/17 21:21, Thomas Gleixner wrote:
> On Tue, 27 Jun 2017, Suravee Suthikulpanit wrote:
>> On 6/27/17 17:48, Borislav Petkov wrote:
>>> On Tue, Jun 27, 2017 at 01:40:52AM -0500, Suravee Suthikulpanit wrote:
>>>> However, this is not the case on AMD family17h multi-die processor
>>>> platforms, which can have up to 4 dies per socket as shown in the
>>>> following system topology.
>>>
>>> So what exactly does that mean? A die is a package on ZN and you can have up
>>> to 4 packages on a physical socket?
>>
>> Yes. 4 packages (or 4 dies, or 4 NUMA nodes) in a socket.
>
> And why is this relevant at all?
>
> The kernel does not care about sockets. Sockets are electromechanical
> components and completely irrelevant.
>
> The kernel cares about :
>
>     Threads	 - Single scheduling unit
>
>     Cores	 - Contains one or more threads
>
>     Packages	 - Contains one or more cores. The cores share L3.
>
>     NUMA Node	 - Contains one or more Packages which share a memory
>     	 	   controller.
>
> 		   I'm not aware of x86 systems which have several Packages
> 		   sharing a memory controller, so Package == NUMA Node
> 		   (but I might be wrong here).

According to the definition of Packages above, x86_has_numa_in_package does not 
map to the topology we are trying to present here since we have 2 L3 caches 
(packages) in a NUMA node. So, I am not sure if x86_has_numa_in_package make 
sense here.

>     Platform	 - Contains one or more Numa Nodes
>
> All the kernel is interested in is the above and the NUMA Node distance so
> it knows about memory access latencies. No sockets, no MCMs, that's all
> completely useless for the scheduler.

Agree to your point here that we do not care about socket, MCMs, and anything 
that has to do with the physical grouping of the cores here. It does not really 
mean anything as long as we can correctly describe the NUMA node topology.

The reason we are trying to present "package == NUMA node (die)" here is because 
the topology.txt defines package to contain a number of cores plus shared 
resources (e.g. DRAM controller, shared caches, etc). Since the 
cpuinfo_x86.phys_proc_id is also defined as the physical ID of the package, and 
it is used in the arch/x86/kernel/smpboot.c: match_die(), we think it would make 
sense to represent package == NUMA node (die) here instead of a socket.

> So if the current CPUID stuff gives you the same phycial package ID for all
> packages in a MCM, then this needs to be fixed at the CPUID/ACPI/BIOS level
> and not hacked around in the kernel.

Unfortunately, the information from CPUID (since the older families) has been 
representing the whole physical package topology, which is why we need fix-up in 
the AMD specific code (in arch/x86/kernel/cpu/amd.c) to correctly derive 
topology information as defined in the topology.txt.

Thanks,
Suravee