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

2012-08-05 Thread Borislav Petkov 
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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-08-05 Thread Ben Hutchings 
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.


signature.asc
Description: This is a digitally signed message part

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

2012-08-04 Thread Ben Hutchings 
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 kjwinches...@gmail.com
  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 b...@alien8.de
 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


signature.asc
Description: This is a digitally signed message part

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

2012-08-04 Thread Henrique de Moraes Holschuh 
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 kjwinches...@gmail.com
   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 b...@alien8.de
  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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-08-04 Thread Ben Hutchings 
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


signature.asc
Description: This is a digitally signed message part

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

2012-08-03 Thread Sven Joachim 
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 borislav.pet...@amd.com

 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 h...@hmh.eng.br
 Cc: Peter Zijlstra pet...@infradead.org
 Signed-off-by: Borislav Petkov borislav.pet...@amd.com
 Link: http://lkml.kernel.org/r/1340280437-7718-2-git-send-email...@amd64.org
 Signed-off-by: H. Peter Anvin h...@zytor.com
 Signed-off-by: Ben Hutchings b...@decadent.org.uk
 ---
  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;
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-08-03 Thread Borislav Petkov 
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 kjwinches...@gmail.com
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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-08-03 Thread Borislav Petkov 
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 kjwinches...@gmail.com
 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 b...@alien8.de
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 kjwinches...@gmail.com
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:

   textdata 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 kjwinches...@gmail.com
Cc: Steffen Persvold s...@numascale.com
Link: 
http://lkml.kernel.org/r/1324428742-12498-1-git-send-email-kjwinches...@gmail.com
Signed-off-by: Ingo Molnar mi...@elte.hu
Signed-off-by: Borislav Petkov borislav.pet...@amd.com
---
 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