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.
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;
> 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?
On Fri, Oct 07, 2005 at 11:52:00AM +0200, Andi Kleen wrote: > > 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.
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; diff -pNru linux-2.6.14-rc3/arch/i386/kernel/cpu/intel_htmc.c linux/arch/i386/kernel/cpu/intel_htmc.c --- linux-2.6.14-rc3/arch/i386/kernel/cpu/intel_htmc.c 1969-12-31 16:00:00.000000000 -0800 +++ linux/arch/i386/kernel/cpu/intel_htmc.c 2005-10-07 16:27:37.708580280 -0700 @@ -0,0 +1,81 @@ +/* + * Copyright (C) 2005 Intel Corp + * Intel Multi Core and Hyper-Threading detection routines + * + * Changes: + * Suresh Siddha : Merge x86 and x86_64 code with some fixes. + */ +#include <linux/config.h> +#include <linux/init.h> + +#include <linux/smp.h> +#include <asm/processor.h> + + +/* + * find out the number of processor cores on the die + */ +int __cpuinit intel_num_cpu_cores(struct cpuinfo_x86 *c) +{ + unsigned int eax, ebx, ecx, edx; + + if (c->cpuid_level < 4) + return 1; + + cpuid_count(4, 0, &eax, &ebx, &ecx, &edx); + + if (eax & 0x1f) + return ((eax >> 26) + 1); + else + return 1; +} + +#ifdef CONFIG_X86_HT +#include <asm/mach_apic.h> + +void __cpuinit detect_ht(struct cpuinfo_x86 *c) +{ + u32 eax, ebx, ecx, edx; + 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; + + 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 ) { + + if (smp_num_siblings > NR_CPUS) { + printk(KERN_WARNING "CPU: Unsupported number of the siblings %d", smp_num_siblings); + smp_num_siblings = 1; + return; + } + + 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; + + index_msb = get_count_order(smp_num_siblings) ; + + core_bits = get_count_order(c->x86_num_cores); + + cpu_core_id[cpu] =
...
On Saturday 08 October 2005 02:52, Siddha, Suresh B wrote:
> On Fri, Oct 07, 2005 at 11:52:00AM +0200, Andi Kleen wrote: > > > 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.
On Sat, Oct 08, 2005 at 12:28:38PM +0200, Andi Kleen wrote: > On Saturday 08 October 2005 02:52, Siddha, Suresh B wrote: > > On Fri, Oct 07, 2005 at 11:52:00AM +0200, Andi Kleen wrote: > > > > 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.
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 :)
> 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.
