Messages in this thread | | | Date | Fri, 22 Mar 2019 16:54:50 +0100 | From | Borislav Petkov <> | Subject | Re: [RFC PATCH] x86/cpu/hygon: Fix phys_proc_id calculation logic for multi-die processor |
| |
On Fri, Mar 22, 2019 at 06:43:00PM +0800, Pu Wen wrote: > 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 codes direct use "phys_proc_id = initial_apicid >> bits", which
Use proper english please - there's no "codes"
> calc phys_proc_id from initial_apicid by shifting *bits*, will get
... or "calc"
> 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. > > ApicId 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.
Sounds to me like you're programming the initial APIC ID not the same way as AMD do...
> Reference: > [1] https://www.amd.com/system/files/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf > > Signed-off-by: Pu Wen <puwen@hygon.cn> > --- > arch/x86/kernel/cpu/hygon.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/x86/kernel/cpu/hygon.c b/arch/x86/kernel/cpu/hygon.c > index cf25405..2df6dd9 100644 > --- a/arch/x86/kernel/cpu/hygon.c > +++ b/arch/x86/kernel/cpu/hygon.c > @@ -87,6 +87,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 >> 6;
That 6 is a magic number, I assume?
-- Regards/Gruss, Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
| |