From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86: always store APIC ID of CPU Date: Thu, 26 Jun 2014 12:02:11 +0100 Message-ID: <53ABFDB3.1060204@citrix.com> References: <53AC0E06020000780001D873@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7647586967021908290==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X07RY-0001ij-SX for xen-devel@lists.xenproject.org; Thu, 26 Jun 2014 11:02:17 +0000 In-Reply-To: <53AC0E06020000780001D873@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============7647586967021908290== Content-Type: multipart/alternative; boundary="------------010701060800050206080203" --------------010701060800050206080203 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 26/06/14 11:11, Jan Beulich wrote: > So far for non-Intel CPUs struct cpuinfo_x86's apicid field didn't get > set, despite MCE code consuming it. > > Do some formatting/ordering adjustment to the touched code at once. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/cpu/common.c > +++ b/xen/arch/x86/cpu/common.c > @@ -152,6 +152,17 @@ static void __cpuinit get_cpu_vendor(str > this_cpu = &default_cpu; > } > > +/* > + * cpuid returns the value latched in the HW at reset, not the APIC ID > + * register's value. For any box whose BIOS changes APIC IDs, like > + * clustered APIC systems, we must use hard_smp_processor_id. > + * > + * See Intel's IA-32 SW Dev's Manual Vol2 under CPUID. > + */ > +static inline u32 phys_pkg_id(u32 cpuid_apic, int index_msb) What is the point of the cpuid_apic parameter? It is unused. index_msb should probably be unsigned as it is used to shift with. ~Andrew > +{ > + return hard_smp_processor_id() >> index_msb; > +} > > /* Do minimum CPU detection early. > Fields really needed: vendor, cpuid_level, family, model, mask, cache alignment. > @@ -216,6 +227,7 @@ static void __cpuinit generic_identify(s > if (c->x86 >= 0x6) > c->x86_model += ((tfms >> 16) & 0xF) << 4; > c->x86_mask = tfms & 15; > + c->apicid = phys_pkg_id((ebx >> 24) & 0xFF, 0); > if ( cpu_has(c, X86_FEATURE_CLFLSH) ) > c->x86_clflush_size = ((ebx >> 8) & 0xff) * 8; > > @@ -339,17 +351,6 @@ void __cpuinit identify_cpu(struct cpuin > } > } > > -/* cpuid returns the value latched in the HW at reset, not the APIC ID > - * register's value. For any box whose BIOS changes APIC IDs, like > - * clustered APIC systems, we must use hard_smp_processor_id. > - * > - * See Intel's IA-32 SW Dev's Manual Vol2 under CPUID. > - */ > -static inline u32 phys_pkg_id(u32 cpuid_apic, int index_msb) > -{ > - return hard_smp_processor_id() >> index_msb; > -} > - > /* leaf 0xb SMT level */ > #define SMT_LEVEL 0 > > @@ -429,14 +430,12 @@ void __cpuinit detect_ht(struct cpuinfo_ > u32 eax, ebx, ecx, edx; > int index_msb, core_bits; > > - cpuid(1, &eax, &ebx, &ecx, &edx); > - > - c->apicid = phys_pkg_id((ebx >> 24) & 0xFF, 0); > - > - if (!cpu_has(c, X86_FEATURE_HT) || cpu_has(c, X86_FEATURE_CMP_LEGACY) > - || cpu_has(c, X86_FEATURE_XTOPOLOGY)) > + if (!cpu_has(c, X86_FEATURE_HT) || > + cpu_has(c, X86_FEATURE_CMP_LEGACY) || > + cpu_has(c, X86_FEATURE_XTOPOLOGY)) > return; > > + cpuid(1, &eax, &ebx, &ecx, &edx); > c->x86_num_siblings = (ebx & 0xff0000) >> 16; > > if (c->x86_num_siblings == 1) { > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------010701060800050206080203 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 26/06/14 11:11, Jan Beulich wrote:
So far for non-Intel CPUs struct cpuinfo_x86's apicid field didn't get
set, despite MCE code consuming it.

Do some formatting/ordering adjustment to the touched code at once.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -152,6 +152,17 @@ static void __cpuinit get_cpu_vendor(str
 	this_cpu = &default_cpu;
 }
 
+/*
+ * cpuid returns the value latched in the HW at reset, not the APIC ID
+ * register's value.  For any box whose BIOS changes APIC IDs, like
+ * clustered APIC systems, we must use hard_smp_processor_id.
+ *
+ * See Intel's IA-32 SW Dev's Manual Vol2 under CPUID.
+ */
+static inline u32 phys_pkg_id(u32 cpuid_apic, int index_msb)

What is the point of the cpuid_apic parameter?  It is unused.

index_msb should probably be unsigned as it is used to shift with.

~Andrew

+{
+	return hard_smp_processor_id() >> index_msb;
+}
 
 /* Do minimum CPU detection early.
    Fields really needed: vendor, cpuid_level, family, model, mask, cache alignment.
@@ -216,6 +227,7 @@ static void __cpuinit generic_identify(s
 	if (c->x86 >= 0x6)
 		c->x86_model += ((tfms >> 16) & 0xF) << 4;
 	c->x86_mask = tfms & 15;
+	c->apicid = phys_pkg_id((ebx >> 24) & 0xFF, 0);
 	if ( cpu_has(c, X86_FEATURE_CLFLSH) )
 		c->x86_clflush_size = ((ebx >> 8) & 0xff) * 8;
 
@@ -339,17 +351,6 @@ void __cpuinit identify_cpu(struct cpuin
 	}
 }
 
-/* cpuid returns the value latched in the HW at reset, not the APIC ID
- * register's value.  For any box whose BIOS changes APIC IDs, like
- * clustered APIC systems, we must use hard_smp_processor_id.
- *
- * See Intel's IA-32 SW Dev's Manual Vol2 under CPUID.
- */
-static inline u32 phys_pkg_id(u32 cpuid_apic, int index_msb)
-{
-	return hard_smp_processor_id() >> index_msb;
-}
-
 /* leaf 0xb SMT level */
 #define SMT_LEVEL       0
 
@@ -429,14 +430,12 @@ void __cpuinit detect_ht(struct cpuinfo_
 	u32 	eax, ebx, ecx, edx;
 	int 	index_msb, core_bits;
 
-	cpuid(1, &eax, &ebx, &ecx, &edx);
-
-	c->apicid = phys_pkg_id((ebx >> 24) & 0xFF, 0);
-
-	if (!cpu_has(c, X86_FEATURE_HT) || cpu_has(c, X86_FEATURE_CMP_LEGACY)
-        || cpu_has(c, X86_FEATURE_XTOPOLOGY))
+	if (!cpu_has(c, X86_FEATURE_HT) ||
+	    cpu_has(c, X86_FEATURE_CMP_LEGACY) ||
+	    cpu_has(c, X86_FEATURE_XTOPOLOGY))
 		return;
 
+	cpuid(1, &eax, &ebx, &ecx, &edx);
 	c->x86_num_siblings = (ebx & 0xff0000) >> 16;
 
 	if (c->x86_num_siblings == 1) {






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------010701060800050206080203-- --===============7647586967021908290== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7647586967021908290==--