Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756326AbdGXOp3 (ORCPT ); Mon, 24 Jul 2017 10:45:29 -0400 Received: from mx2.suse.de ([195.135.220.15]:47057 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756304AbdGXOpU (ORCPT ); Mon, 24 Jul 2017 10:45:20 -0400 Date: Mon, 24 Jul 2017 16:44:45 +0200 From: Borislav Petkov To: Suravee Suthikulpanit Cc: linux-kernel@vger.kernel.org, x86@kernel.org, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, peterz@infradead.org, Yazen.Ghannam@amd.com, Andreas Herrmann Subject: Re: [PATCH v4 2/2] x86/amd: Fixup cpu_core_id for family17h downcore configuration Message-ID: <20170724144445.GA31300@nazgul.tnic> References: <1500888165-2345-1-git-send-email-suravee.suthikulpanit@amd.com> <1500888165-2345-3-git-send-email-suravee.suthikulpanit@amd.com> <20170724111408.GC28024@nazgul.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4679 Lines: 110 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) --