linux-kernel - Re: [PATCH v4 2/2] x86/amd: Fixup cpu_core_id for family17h downcore configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170724144445.GA31300@nazgul.tnic>
Date:   Mon, 24 Jul 2017 16:44:45 +0200
From:   Borislav Petkov <bp@...e.de>
To:     Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de,
        mingo@...hat.com, hpa@...or.com, peterz@...radead.org,
        Yazen.Ghannam@....com, Andreas Herrmann <aherrmann@...e.com>
Subject: Re: [PATCH v4 2/2] x86/amd: Fixup cpu_core_id for family17h downcore
 configuration

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)
--