Skip to contentSkip to conversation

x86/microcode/intel: Quieten down microcode updates on large systems

PATCH In review Andi Kleen 4 participants 4 messages Jun 9, 2016 – Jun 14, 2016

Subsystems (2)
lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 x86/microcode/intel: Quieten down microcode updates on large systems

From: Andi Kleen <[email protected]>

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"

---
 arch/x86/kernel/cpu/microcode/intel.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +18 -7
index cbb3cf0..54f5f6c 100644
@@ -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);
 
Signature · 1 line
2.8.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 lines · Andi Kleen

From: Andi Kleen <[email protected]>

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.

Quoted 8 lines

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"

Thanks, for WLIW,

Quoted 61 lines
---
 arch/x86/kernel/cpu/microcode/intel.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +18 -7
index cbb3cf0..54f5f6c 100644
@@ -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);
 
Signature · 4 lines
  "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
maintainer
#2 lore raw
In reply to Andi Kleen
Quoted 31 lines · Andi Kleen

From: Andi Kleen <[email protected]>

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"

---
 arch/x86/kernel/cpu/microcode/intel.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +1 -0
index cbb3cf0..54f5f6c 100644
@@ -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];
 

arch/x86/kernel/cpu/microcode/intel.c static int collect_cpu_info(int cpu_num, struct cpu_signature *csig) · line 816

Quoted 10 lines
arch/x86/kernel/cpu/microcode/intel.c +5 -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.

Signature · 4 lines
Regards/Gruss,
    Boris.

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

[tip:x86/microcode] x86/microcode/intel: Do not issue microcode updates messages on each CPU

In reply to Andi Kleen
x86/microcode/intel: Do not issue microcode updates messages on each CPU
Commit-ID:  354542d034ab2a849a284edcc661e76b753a57dc
Gitweb:     http://git.kernel.org/tip/354542d034ab2a849a284edcc661e76b753a57dc
Author:     Andi Kleen <[email protected]>
AuthorDate: Thu, 9 Jun 2016 06:41:41 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 14 Jun 2016 10:51:43 +0200

x86/microcode/intel: Do not issue microcode updates messages on each CPU

On large systems the microcode driver is very noisy, because it prints a line for each CPU. The lines are redundant 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.

---
 arch/x86/kernel/cpu/microcode/intel.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +17 -7
index 8962d6a..6515c80 100644
@@ -843,6 +843,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];
 
@@ -857,8 +858,13 @@ 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("sig=0x%x, pf=0x%x, revision=0x%x\n",
+			csig->sig, csig->pf, csig->rev);
+		prev = *csig;
+	}
 
 	return 0;
 }
@@ -887,6 +893,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))
@@ -921,11 +928,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("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);