lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 26 Apr 2018 11:11:59 +0200 (CEST) From: Thomas Gleixner <tglx@...utronix.de> To: David Wang <davidwang@...oxin.com> cc: mingo@...hat.com, hpa@...or.com, gregkh@...uxfoundation.org, x86@...nel.org, linux-kernel@...r.kernel.org, brucechang@...-alliance.com, cooperyan@...oxin.com, qiyuanwang@...oxin.com, benjaminpan@...tech.com, lukelin@...cpu.com, timguo@...oxin.com Subject: Re: [PATCH v2] x86/centaur: report correct CPU/cache topology On Wed, 25 Apr 2018, David Wang wrote: > > +static void early_init_centaur_mc(struct cpuinfo_x86 *c) > +{ > +#ifdef CONFIG_SMP > + unsigned int eax, ebx, ecx, edx; > + > + if (c->cpuid_level < 4) > + return; > + > + cpuid_count(4, 0, &eax, &ebx, &ecx, &edx); > + if (eax & 0x1f) > + c->x86_max_cores = (eax >> 26) + 1; > + else > + return; > +#endif > +} My review comment from last time still stands: > > This is a bad copy of intel_num_cpu_cores(). See for the subtle > > difference. Please rename the intel function and move it to common.c In other words: Make a patch which moves intel_num_cpu_cores() into common.c. Rename the function into something like detect_num_cpu_cores() and fix up the call site in intel.c. Then add your patch and use the very same function. > + > static void early_init_centaur(struct cpuinfo_x86 *c) > { > + early_init_centaur_mc(c); > switch (c->x86) { > #ifdef CONFIG_X86_32 > case 5: > @@ -146,6 +163,7 @@ static void centaur_detect_vmx_virtcap(struct cpuinfo_x86 *c) > > static void init_centaur(struct cpuinfo_x86 *c) > { > + unsigned int l2 = 0; > #ifdef CONFIG_X86_32 > char *name; > u32 fcr_set = 0; > @@ -161,6 +179,17 @@ static void init_centaur(struct cpuinfo_x86 *c) > #endif > early_init_centaur(c); > > + l2 = init_intel_cacheinfo(c); > + > + /* Detect legacy cache sizes if init_intel_cacheinfo did not */ > + if (l2 == 0) { > + cpu_detect_cache_sizes(c); > + } Aside of the pointless parentheses, this really wants to be cleaned up as well. init_intel_cacheinfo() is called from the intel init code and it does the same silly thing. So the right thing to do is in a separate patch first --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -679,12 +679,6 @@ static void init_intel(struct cpuinfo_x8 l2 = init_intel_cacheinfo(c); - /* Detect legacy cache sizes if init_intel_cacheinfo did not */ - if (l2 == 0) { - cpu_detect_cache_sizes(c); - l2 = c->x86_cache_size; - } - if (c->cpuid_level > 9) { unsigned eax = cpuid_eax(10); /* Check for version and the number of counters */ --- a/arch/x86/kernel/cpu/intel_cacheinfo.c +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c @@ -802,6 +802,11 @@ unsigned int init_intel_cacheinfo(struct c->x86_cache_size = l3 ? l3 : (l2 ? l2 : (l1i+l1d)); + /* Detect legacy cache sizes if the above did not work */ + if (!l2) { + cpu_detect_cache_sizes(c); + l2 = c->x86_cache_size; + } return l2; } Hmm? tglx
Powered by blists - more mailing lists