x86/microcode: Update microcode for all cores in parallel
diff mbox series

Message ID 1566506627-16536-2-git-send-email-mihai.carabas@oracle.com
State New, archived
Headers show
Series
  • x86/microcode: Update microcode for all cores in parallel
Related show

Commit Message

Mihai Carabas Aug. 22, 2019, 8:43 p.m. UTC
From: Ashok Raj <ashok.raj@intel.com>

Microcode update was changed to be serialized due to restrictions after
Spectre days. Updating serially on a large multi-socket system can be
painful since we do this one CPU at a time. Cloud customers have expressed
discontent as services disappear for a prolonged time. The restriction is
that only one core goes through the update while other cores are quiesced.
The update is now done only on the first thread of each core while other
siblings simply wait for this to complete.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
---
 arch/x86/kernel/cpu/microcode/core.c  | 44 ++++++++++++++++++++++++-----------
 arch/x86/kernel/cpu/microcode/intel.c | 14 ++++-------
 2 files changed, 36 insertions(+), 22 deletions(-)

Comments

Borislav Petkov Aug. 24, 2019, 8:51 a.m. UTC | #1
On Thu, Aug 22, 2019 at 11:43:47PM +0300, Mihai Carabas wrote:
> From: Ashok Raj <ashok.raj@intel.com>
> 
> Microcode update was changed to be serialized due to restrictions after
> Spectre days. Updating serially on a large multi-socket system can be
> painful since we do this one CPU at a time. Cloud customers have expressed
> discontent as services disappear for a prolonged time. The restriction is
> that only one core goes through the update while other cores are quiesced.
> The update is now done only on the first thread of each core while other
> siblings simply wait for this to complete.
> 
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> ---
>  arch/x86/kernel/cpu/microcode/core.c  | 44 ++++++++++++++++++++++++-----------
>  arch/x86/kernel/cpu/microcode/intel.c | 14 ++++-------
>  2 files changed, 36 insertions(+), 22 deletions(-)

Thanks, I did some cleaning up and smoke testing. It looks ok so far.
The versions I'm going to hammer on more - and I'd appreciate it if you
did so too, when possible - as a reply to this message.

Patch
diff mbox series

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index cb0fdca..577b223 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -63,11 +63,6 @@ 
  */
 static DEFINE_MUTEX(microcode_mutex);
 
-/*
- * Serialize late loading so that CPUs get updated one-by-one.
- */
-static DEFINE_RAW_SPINLOCK(update_lock);
-
 struct ucode_cpu_info		ucode_cpu_info[NR_CPUS];
 
 struct cpu_info_ctx {
@@ -566,9 +561,23 @@  static int __reload_late(void *info)
 	if (__wait_for_cpus(&late_cpus_in, NSEC_PER_SEC))
 		return -1;
 
-	raw_spin_lock(&update_lock);
-	apply_microcode_local(&err);
-	raw_spin_unlock(&update_lock);
+	/*
+	 * Update just on the first CPU in the core. Other siblings
+	 * get the update automatically according to Intel SDM 9.11.6.3
+	 * Update in a System Supporting Intel Hyper-Threading Technology
+	 * Intel Hyper-Threading Technology has implications on the loading of the
+	 * microcode update. The update must be loaded for each core in a physical
+	 * processor. Thus, for a processor supporting Intel Hyper-Threading
+	 * Technology, only one logical processor per core is required to load the
+	 * microcode update. Each individual logical processor can independently
+	 * load the update. However, MP initialization must provide some mechanism
+	 * (e.g. a software semaphore) to force serialization of microcode update
+	 * loads and to prevent simultaneous load attempts to the same core.
+	 */
+	if (cpumask_first(topology_sibling_cpumask(cpu)) == cpu)
+		apply_microcode_local(&err);
+	else
+		goto wait_for_siblings;
 
 	/* siblings return UCODE_OK because their engine got updated already */
 	if (err > UCODE_NFOUND) {
@@ -578,15 +587,24 @@  static int __reload_late(void *info)
 		ret = 1;
 	}
 
+wait_for_siblings:
 	/*
-	 * Increase the wait timeout to a safe value here since we're
-	 * serializing the microcode update and that could take a while on a
-	 * large number of CPUs. And that is fine as the *actual* timeout will
-	 * be determined by the last CPU finished updating and thus cut short.
+	 * Since we are doing all cores in parallel, and the other
+	 * sibling threads just do a rev update, there is no need to
+	 * increase the timeout
 	 */
-	if (__wait_for_cpus(&late_cpus_out, NSEC_PER_SEC * num_online_cpus()))
+	if (__wait_for_cpus(&late_cpus_out, NSEC_PER_SEC))
 		panic("Timeout during microcode update!\n");
 
+	/*
+	 * At least one thread has completed update in each core.
+	 * For others, simply call the update to make sure the
+	 * per-cpu cpuinfo can be updated with right microcode
+	 * revision.
+	 */
+	 if (cpumask_first(topology_sibling_cpumask(cpu)) != cpu)
+		apply_microcode_local(&err);
+
 	return ret;
 }
 
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index ce799cf..884d02d 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -793,7 +793,6 @@  static enum ucode_state apply_microcode_intel(int cpu)
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
 	struct microcode_intel *mc;
 	enum ucode_state ret;
-	static int prev_rev;
 	u32 rev;
 
 	/* We should bind the task to the CPU */
@@ -836,14 +835,11 @@  static enum ucode_state apply_microcode_intel(int cpu)
 		return UCODE_ERROR;
 	}
 
-	if (rev != prev_rev) {
-		pr_info("updated to revision 0x%x, date = %04x-%02x-%02x\n",
-			rev,
-			mc->hdr.date & 0xffff,
-			mc->hdr.date >> 24,
-			(mc->hdr.date >> 16) & 0xff);
-		prev_rev = rev;
-	}
+	pr_info_once("updated to revision 0x%x, date = %04x-%02x-%02x\n",
+		rev,
+		mc->hdr.date & 0xffff,
+		mc->hdr.date >> 24,
+		(mc->hdr.date >> 16) & 0xff);
 
 	ret = UCODE_UPDATED;