[RFC PATCH v2] x86/cpu/hygon: Fix phys_proc_id calculation logic for multi-die processor

[RFC PATCH v2] x86/cpu/hygon: Fix phys_proc_id calculation logic for multi-die processor

[Pu Wen]

v1->v2:
  - Define bit 6 of ApicId with a descriptive name.
  - Add description about why Hygon need to do something different than
    what AMD does.


For Hygon family 18h multi-die processor platform, which support 1-Die/
2-Die/4-Die per socket, the system view is shown in the following system
topology.

System View (with 1-Die 2-Socket):
           |------------|
         ------       -----
 SOCKET0 | D0 |       | D1 |  SOCKET1
         ------       -----

System View (with 2-Die 2-socket):
           --------------------
           |     -------------|------
           |     |            |     |
         ------------       ------------
 SOCKET0 | D1 -- D0 |       | D3 -- D2 | SOCKET1
         ------------       ------------

System View (with 4-Die 2-Socket) :
           --------------------
           |     -------------|------
           |     |            |     |
         ------------       ------------
         | D1 -- D0 |       | D7 -- D6 |
         | |  \/ |  |       | |  \/ |  |
 SOCKET0 | |  /\ |  |       | |  /\ |  | SOCKET1
         | D2 -- D3 |       | D4 -- D5 |
         ------------       ------------
           |     |            |     |
           ------|------------|     |
                 --------------------

Current code direct use "phys_proc_id = initial_apicid >> bits", which
calculate phys_proc_id from initial_apicid by shifting *bits*, will get
wrong phys_proc_id for Hygon family 18h 1-Die/2-Die 2-socket system.

According to document [1] section 2.1.11.1, the *bits* is the value of
CPUID_Fn80000008_ECX[12:15]. The value may be 4, 5 or 6. The definitions
are: 4 - 1 die, 5 - 2 die, 6 - 3/4 die.

Hygon programs the initial ApicId in the same way as AMD. The ApicId is
read from CPUID_Fn00000001_EBX (see section 2.1.11.1 of referrence [1])
and the definition is as below (see section 2.1.10.2.1.3 of [1]):
             -------------------------------------------------
         Bit |     6     |   5  4  |    3   |    2   1   0   |
             |-----------|---------|--------|----------------|
         IDs | Socket ID | Node ID | CCX ID | Core/Thread ID |
             -------------------------------------------------

So for 3/4-Die CPU the *bits* is 6, which is same to ApicID definition
field, which still can get correct socket ID for 2-socket system. But
for 1-Die or 2-Die CPU the *bits* is 4 or 5, which will cause the right
shifted result is not exactly the value of socket ID.

For Hygon family 18h CPU the socket ID should be obtained from ApicId[6].
To fix the problem and match ApicID field definition, adjust the shift
bits to 6 for all Hygon family 18h multi-die CPUs.

Because AMD doesn't have 2-Socket system with 1-Die/2-Die processors(see
reference [2]), so it will return the right result when getting physical
ID in AMD system. Therefore this doesn't need to be changed in AMD code
but only for Hygon.

Reference:
[1] https://www.amd.com/system/files/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf
[2] https://www.amd.com/en/products/specifications/processors

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 arch/x86/kernel/cpu/hygon.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/cpu/hygon.c b/arch/x86/kernel/cpu/hygon.c
index cf25405..415621d 100644
--- a/arch/x86/kernel/cpu/hygon.c
+++ b/arch/x86/kernel/cpu/hygon.c
@@ -19,6 +19,8 @@
 
 #include "cpu.h"
 
+#define APICID_SOCKET_ID_BIT 6
+
 /*
  * nodes_per_socket: Stores the number of nodes per socket.
  * Refer to CPUID Fn8000_001E_ECX Node Identifiers[10:8]
@@ -87,6 +89,9 @@ static void hygon_get_topology(struct cpuinfo_x86 *c)
 		if (!err)
 			c->x86_coreid_bits = get_count_order(c->x86_max_cores);
 
+		/* Socket ID is ApicId[6] for these processors. */
+		c->phys_proc_id = c->apicid >> APICID_SOCKET_ID_BIT;
+
 		cacheinfo_hygon_init_llc_id(c, cpu, node_id);
 	} else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {
 		u64 value;
-- 
2.7.4

Re: [RFC PATCH v2] x86/cpu/hygon: Fix phys_proc_id calculation logic for multi-die processor

[Borislav Petkov]

On Sat, Mar 23, 2019 at 11:42:20PM +0800, Pu Wen wrote:
> 
> v1->v2:
>   - Define bit 6 of ApicId with a descriptive name.
>   - Add description about why Hygon need to do something different than
>     what AMD does.

In the future, such patch changelogs belong right...

...

> Signed-off-by: Pu Wen <puwen@hygon.cn>
> ---

<--- here.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH v2] x86/cpu/hygon: Fix phys_proc_id calculation logic for multi-die processor

[Pu Wen]

On 2019/3/24 0:40, Borislav Petkov wrote:
> On Sat, Mar 23, 2019 at 11:42:20PM +0800, Pu Wen wrote:
>>
>> v1->v2:
>>    - Define bit 6 of ApicId with a descriptive name.
>>    - Add description about why Hygon need to do something different than
>>      what AMD does.
> 
> In the future, such patch changelogs belong right...
> 
> ...
> 
>> Signed-off-by: Pu Wen <puwen@hygon.cn>
>> ---
> 
> <--- here.

Okay, thanks for the suggestion.

-- 
Regards,
Pu Wen

[tip:x86/cpu] x86/CPU/hygon: Fix phys_proc_id calculation logic for multi-die processors

[tip-bot for Pu Wen]

Commit-ID:  e0ceeae708cebf22c990c3d703a4ca187dc837f5
Gitweb:     https://git.kernel.org/tip/e0ceeae708cebf22c990c3d703a4ca187dc837f5
Author:     Pu Wen <puwen@hygon.cn>
AuthorDate: Sat, 23 Mar 2019 23:42:20 +0800
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Sat, 23 Mar 2019 17:41:09 +0100

x86/CPU/hygon: Fix phys_proc_id calculation logic for multi-die processors

The Hygon family 18h multi-die processor platform supports 1, 2 or
4-Dies per socket. The topology looks like this:

  System View (with 1-Die 2-Socket):
             |------------|
           ------       -----
   SOCKET0 | D0 |       | D1 |  SOCKET1
           ------       -----

  System View (with 2-Die 2-socket):
             --------------------
             |     -------------|------
             |     |            |     |
           ------------       ------------
   SOCKET0 | D1 -- D0 |       | D3 -- D2 | SOCKET1
           ------------       ------------

  System View (with 4-Die 2-Socket) :
             --------------------
             |     -------------|------
             |     |            |     |
           ------------       ------------
           | D1 -- D0 |       | D7 -- D6 |
           | |  \/ |  |       | |  \/ |  |
   SOCKET0 | |  /\ |  |       | |  /\ |  | SOCKET1
           | D2 -- D3 |       | D4 -- D5 |
           ------------       ------------
             |     |            |     |
             ------|------------|     |
                   --------------------

Currently

  phys_proc_id = initial_apicid >> bits

calculates the physical processor ID from the initial_apicid by shifting
*bits*.

However, this does not work for 1-Die and 2-Die 2-socket systems.

According to document [1] section 2.1.11.1, the bits is the value of
CPUID_Fn80000008_ECX[12:15]. The possible values are 4, 5 or 6 which
mean:

  4 - 1 die
  5 - 2 dies
  6 - 3/4 dies.

Hygon programs the initial ApicId the same way as AMD. The ApicId is
read from CPUID_Fn00000001_EBX (see section 2.1.11.1 of referrence [1])
and the definition is as below (see section 2.1.10.2.1.3 of [1]):

      -------------------------------------------------
  Bit |     6     |   5  4  |    3   |    2   1   0   |
      |-----------|---------|--------|----------------|
  IDs | Socket ID | Node ID | CCX ID | Core/Thread ID |
      -------------------------------------------------

So for 3/4-Die configurations, the bits variable is 6, which is the same
as the ApicID definition field.

For 1-Die and 2-Die configurations, bits is 4 or 5, which will cause the
right shifted result to not be exactly the value of socket ID.

However, the socket ID should be obtained from ApicId[6]. To fix the
problem and match the ApicID field definition, set the shift bits to 6
for all Hygon family 18h multi-die CPUs.

Because AMD doesn't have 2-Socket systems with 1-Die/2-Die processors
(see reference [2]), this doesn't need to be changed on the AMD side but
only for Hygon.

References:
[1] https://www.amd.com/system/files/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf
[2] https://www.amd.com/en/products/specifications/processors

 [bp: heavily massage commit message. ]

Signed-off-by: Pu Wen <puwen@hygon.cn>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Lendacky <Thomas.Lendacky@amd.com>
Cc: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/1553355740-19999-1-git-send-email-puwen@hygon.cn
---
 arch/x86/kernel/cpu/hygon.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/cpu/hygon.c b/arch/x86/kernel/cpu/hygon.c
index cf25405444ab..415621ddb8a2 100644
--- a/arch/x86/kernel/cpu/hygon.c
+++ b/arch/x86/kernel/cpu/hygon.c
@@ -19,6 +19,8 @@
 
 #include "cpu.h"
 
+#define APICID_SOCKET_ID_BIT 6
+
 /*
  * nodes_per_socket: Stores the number of nodes per socket.
  * Refer to CPUID Fn8000_001E_ECX Node Identifiers[10:8]
@@ -87,6 +89,9 @@ static void hygon_get_topology(struct cpuinfo_x86 *c)
 		if (!err)
 			c->x86_coreid_bits = get_count_order(c->x86_max_cores);
 
+		/* Socket ID is ApicId[6] for these processors. */
+		c->phys_proc_id = c->apicid >> APICID_SOCKET_ID_BIT;
+
 		cacheinfo_hygon_init_llc_id(c, cpu, node_id);
 	} else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {
 		u64 value;