All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems
@ 2016-05-31 21:22 Andi Kleen
  2016-06-09  4:34 ` Elliott, Robert (Persistent Memory)
  0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2016-05-31 21:22 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

On large systems the microcode driver is very noisy, because it prints
a line for each CPU. The lines are redundant because because usually
all CPUs are updated to the same microcode revision.

All other subsystems have been patched previously to not print
a line for each CPU. Only the microcode driver is left.

Only print an microcode revision update when something changed. This results
in typically only a single line being printed.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/cpu/microcode/intel.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index cbb3cf0..952c8a3 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -794,6 +794,7 @@ void reload_ucode_intel(void)
 
 static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
 {
+	static struct cpu_signature prev;
 	struct cpuinfo_x86 *c = &cpu_data(cpu_num);
 	unsigned int val[2];
 
@@ -808,8 +809,14 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
 	}
 
 	csig->rev = c->microcode;
-	pr_info("CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
-		cpu_num, csig->sig, csig->pf, csig->rev);
+
+	/* No extra locking on prev, races are harmless. */
+	if (csig->sig != prev.sig || csig->pf != prev.pf ||
+	    csig->rev != prev.rev) {
+		pr_info("CPU sig=0x%x, pf=0x%x, revision=0x%x\n",
+			csig->sig, csig->pf, csig->rev);
+		prev = *csig;
+	}
 
 	return 0;
 }
@@ -838,6 +845,7 @@ static int apply_microcode_intel(int cpu)
 	struct ucode_cpu_info *uci;
 	struct cpuinfo_x86 *c;
 	unsigned int val[2];
+	static int prev_rev;
 
 	/* We should bind the task to the CPU */
 	if (WARN_ON(raw_smp_processor_id() != cpu))
@@ -872,11 +880,14 @@ static int apply_microcode_intel(int cpu)
 		return -1;
 	}
 
-	pr_info("CPU%d updated to revision 0x%x, date = %04x-%02x-%02x\n",
-		cpu, val[1],
-		mc->hdr.date & 0xffff,
-		mc->hdr.date >> 24,
-		(mc->hdr.date >> 16) & 0xff);
+	if (val[1] != prev_rev) {
+		pr_info("CPU updated to revision 0x%x, date = %04x-%02x-%02x\n",
+			val[1],
+			mc->hdr.date & 0xffff,
+			mc->hdr.date >> 24,
+			(mc->hdr.date >> 16) & 0xff);
+		prev_rev = val[1];
+	}
 
 	c = &cpu_data(cpu);
 
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems
  2016-05-31 21:22 [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems Andi Kleen
@ 2016-06-09  4:34 ` Elliott, Robert (Persistent Memory)
  2016-06-09 15:47   ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 6+ messages in thread
From: Elliott, Robert (Persistent Memory) @ 2016-06-09  4:34 UTC (permalink / raw)
  To: Andi Kleen, x86; +Cc: linux-kernel, Andi Kleen



> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Andi Kleen
> Sent: Tuesday, May 31, 2016 4:23 PM
> To: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org; Andi Kleen <ak@linux.intel.com>
> Subject: [PATCH] x86/microcode/intel: Quieten down microcode updates
> on large systems
...
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c
> @@ -808,8 +809,14 @@ static int collect_cpu_info(int cpu_num, struct
> cpu_signature *csig)
>  	}
> 
>  	csig->rev = c->microcode;
> -	pr_info("CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
> -		cpu_num, csig->sig, csig->pf, csig->rev);
> +
> +	/* No extra locking on prev, races are harmless. */
> +	if (csig->sig != prev.sig || csig->pf != prev.pf ||
> +	    csig->rev != prev.rev) {
> +		pr_info("CPU sig=0x%x, pf=0x%x, revision=0x%x\n",
> +			csig->sig, csig->pf, csig->rev);
> +		prev = *csig;
> +	}
...
> -	pr_info("CPU%d updated to revision 0x%x, date = %04x-%02x-
> %02x\n",
> -		cpu, val[1],
> -		mc->hdr.date & 0xffff,
> -		mc->hdr.date >> 24,
> -		(mc->hdr.date >> 16) & 0xff);
> +	if (val[1] != prev_rev) {
> +		pr_info("CPU updated to revision 0x%x, date = %04x-%02x-
> %02x\n",

"CPU(s)" would help convey that the messages now apply to one or 
more CPUs.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems
  2016-06-09  4:34 ` Elliott, Robert (Persistent Memory)
@ 2016-06-09 15:47   ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 6+ messages in thread
From: Henrique de Moraes Holschuh @ 2016-06-09 15:47 UTC (permalink / raw)
  To: Elliott, Robert (Persistent Memory)
  Cc: Andi Kleen, x86, linux-kernel, Andi Kleen

On Thu, 09 Jun 2016, Elliott, Robert (Persistent Memory) wrote:
> > +	if (val[1] != prev_rev) {
> > +		pr_info("CPU updated to revision 0x%x, date = %04x-%02x-
> > %02x\n",
> 
> "CPU(s)" would help convey that the messages now apply to one or 
> more CPUs.

Agreed, especially because they *already* apply to more than one "cpu"
(because cpu for the kernel is a hardware thread for the hardware, and
updating the microcode for a core usually updates all threads hosted by
that core, thus "several cpus").

And yes, Debian got one or two bugs about people getting confused
because some "cpus" were never "listed" as being updated...

Obviously, it is fine if a simple follow-up patch fixes this message to
say "CPU(s)", or, if we really want to go for minimum chance of
confusion: "One or more CPUs updated to...", as that will help a lot in
mixed-stepping configurations, where you will get a few such messages,
and just saying "CPU(s)" might cause people to get the wrong idea that
the same set of CPUs have been updated several times.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems
  2016-06-09 13:41 Andi Kleen
  2016-06-09 23:58 ` Henrique de Moraes Holschuh
@ 2016-06-10  9:54 ` Borislav Petkov
  1 sibling, 0 replies; 6+ messages in thread
From: Borislav Petkov @ 2016-06-10  9:54 UTC (permalink / raw)
  To: Andi Kleen; +Cc: x86, hmh, elliott, linux-kernel, Andi Kleen

On Thu, Jun 09, 2016 at 06:41:41AM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> On large systems the microcode driver is very noisy, because it prints
> a line for each CPU. The lines are redundant because because usually
> all CPUs are updated to the same microcode revision.
> 
> All other subsystems have been patched previously to not print
> a line for each CPU. Only the microcode driver is left.
> 
> Only print an microcode revision update when something changed. This results
> in typically only a single line being printed.
> 
> v2: Change message to "One or more CPUs"
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/microcode/intel.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index cbb3cf0..54f5f6c 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -794,6 +794,7 @@ void reload_ucode_intel(void)
>  
>  static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
>  {
> +	static struct cpu_signature prev;
>  	struct cpuinfo_x86 *c = &cpu_data(cpu_num);
>  	unsigned int val[2];
>  
> @@ -808,8 +809,14 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
>  	}
>  
>  	csig->rev = c->microcode;
> -	pr_info("CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
> -		cpu_num, csig->sig, csig->pf, csig->rev);
> +
> +	/* No extra locking on prev, races are harmless. */
> +	if (csig->sig != prev.sig || csig->pf != prev.pf ||
> +	    csig->rev != prev.rev) {
> +		pr_info("One or more CPUs sig=0x%x, pf=0x%x, revision=0x%x\n",

This "One or more CPUs" is just silly. I've removed it while applying.
This way, there's no mentioning of CPUs and people can check
/proc/cpuinfo for that.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems
  2016-06-09 13:41 Andi Kleen
@ 2016-06-09 23:58 ` Henrique de Moraes Holschuh
  2016-06-10  9:54 ` Borislav Petkov
  1 sibling, 0 replies; 6+ messages in thread
From: Henrique de Moraes Holschuh @ 2016-06-09 23:58 UTC (permalink / raw)
  To: Andi Kleen; +Cc: x86, elliott, linux-kernel, Andi Kleen

On Thu, 09 Jun 2016, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> On large systems the microcode driver is very noisy, because it prints
> a line for each CPU. The lines are redundant because because usually
> all CPUs are updated to the same microcode revision.
> 
> All other subsystems have been patched previously to not print
> a line for each CPU. Only the microcode driver is left.
> 
> Only print an microcode revision update when something changed. This results
> in typically only a single line being printed.
> 
> v2: Change message to "One or more CPUs"
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Thanks, for WLIW,
Reviewed-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

> ---
>  arch/x86/kernel/cpu/microcode/intel.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index cbb3cf0..54f5f6c 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -794,6 +794,7 @@ void reload_ucode_intel(void)
>  
>  static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
>  {
> +	static struct cpu_signature prev;
>  	struct cpuinfo_x86 *c = &cpu_data(cpu_num);
>  	unsigned int val[2];
>  
> @@ -808,8 +809,14 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
>  	}
>  
>  	csig->rev = c->microcode;
> -	pr_info("CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
> -		cpu_num, csig->sig, csig->pf, csig->rev);
> +
> +	/* No extra locking on prev, races are harmless. */
> +	if (csig->sig != prev.sig || csig->pf != prev.pf ||
> +	    csig->rev != prev.rev) {
> +		pr_info("One or more CPUs sig=0x%x, pf=0x%x, revision=0x%x\n",
> +			csig->sig, csig->pf, csig->rev);
> +		prev = *csig;
> +	}
>  
>  	return 0;
>  }
> @@ -838,6 +845,7 @@ static int apply_microcode_intel(int cpu)
>  	struct ucode_cpu_info *uci;
>  	struct cpuinfo_x86 *c;
>  	unsigned int val[2];
> +	static int prev_rev;
>  
>  	/* We should bind the task to the CPU */
>  	if (WARN_ON(raw_smp_processor_id() != cpu))
> @@ -872,11 +880,14 @@ static int apply_microcode_intel(int cpu)
>  		return -1;
>  	}
>  
> -	pr_info("CPU%d updated to revision 0x%x, date = %04x-%02x-%02x\n",
> -		cpu, val[1],
> -		mc->hdr.date & 0xffff,
> -		mc->hdr.date >> 24,
> -		(mc->hdr.date >> 16) & 0xff);
> +	if (val[1] != prev_rev) {
> +		pr_info("One or more CPUs updated to revision 0x%x, date = %04x-%02x-%02x\n",
> +			val[1],
> +			mc->hdr.date & 0xffff,
> +			mc->hdr.date >> 24,
> +			(mc->hdr.date >> 16) & 0xff);
> +		prev_rev = val[1];
> +	}
>  
>  	c = &cpu_data(cpu);
>  

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems
@ 2016-06-09 13:41 Andi Kleen
  2016-06-09 23:58 ` Henrique de Moraes Holschuh
  2016-06-10  9:54 ` Borislav Petkov
  0 siblings, 2 replies; 6+ messages in thread
From: Andi Kleen @ 2016-06-09 13:41 UTC (permalink / raw)
  To: x86; +Cc: hmh, elliott, linux-kernel, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

On large systems the microcode driver is very noisy, because it prints
a line for each CPU. The lines are redundant because because usually
all CPUs are updated to the same microcode revision.

All other subsystems have been patched previously to not print
a line for each CPU. Only the microcode driver is left.

Only print an microcode revision update when something changed. This results
in typically only a single line being printed.

v2: Change message to "One or more CPUs"
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/cpu/microcode/intel.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index cbb3cf0..54f5f6c 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -794,6 +794,7 @@ void reload_ucode_intel(void)
 
 static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
 {
+	static struct cpu_signature prev;
 	struct cpuinfo_x86 *c = &cpu_data(cpu_num);
 	unsigned int val[2];
 
@@ -808,8 +809,14 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
 	}
 
 	csig->rev = c->microcode;
-	pr_info("CPU%d sig=0x%x, pf=0x%x, revision=0x%x\n",
-		cpu_num, csig->sig, csig->pf, csig->rev);
+
+	/* No extra locking on prev, races are harmless. */
+	if (csig->sig != prev.sig || csig->pf != prev.pf ||
+	    csig->rev != prev.rev) {
+		pr_info("One or more CPUs sig=0x%x, pf=0x%x, revision=0x%x\n",
+			csig->sig, csig->pf, csig->rev);
+		prev = *csig;
+	}
 
 	return 0;
 }
@@ -838,6 +845,7 @@ static int apply_microcode_intel(int cpu)
 	struct ucode_cpu_info *uci;
 	struct cpuinfo_x86 *c;
 	unsigned int val[2];
+	static int prev_rev;
 
 	/* We should bind the task to the CPU */
 	if (WARN_ON(raw_smp_processor_id() != cpu))
@@ -872,11 +880,14 @@ static int apply_microcode_intel(int cpu)
 		return -1;
 	}
 
-	pr_info("CPU%d updated to revision 0x%x, date = %04x-%02x-%02x\n",
-		cpu, val[1],
-		mc->hdr.date & 0xffff,
-		mc->hdr.date >> 24,
-		(mc->hdr.date >> 16) & 0xff);
+	if (val[1] != prev_rev) {
+		pr_info("One or more CPUs updated to revision 0x%x, date = %04x-%02x-%02x\n",
+			val[1],
+			mc->hdr.date & 0xffff,
+			mc->hdr.date >> 24,
+			(mc->hdr.date >> 16) & 0xff);
+		prev_rev = val[1];
+	}
 
 	c = &cpu_data(cpu);
 
-- 
2.8.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-10 10:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-31 21:22 [PATCH] x86/microcode/intel: Quieten down microcode updates on large systems Andi Kleen
2016-06-09  4:34 ` Elliott, Robert (Persistent Memory)
2016-06-09 15:47   ` Henrique de Moraes Holschuh
2016-06-09 13:41 Andi Kleen
2016-06-09 23:58 ` Henrique de Moraes Holschuh
2016-06-10  9:54 ` Borislav Petkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.