Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753769Ab2HCM1l (ORCPT ); Fri, 3 Aug 2012 08:27:41 -0400 Received: from osrc3.amd.com ([217.9.48.20]:46624 "EHLO mail.x86-64.org" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1752922Ab2HCM1i (ORCPT ); Fri, 3 Aug 2012 08:27:38 -0400 Date: Fri, 3 Aug 2012 14:27:27 +0200 From: Borislav Petkov To: Ben Hutchings Cc: Sven Joachim , linux-kernel@vger.kernel.org, stable@vger.kernel.org, torvalds@linux-foundation.org, akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk, Henrique de Moraes Holschuh , Peter Zijlstra , "H. Peter Anvin" , Kevin Winchester Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface Message-ID: <20120803122727.GA32299@aftab.osrc.amd.com> References: <20120731044310.013763753@decadent.org.uk> <20120731044316.585695559@decadent.org.uk> <87y5lw73eh.fsf@turtle.gmx.de> <20120803094314.GB31792@aftab.osrc.amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120803094314.GB31792@aftab.osrc.amd.com> 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 Content-Length: 11751 Lines: 363 On Fri, Aug 03, 2012 at 11:43:14AM +0200, Borislav Petkov wrote: > On Fri, Aug 03, 2012 at 11:04:06AM +0200, Sven Joachim wrote: > > On 2012-07-31 06:43 +0200, Ben Hutchings wrote: > > > > > 3.2-stable review patch. If anyone has any objections, please let me know. > > > > Alas, this does not build if CONFIG_SMP is unset: > > > > ,---- > > | arch/x86/kernel/microcode_core.c: In function 'reload_store': > > | arch/x86/kernel/microcode_core.c:304:19: error: 'struct cpuinfo_x86' has no member named 'cpu_index' > > `---- > > Crap. :-( > > 3.2 still has this: > > : > ... > #ifdef CONFIG_SMP > /* number of cores as seen by the OS: */ > u16 booted_cores; > /* Physical processor id: */ > u16 phys_proc_id; > /* Core id: */ > u16 cpu_core_id; > /* Compute unit id */ > u8 compute_unit_id; > /* Index into per_cpu list: */ > u16 cpu_index; > #endif > u32 microcode; > } __attribute__((__aligned__(SMP_CACHE_BYTES))); > --- > > which got removed by > > commit 141168c36cdee3ff23d9c7700b0edc47cb65479f > Author: Kevin Winchester > Date: Tue Dec 20 20:52:22 2011 -0400 > > x86: Simplify code by removing a !SMP #ifdefs from 'struct cpuinfo_x86' > > Ben, you might want to backport this one too... I'll run a couple of 3.2 > builds with it ontop of 3.2 to verify nothing else breaks. Ok, 141168c36cdee3ff23d9c7700b0edc47cb65479f doesn't apply cleanly to 3.2-stable, as expected. I've attached a partly backported version. Why partly? Well, it broke an UP build in mainline which got fixed later by commit 3f806e50981825fa56a7f1938f24c0680816be45 Author: Borislav Petkov Date: Fri Feb 3 20:18:01 2012 +0100 x86/mce/AMD: Fix UP build error 141168c36cde ("x86: Simplify code by removing a !SMP #ifdefs from 'struct cpuinfo_x86'") removed a bunch of CONFIG_SMP ifdefs around code touching struct cpuinfo_x86 members but also caused the following build error with Randy's randconfigs: mce_amd.c:(.cpuinit.text+0x4723): undefined reference to `cpu_llc_shared_map' --- which reverted what the original patch removed. So I've taken out the parts that introduce the breakage from the backport. And the attached version survives a bunch of smoke tests like all{yes,no,mod}config builds on 32 and 64-bit. @Sven: it should fix the issue on your box too. HTH. -- >From 5e2fe6b301f5f969f25e4404a6b9d069957b8aeb Mon Sep 17 00:00:00 2001 From: Kevin Winchester Date: Tue, 20 Dec 2011 20:52:22 -0400 Subject: [PATCH] x86: Simplify code by removing a !SMP #ifdefs from 'struct cpuinfo_x86' Upstream commit: 141168c36cdee3ff23d9c7700b0edc47cb65479f Several fields in struct cpuinfo_x86 were not defined for the !SMP case, likely to save space. However, those fields still have some meaning for UP, and keeping them allows some #ifdef removal from other files. The additional size of the UP kernel from this change is not significant enough to worry about keeping up the distinction: text data bss dec hex filename 4737168 506459 972040 6215667 5ed7f3 vmlinux.o.before 4737444 506459 972040 6215943 5ed907 vmlinux.o.after for a difference of 276 bytes for an example UP config. If someone wants those 276 bytes back badly then it should be implemented in a cleaner way. Signed-off-by: Kevin Winchester Cc: Steffen Persvold Link: http://lkml.kernel.org/r/1324428742-12498-1-git-send-email-kjwinchester@gmail.com Signed-off-by: Ingo Molnar Signed-off-by: Borislav Petkov --- arch/x86/include/asm/processor.h | 2 -- arch/x86/kernel/amd_nb.c | 8 ++------ arch/x86/kernel/cpu/amd.c | 2 -- arch/x86/kernel/cpu/common.c | 5 ----- arch/x86/kernel/cpu/intel.c | 2 -- arch/x86/kernel/cpu/mcheck/mce.c | 2 -- arch/x86/kernel/cpu/mcheck/mce_amd.c | 5 +---- arch/x86/kernel/cpu/proc.c | 4 +--- drivers/edac/sb_edac.c | 2 -- drivers/hwmon/coretemp.c | 7 +++---- 10 files changed, 7 insertions(+), 32 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index bb3ee3629a0f..f7c89e231c6c 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -99,7 +99,6 @@ struct cpuinfo_x86 { u16 apicid; u16 initial_apicid; u16 x86_clflush_size; -#ifdef CONFIG_SMP /* number of cores as seen by the OS: */ u16 booted_cores; /* Physical processor id: */ @@ -110,7 +109,6 @@ struct cpuinfo_x86 { u8 compute_unit_id; /* Index into per_cpu list: */ u16 cpu_index; -#endif u32 microcode; } __attribute__((__aligned__(SMP_CACHE_BYTES))); diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index bae1efe6d515..be16854591cc 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -154,16 +154,14 @@ int amd_get_subcaches(int cpu) { struct pci_dev *link = node_to_amd_nb(amd_get_nb_id(cpu))->link; unsigned int mask; - int cuid = 0; + int cuid; if (!amd_nb_has_feature(AMD_NB_L3_PARTITIONING)) return 0; pci_read_config_dword(link, 0x1d4, &mask); -#ifdef CONFIG_SMP cuid = cpu_data(cpu).compute_unit_id; -#endif return (mask >> (4 * cuid)) & 0xf; } @@ -172,7 +170,7 @@ int amd_set_subcaches(int cpu, int mask) static unsigned int reset, ban; struct amd_northbridge *nb = node_to_amd_nb(amd_get_nb_id(cpu)); unsigned int reg; - int cuid = 0; + int cuid; if (!amd_nb_has_feature(AMD_NB_L3_PARTITIONING) || mask > 0xf) return -EINVAL; @@ -190,9 +188,7 @@ int amd_set_subcaches(int cpu, int mask) pci_write_config_dword(nb->misc, 0x1b8, reg & ~0x180000); } -#ifdef CONFIG_SMP cuid = cpu_data(cpu).compute_unit_id; -#endif mask <<= 4 * cuid; mask |= (0xf ^ (1 << cuid)) << 26; diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 3524e1f5e960..ff8557e2e101 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -148,7 +148,6 @@ static void __cpuinit init_amd_k6(struct cpuinfo_x86 *c) static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c) { -#ifdef CONFIG_SMP /* calling is from identify_secondary_cpu() ? */ if (!c->cpu_index) return; @@ -192,7 +191,6 @@ static void __cpuinit amd_k7_smp_check(struct cpuinfo_x86 *c) valid_k7: ; -#endif } static void __cpuinit init_amd_k7(struct cpuinfo_x86 *c) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index aa003b13a831..ca93cc79fbc6 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -676,9 +676,7 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c) if (this_cpu->c_early_init) this_cpu->c_early_init(c); -#ifdef CONFIG_SMP c->cpu_index = 0; -#endif filter_cpuid_features(c, false); setup_smep(c); @@ -764,10 +762,7 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c) c->apicid = c->initial_apicid; # endif #endif - -#ifdef CONFIG_X86_HT c->phys_proc_id = c->initial_apicid; -#endif } setup_smep(c); diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 523131213f08..3e6ff6cbf42a 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -181,7 +181,6 @@ static void __cpuinit trap_init_f00f_bug(void) static void __cpuinit intel_smp_check(struct cpuinfo_x86 *c) { -#ifdef CONFIG_SMP /* calling is from identify_secondary_cpu() ? */ if (!c->cpu_index) return; @@ -198,7 +197,6 @@ static void __cpuinit intel_smp_check(struct cpuinfo_x86 *c) WARN_ONCE(1, "WARNING: SMP operation may be unreliable" "with B stepping processors.\n"); } -#endif } static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c) diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index b0f1271e3138..3b678770dba5 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -119,9 +119,7 @@ void mce_setup(struct mce *m) m->time = get_seconds(); m->cpuvendor = boot_cpu_data.x86_vendor; m->cpuid = cpuid_eax(1); -#ifdef CONFIG_SMP m->socketid = cpu_data(m->extcpu).phys_proc_id; -#endif m->apicid = cpu_data(m->extcpu).initial_apicid; rdmsrl(MSR_IA32_MCG_CAP, m->mcgcap); } diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c index 445a61c39dff..d4444be98912 100644 --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c @@ -65,11 +65,9 @@ struct threshold_bank { }; static DEFINE_PER_CPU(struct threshold_bank * [NR_BANKS], threshold_banks); -#ifdef CONFIG_SMP static unsigned char shared_bank[NR_BANKS] = { 0, 0, 0, 0, 1 }; -#endif static DEFINE_PER_CPU(unsigned char, bank_map); /* see which banks are on */ @@ -227,10 +225,9 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c) if (!block) per_cpu(bank_map, cpu) |= (1 << bank); -#ifdef CONFIG_SMP + if (shared_bank[bank] && c->cpu_core_id) break; -#endif memset(&b, 0, sizeof(b)); b.cpu = cpu; diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index 14b23140e81f..8022c6681485 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -64,12 +64,10 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c) static int show_cpuinfo(struct seq_file *m, void *v) { struct cpuinfo_x86 *c = v; - unsigned int cpu = 0; + unsigned int cpu; int i; -#ifdef CONFIG_SMP cpu = c->cpu_index; -#endif seq_printf(m, "processor\t: %u\n" "vendor_id\t: %s\n" "cpu family\t: %d\n" diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c index 18a129391c0f..0db57b594c62 100644 --- a/drivers/edac/sb_edac.c +++ b/drivers/edac/sb_edac.c @@ -1609,11 +1609,9 @@ static int sbridge_mce_check_error(struct notifier_block *nb, unsigned long val, mce->cpuvendor, mce->cpuid, mce->time, mce->socketid, mce->apicid); -#ifdef CONFIG_SMP /* Only handle if it is the right mc controller */ if (cpu_data(mce->cpu).phys_proc_id != pvt->sbridge_dev->mc) return NOTIFY_DONE; -#endif smp_rmb(); if ((pvt->mce_out + 1) % MCE_LOG_LEN == pvt->mce_in) { diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c index 0790c98e294e..19b4412ed534 100644 --- a/drivers/hwmon/coretemp.c +++ b/drivers/hwmon/coretemp.c @@ -57,16 +57,15 @@ MODULE_PARM_DESC(tjmax, "TjMax value in degrees Celsius"); #define TOTAL_ATTRS (MAX_CORE_ATTRS + 1) #define MAX_CORE_DATA (NUM_REAL_CORES + BASE_SYSFS_ATTR_NO) -#ifdef CONFIG_SMP #define TO_PHYS_ID(cpu) cpu_data(cpu).phys_proc_id #define TO_CORE_ID(cpu) cpu_data(cpu).cpu_core_id +#define TO_ATTR_NO(cpu) (TO_CORE_ID(cpu) + BASE_SYSFS_ATTR_NO) + +#ifdef CONFIG_SMP #define for_each_sibling(i, cpu) for_each_cpu(i, cpu_sibling_mask(cpu)) #else -#define TO_PHYS_ID(cpu) (cpu) -#define TO_CORE_ID(cpu) (cpu) #define for_each_sibling(i, cpu) for (i = 0; false; ) #endif -#define TO_ATTR_NO(cpu) (TO_CORE_ID(cpu) + BASE_SYSFS_ATTR_NO) /* * Per-Core Temperature Data -- 1.7.11.rc1 -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/