Re: [PATCH v4 2/2] x86/amd: Fixup cpu_core_id for family17h downcore configuration
From: Borislav Petkov
Date: Mon Jul 24 2017 - 10:45:34 EST
On Mon, Jul 24, 2017 at 09:14:18PM +0700, Suravee Suthikulpanit wrote:
> Actually, this is not totally accurate. My apology. This patch is
> mainly fix to incorrect core ID in /proc/cpuinfo.
So you're "fixing" only some numbering thing. Because core_id doesn't
have any influence on anything. Here's on an Intel box I have here:
processor : 0 physical id : 0 core id : 0
processor : 1 physical id : 1 core id : 0
processor : 2 physical id : 2 core id : 0
processor : 3 physical id : 3 core id : 0
processor : 4 physical id : 0 core id : 8
processor : 5 physical id : 1 core id : 8
processor : 6 physical id : 2 core id : 8
processor : 7 physical id : 3 core id : 8
processor : 8 physical id : 0 core id : 2
processor : 9 physical id : 1 core id : 2
processor : 10 physical id : 2 core id : 2
processor : 11 physical id : 3 core id : 2
processor : 12 physical id : 0 core id : 10
processor : 13 physical id : 1 core id : 10
processor : 14 physical id : 2 core id : 10
processor : 15 physical id : 3 core id : 10
processor : 16 physical id : 0 core id : 1
processor : 17 physical id : 1 core id : 1
processor : 18 physical id : 2 core id : 1
processor : 19 physical id : 3 core id : 1
processor : 20 physical id : 0 core id : 9
processor : 21 physical id : 1 core id : 9
processor : 22 physical id : 2 core id : 9
processor : 23 physical id : 3 core id : 9
processor : 24 physical id : 0 core id : 3
processor : 25 physical id : 1 core id : 3
processor : 26 physical id : 2 core id : 3
processor : 27 physical id : 3 core id : 3
processor : 28 physical id : 0 core id : 11
processor : 29 physical id : 1 core id : 11
processor : 30 physical id : 2 core id : 11
processor : 31 physical id : 3 core id : 11
processor : 32 physical id : 0 core id : 0
processor : 33 physical id : 1 core id : 0
processor : 34 physical id : 2 core id : 0
processor : 35 physical id : 3 core id : 0
processor : 36 physical id : 0 core id : 8
processor : 37 physical id : 1 core id : 8
processor : 38 physical id : 2 core id : 8
processor : 39 physical id : 3 core id : 8
processor : 40 physical id : 0 core id : 2
processor : 41 physical id : 1 core id : 2
processor : 42 physical id : 2 core id : 2
processor : 43 physical id : 3 core id : 2
processor : 44 physical id : 0 core id : 10
processor : 45 physical id : 1 core id : 10
processor : 46 physical id : 2 core id : 10
processor : 47 physical id : 3 core id : 10
processor : 48 physical id : 0 core id : 1
processor : 49 physical id : 1 core id : 1
processor : 50 physical id : 2 core id : 1
processor : 51 physical id : 3 core id : 1
processor : 52 physical id : 0 core id : 9
processor : 53 physical id : 1 core id : 9
processor : 54 physical id : 2 core id : 9
processor : 55 physical id : 3 core id : 9
processor : 56 physical id : 0 core id : 3
processor : 57 physical id : 1 core id : 3
processor : 58 physical id : 2 core id : 3
processor : 59 physical id : 3 core id : 3
processor : 60 physical id : 0 core id : 11
processor : 61 physical id : 1 core id : 11
processor : 62 physical id : 2 core id : 11
processor : 63 physical id : 3 core id : 11
So those core id numbers can be anything as long as the cpumasks used by
the scheduler are correct.
> This is due to the cpu_core_id fixup in amd_get_topology() below:
>
> /* fixup multi-node processor information */
> if (nodes_per_socket > 1) {
> u32 cus_per_node;
>
> set_cpu_cap(c, X86_FEATURE_AMD_DCM);
> cus_per_node = c->x86_max_cores / nodes_per_socket;
>
> /* core id has to be in the [0 .. cores_per_node - 1] range */
> c->cpu_core_id %= cus_per_node;
> }
AFAICT, Andreas did this for MC at the time:
4a376ec3a259 ("x86: Fix CPU llc_shared_map information for AMD Magny-Cours")
but I don't think we need to care about core_ids fitting into the node
range anymore. For the above reason - topology doesn't use core ids.
So you can just as well let ->cpu_core_id be derived from the
->initial_apicid as it is being done now in amd_detect_cmp().
In order not to cause any more confusion, you can limit the above fixup
to anything below F17h so that we don't upset existing users and add a
big fat comment as to why we're doing this. But if it is only a silly
numbering thing, I don't see the need for doing that jumping through
hoops.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix ImendÃrffer, Jane Smithard, Graham Norton, HRB 21284 (AG NÃrnberg)
--