2012-08-03 09:04:13

by Sven Joachim

[permalink] [raw]
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

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'
`----

Cheers,
Sven


> From: Borislav Petkov <[email protected]>
>
> commit c9fc3f778a6a215ace14ee556067c73982b6d40f upstream.
>
> Microcode reloading in a per-core manner is a very bad idea for both
> major x86 vendors. And the thing is, we have such interface with which
> we can end up with different microcode versions applied on different
> cores of an otherwise homogeneous wrt (family,model,stepping) system.
>
> So turn off the possibility of doing that per core and allow it only
> system-wide.
>
> This is a minimal fix which we'd like to see in stable too thus the
> more-or-less arbitrary decision to allow system-wide reloading only on
> the BSP:
>
> $ echo 1 > /sys/devices/system/cpu/cpu0/microcode/reload
> ...
>
> and disable the interface on the other cores:
>
> $ echo 1 > /sys/devices/system/cpu/cpu23/microcode/reload
> -bash: echo: write error: Invalid argument
>
> Also, allowing the reload only from one CPU (the BSP in
> that case) doesn't allow the reload procedure to degenerate
> into an O(n^2) deal when triggering reloads from all
> /sys/devices/system/cpu/cpuX/microcode/reload sysfs nodes
> simultaneously.
>
> A more generic fix will follow.
>
> Cc: Henrique de Moraes Holschuh <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> Link: http://lkml.kernel.org/r/[email protected]
> Signed-off-by: H. Peter Anvin <[email protected]>
> Signed-off-by: Ben Hutchings <[email protected]>
> ---
> arch/x86/kernel/microcode_core.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
> index fbdfc69..24b852b 100644
> --- a/arch/x86/kernel/microcode_core.c
> +++ b/arch/x86/kernel/microcode_core.c
> @@ -298,19 +298,31 @@ static ssize_t reload_store(struct device *dev,
> const char *buf, size_t size)
> {
> unsigned long val;
> - int cpu = dev->id;
> - ssize_t ret = 0;
> + int cpu;
> + ssize_t ret = 0, tmp_ret;
> +
> + /* allow reload only from the BSP */
> + if (boot_cpu_data.cpu_index != dev->id)
> + return -EINVAL;
>
> ret = kstrtoul(buf, 0, &val);
> if (ret)
> return ret;
>
> - if (val == 1) {
> - get_online_cpus();
> - if (cpu_online(cpu))
> - ret = reload_for_cpu(cpu);
> - put_online_cpus();
> + if (val != 1)
> + return size;
> +
> + get_online_cpus();
> + for_each_online_cpu(cpu) {
> + tmp_ret = reload_for_cpu(cpu);
> + if (tmp_ret != 0)
> + pr_warn("Error reloading microcode on CPU %d\n", cpu);
> +
> + /* save retval of the first encountered reload error */
> + if (!ret)
> + ret = tmp_ret;
> }
> + put_online_cpus();
>
> if (!ret)
> ret = size;


2012-08-03 09:43:26

by Borislav Petkov

[permalink] [raw]
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

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:

<arch/x86/include/asm/processor.h>:
...
#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 <[email protected]>
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.

Thanks.

--
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

2012-08-03 12:27:41

by Borislav Petkov

[permalink] [raw]
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

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:
>
> <arch/x86/include/asm/processor.h>:
> ...
> #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 <[email protected]>
> 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Cc: Steffen Persvold <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
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

2012-08-04 15:41:42

by Ben Hutchings

[permalink] [raw]
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

On Fri, 2012-08-03 at 14:27 +0200, Borislav Petkov wrote:
> 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:
> >
> > <arch/x86/include/asm/processor.h>:
> > ...
> > #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 <[email protected]>
> > 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 <[email protected]>
> 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.
[...]

Thanks everyone for working this out.

If you combine multiple mainline commits like this, the new commit
message should refer to all of them. I've fixed that up this time.

Ben.

--
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
- Carolyn Scheppner


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

On Sat, 04 Aug 2012, Ben Hutchings wrote:
> On Fri, 2012-08-03 at 14:27 +0200, Borislav Petkov wrote:
> > 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:
> > >
> > > <arch/x86/include/asm/processor.h>:
> > > ...
> > > #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 <[email protected]>
> > > 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 <[email protected]>
> > 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.
> [...]
>
> Thanks everyone for working this out.
>
> If you combine multiple mainline commits like this, the new commit
> message should refer to all of them. I've fixed that up this time.

Ben, the backport is also needed on 3.0 and 3.4, do you have your patch
queue available for download/pull somewhere?

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2012-08-04 17:23:56

by Ben Hutchings

[permalink] [raw]
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

On Sat, 2012-08-04 at 13:07 -0300, Henrique de Moraes Holschuh wrote:
> On Sat, 04 Aug 2012, Ben Hutchings wrote:
> > On Fri, 2012-08-03 at 14:27 +0200, Borislav Petkov wrote:
> > > 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'
> > > > > `----
[...]
> >
> > Thanks everyone for working this out.
> >
> > If you combine multiple mainline commits like this, the new commit
> > message should refer to all of them. I've fixed that up this time.
>
> Ben, the backport is also needed on 3.0 and 3.4, do you have your patch
> queue available for download/pull somewhere?

This is in v3.2.26, tagged in git
<git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-3.2.y.git>.
I'll wait for Greg to generate tarballs etc. before sending the
announcement.

Ben.

--
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
- Carolyn Scheppner


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2012-08-05 09:21:23

by Borislav Petkov

[permalink] [raw]
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

On Sat, Aug 04, 2012 at 06:23:41PM +0100, Ben Hutchings wrote:

[ … ]

> > > Thanks everyone for working this out.
> > >
> > > If you combine multiple mainline commits like this, the new commit
> > > message should refer to all of them. I've fixed that up this time.

Thanks.

> > Ben, the backport is also needed on 3.0 and 3.4, do you have your patch
> > queue available for download/pull somewhere?
>
> This is in v3.2.26, tagged in git
> <git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-3.2.y.git>.
> I'll wait for Greg to generate tarballs etc. before sending the
> announcement.

Ok, guys.

Pls let me know if I should send the backported patch for 3.0 and 3.4 to
Greg or you are doing this.

Thanks.

--
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

2012-08-05 18:56:31

by Ben Hutchings

[permalink] [raw]
Subject: Re: [ 33/73] x86, microcode: Sanitize per-cpu microcode reloading interface

On Sun, 2012-08-05 at 11:21 +0200, Borislav Petkov wrote:
> On Sat, Aug 04, 2012 at 06:23:41PM +0100, Ben Hutchings wrote:
>
> [ … ]
>
> > > > Thanks everyone for working this out.
> > > >
> > > > If you combine multiple mainline commits like this, the new commit
> > > > message should refer to all of them. I've fixed that up this time.
>
> Thanks.
>
> > > Ben, the backport is also needed on 3.0 and 3.4, do you have your patch
> > > queue available for download/pull somewhere?
> >
> > This is in v3.2.26, tagged in git
> > <git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-3.2.y.git>.
> > I'll wait for Greg to generate tarballs etc. before sending the
> > announcement.
>
> Ok, guys.
>
> Pls let me know if I should send the backported patch for 3.0 and 3.4 to
> Greg or you are doing this.

Please do that. They will both need commit
e826abd523913f63eb03b59746ffb16153c53dc4 ('x86, microcode:
microcode_core.c simple_strtoul cleanup') and 3.0 needs the !SMP fix.

Ben.

--
Ben Hutchings
Computers are not intelligent. They only think they are.


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part