From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933072AbcFBWyo (ORCPT ); Thu, 2 Jun 2016 18:54:44 -0400 Received: from one.firstfloor.org ([193.170.194.197]:46111 "EHLO one.firstfloor.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932166AbcFBWyn (ORCPT ); Thu, 2 Jun 2016 18:54:43 -0400 Date: Thu, 2 Jun 2016 15:54:40 -0700 From: Andi Kleen To: Henrique de Moraes Holschuh Cc: Andi Kleen , x86@kernel.org, linux-kernel@vger.kernel.org, Andi Kleen Subject: Re: [PATCH] x86: Report Intel platform_id in /proc/cpuinfo Message-ID: <20160602225439.GF13997@two.firstfloor.org> References: <1464729894-108356-1-git-send-email-andi@firstfloor.org> <20160602182707.GA15431@khazad-dum.debian.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160602182707.GA15431@khazad-dum.debian.net> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 02, 2016 at 03:27:07PM -0300, Henrique de Moraes Holschuh wrote: > On Tue, 31 May 2016, Andi Kleen wrote: > > We have a need to distinguish systems based on their platform ID. > > For example this is useful to distinguish systems with L4 cache > > versus ones without. > > > > There is a 5 bit identifier (also called processor flags) in > > There is a 3 bit identifier... Thanks. > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c > > index ee81c54..6244a88 100644 > > --- a/arch/x86/kernel/cpu/microcode/intel.c > > +++ b/arch/x86/kernel/cpu/microcode/intel.c > > @@ -812,6 +812,7 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) > > /* get processor flags from MSR 0x17 */ > > rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); > > csig->pf = 1 << ((val[1] >> 18) & 7); > > + cpu_data(cpu_num).platform_id = (val[1] >> 18) & 7; > > See below. It might be better to have "cpu_data(cpu_num).platform_id = > csig->pf" instead. Ok. > > > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c > > index 18ca99f..1c4e4f5 100644 > > --- a/arch/x86/kernel/cpu/proc.c > > +++ b/arch/x86/kernel/cpu/proc.c > > @@ -76,6 +76,8 @@ static int show_cpuinfo(struct seq_file *m, void *v) > > seq_puts(m, "stepping\t: unknown\n"); > > if (c->microcode) > > seq_printf(m, "microcode\t: 0x%x\n", c->microcode); > > + if (c->platform_id) > > + seq_printf(m, "platform_id\t: %d\n", c->platform_id); > > platform_id can meaningfully be zero on Intel, and in fact it often is (just > look for output from the microcode driver that logs pf=0x1). In those > processors, (MSR(17) >> 18 & 7) is zero, and csig->pf is 1. > > May I humbly suggest using the mask "(1 << value)" notation for platform_id > as used by the microcode driver? We already do it for csig->pf, and it has > the advantage that mask notation will never be zero (so, the field is always > non-zero where relevant). That would be confusing because the value is used in some documents. I would prefer to match them. > > Alterntively, the patch could be changed to always print platform_id on > Intel processors, instead of just printing it out when it is non-zero. What I can do is to add an extra flag and print it when the flag is set, even if it is zero. Thanks for the review. -Andi