Skip to contentSkip to conversation

x86, x86_64: Intel HT, Multi core detection code cleanup

PATCH In review Siddha, Suresh B 2 participants 14 messages Oct 5, 2005 – Oct 13, 2005 Bitops API

lore
Share & tools
lore

Siddha, Suresh B

author
#0 lore raw

PATCH 1/1 x86, x86_64: Intel HT, Multi core detection code cleanup

Andrew, Please apply this to your tree. This is 2.6.15 material.

Thanks.
--

This patch cleans up the x86 and x86_64 Intel HT and Multi Core detection code. These are the areas that this patch touches.

a) Cleanup and merge HT and Mutli Core detection code for x86 and x86_64

b) Fields obtained through cpuid vector 0x1(ebx[16:23]) and
vector 0x4(eax[14:25], eax[26:31]) indicate the maximum values and might not always be the same as what is available and what OS sees. So make sure "siblings" and "cpu cores" values in /proc/cpuinfo reflect the values as seen by OS instead of what cpuid instruction says. This will also fix the buggy BIOS cases (for example where cpuid on a single core cpu says there are "2" siblings, even when HT is disabled in the BIOS. http://bugzilla.kernel.org/show_bug.cgi?id=4359)

c) Fix the cache detection code assumption that number of threads sharing the cache will either be equal to number of HT or core siblings.

Signed-off-by Suresh Siddha <[email protected]>
arch/i386/kernel/Makefile +1 -1
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/Makefile linux-2.6.14-rc3-core/arch/i386/kernel/Makefile
@@ -18,7 +18,7 @@ obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
 obj-$(CONFIG_MICROCODE)		+= microcode.o
 obj-$(CONFIG_APM)		+= apm.o
-obj-$(CONFIG_X86_SMP)		+= smp.o smpboot.o
+obj-$(CONFIG_X86_SMP)		+= smp.o smpboot.o sibling-map.o
 obj-$(CONFIG_X86_TRAMPOLINE)	+= trampoline.o
 obj-$(CONFIG_X86_MPPARSE)	+= mpparse.o
 obj-$(CONFIG_X86_LOCAL_APIC)	+= apic.o nmi.o
arch/i386/kernel/cpu/Makefile +1 -1
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/Makefile linux-2.6.14-rc3-core/arch/i386/kernel/cpu/Makefile
@@ -8,7 +8,7 @@ obj-y	+=	amd.o
 obj-y	+=	cyrix.o
 obj-y	+=	centaur.o
 obj-y	+=	transmeta.o
-obj-y	+=	intel.o intel_cacheinfo.o
+obj-y	+=	intel.o intel_cacheinfo.o intel_htmc.o
 obj-y	+=	rise.o
 obj-y	+=	nexgen.o
 obj-y	+=	umc.o
arch/i386/kernel/cpu/common.c +0 -55

61 lines not on this page · raw message

arch/i386/kernel/cpu/intel.c +1 -20
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/intel.c linux-2.6.14-rc3-core/arch/i386/kernel/cpu/intel.c
@@ -76,25 +76,6 @@ static void __devinit Intel_errata_worka
 	}
 }
 
-
-/*
- * find out the number of processor cores on the die
- */
-static int __devinit num_cpu_cores(struct cpuinfo_x86 *c)
-{
-	unsigned int eax, ebx, ecx, edx;
-
-	if (c->cpuid_level < 4)
-		return 1;
-
-	/* Intel has a non-standard dependency on %ecx for this CPUID level. */
-	cpuid_count(4, 0, &eax, &ebx, &ecx, &edx);
-	if (eax & 0x1f)
-		return ((eax >> 26) + 1);
-	else
-		return 1;
-}
-
 static void __devinit init_intel(struct cpuinfo_x86 *c)
 {
 	unsigned int l2 = 0;

@@ -157,7 +138,7 @@ static void __devinit init_intel(struct 
 	if ( p )
 		strcpy(c->x86_model_id, p);
 	
-	c->x86_num_cores = num_cpu_cores(c);
+	c->x86_num_cores = intel_num_cpu_cores(c);
 
 	detect_ht(c);
arch/i386/kernel/cpu/intel_cacheinfo.c +32 -14

69 lines not on this page · raw message

arch/i386/kernel/cpu/intel_htmc.c +96 -0

96 lines not on this page · raw message

arch/i386/kernel/cpu/proc.c +2 -2
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/proc.c linux-2.6.14-rc3-core/arch/i386/kernel/cpu/proc.c
@@ -97,9 +97,9 @@ static int show_cpuinfo(struct seq_file 
 	if (c->x86_num_cores * smp_num_siblings > 1) {
 		seq_printf(m, "physical id\t: %d\n", phys_proc_id[n]);
 		seq_printf(m, "siblings\t: %d\n",
-				c->x86_num_cores * smp_num_siblings);
+				cpus_weight(cpu_core_map[n]));
 		seq_printf(m, "core id\t\t: %d\n", cpu_core_id[n]);
-		seq_printf(m, "cpu cores\t: %d\n", c->x86_num_cores);
+		seq_printf(m, "cpu cores\t: %d\n", c->booted_cores);
 	}
 #endif
arch/i386/kernel/sibling-map.c +93 -0

93 lines not on this page · raw message

arch/i386/kernel/smpboot.c +1 -52

71 lines not on this page · raw message

arch/x86_64/kernel/Makefile +4 -2
diff -pNru linux-2.6.14-rc3/arch/x86_64/kernel/Makefile linux-2.6.14-rc3-core/arch/x86_64/kernel/Makefile
@@ -16,7 +16,7 @@ obj-$(CONFIG_ACPI)		+= acpi/
 obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_MICROCODE)		+= microcode.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
-obj-$(CONFIG_SMP)		+= smp.o smpboot.o trampoline.o
+obj-$(CONFIG_SMP)		+= smp.o smpboot.o trampoline.o sibling-map.o
 obj-$(CONFIG_X86_LOCAL_APIC)	+= apic.o  nmi.o
 obj-$(CONFIG_X86_IO_APIC)	+= io_apic.o mpparse.o \
 		genapic.o genapic_cluster.o genapic_flat.o

@@ -34,7 +34,7 @@ obj-$(CONFIG_X86_PM_TIMER)	+= pmtimer.o
 obj-$(CONFIG_MODULES)		+= module.o
 
 obj-y				+= topology.o
-obj-y				+= intel_cacheinfo.o
+obj-y				+= intel_cacheinfo.o intel_htmc.o
 
 CFLAGS_vsyscall.o		:= $(PROFILING) -g0
 

@@ -47,3 +47,5 @@ intel_cacheinfo-y		+= ../../i386/kernel/
 quirks-y			+= ../../i386/kernel/quirks.o
 i8237-y				+= ../../i386/kernel/i8237.o
 msr-$(subst m,y,$(CONFIG_X86_MSR))  += ../../i386/kernel/msr.o
+sibling-map-y			+= ../../i386/kernel/sibling-map.o
+intel_htmc-y			+= ../../i386/kernel/cpu/intel_htmc.o
arch/x86_64/kernel/setup.c +2 -81

96 lines not on this page · raw message

arch/x86_64/kernel/smpboot.c +2 -43

69 lines not on this page · raw message

include/asm-i386/processor.h +3 -0
diff -pNru linux-2.6.14-rc3/include/asm-i386/processor.h linux-2.6.14-rc3-core/include/asm-i386/processor.h
@@ -66,6 +66,8 @@ struct cpuinfo_x86 {
 	int	coma_bug;
 	unsigned long loops_per_jiffy;
 	unsigned char x86_num_cores;
+	unsigned char booted_cores;
+	unsigned char apicid;
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
 
 #define X86_VENDOR_INTEL 0

@@ -104,6 +106,7 @@ extern char ignore_fpu_irq;
 extern void identify_cpu(struct cpuinfo_x86 *);
 extern void print_cpu_info(struct cpuinfo_x86 *);
 extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
+extern int intel_num_cpu_cores(struct cpuinfo_x86 *c);
 
 #ifdef CONFIG_X86_HT
 extern void detect_ht(struct cpuinfo_x86 *c);
include/asm-i386/smp.h +3 -0
diff -pNru linux-2.6.14-rc3/include/asm-i386/smp.h linux-2.6.14-rc3-core/include/asm-i386/smp.h
@@ -37,6 +37,9 @@ extern int smp_num_siblings;
 extern cpumask_t cpu_sibling_map[];
 extern cpumask_t cpu_core_map[];
 
+extern void set_cpu_sibling_map(int);
+extern void remove_siblinginfo(int);
+
 extern void (*mtrr_hook) (void);
 extern void zap_low_mappings (void);
 extern void lock_ipi_call_lock(void);
include/asm-x86_64/processor.h +10 -0
diff -pNru linux-2.6.14-rc3/include/asm-x86_64/processor.h linux-2.6.14-rc3-core/include/asm-x86_64/processor.h
@@ -65,6 +65,8 @@ struct cpuinfo_x86 {
         __u32   x86_power; 	
 	__u32   extended_cpuid_level;	/* Max extended CPUID function supported */
 	unsigned long loops_per_jiffy;
+	__u8	apicid;
+	__u8	booted_cores;
 } ____cacheline_aligned;
 
 #define X86_VENDOR_INTEL 0

@@ -91,6 +93,14 @@ extern char ignore_irq13;
 extern void identify_cpu(struct cpuinfo_x86 *);
 extern void print_cpu_info(struct cpuinfo_x86 *);
 extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
+extern int intel_num_cpu_cores(struct cpuinfo_x86 *c);
+
+#ifdef CONFIG_X86_HT
+extern void detect_ht(struct cpuinfo_x86 *c);
+#else
+static inline void detect_ht(struct cpuinfo_x86 *c) {}
+#endif
+
 
 /*
  * EFLAGS bits
include/asm-x86_64/smp.h +3 -0
diff -pNru linux-2.6.14-rc3/include/asm-x86_64/smp.h linux-2.6.14-rc3-core/include/asm-x86_64/smp.h
@@ -57,6 +57,9 @@ extern cpumask_t cpu_core_map[NR_CPUS];
 extern u8 phys_proc_id[NR_CPUS];
 extern u8 cpu_core_id[NR_CPUS];
 
+extern void set_cpu_sibling_map(int);
+extern void remove_siblinginfo(int);
+
 #define SMP_TRAMPOLINE_BASE 0x6000
 
 /*
include/linux/bitops.h +10 -0
diff -pNru linux-2.6.14-rc3/include/linux/bitops.h linux-2.6.14-rc3-core/include/linux/bitops.h
@@ -84,6 +84,16 @@ static __inline__ int get_bitmask_order(
 	return order;	/* We could be slightly more clever with -1 here... */
 }
 
+static __inline__ int get_count_order(unsigned int count)
+{
+	int order;
+	
+	order = fls(count) - 1;
+	if (count & (count - 1))
+		order++;
+	return order;
+}
+
 /*
  * hweightN: returns the hamming weight (i.e. the number
  * of bits set) of a N-bit word
#1 lore raw

arch/i386/kernel/cpu/intel_htmc.c line 38

Quoted 7 lines · Suresh B
arch/i386/kernel/cpu/intel_htmc.c +7 -0
+
+#ifdef CONFIG_X86_HT
+#ifndef CONFIG_X86_64
+#include <mach_apic.h>
+#else
+#include <asm/mach_apic.h>
+#endif

Having such ifdefs is a clear cue that the code shouldn't be shared between architectures.

-Andi

author
#2 lore raw
Quoted 12 lines · Andi Kleen

arch/i386/kernel/cpu/intel_htmc.c line 38

Quoted 7 lines · Suresh B
arch/i386/kernel/cpu/intel_htmc.c +7 -0
+
+#ifdef CONFIG_X86_HT
+#ifndef CONFIG_X86_64
+#include <mach_apic.h>
+#else
+#include <asm/mach_apic.h>
+#endif

Having such ifdefs is a clear cue that the code shouldn't be shared between architectures.

Andi, Those headers are for phys_pkg_id(). And its meaning is same on both x86 and x86_64(though its API is different. We can make it consistent across x86 and x86_64. But that will not solve the above pointed out ifdef hunk)

Unfortunately x86 mach specific vector implementation is not as simple as x86_64, resulting in the above ifdefs mess...

I can fix the API mess. Is there anything else you want me to do?

thanks,
suresh

#3 lore raw
Quoted 1 line

I can fix the API mess. Is there anything else you want me to do?

I think you overdid the sharing. Can you limit it to one file and copy the stuff that doesn't fit easily?

-Andi

author
#4 lore raw
Quoted 4 lines · Andi Kleen
Quoted 1 line

I can fix the API mess. Is there anything else you want me to do?

I think you overdid the sharing. Can you limit it to one file and copy the stuff that doesn't fit easily?

Andi, This stuff is very much common to x86 and x86_64. Shared code is split into two files because setting up sibling map code is generic and HT/core detection code is very specific to Intel.

How about the appended patch?

thanks,
suresh
--

This patch cleans up the Intel HT and Multi Core detection code in x86 and x86_64. These are the areas that this patch touches.

a) Cleanup and merge HT and Mutli Core detection code for x86 and x86_64

b) Fields obtained through cpuid vector 0x1(ebx[16:23]) and
vector 0x4(eax[14:25], eax[26:31]) indicate the maximum values and might not always be the same as what is available and what OS sees. So make sure "siblings" and "cpu cores" values in /proc/cpuinfo reflect the values as seen by OS instead of what cpuid instruction says. This will also fix the buggy BIOS cases (for example where cpuid says there are "2" siblings, even when HT is disabled in the BIOS. http://bugzilla.kernel.org/show_bug.cgi?id=4359)

c) Fix the cache detection code assumption that number of threads sharing the cache will either be equal to number of HT or core siblings.

Signed-off-by Suresh Siddha <[email protected]>
arch/i386/kernel/Makefile +1 -1
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/Makefile linux/arch/i386/kernel/Makefile
@@ -18,7 +18,7 @@ obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
 obj-$(CONFIG_MICROCODE)		+= microcode.o
 obj-$(CONFIG_APM)		+= apm.o
-obj-$(CONFIG_X86_SMP)		+= smp.o smpboot.o
+obj-$(CONFIG_X86_SMP)		+= smp.o smpboot.o sibling-map.o
 obj-$(CONFIG_X86_TRAMPOLINE)	+= trampoline.o
 obj-$(CONFIG_X86_MPPARSE)	+= mpparse.o
 obj-$(CONFIG_X86_LOCAL_APIC)	+= apic.o nmi.o
arch/i386/kernel/cpu/Makefile +1 -1
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/Makefile linux/arch/i386/kernel/cpu/Makefile
@@ -8,7 +8,7 @@ obj-y	+=	amd.o
 obj-y	+=	cyrix.o
 obj-y	+=	centaur.o
 obj-y	+=	transmeta.o
-obj-y	+=	intel.o intel_cacheinfo.o
+obj-y	+=	intel.o intel_cacheinfo.o intel_htmc.o
 obj-y	+=	rise.o
 obj-y	+=	nexgen.o
 obj-y	+=	umc.o
arch/i386/kernel/cpu/common.c +0 -55

61 lines not on this page · raw message

arch/i386/kernel/cpu/intel.c +1 -20
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/intel.c linux/arch/i386/kernel/cpu/intel.c
@@ -76,25 +76,6 @@ static void __devinit Intel_errata_worka
 	}
 }
 
-
-/*
- * find out the number of processor cores on the die
- */
-static int __devinit num_cpu_cores(struct cpuinfo_x86 *c)
-{
-	unsigned int eax, ebx, ecx, edx;
-
-	if (c->cpuid_level < 4)
-		return 1;
-
-	/* Intel has a non-standard dependency on %ecx for this CPUID level. */
-	cpuid_count(4, 0, &eax, &ebx, &ecx, &edx);
-	if (eax & 0x1f)
-		return ((eax >> 26) + 1);
-	else
-		return 1;
-}
-
 static void __devinit init_intel(struct cpuinfo_x86 *c)
 {
 	unsigned int l2 = 0;

@@ -157,7 +138,7 @@ static void __devinit init_intel(struct 
 	if ( p )
 		strcpy(c->x86_model_id, p);
 	
-	c->x86_num_cores = num_cpu_cores(c);
+	c->x86_num_cores = intel_num_cpu_cores(c);
 
 	detect_ht(c);
arch/i386/kernel/cpu/intel_cacheinfo.c +32 -14

69 lines not on this page · raw message

arch/i386/kernel/cpu/intel_htmc.c +81 -0

81 lines not on this page · raw message

arch/i386/kernel/cpu/proc.c +2 -2
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/proc.c linux/arch/i386/kernel/cpu/proc.c
@@ -97,9 +97,9 @@ static int show_cpuinfo(struct seq_file 
 	if (c->x86_num_cores * smp_num_siblings > 1) {
 		seq_printf(m, "physical id\t: %d\n", phys_proc_id[n]);
 		seq_printf(m, "siblings\t: %d\n",
-				c->x86_num_cores * smp_num_siblings);
+				cpus_weight(cpu_core_map[n]));
 		seq_printf(m, "core id\t\t: %d\n", cpu_core_id[n]);
-		seq_printf(m, "cpu cores\t: %d\n", c->x86_num_cores);
+		seq_printf(m, "cpu cores\t: %d\n", c->booted_cores);
 	}
 #endif
arch/i386/kernel/sibling-map.c +93 -0

93 lines not on this page · raw message

arch/i386/kernel/smpboot.c +1 -52

71 lines not on this page · raw message

arch/x86_64/kernel/Makefile +4 -2
diff -pNru linux-2.6.14-rc3/arch/x86_64/kernel/Makefile linux/arch/x86_64/kernel/Makefile
@@ -16,7 +16,7 @@ obj-$(CONFIG_ACPI)		+= acpi/
 obj-$(CONFIG_X86_MSR)		+= msr.o
 obj-$(CONFIG_MICROCODE)		+= microcode.o
 obj-$(CONFIG_X86_CPUID)		+= cpuid.o
-obj-$(CONFIG_SMP)		+= smp.o smpboot.o trampoline.o
+obj-$(CONFIG_SMP)		+= smp.o smpboot.o trampoline.o sibling-map.o
 obj-$(CONFIG_X86_LOCAL_APIC)	+= apic.o  nmi.o
 obj-$(CONFIG_X86_IO_APIC)	+= io_apic.o mpparse.o \
 		genapic.o genapic_cluster.o genapic_flat.o

@@ -34,7 +34,7 @@ obj-$(CONFIG_X86_PM_TIMER)	+= pmtimer.o
 obj-$(CONFIG_MODULES)		+= module.o
 
 obj-y				+= topology.o
-obj-y				+= intel_cacheinfo.o
+obj-y				+= intel_cacheinfo.o intel_htmc.o
 
 CFLAGS_vsyscall.o		:= $(PROFILING) -g0
 

@@ -47,3 +47,5 @@ intel_cacheinfo-y		+= ../../i386/kernel/
 quirks-y			+= ../../i386/kernel/quirks.o
 i8237-y				+= ../../i386/kernel/i8237.o
 msr-$(subst m,y,$(CONFIG_X86_MSR))  += ../../i386/kernel/msr.o
+sibling-map-y			+= ../../i386/kernel/sibling-map.o
+intel_htmc-y			+= ../../i386/kernel/cpu/intel_htmc.o
arch/x86_64/kernel/genapic_cluster.c +1 -1
diff -pNru linux-2.6.14-rc3/arch/x86_64/kernel/genapic_cluster.c linux/arch/x86_64/kernel/genapic_cluster.c
@@ -113,7 +113,7 @@ static unsigned int cluster_cpu_mask_to_
  *
  * See Intel's IA-32 SW Dev's Manual Vol2 under CPUID.
  */
-static unsigned int phys_pkg_id(int index_msb)
+static unsigned int phys_pkg_id(u32 cpu_apicid, int index_msb)
 {
 	return hard_smp_processor_id() >> index_msb;
 }
arch/x86_64/kernel/genapic_flat.c +2 -5
diff -pNru linux-2.6.14-rc3/arch/x86_64/kernel/genapic_flat.c linux/arch/x86_64/kernel/genapic_flat.c
@@ -107,12 +107,9 @@ static unsigned int flat_cpu_mask_to_api
 	return cpus_addr(cpumask)[0] & APIC_ALL_CPUS;
 }
 
-static unsigned int phys_pkg_id(int index_msb)
+static unsigned int phys_pkg_id(u32 cpu_apicid, int index_msb)
 {
-	u32 ebx;
-
-	ebx = cpuid_ebx(1);
-	return ((ebx >> 24) & 0xFF) >> index_msb;
+	return cpu_apicid >> index_msb;
 }
 
 struct genapic apic_flat =  {
arch/x86_64/kernel/setup.c +2 -81

96 lines not on this page · raw message

arch/x86_64/kernel/smpboot.c +2 -43

69 lines not on this page · raw message

include/asm-i386/mach_apic.h +5 -0
diff -pNru linux-2.6.14-rc3/include/asm-i386/mach_apic.h linux/include/asm-i386/mach_apic.h
@@ -0,0 +1,5 @@
+/* Magic for including machine specific mach_apic.h from C files,
+ * so that shared code between i386 and x86_64 can simply include 
+ * <asm/mach_apic.h> without complicated ifdefs
+ */
+#include <mach_apic.h>
include/asm-i386/processor.h +3 -0
diff -pNru linux-2.6.14-rc3/include/asm-i386/processor.h linux/include/asm-i386/processor.h
@@ -66,6 +66,8 @@ struct cpuinfo_x86 {
 	int	coma_bug;
 	unsigned long loops_per_jiffy;
 	unsigned char x86_num_cores;
+	unsigned char booted_cores;
+	unsigned char apicid;
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
 
 #define X86_VENDOR_INTEL 0

@@ -104,6 +106,7 @@ extern char ignore_fpu_irq;
 extern void identify_cpu(struct cpuinfo_x86 *);
 extern void print_cpu_info(struct cpuinfo_x86 *);
 extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
+extern int intel_num_cpu_cores(struct cpuinfo_x86 *c);
 
 #ifdef CONFIG_X86_HT
 extern void detect_ht(struct cpuinfo_x86 *c);
include/asm-i386/smp.h +3 -0
diff -pNru linux-2.6.14-rc3/include/asm-i386/smp.h linux/include/asm-i386/smp.h
@@ -37,6 +37,9 @@ extern int smp_num_siblings;
 extern cpumask_t cpu_sibling_map[];
 extern cpumask_t cpu_core_map[];
 
+extern void set_cpu_sibling_map(int);
+extern void remove_siblinginfo(int);
+
 extern void (*mtrr_hook) (void);
 extern void zap_low_mappings (void);
 extern void lock_ipi_call_lock(void);
include/asm-x86_64/genapic.h +1 -1
diff -pNru linux-2.6.14-rc3/include/asm-x86_64/genapic.h linux/include/asm-x86_64/genapic.h
@@ -26,7 +26,7 @@ struct genapic {
 	void (*send_IPI_all)(int vector);
 	/* */
 	unsigned int (*cpu_mask_to_apicid)(cpumask_t cpumask);
-	unsigned int (*phys_pkg_id)(int index_msb);
+	unsigned int (*phys_pkg_id)(u32 cpu_apicid, int index_msb);
 };
include/asm-x86_64/processor.h +10 -0
diff -pNru linux-2.6.14-rc3/include/asm-x86_64/processor.h linux/include/asm-x86_64/processor.h
@@ -65,6 +65,8 @@ struct cpuinfo_x86 {
         __u32   x86_power; 	
 	__u32   extended_cpuid_level;	/* Max extended CPUID function supported */
 	unsigned long loops_per_jiffy;
+	__u8	booted_cores;
+	__u8	apicid;
 } ____cacheline_aligned;
 
 #define X86_VENDOR_INTEL 0

@@ -91,6 +93,14 @@ extern char ignore_irq13;
 extern void identify_cpu(struct cpuinfo_x86 *);
 extern void print_cpu_info(struct cpuinfo_x86 *);
 extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
+extern int intel_num_cpu_cores(struct cpuinfo_x86 *c);
+
+#ifdef CONFIG_X86_HT
+extern void detect_ht(struct cpuinfo_x86 *c);
+#else
+static inline void detect_ht(struct cpuinfo_x86 *c) {}
+#endif
+
 
 /*
  * EFLAGS bits
include/asm-x86_64/smp.h +3 -0
diff -pNru linux-2.6.14-rc3/include/asm-x86_64/smp.h linux/include/asm-x86_64/smp.h
@@ -57,6 +57,9 @@ extern cpumask_t cpu_core_map[NR_CPUS];
 extern u8 phys_proc_id[NR_CPUS];
 extern u8 cpu_core_id[NR_CPUS];
 
+extern void set_cpu_sibling_map(int);
+extern void remove_siblinginfo(int);
+
 #define SMP_TRAMPOLINE_BASE 0x6000
 
 /*
include/linux/bitops.h +10 -0
diff -pNru linux-2.6.14-rc3/include/linux/bitops.h linux/include/linux/bitops.h
@@ -84,6 +84,16 @@ static __inline__ int get_bitmask_order(
 	return order;	/* We could be slightly more clever with -1 here... */
 }
 
+static __inline__ int get_count_order(unsigned int count)
+{
+	int order;
+	
+	order = fls(count) - 1;
+	if (count & (count - 1))
+		order++;
+	return order;
+}
+
 /*
  * hweightN: returns the hamming weight (i.e. the number
  * of bits set) of a N-bit word
#5 lore raw
Quoted 11 lines · Suresh B
Quoted 4 lines · Andi Kleen
Quoted 1 line

I can fix the API mess. Is there anything else you want me to do?

I think you overdid the sharing. Can you limit it to one file and copy the stuff that doesn't fit easily?

Andi, This stuff is very much common to x86 and x86_64. Shared code is split into two files because setting up sibling map code is generic and HT/core detection code is very specific to Intel.

How about the appended patch?

I would prefer if the Intel CPU detection support wasn't distributed over so many small files. If you prefer to share it put it all into a single file and share that. But please only for code that can be cleanly shared without ifdefs.

Also in general it would be better if you first did the cleanup and then as separate patches the various functionality enhancements.That makes the changes easier to be reviewed and it helps in binary search when something goes wrong.

-Andi

author
#6 lore raw
Quoted 11 lines · Andi Kleen
Quoted 10 lines · Suresh B
Quoted 4 lines · Andi Kleen
Quoted 1 line

I can fix the API mess. Is there anything else you want me to do?

I think you overdid the sharing. Can you limit it to one file and copy the stuff that doesn't fit easily?

Andi, This stuff is very much common to x86 and x86_64. Shared code is split into two files because setting up sibling map code is generic and HT/core detection code is very specific to Intel.

Quoted 6 lines
Quoted 1 line

How about the appended patch?

I would prefer if the Intel CPU detection support wasn't distributed over so many small files. If you prefer to share it put it all into a single file and share that. But please only for code that can be cleanly shared without ifdefs.

Lets defer this code sharing to some other time. I want to make sure that -mm tree (and finally 2.6.15) picks up these enhancements first, before I start my vacation :)

Quoted 4 lines

Also in general it would be better if you first did the cleanup and then as separate patches the various functionality enhancements.That makes the changes easier to be reviewed and it helps in binary search when something goes wrong.

I am going to send two follow up patches which addresses the functionality enhancements.

thanks,
suresh

author
#7 lore raw

PATCH 1/2 x86, x86_64: Intel HT, Multi core detection fixes

In reply to Andi Kleen

Andrew, Please apply.

thanks,
suresh
--

Fields obtained through cpuid vector 0x1(ebx[16:23]) and
vector 0x4(eax[14:25], eax[26:31]) indicate the maximum values and might not always be the same as what is available and what OS sees. So make sure "siblings" and "cpu cores" values in /proc/cpuinfo reflect the values as seen by OS instead of what cpuid instruction says. This will also fix the buggy BIOS cases (for example where cpuid on a single core cpu says there are "2" siblings, even when HT is disabled in the BIOS.
http://bugzilla.kernel.org/show_bug.cgi?id=4359)

Signed-off-by Suresh Siddha <[email protected]>
arch/i386/kernel/cpu/common.c +11 -19
diff -pNru linux-2.6.14-rc4/arch/i386/kernel/cpu/common.c linux-2.6.14-rc4-dc/arch/i386/kernel/cpu/common.c
@@ -446,32 +446,29 @@ void __devinit identify_cpu(struct cpuin
 void __devinit detect_ht(struct cpuinfo_x86 *c)
 {
 	u32 	eax, ebx, ecx, edx;
-	int 	index_msb, tmp;
+	int 	index_msb, core_bits;
 	int 	cpu = smp_processor_id();
 
+	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))
 		return;
 
-	cpuid(1, &eax, &ebx, &ecx, &edx);
 	smp_num_siblings = (ebx & 0xff0000) >> 16;
 
 	if (smp_num_siblings == 1) {
 		printk(KERN_INFO  "CPU: Hyper-Threading is disabled\n");
 	} else if (smp_num_siblings > 1 ) {
-		index_msb = 31;
 
 		if (smp_num_siblings > NR_CPUS) {
 			printk(KERN_WARNING "CPU: Unsupported number of the siblings %d", smp_num_siblings);
 			smp_num_siblings = 1;
 			return;
 		}
-		tmp = smp_num_siblings;
-		while ((tmp & 0x80000000 ) == 0) {
-			tmp <<=1 ;
-			index_msb--;
-		}
-		if (smp_num_siblings & (smp_num_siblings - 1))
-			index_msb++;
+
+		index_msb = get_count_order(smp_num_siblings);
 		phys_proc_id[cpu] = phys_pkg_id((ebx >> 24) & 0xFF, index_msb);
 
 		printk(KERN_INFO  "CPU: Physical Processor ID: %d\n",

@@ -479,17 +476,12 @@ void __devinit detect_ht(struct cpuinfo_
 
 		smp_num_siblings = smp_num_siblings / c->x86_num_cores;
 
-		tmp = smp_num_siblings;
-		index_msb = 31;
-		while ((tmp & 0x80000000) == 0) {
-			tmp <<=1 ;
-			index_msb--;
-		}
+		index_msb = get_count_order(smp_num_siblings) ;
 
-		if (smp_num_siblings & (smp_num_siblings - 1))
-			index_msb++;
+		core_bits = get_count_order(c->x86_num_cores);
 
-		cpu_core_id[cpu] = phys_pkg_id((ebx >> 24) & 0xFF, index_msb);
+		cpu_core_id[cpu] = phys_pkg_id((ebx >> 24) & 0xFF, index_msb) &
+					       ((1 << core_bits) - 1);
 
 		if (c->x86_num_cores > 1)
 			printk(KERN_INFO  "CPU: Processor Core ID: %d\n",
arch/i386/kernel/cpu/proc.c +2 -3
diff -pNru linux-2.6.14-rc4/arch/i386/kernel/cpu/proc.c linux-2.6.14-rc4-dc/arch/i386/kernel/cpu/proc.c
@@ -96,10 +96,9 @@ static int show_cpuinfo(struct seq_file 
 #ifdef CONFIG_X86_HT
 	if (c->x86_num_cores * smp_num_siblings > 1) {
 		seq_printf(m, "physical id\t: %d\n", phys_proc_id[n]);
-		seq_printf(m, "siblings\t: %d\n",
-				c->x86_num_cores * smp_num_siblings);
+		seq_printf(m, "siblings\t: %d\n", cpus_weight(cpu_core_map[n]));
 		seq_printf(m, "core id\t\t: %d\n", cpu_core_id[n]);
-		seq_printf(m, "cpu cores\t: %d\n", c->x86_num_cores);
+		seq_printf(m, "cpu cores\t: %d\n", c->booted_cores);
 	}
 #endif
arch/i386/kernel/smpboot.c +50 -20

111 lines not on this page · raw message

arch/x86_64/kernel/setup.c +19 -30

81 lines not on this page · raw message

arch/x86_64/kernel/smpboot.c +50 -14

109 lines not on this page · raw message

include/asm-i386/processor.h +2 -0
diff -pNru linux-2.6.14-rc4/include/asm-i386/processor.h linux-2.6.14-rc4-dc/include/asm-i386/processor.h
@@ -66,6 +66,8 @@ struct cpuinfo_x86 {
 	int	coma_bug;
 	unsigned long loops_per_jiffy;
 	unsigned char x86_num_cores;
+	unsigned char booted_cores;
+	unsigned char apicid;
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
 
 #define X86_VENDOR_INTEL 0
include/asm-x86_64/processor.h +2 -0
diff -pNru linux-2.6.14-rc4/include/asm-x86_64/processor.h linux-2.6.14-rc4-dc/include/asm-x86_64/processor.h
@@ -65,6 +65,8 @@ struct cpuinfo_x86 {
         __u32   x86_power; 	
 	__u32   extended_cpuid_level;	/* Max extended CPUID function supported */
 	unsigned long loops_per_jiffy;
+	__u8	apicid;
+	__u8	booted_cores;
 } ____cacheline_aligned;
 
 #define X86_VENDOR_INTEL 0
include/linux/bitops.h +10 -0
diff -pNru linux-2.6.14-rc4/include/linux/bitops.h linux-2.6.14-rc4-dc/include/linux/bitops.h
@@ -84,6 +84,16 @@ static __inline__ int get_bitmask_order(
 	return order;	/* We could be slightly more clever with -1 here... */
 }
 
+static __inline__ int get_count_order(unsigned int count)
+{
+	int order;
+	
+	order = fls(count) - 1;
+	if (count & (count - 1))
+		order++;
+	return order;
+}
+
 /*
  * hweightN: returns the hamming weight (i.e. the number
  * of bits set) of a N-bit word
#8 lore raw

Re: [discuss] [Patch 1/2] x86, x86_64: Intel HT, Multi core detection fixes

Quoted 8 lines · Suresh B

Fields obtained through cpuid vector 0x1(ebx[16:23]) and
vector 0x4(eax[14:25], eax[26:31]) indicate the maximum values and might not always be the same as what is available and what OS sees. So make sure "siblings" and "cpu cores" values in /proc/cpuinfo reflect the values as seen by OS instead of what cpuid instruction says. This will also fix the buggy BIOS cases (for example where cpuid on a single core cpu says there are "2" siblings, even when HT is disabled in the BIOS.
http://bugzilla.kernel.org/show_bug.cgi?id=4359)

I'm not too fond of this new booted_core variable. How about you just put the true number of cores into x86_num_cores? What should x86_num_cores be in your setup anyways if not "booted cores"?

Also I must admit the number of different variables to keep track of multicore and siblingness starts to become mindboggling, so I would recommend you add a fat overview comment somewhere that describes their definition and relationship. Or better put something into Documentation, it is probably as confusing for user space /proc/cpuinfo consumer too.

-Andi

author
#9 lore raw

Re: [discuss] [Patch 1/2] x86, x86_64: Intel HT, Multi core detection fixes

Quoted 11 lines · Andi Kleen
Quoted 8 lines · Suresh B

Fields obtained through cpuid vector 0x1(ebx[16:23]) and
vector 0x4(eax[14:25], eax[26:31]) indicate the maximum values and might not always be the same as what is available and what OS sees. So make sure "siblings" and "cpu cores" values in /proc/cpuinfo reflect the values as seen by OS instead of what cpuid instruction says. This will also fix the buggy BIOS cases (for example where cpuid on a single core cpu says there are "2" siblings, even when HT is disabled in the BIOS.
http://bugzilla.kernel.org/show_bug.cgi?id=4359)

Quoted 4 lines

I'm not too fond of this new booted_core variable. How about you just put the true number of cores into x86_num_cores? What should x86_num_cores be in your setup anyways if not "booted cores"?

x86_num_cores (cpuid returned value) is required while right shifting the apicid to get the core-id and the package-id. booted_cores will indicate how many cores actually cameup. x86_num_cores can differ from "booted cores" in these scenarios

a) x86_num_cores may not be '1' when multi-core is disabled in the BIOS
b) "maxcpus=" kernel boot param
c) logical hotplug

Quoted 6 lines

Also I must admit the number of different variables to keep track of multicore and siblingness starts to become mindboggling, so I would recommend you add a fat overview comment somewhere that describes their definition and relationship. Or better put something into Documentation, it is probably as confusing for user space /proc/cpuinfo consumer too.

I will send a new documentation patch.

thanks,
suresh

#10 lore raw

Re: [discuss] [Patch 1/2] x86, x86_64: Intel HT, Multi core detection fixes

Quoted 4 lines · Suresh B

x86_num_cores (cpuid returned value) is required while right shifting the apicid to get the core-id and the package-id. booted_cores will indicate how many cores actually cameup. x86_num_cores can differ from "booted cores" in these scenarios

The original idea was to round it up to the next power of two before shifting. But I can see that it will break when the number of online cores differs from the max number by more than a single power of two (e.g. quadcore with only a single CPU booted)

Ok, if you rename the variable to make it clear

x86_num_cores -> x86_max_cores

-Andi

author
#11 lore raw

Re: [discuss] [Patch 1/2] x86, x86_64: Intel HT, Multi core detection fixes

Quoted 3 lines · Andi Kleen

Ok, if you rename the variable to make it clear

x86_num_cores -> x86_max_cores

Andrew, please apply.

thanks,
suresh
--

Fields obtained through cpuid vector 0x1(ebx[16:23]) and
vector 0x4(eax[14:25], eax[26:31]) indicate the maximum values and might not always be the same as what is available and what OS sees. So make sure "siblings" and "cpu cores" values in /proc/cpuinfo reflect the values as seen by OS instead of what cpuid instruction says. This will also fix the buggy BIOS cases (for example where cpuid on a single core cpu says there are "2" siblings, even when HT is disabled in the BIOS.
http://bugzilla.kernel.org/show_bug.cgi?id=4359)

Signed-off-by Suresh Siddha <[email protected]>
arch/i386/kernel/cpu/amd.c +6 -6
diff -pNru linux-2.6.14-rc4/arch/i386/kernel/cpu/amd.c linux-2.6.14-rc4-dc/arch/i386/kernel/cpu/amd.c
@@ -206,9 +206,9 @@ static void __init init_amd(struct cpuin
 	display_cacheinfo(c);
 
 	if (cpuid_eax(0x80000000) >= 0x80000008) {
-		c->x86_num_cores = (cpuid_ecx(0x80000008) & 0xff) + 1;
-		if (c->x86_num_cores & (c->x86_num_cores - 1))
-			c->x86_num_cores = 1;
+		c->x86_max_cores = (cpuid_ecx(0x80000008) & 0xff) + 1;
+		if (c->x86_max_cores & (c->x86_max_cores - 1))
+			c->x86_max_cores = 1;
 	}
 
 #ifdef CONFIG_X86_HT

@@ -217,15 +217,15 @@ static void __init init_amd(struct cpuin
 	 * distingush the cores.  Assumes number of cores is a power
 	 * of two.
 	 */
-	if (c->x86_num_cores > 1) {
+	if (c->x86_max_cores > 1) {
 		int cpu = smp_processor_id();
 		unsigned bits = 0;
-		while ((1 << bits) < c->x86_num_cores)
+		while ((1 << bits) < c->x86_max_cores)
 			bits++;
 		cpu_core_id[cpu] = phys_proc_id[cpu] & ((1<<bits)-1);
 		phys_proc_id[cpu] >>= bits;
 		printk(KERN_INFO "CPU %d(%d) -> Core %d\n",
-		       cpu, c->x86_num_cores, cpu_core_id[cpu]);
+		       cpu, c->x86_max_cores, cpu_core_id[cpu]);
 	}
 #endif
 }
arch/i386/kernel/cpu/common.c +14 -22
diff -pNru linux-2.6.14-rc4/arch/i386/kernel/cpu/common.c linux-2.6.14-rc4-dc/arch/i386/kernel/cpu/common.c
@@ -335,7 +335,7 @@ void __devinit identify_cpu(struct cpuin
 	c->x86_model = c->x86_mask = 0;	/* So far unknown... */
 	c->x86_vendor_id[0] = '\0'; /* Unset */
 	c->x86_model_id[0] = '\0';  /* Unset */
-	c->x86_num_cores = 1;
+	c->x86_max_cores = 1;
 	memset(&c->x86_capability, 0, sizeof c->x86_capability);
 
 	if (!have_cpuid_p()) {

@@ -446,52 +446,44 @@ void __devinit identify_cpu(struct cpuin
 void __devinit detect_ht(struct cpuinfo_x86 *c)
 {
 	u32 	eax, ebx, ecx, edx;
-	int 	index_msb, tmp;
+	int 	index_msb, core_bits;
 	int 	cpu = smp_processor_id();
 
+	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))
 		return;
 
-	cpuid(1, &eax, &ebx, &ecx, &edx);
 	smp_num_siblings = (ebx & 0xff0000) >> 16;
 
 	if (smp_num_siblings == 1) {
 		printk(KERN_INFO  "CPU: Hyper-Threading is disabled\n");
 	} else if (smp_num_siblings > 1 ) {
-		index_msb = 31;
 
 		if (smp_num_siblings > NR_CPUS) {
 			printk(KERN_WARNING "CPU: Unsupported number of the siblings %d", smp_num_siblings);
 			smp_num_siblings = 1;
 			return;
 		}
-		tmp = smp_num_siblings;
-		while ((tmp & 0x80000000 ) == 0) {
-			tmp <<=1 ;
-			index_msb--;
-		}
-		if (smp_num_siblings & (smp_num_siblings - 1))
-			index_msb++;
+
+		index_msb = get_count_order(smp_num_siblings);
 		phys_proc_id[cpu] = phys_pkg_id((ebx >> 24) & 0xFF, index_msb);
 
 		printk(KERN_INFO  "CPU: Physical Processor ID: %d\n",
 		       phys_proc_id[cpu]);
 
-		smp_num_siblings = smp_num_siblings / c->x86_num_cores;
+		smp_num_siblings = smp_num_siblings / c->x86_max_cores;
 
-		tmp = smp_num_siblings;
-		index_msb = 31;
-		while ((tmp & 0x80000000) == 0) {
-			tmp <<=1 ;
-			index_msb--;
-		}
+		index_msb = get_count_order(smp_num_siblings) ;
 
-		if (smp_num_siblings & (smp_num_siblings - 1))
-			index_msb++;
+		core_bits = get_count_order(c->x86_max_cores);
 
-		cpu_core_id[cpu] = phys_pkg_id((ebx >> 24) & 0xFF, index_msb);
+		cpu_core_id[cpu] = phys_pkg_id((ebx >> 24) & 0xFF, index_msb) &
+					       ((1 << core_bits) - 1);
 
-		if (c->x86_num_cores > 1)
+		if (c->x86_max_cores > 1)
 			printk(KERN_INFO  "CPU: Processor Core ID: %d\n",
 			       cpu_core_id[cpu]);
 	}
arch/i386/kernel/cpu/intel.c +1 -1
diff -pNru linux-2.6.14-rc4/arch/i386/kernel/cpu/intel.c linux-2.6.14-rc4-dc/arch/i386/kernel/cpu/intel.c
@@ -157,7 +157,7 @@ static void __devinit init_intel(struct 
 	if ( p )
 		strcpy(c->x86_model_id, p);
 	
-	c->x86_num_cores = num_cpu_cores(c);
+	c->x86_max_cores = num_cpu_cores(c);
 
 	detect_ht(c);
arch/i386/kernel/cpu/intel_cacheinfo.c +1 -1
diff -pNru linux-2.6.14-rc4/arch/i386/kernel/cpu/intel_cacheinfo.c linux-2.6.14-rc4-dc/arch/i386/kernel/cpu/intel_cacheinfo.c
@@ -317,7 +317,7 @@ static void __devinit cache_shared_cpu_m
 #ifdef CONFIG_X86_HT
 	else if (num_threads_sharing == smp_num_siblings)
 		this_leaf->shared_cpu_map = cpu_sibling_map[cpu];
-	else if (num_threads_sharing == (c->x86_num_cores * smp_num_siblings))
+	else if (num_threads_sharing == (c->x86_max_cores * smp_num_siblings))
 		this_leaf->shared_cpu_map = cpu_core_map[cpu];
 	else
 		printk(KERN_DEBUG "Number of CPUs sharing cache didn't match "
arch/i386/kernel/cpu/proc.c +3 -4
diff -pNru linux-2.6.14-rc4/arch/i386/kernel/cpu/proc.c linux-2.6.14-rc4-dc/arch/i386/kernel/cpu/proc.c
@@ -94,12 +94,11 @@ static int show_cpuinfo(struct seq_file 
 	if (c->x86_cache_size >= 0)
 		seq_printf(m, "cache size\t: %d KB\n", c->x86_cache_size);
 #ifdef CONFIG_X86_HT
-	if (c->x86_num_cores * smp_num_siblings > 1) {
+	if (c->x86_max_cores * smp_num_siblings > 1) {
 		seq_printf(m, "physical id\t: %d\n", phys_proc_id[n]);
-		seq_printf(m, "siblings\t: %d\n",
-				c->x86_num_cores * smp_num_siblings);
+		seq_printf(m, "siblings\t: %d\n", cpus_weight(cpu_core_map[n]));
 		seq_printf(m, "core id\t\t: %d\n", cpu_core_id[n]);
-		seq_printf(m, "cpu cores\t: %d\n", c->x86_num_cores);
+		seq_printf(m, "cpu cores\t: %d\n", c->booted_cores);
 	}
 #endif
arch/i386/kernel/smpboot.c +53 -20

123 lines not on this page · raw message

arch/x86_64/kernel/setup.c +29 -40

133 lines not on this page · raw message

arch/x86_64/kernel/smpboot.c +55 -14

127 lines not on this page · raw message

include/asm-i386/processor.h +3 -1
diff -pNru linux-2.6.14-rc4/include/asm-i386/processor.h linux-2.6.14-rc4-dc/include/asm-i386/processor.h
@@ -65,7 +65,9 @@ struct cpuinfo_x86 {
 	int	f00f_bug;
 	int	coma_bug;
 	unsigned long loops_per_jiffy;
-	unsigned char x86_num_cores;
+	unsigned char x86_max_cores;	/* cpuid returned max cores value */
+	unsigned char booted_cores;	/* number of cores as seen by OS */
+	unsigned char apicid;
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
 
 #define X86_VENDOR_INTEL 0
include/asm-x86_64/processor.h +3 -1
diff -pNru linux-2.6.14-rc4/include/asm-x86_64/processor.h linux-2.6.14-rc4-dc/include/asm-x86_64/processor.h
@@ -61,10 +61,12 @@ struct cpuinfo_x86 {
 	int	x86_cache_alignment;
 	int	x86_tlbsize;	/* number of 4K pages in DTLB/ITLB combined(in pages)*/
         __u8    x86_virt_bits, x86_phys_bits;
-	__u8	x86_num_cores;
+	__u8	x86_max_cores;	/* cpuid returned max cores value */
         __u32   x86_power; 	
 	__u32   extended_cpuid_level;	/* Max extended CPUID function supported */
 	unsigned long loops_per_jiffy;
+	__u8	apicid;
+	__u8	booted_cores;	/* number of cores as seen by OS */
 } ____cacheline_aligned;
 
 #define X86_VENDOR_INTEL 0
include/linux/bitops.h +10 -0
diff -pNru linux-2.6.14-rc4/include/linux/bitops.h linux-2.6.14-rc4-dc/include/linux/bitops.h
@@ -84,6 +84,16 @@ static __inline__ int get_bitmask_order(
 	return order;	/* We could be slightly more clever with -1 here... */
 }
 
+static __inline__ int get_count_order(unsigned int count)
+{
+	int order;
+	
+	order = fls(count) - 1;
+	if (count & (count - 1))
+		order++;
+	return order;
+}
+
 /*
  * hweightN: returns the hamming weight (i.e. the number
  * of bits set) of a N-bit word
author
#12 lore raw

PATCH 2/2 x86, x86_64: fix Intel cache detection code assumption about threads sharing

In reply to Andi Kleen
Quoted 3 lines · Andi Kleen

Ok, if you rename the variable to make it clear

x86_num_cores -> x86_max_cores

Updated patch 2/2 in the context of this change.

Andrew, please apply.

thanks,
suresh
--

Fix the Intel cache detection code assumption that number of threads sharing the cache will either be equal to number of HT or core siblings.

Signed-off-by Suresh Siddha <[email protected]>
arch/i386/kernel/cpu/intel_cacheinfo.c +32 -14
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/intel_cacheinfo.c linux/arch/i386/kernel/cpu/intel_cacheinfo.c
@@ -303,29 +303,45 @@ static struct _cpuid4_info *cpuid4_info[
 #ifdef CONFIG_SMP
 static void __devinit cache_shared_cpu_map_setup(unsigned int cpu, int index)
 {
-	struct _cpuid4_info	*this_leaf;
+	struct _cpuid4_info	*this_leaf, *sibling_leaf;
 	unsigned long num_threads_sharing;
-#ifdef CONFIG_X86_HT
-	struct cpuinfo_x86 *c = cpu_data + cpu;
-#endif
+	int index_msb, i;
+	struct cpuinfo_x86 *c = cpu_data;
 
 	this_leaf = CPUID4_INFO_IDX(cpu, index);
 	num_threads_sharing = 1 + this_leaf->eax.split.num_threads_sharing;
 
 	if (num_threads_sharing == 1)
 		cpu_set(cpu, this_leaf->shared_cpu_map);
-#ifdef CONFIG_X86_HT
-	else if (num_threads_sharing == smp_num_siblings)
-		this_leaf->shared_cpu_map = cpu_sibling_map[cpu];
-	else if (num_threads_sharing == (c->x86_max_cores * smp_num_siblings))
-		this_leaf->shared_cpu_map = cpu_core_map[cpu];
-	else
-		printk(KERN_DEBUG "Number of CPUs sharing cache didn't match "
-				"any known set of CPUs\n");
-#endif
+	else {
+		index_msb = get_count_order(num_threads_sharing);
+
+		for_each_online_cpu(i) {
+			if (c[i].apicid >> index_msb ==
+			    c[cpu].apicid >> index_msb) {
+				cpu_set(i, this_leaf->shared_cpu_map);
+				if (i != cpu && cpuid4_info[i])  {
+					sibling_leaf = CPUID4_INFO_IDX(i, index);
+					cpu_set(cpu, sibling_leaf->shared_cpu_map);
+				}
+			}
+		}
+	}
+}
+static void __devinit cache_remove_shared_cpu_map(unsigned int cpu, int index)
+{
+	struct _cpuid4_info	*this_leaf, *sibling_leaf;
+	int sibling;
+
+	this_leaf = CPUID4_INFO_IDX(cpu, index);
+	for_each_cpu_mask(sibling, this_leaf->shared_cpu_map) {
+		sibling_leaf = CPUID4_INFO_IDX(sibling, index);	
+		cpu_clear(cpu, sibling_leaf->shared_cpu_map);
+	}
 }
 #else
 static void __init cache_shared_cpu_map_setup(unsigned int cpu, int index) {}
+static void __init cache_remove_shared_cpu_map(unsigned int cpu, int index) {}
 #endif
 
 static void free_cache_attributes(unsigned int cpu)

@@ -584,8 +600,10 @@ static int __devexit cache_remove_dev(st
 	unsigned int cpu = sys_dev->id;
 	unsigned long i;
 
-	for (i = 0; i < num_cache_leaves; i++)
+	for (i = 0; i < num_cache_leaves; i++) {
+		cache_remove_shared_cpu_map(cpu, i);
 		kobject_unregister(&(INDEX_KOBJECT_PTR(cpu,i)->kobj));
+	}
 	kobject_unregister(cache_kobject[cpu]);
 	cpuid4_cache_sysfs_exit(cpu);
 	return 0;
author
#13 lore raw

PATCH 2/2 x86, x86_64: fix Intel cache detection code assumption about threads sharing

In reply to Andi Kleen

Andrew, Please apply.

thanks,
suresh
--

Fix the Intel cache detection code assumption that number of threads sharing the cache will either be equal to number of HT or core siblings.

Signed-off-by Suresh Siddha <[email protected]>
arch/i386/kernel/cpu/intel_cacheinfo.c +32 -14
diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/intel_cacheinfo.c linux/arch/i386/kernel/cpu/intel_cacheinfo.c
@@ -303,29 +303,45 @@ static struct _cpuid4_info *cpuid4_info[
 #ifdef CONFIG_SMP
 static void __devinit cache_shared_cpu_map_setup(unsigned int cpu, int index)
 {
-	struct _cpuid4_info	*this_leaf;
+	struct _cpuid4_info	*this_leaf, *sibling_leaf;
 	unsigned long num_threads_sharing;
-#ifdef CONFIG_X86_HT
-	struct cpuinfo_x86 *c = cpu_data + cpu;
-#endif
+	int index_msb, i;
+	struct cpuinfo_x86 *c = cpu_data;
 
 	this_leaf = CPUID4_INFO_IDX(cpu, index);
 	num_threads_sharing = 1 + this_leaf->eax.split.num_threads_sharing;
 
 	if (num_threads_sharing == 1)
 		cpu_set(cpu, this_leaf->shared_cpu_map);
-#ifdef CONFIG_X86_HT
-	else if (num_threads_sharing == smp_num_siblings)
-		this_leaf->shared_cpu_map = cpu_sibling_map[cpu];
-	else if (num_threads_sharing == (c->x86_num_cores * smp_num_siblings))
-		this_leaf->shared_cpu_map = cpu_core_map[cpu];
-	else
-		printk(KERN_DEBUG "Number of CPUs sharing cache didn't match "
-				"any known set of CPUs\n");
-#endif
+	else {
+		index_msb = get_count_order(num_threads_sharing);
+
+		for_each_online_cpu(i) {
+			if (c[i].apicid >> index_msb ==
+			    c[cpu].apicid >> index_msb) {
+				cpu_set(i, this_leaf->shared_cpu_map);
+				if (i != cpu && cpuid4_info[i])  {
+					sibling_leaf = CPUID4_INFO_IDX(i, index);
+					cpu_set(cpu, sibling_leaf->shared_cpu_map);
+				}
+			}
+		}
+	}
+}
+static void __devinit cache_remove_shared_cpu_map(unsigned int cpu, int index)
+{
+	struct _cpuid4_info	*this_leaf, *sibling_leaf;
+	int sibling;
+
+	this_leaf = CPUID4_INFO_IDX(cpu, index);
+	for_each_cpu_mask(sibling, this_leaf->shared_cpu_map) {
+		sibling_leaf = CPUID4_INFO_IDX(sibling, index);	
+		cpu_clear(cpu, sibling_leaf->shared_cpu_map);
+	}
 }
 #else
 static void __init cache_shared_cpu_map_setup(unsigned int cpu, int index) {}
+static void __init cache_remove_shared_cpu_map(unsigned int cpu, int index) {}
 #endif
 
 static void free_cache_attributes(unsigned int cpu)

@@ -584,8 +600,10 @@ static int __devexit cache_remove_dev(st
 	unsigned int cpu = sys_dev->id;
 	unsigned long i;
 
-	for (i = 0; i < num_cache_leaves; i++)
+	for (i = 0; i < num_cache_leaves; i++) {
+		cache_remove_shared_cpu_map(cpu, i);
 		kobject_unregister(&(INDEX_KOBJECT_PTR(cpu,i)->kobj));
+	}
 	kobject_unregister(cache_kobject[cpu]);
 	cpuid4_cache_sysfs_exit(cpu);
 	return 0;