Skip to contentSkip to conversation

x86/microcode: Improve late loading

PATCH In review Borislav Petkov 5 participants 26 messages Feb 28, 2018 – Mar 8, 2018

Subsystems (2)
lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

cover x86/microcode: Improve late loading

From: Borislav Petkov <[email protected]>

Hi,

here are a bunch of patches which improve microcode late loading.

Before you read any further: the early loading method is still the preferred one and you should always do that. This patchset is improving the late loading mechanism for long running jobs and cloud use cases - i.e., use cases where early loading is, hm, a bit problematic.

Ashok Raj (4):
x86/microcode/intel: Check microcode revision before updating sibling
threads
x86/microcode/intel: Writeback and invalidate caches before updating
microcode
x86/microcode: Do not upload microcode if CPUs are offline x86/microcode: Synchronize late microcode loading

Borislav Petkov (3):
x86/microcode: Get rid of struct apply_microcode_ctx x86/microcode/intel: Look into the patch cache first x86/microcode: Request microcode on the BSP

 arch/x86/kernel/cpu/microcode/core.c  | 158 +++++++++++++++++++++++++---------
 arch/x86/kernel/cpu/microcode/intel.c |  48 +++++++++--
 2 files changed, 159 insertions(+), 47 deletions(-)
Signature · 1 line
2.13.0
author maintainer
#1 lore raw

PATCH 1/7 x86/microcode: Get rid of struct apply_microcode_ctx

From: Borislav Petkov <[email protected]>

It is a useless remnant from earlier times. Use the ucode_state enum directly.

No functionality change.

---
 arch/x86/kernel/cpu/microcode/core.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +8 -11
index aa1b9a422f2b..63370651e376 100644
@@ -373,26 +373,23 @@ static int collect_cpu_info(int cpu)
 	return ret;
 }
 
-struct apply_microcode_ctx {
-	enum ucode_state err;
-};
-
 static void apply_microcode_local(void *arg)
 {
-	struct apply_microcode_ctx *ctx = arg;
+	enum ucode_state *err = arg;
 
-	ctx->err = microcode_ops->apply_microcode(smp_processor_id());
+	*err = microcode_ops->apply_microcode(smp_processor_id());
 }
 
 static int apply_microcode_on_target(int cpu)
 {
-	struct apply_microcode_ctx ctx = { .err = 0 };
+	enum ucode_state err;
 	int ret;
 
-	ret = smp_call_function_single(cpu, apply_microcode_local, &ctx, 1);
-	if (!ret)
-		ret = ctx.err;
-
+	ret = smp_call_function_single(cpu, apply_microcode_local, &err, 1);
+	if (!ret) {
+		if (err == UCODE_ERROR)
+			ret = 1;
+	}
 	return ret;
 }
 
Signature · 1 line
2.13.0
#2 lore raw

[tip:x86/pti] x86/microcode: Get rid of struct apply_microcode_ctx

x86/microcode: Get rid of struct apply_microcode_ctx
Commit-ID:  854857f5944c59a881ff607b37ed9ed41d031a3b
Gitweb:     https://git.kernel.org/tip/854857f5944c59a881ff607b37ed9ed41d031a3b
Author:     Borislav Petkov <[email protected]>
AuthorDate: Wed, 28 Feb 2018 11:28:40 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 8 Mar 2018 10:19:25 +0100

x86/microcode: Get rid of struct apply_microcode_ctx

It is a useless remnant from earlier times. Use the ucode_state enum directly.

No functional change.

---
 arch/x86/kernel/cpu/microcode/core.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +8 -11
index aa1b9a422f2b..63370651e376 100644
@@ -373,26 +373,23 @@ static int collect_cpu_info(int cpu)
 	return ret;
 }
 
-struct apply_microcode_ctx {
-	enum ucode_state err;
-};
-
 static void apply_microcode_local(void *arg)
 {
-	struct apply_microcode_ctx *ctx = arg;
+	enum ucode_state *err = arg;
 
-	ctx->err = microcode_ops->apply_microcode(smp_processor_id());
+	*err = microcode_ops->apply_microcode(smp_processor_id());
 }
 
 static int apply_microcode_on_target(int cpu)
 {
-	struct apply_microcode_ctx ctx = { .err = 0 };
+	enum ucode_state err;
 	int ret;
 
-	ret = smp_call_function_single(cpu, apply_microcode_local, &ctx, 1);
-	if (!ret)
-		ret = ctx.err;
-
+	ret = smp_call_function_single(cpu, apply_microcode_local, &err, 1);
+	if (!ret) {
+		if (err == UCODE_ERROR)
+			ret = 1;
+	}
 	return ret;
 }
 
author maintainer
#3 lore raw

PATCH 2/7 x86/microcode/intel: Check microcode revision before updating sibling threads

In reply to Borislav Petkov

From: Ashok Raj <[email protected]>

After updating microcode on one of the threads of a core, the other thread sibling automatically gets the update since the microcode resources on a hyperthreaded core are shared between the two threads.

Check the microcode revision on the CPU before performing a microcode update and thus save us the WRMSR 0x79 because it is a particularly expensive operation.

---
 arch/x86/kernel/cpu/microcode/intel.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +24 -3
index 923054a6b760..87bd6dc94081 100644
@@ -589,6 +589,17 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early)
 	if (!mc)
 		return 0;
 
+	/*
+	 * Save us the MSR write below - which is a particular expensive
+	 * operation - when the other hyperthread has updated the microcode
+	 * already.
+	 */
+	rev = intel_get_microcode_revision();
+	if (rev >= mc->hdr.rev) {
+		uci->cpu_sig.rev = rev;
+		return UCODE_OK;
+	}
+
 	/* write microcode via MSR 0x79 */
 	native_wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
@@ -776,7 +787,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
 {
 	struct microcode_intel *mc;
 	struct ucode_cpu_info *uci;
-	struct cpuinfo_x86 *c;
+	struct cpuinfo_x86 *c = &cpu_data(cpu);
 	static int prev_rev;
 	u32 rev;
 
@@ -793,6 +804,18 @@ static enum ucode_state apply_microcode_intel(int cpu)
 			return UCODE_NFOUND;
 	}
 
+	/*
+	 * Save us the MSR write below - which is a particular expensive
+	 * operation - when the other hyperthread has updated the microcode
+	 * already.
+	 */
+	rev = intel_get_microcode_revision();
+	if (rev >= mc->hdr.rev) {
+		uci->cpu_sig.rev = rev;
+		c->microcode = rev;
+		return UCODE_OK;
+	}
+
 	/* write microcode via MSR 0x79 */
 	wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
@@ -813,8 +836,6 @@ static enum ucode_state apply_microcode_intel(int cpu)
 		prev_rev = rev;
 	}
 
-	c = &cpu_data(cpu);
-
 	uci->cpu_sig.rev = rev;
 	c->microcode = rev;
 
Signature · 1 line
2.13.0
#4 lore raw

[tip:x86/pti] x86/microcode/intel: Check microcode revision before updating sibling threads

x86/microcode/intel: Check microcode revision before updating sibling threads
Commit-ID:  c182d2b7d0ca48e0d6ff16f7d883161238c447ed
Gitweb:     https://git.kernel.org/tip/c182d2b7d0ca48e0d6ff16f7d883161238c447ed
Author:     Ashok Raj <[email protected]>
AuthorDate: Wed, 28 Feb 2018 11:28:41 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 8 Mar 2018 10:19:25 +0100

x86/microcode/intel: Check microcode revision before updating sibling threads

After updating microcode on one of the threads of a core, the other thread sibling automatically gets the update since the microcode resources on a hyperthreaded core are shared between the two threads.

Check the microcode revision on the CPU before performing a microcode update and thus save us the WRMSR 0x79 because it is a particularly expensive operation.

[ Borislav: Massage changelog and coding style. ]

---
 arch/x86/kernel/cpu/microcode/intel.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +24 -3
index 923054a6b760..87bd6dc94081 100644
@@ -589,6 +589,17 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early)
 	if (!mc)
 		return 0;
 
+	/*
+	 * Save us the MSR write below - which is a particular expensive
+	 * operation - when the other hyperthread has updated the microcode
+	 * already.
+	 */
+	rev = intel_get_microcode_revision();
+	if (rev >= mc->hdr.rev) {
+		uci->cpu_sig.rev = rev;
+		return UCODE_OK;
+	}
+
 	/* write microcode via MSR 0x79 */
 	native_wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
@@ -776,7 +787,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
 {
 	struct microcode_intel *mc;
 	struct ucode_cpu_info *uci;
-	struct cpuinfo_x86 *c;
+	struct cpuinfo_x86 *c = &cpu_data(cpu);
 	static int prev_rev;
 	u32 rev;
 
@@ -793,6 +804,18 @@ static enum ucode_state apply_microcode_intel(int cpu)
 			return UCODE_NFOUND;
 	}
 
+	/*
+	 * Save us the MSR write below - which is a particular expensive
+	 * operation - when the other hyperthread has updated the microcode
+	 * already.
+	 */
+	rev = intel_get_microcode_revision();
+	if (rev >= mc->hdr.rev) {
+		uci->cpu_sig.rev = rev;
+		c->microcode = rev;
+		return UCODE_OK;
+	}
+
 	/* write microcode via MSR 0x79 */
 	wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
@@ -813,8 +836,6 @@ static enum ucode_state apply_microcode_intel(int cpu)
 		prev_rev = rev;
 	}
 
-	c = &cpu_data(cpu);
-
 	uci->cpu_sig.rev = rev;
 	c->microcode = rev;
 
author maintainer
#5 lore raw

PATCH 3/7 x86/microcode/intel: Writeback and invalidate caches before updating microcode

In reply to Borislav Petkov

From: Ashok Raj <[email protected]>

Updating microcode is less error prone when caches have been flushed and depending on what exactly the microcode is updating. For example, some of the issues around certain Broadwell parts can be addressed by doing a full cache flush.

Cc X86 ML <[email protected]>
Cc Tony Luck <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Tom Lendacky <[email protected]>
Cc Arjan Van De Ven <[email protected]>
[ Massage it and use native_wbinvd() in both cases. ]
---
 arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
arch/x86/kernel/cpu/microcode/intel.c +12 -0
index 87bd6dc94081..e2864bc2d575 100644
@@ -600,6 +600,12 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early)
 		return UCODE_OK;
 	}
 
+	/*
+	 * Writeback and invalidate caches before updating microcode to avoid
+	 * internal issues depending on what the microcode is updating.
+	 */
+	native_wbinvd();
+
 	/* write microcode via MSR 0x79 */
 	native_wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
@@ -816,6 +822,12 @@ static enum ucode_state apply_microcode_intel(int cpu)
 		return UCODE_OK;
 	}
 
+	/*
+	 * Writeback and invalidate caches before updating microcode to avoid
+	 * internal issues depending on what the microcode is updating.
+	 */
+	native_wbinvd();
+
 	/* write microcode via MSR 0x79 */
 	wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
Signature · 1 line
2.13.0
#6 lore raw

[tip:x86/pti] x86/microcode/intel: Writeback and invalidate caches before updating microcode

x86/microcode/intel: Writeback and invalidate caches before updating microcode
Commit-ID:  91df9fdf51492aec9fed6b4cbd33160886740f47
Gitweb:     https://git.kernel.org/tip/91df9fdf51492aec9fed6b4cbd33160886740f47
Author:     Ashok Raj <[email protected]>
AuthorDate: Wed, 28 Feb 2018 11:28:42 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 8 Mar 2018 10:19:25 +0100

x86/microcode/intel: Writeback and invalidate caches before updating microcode

Updating microcode is less error prone when caches have been flushed and depending on what exactly the microcode is updating. For example, some of the issues around certain Broadwell parts can be addressed by doing a full cache flush.

[ Borislav: Massage it and use native_wbinvd() in both cases. ]

---
 arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
arch/x86/kernel/cpu/microcode/intel.c +12 -0
index 87bd6dc94081..e2864bc2d575 100644
@@ -600,6 +600,12 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early)
 		return UCODE_OK;
 	}
 
+	/*
+	 * Writeback and invalidate caches before updating microcode to avoid
+	 * internal issues depending on what the microcode is updating.
+	 */
+	native_wbinvd();
+
 	/* write microcode via MSR 0x79 */
 	native_wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
@@ -816,6 +822,12 @@ static enum ucode_state apply_microcode_intel(int cpu)
 		return UCODE_OK;
 	}
 
+	/*
+	 * Writeback and invalidate caches before updating microcode to avoid
+	 * internal issues depending on what the microcode is updating.
+	 */
+	native_wbinvd();
+
 	/* write microcode via MSR 0x79 */
 	wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
author maintainer
#7 lore raw

PATCH 4/7 x86/microcode: Do not upload microcode if CPUs are offline

In reply to Borislav Petkov

From: Ashok Raj <[email protected]>

Avoid loading microcode if any of the CPUs are offline, and issue a warning. Having different microcode revisions on the system at any time is outright dangerous.

---
 arch/x86/kernel/cpu/microcode/core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
arch/x86/kernel/cpu/microcode/core.c +18 -0
index 63370651e376..fa32cb3dcca5 100644
@@ -486,6 +486,16 @@ static void __exit microcode_dev_exit(void)
 /* fake device for request_firmware */
 static struct platform_device	*microcode_pdev;
 
+static int check_online_cpus(void)
+{
+	if (num_online_cpus() == num_present_cpus())
+		return 0;
+
+	pr_err("Not all CPUs online, aborting microcode update.\n");
+
+	return -EINVAL;
+}
+
 static enum ucode_state reload_for_cpu(int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -519,7 +529,13 @@ static ssize_t reload_store(struct device *dev,
 		return size;
 
 	get_online_cpus();
+
+	ret = check_online_cpus();
+	if (ret)
+		goto put;
+
 	mutex_lock(&microcode_mutex);
+
 	for_each_online_cpu(cpu) {
 		tmp_ret = reload_for_cpu(cpu);
 		if (tmp_ret > UCODE_NFOUND) {
@@ -538,6 +554,8 @@ static ssize_t reload_store(struct device *dev,
 		microcode_check();
 
 	mutex_unlock(&microcode_mutex);
+
+put:
 	put_online_cpus();
 
 	if (!ret)
Signature · 1 line
2.13.0
#8 lore raw

Re: [PATCH 4/7] x86/microcode: Do not upload microcode if CPUs are offline

Quoted 3 lines · Borislav Petkov

Avoid loading microcode if any of the CPUs are offline, and issue a warning. Having different microcode revisions on the system at any time is outright dangerous.

Even if we update that microcode during CPU early bring-up, before we mark it on-line and start using it?

AFAIK, late-loading or not, this is what should happen in the current
code: APs that are brought up after a microcode update is loaded (either by the early or late driver, it doesn't matter) will be always *early-updated* to the new microcode.

Is it dangerous to have an offline core at an older microcode revision than the online cores?

I am not against the patch, mind you, but I am curious about why it is supposed to be dangerous if we're updating the CPUs before we start using them *anyway*.

Also, if this is really dangerous, does it means safe CPU hotplug isn't possible? AFAICT, the firmware would have to do it for us, but it *doesn't* have the up-to-date microcode (*we* had to update it)...

Signature · 1 line
  Henrique Holschuh
#9 lore raw

Re: [PATCH 4/7] x86/microcode: Do not upload microcode if CPUs are offline

Quoted 7 lines · Henrique de Moraes Holschuh
Quoted 3 lines

Avoid loading microcode if any of the CPUs are offline, and issue a warning. Having different microcode revisions on the system at any time is outright dangerous.

Even if we update that microcode during CPU early bring-up, before we mark it on-line and start using it?

Quoted 7 lines

AFAIK, late-loading or not, this is what should happen in the current
code: APs that are brought up after a microcode update is loaded (either by the early or late driver, it doesn't matter) will be always *early-updated* to the new microcode.

Is it dangerous to have an offline core at an older microcode revision than the online cores?

We don't want to leave a system and allow the user to update microcode when some cpus are offline. Remember cpu_offline in linux is only logical offlining.. so the cpu is still in the system.. it can even participate in MCE for e.g. It is very much alive. Its not a question that "Would it not work" but its not worth to leave an open door and being paranoid is best!

Quoted 8 lines

I am not against the patch, mind you, but I am curious about why it is supposed to be dangerous if we're updating the CPUs before we start using them *anyway*.

Also, if this is really dangerous, does it means safe CPU hotplug isn't possible? AFAICT, the firmware would have to do it for us, but it *doesn't* have the up-to-date microcode (*we* had to update it)...

The difference is hot-adding you know you are going to update the current microcode. But leaving a cpu in offline state is leaving it stale for a long time without realizing that you have some stale cores.

#10 lore raw

Re: [PATCH 4/7] x86/microcode: Do not upload microcode if CPUs are offline

Quoted 16 lines · Ashok
Quoted 7 lines · Henrique de Moraes Holschuh
Quoted 3 lines

Avoid loading microcode if any of the CPUs are offline, and issue a warning. Having different microcode revisions on the system at any time is outright dangerous.

Even if we update that microcode during CPU early bring-up, before we mark it on-line and start using it?

Quoted 7 lines

AFAIK, late-loading or not, this is what should happen in the current
code: APs that are brought up after a microcode update is loaded (either by the early or late driver, it doesn't matter) will be always *early-updated* to the new microcode.

Is it dangerous to have an offline core at an older microcode revision than the online cores?

Quoted 6 lines

We don't want to leave a system and allow the user to update microcode when some cpus are offline. Remember cpu_offline in linux is only logical offlining.. so the cpu is still in the system.. it can even participate in MCE for e.g. It is very much alive. Its not a question that "Would it not work" but its not worth to leave an open door and being paranoid is best!

I see. Thanks for the explanation!

Quoted 11 lines
Quoted 7 lines

I am not against the patch, mind you, but I am curious about why it is supposed to be dangerous if we're updating the CPUs before we start using them *anyway*.

Also, if this is really dangerous, does it means safe CPU hotplug isn't possible? AFAICT, the firmware would have to do it for us, but it *doesn't* have the up-to-date microcode (*we* had to update it)...

The difference is hot-adding you know you are going to update the current microcode. But leaving a cpu in offline state is leaving it stale for a long time without realizing that you have some stale cores.

That begs the question: do we have any reasons to not update the microcode even the offline cores?

Signature · 1 line
  Henrique Holschuh
Reviewed-by on v1 4/7
#11 lore raw

Re: [PATCH 4/7] x86/microcode: Do not upload microcode if CPUs are offline

In reply to Borislav Petkov
Quoted 6 lines · Borislav Petkov

From: Ashok Raj <[email protected]>

Avoid loading microcode if any of the CPUs are offline, and issue a warning. Having different microcode revisions on the system at any time is outright dangerous.

Quoted 9 lines
---
 arch/x86/kernel/cpu/microcode/core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Quoted 45 lines
arch/x86/kernel/cpu/microcode/core.c +18 -0
index 63370651e376..fa32cb3dcca5 100644
@@ -486,6 +486,16 @@ static void __exit microcode_dev_exit(void)
 /* fake device for request_firmware */
 static struct platform_device	*microcode_pdev;
 
+static int check_online_cpus(void)
+{
+	if (num_online_cpus() == num_present_cpus())
+		return 0;
+
+	pr_err("Not all CPUs online, aborting microcode update.\n");
+
+	return -EINVAL;
+}
+
 static enum ucode_state reload_for_cpu(int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -519,7 +529,13 @@ static ssize_t reload_store(struct device *dev,
 		return size;
 
 	get_online_cpus();
+
+	ret = check_online_cpus();
+	if (ret)
+		goto put;
+
 	mutex_lock(&microcode_mutex);
+
 	for_each_online_cpu(cpu) {
 		tmp_ret = reload_for_cpu(cpu);
 		if (tmp_ret > UCODE_NFOUND) {
@@ -538,6 +554,8 @@ static ssize_t reload_store(struct device *dev,
 		microcode_check();
 
 	mutex_unlock(&microcode_mutex);
+
+put:
 	put_online_cpus();
 
 	if (!ret)
#12 lore raw

[tip:x86/pti] x86/microcode: Do not upload microcode if CPUs are offline

In reply to Borislav Petkov
x86/microcode: Do not upload microcode if CPUs are offline
Commit-ID:  30ec26da9967d0d785abc24073129a34c3211777
Gitweb:     https://git.kernel.org/tip/30ec26da9967d0d785abc24073129a34c3211777
Author:     Ashok Raj <[email protected]>
AuthorDate: Wed, 28 Feb 2018 11:28:43 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 8 Mar 2018 10:19:26 +0100

x86/microcode: Do not upload microcode if CPUs are offline

Avoid loading microcode if any of the CPUs are offline, and issue a warning. Having different microcode revisions on the system at any time is outright dangerous.

[ Borislav: Massage changelog. ]

---
 arch/x86/kernel/cpu/microcode/core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
arch/x86/kernel/cpu/microcode/core.c +18 -0
index 63370651e376..fa32cb3dcca5 100644
@@ -486,6 +486,16 @@ static void __exit microcode_dev_exit(void)
 /* fake device for request_firmware */
 static struct platform_device	*microcode_pdev;
 
+static int check_online_cpus(void)
+{
+	if (num_online_cpus() == num_present_cpus())
+		return 0;
+
+	pr_err("Not all CPUs online, aborting microcode update.\n");
+
+	return -EINVAL;
+}
+
 static enum ucode_state reload_for_cpu(int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -519,7 +529,13 @@ static ssize_t reload_store(struct device *dev,
 		return size;
 
 	get_online_cpus();
+
+	ret = check_online_cpus();
+	if (ret)
+		goto put;
+
 	mutex_lock(&microcode_mutex);
+
 	for_each_online_cpu(cpu) {
 		tmp_ret = reload_for_cpu(cpu);
 		if (tmp_ret > UCODE_NFOUND) {
@@ -538,6 +554,8 @@ static ssize_t reload_store(struct device *dev,
 		microcode_check();
 
 	mutex_unlock(&microcode_mutex);
+
+put:
 	put_online_cpus();
 
 	if (!ret)
author maintainer
#13 lore raw

PATCH 5/7 x86/microcode/intel: Look into the patch cache first

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

The cache might contain a newer patch - look in there first.

A follow-on change will make sure newest patches are loaded into the cache of microcode patches.

---
 arch/x86/kernel/cpu/microcode/intel.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +5 -6
index e2864bc2d575..2aded9db1d42 100644
@@ -791,9 +791,9 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
 
 static enum ucode_state apply_microcode_intel(int cpu)
 {
-	struct microcode_intel *mc;
-	struct ucode_cpu_info *uci;
+	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
+	struct microcode_intel *mc;
 	static int prev_rev;
 	u32 rev;
 
@@ -801,11 +801,10 @@ static enum ucode_state apply_microcode_intel(int cpu)
 	if (WARN_ON(raw_smp_processor_id() != cpu))
 		return UCODE_ERROR;
 
-	uci = ucode_cpu_info + cpu;
-	mc = uci->mc;
+	/* Look for a newer patch in our cache: */
+	mc = find_patch(uci);
 	if (!mc) {
-		/* Look for a newer patch in our cache: */
-		mc = find_patch(uci);
+		mc = uci->mc;
 		if (!mc)
 			return UCODE_NFOUND;
 	}
Signature · 1 line
2.13.0
#14 lore raw

[tip:x86/pti] x86/microcode/intel: Look into the patch cache first

x86/microcode/intel: Look into the patch cache first
Commit-ID:  d8c3b52c00a05036e0a6b315b4b17921a7b67997
Gitweb:     https://git.kernel.org/tip/d8c3b52c00a05036e0a6b315b4b17921a7b67997
Author:     Borislav Petkov <[email protected]>
AuthorDate: Wed, 28 Feb 2018 11:28:44 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 8 Mar 2018 10:19:26 +0100

x86/microcode/intel: Look into the patch cache first

The cache might contain a newer patch - look in there first.

A follow-on change will make sure newest patches are loaded into the cache of microcode patches.

---
 arch/x86/kernel/cpu/microcode/intel.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
arch/x86/kernel/cpu/microcode/intel.c +5 -6
index e2864bc2d575..2aded9db1d42 100644
@@ -791,9 +791,9 @@ static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
 
 static enum ucode_state apply_microcode_intel(int cpu)
 {
-	struct microcode_intel *mc;
-	struct ucode_cpu_info *uci;
+	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
+	struct microcode_intel *mc;
 	static int prev_rev;
 	u32 rev;
 
@@ -801,11 +801,10 @@ static enum ucode_state apply_microcode_intel(int cpu)
 	if (WARN_ON(raw_smp_processor_id() != cpu))
 		return UCODE_ERROR;
 
-	uci = ucode_cpu_info + cpu;
-	mc = uci->mc;
+	/* Look for a newer patch in our cache: */
+	mc = find_patch(uci);
 	if (!mc) {
-		/* Look for a newer patch in our cache: */
-		mc = find_patch(uci);
+		mc = uci->mc;
 		if (!mc)
 			return UCODE_NFOUND;
 	}
author maintainer
#15 lore raw

PATCH 6/7 x86/microcode: Request microcode on the BSP

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

... so that any newer version can land in the cache and can later be fished out by the application functions. Do that before grabbing the hotplug lock.

---
 arch/x86/kernel/cpu/microcode/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +5 -6
index fa32cb3dcca5..5dd157d48606 100644
@@ -499,15 +499,10 @@ static int check_online_cpus(void)
 static enum ucode_state reload_for_cpu(int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
-	enum ucode_state ustate;
 
 	if (!uci->valid)
 		return UCODE_OK;
 
-	ustate = microcode_ops->request_microcode_fw(cpu, &microcode_pdev->dev, true);
-	if (ustate != UCODE_OK)
-		return ustate;
-
 	return apply_microcode_on_target(cpu);
 }
 
@@ -515,11 +510,11 @@ static ssize_t reload_store(struct device *dev,
 			    struct device_attribute *attr,
 			    const char *buf, size_t size)
 {
+	int cpu, bsp = boot_cpu_data.cpu_index;
 	enum ucode_state tmp_ret = UCODE_OK;
 	bool do_callback = false;
 	unsigned long val;
 	ssize_t ret = 0;
-	int cpu;
 
 	ret = kstrtoul(buf, 0, &val);
 	if (ret)
@@ -528,6 +523,10 @@ static ssize_t reload_store(struct device *dev,
 	if (val != 1)
 		return size;
 
+	tmp_ret = microcode_ops->request_microcode_fw(bsp, &microcode_pdev->dev, true);
+	if (tmp_ret != UCODE_OK)
+		return size;
+
 	get_online_cpus();
 
 	ret = check_online_cpus();
Signature · 1 line
2.13.0
Reviewed-by on v1 6/7
#16 lore raw

Re: [PATCH 6/7] x86/microcode: Request microcode on the BSP

Quoted 11 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

... so that any newer version can land in the cache and can later be fished out by the application functions. Do that before grabbing the hotplug lock.

---
 arch/x86/kernel/cpu/microcode/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
Quoted 45 lines
arch/x86/kernel/cpu/microcode/core.c +5 -6
index fa32cb3dcca5..5dd157d48606 100644
@@ -499,15 +499,10 @@ static int check_online_cpus(void)
 static enum ucode_state reload_for_cpu(int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
-	enum ucode_state ustate;
 
 	if (!uci->valid)
 		return UCODE_OK;
 
-	ustate = microcode_ops->request_microcode_fw(cpu, &microcode_pdev->dev, true);
-	if (ustate != UCODE_OK)
-		return ustate;
-
 	return apply_microcode_on_target(cpu);
 }
 
@@ -515,11 +510,11 @@ static ssize_t reload_store(struct device *dev,
 			    struct device_attribute *attr,
 			    const char *buf, size_t size)
 {
+	int cpu, bsp = boot_cpu_data.cpu_index;
 	enum ucode_state tmp_ret = UCODE_OK;
 	bool do_callback = false;
 	unsigned long val;
 	ssize_t ret = 0;
-	int cpu;
 
 	ret = kstrtoul(buf, 0, &val);
 	if (ret)
@@ -528,6 +523,10 @@ static ssize_t reload_store(struct device *dev,
 	if (val != 1)
 		return size;
 
+	tmp_ret = microcode_ops->request_microcode_fw(bsp, &microcode_pdev->dev, true);
+	if (tmp_ret != UCODE_OK)
+		return size;
+
 	get_online_cpus();
 
 	ret = check_online_cpus();
#17 lore raw

[tip:x86/pti] x86/microcode: Request microcode on the BSP

In reply to Borislav Petkov
x86/microcode: Request microcode on the BSP
Commit-ID:  cfb52a5a09c8ae3a1dafb44ce549fde5b69e8117
Gitweb:     https://git.kernel.org/tip/cfb52a5a09c8ae3a1dafb44ce549fde5b69e8117
Author:     Borislav Petkov <[email protected]>
AuthorDate: Wed, 28 Feb 2018 11:28:45 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 8 Mar 2018 10:19:26 +0100

x86/microcode: Request microcode on the BSP

... so that any newer version can land in the cache and can later be fished out by the application functions. Do that before grabbing the hotplug lock.

---
 arch/x86/kernel/cpu/microcode/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +5 -6
index fa32cb3dcca5..5dd157d48606 100644
@@ -499,15 +499,10 @@ static int check_online_cpus(void)
 static enum ucode_state reload_for_cpu(int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
-	enum ucode_state ustate;
 
 	if (!uci->valid)
 		return UCODE_OK;
 
-	ustate = microcode_ops->request_microcode_fw(cpu, &microcode_pdev->dev, true);
-	if (ustate != UCODE_OK)
-		return ustate;
-
 	return apply_microcode_on_target(cpu);
 }
 
@@ -515,11 +510,11 @@ static ssize_t reload_store(struct device *dev,
 			    struct device_attribute *attr,
 			    const char *buf, size_t size)
 {
+	int cpu, bsp = boot_cpu_data.cpu_index;
 	enum ucode_state tmp_ret = UCODE_OK;
 	bool do_callback = false;
 	unsigned long val;
 	ssize_t ret = 0;
-	int cpu;
 
 	ret = kstrtoul(buf, 0, &val);
 	if (ret)
@@ -528,6 +523,10 @@ static ssize_t reload_store(struct device *dev,
 	if (val != 1)
 		return size;
 
+	tmp_ret = microcode_ops->request_microcode_fw(bsp, &microcode_pdev->dev, true);
+	if (tmp_ret != UCODE_OK)
+		return size;
+
 	get_online_cpus();
 
 	ret = check_online_cpus();
author maintainer
#18 lore raw

PATCH 7/7 x86/microcode: Synchronize late microcode loading

In reply to Borislav Petkov

From: Ashok Raj <[email protected]>

Original idea by Ashok, completely rewritten by Borislav.

Before you read any further: the early loading method is still the preferred one and you should always do that. The following patch is improving the late loading mechanism for long running jobs and cloud use cases.

Gather all cores and serialize the microcode update on them by doing it one-by-one to make the late update process as reliable as possible and avoid potential issues caused by the microcode update.

---
 arch/x86/kernel/cpu/microcode/core.c | 118 +++++++++++++++++++++++++++--------
 1 file changed, 92 insertions(+), 26 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +92 -26
index 5dd157d48606..70ecbc8099c9 100644
@@ -22,13 +22,16 @@
 #define pr_fmt(fmt) "microcode: " fmt
 
 #include <linux/platform_device.h>
+#include <linux/stop_machine.h>
 #include <linux/syscore_ops.h>
 #include <linux/miscdevice.h>
 #include <linux/capability.h>
 #include <linux/firmware.h>
 #include <linux/kernel.h>
+#include <linux/delay.h>
 #include <linux/mutex.h>
 #include <linux/cpu.h>
+#include <linux/nmi.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
 
@@ -64,6 +67,11 @@ LIST_HEAD(microcode_cache);
  */
 static DEFINE_MUTEX(microcode_mutex);
 
+/*
+ * Serialize late loading so that CPUs get updated one-by-one.
+ */
+static DEFINE_SPINLOCK(update_lock);
+
 struct ucode_cpu_info		ucode_cpu_info[NR_CPUS];
 
 struct cpu_info_ctx {
@@ -486,6 +494,19 @@ static void __exit microcode_dev_exit(void)
 /* fake device for request_firmware */
 static struct platform_device	*microcode_pdev;
 
+/*
+ * Late loading dance. Why the heavy-handed stomp_machine effort?
+ *
+ * - HT siblings must be idle and not execute other code while the other sibling
+ *   is loading microcode in order to avoid any negative interactions caused by
+ *   the loading.
+ *
+ * - In addition, microcode update on the cores must be serialized until this
+ *   requirement can be relaxed in the future. Right now, this is conservative
+ *   and good.
+ */
+#define SPINUNIT 100 /* 100 nsec */
+
 static int check_online_cpus(void)
 {
 	if (num_online_cpus() == num_present_cpus())
@@ -496,23 +517,85 @@ static int check_online_cpus(void)
 	return -EINVAL;
 }
 
-static enum ucode_state reload_for_cpu(int cpu)
+static atomic_t late_cpus;
+
+/*
+ * Returns:
+ * < 0 - on error
+ *   0 - no update done
+ *   1 - microcode was updated
+ */
+static int __reload_late(void *info)
 {
-	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
+	unsigned int timeout = NSEC_PER_SEC;
+	int all_cpus = num_online_cpus();
+	int cpu = smp_processor_id();
+	enum ucode_state err;
+	int ret = 0;
 
-	if (!uci->valid)
-		return UCODE_OK;
+	atomic_dec(&late_cpus);
+
+	/*
+	 * Wait for all CPUs to arrive. A load will not be attempted unless all
+	 * CPUs show up.
+	 * */
+	while (atomic_read(&late_cpus)) {
+		if (timeout < SPINUNIT) {
+			pr_err("Timeout while waiting for CPUs rendezvous, remaining: %d\n",
+				atomic_read(&late_cpus));
+			return -1;
+		}
+
+		ndelay(SPINUNIT);
+		timeout -= SPINUNIT;
+
+		touch_nmi_watchdog();
+	}
+
+	spin_lock(&update_lock);
+	apply_microcode_local(&err);
+	spin_unlock(&update_lock);
+
+	if (err > UCODE_NFOUND) {
+		pr_warn("Error reloading microcode on CPU %d\n", cpu);
+		ret = -1;
+	} else if (err == UCODE_UPDATED) {
+		ret = 1;
+	}
 
-	return apply_microcode_on_target(cpu);
+	atomic_inc(&late_cpus);
+
+	while (atomic_read(&late_cpus) != all_cpus)
+		cpu_relax();
+
+	return ret;
+}
+
+/*
+ * Reload microcode late on all CPUs. Wait for a sec until they
+ * all gather together.
+ */
+static int microcode_reload_late(void)
+{
+	int ret;
+
+	atomic_set(&late_cpus, num_online_cpus());
+
+	ret = stop_machine_cpuslocked(__reload_late, NULL, cpu_online_mask);
+	if (ret < 0)
+		return ret;
+	else if (ret > 0)
+		microcode_check();
+
+	return ret;
 }
 
 static ssize_t reload_store(struct device *dev,
 			    struct device_attribute *attr,
 			    const char *buf, size_t size)
 {
-	int cpu, bsp = boot_cpu_data.cpu_index;
 	enum ucode_state tmp_ret = UCODE_OK;
-	bool do_callback = false;
+	int bsp = boot_cpu_data.cpu_index;
 	unsigned long val;
 	ssize_t ret = 0;
 
@@ -534,30 +617,13 @@ static ssize_t reload_store(struct device *dev,
 		goto put;
 
 	mutex_lock(&microcode_mutex);
-
-	for_each_online_cpu(cpu) {
-		tmp_ret = reload_for_cpu(cpu);
-		if (tmp_ret > UCODE_NFOUND) {
-			pr_warn("Error reloading microcode on CPU %d\n", cpu);
-
-			/* set retval for the first encountered reload error */
-			if (!ret)
-				ret = -EINVAL;
-		}
-
-		if (tmp_ret == UCODE_UPDATED)
-			do_callback = true;
-	}
-
-	if (!ret && do_callback)
-		microcode_check();
-
+	ret = microcode_reload_late();
 	mutex_unlock(&microcode_mutex);
 
 put:
 	put_online_cpus();
 
-	if (!ret)
+	if (ret >= 0)
 		ret = size;
 
 	return ret;
Signature · 1 line
2.13.0
#19 lore raw

Re: [PATCH 7/7] x86/microcode: Synchronize late microcode loading

Quoted 9 lines · Borislav Petkov

+ * Late loading dance. Why the heavy-handed stomp_machine effort?
+ *
+ * - HT siblings must be idle and not execute other code while the other sibling
+ * is loading microcode in order to avoid any negative interactions caused by
+ * the loading.
+ *
+ * - In addition, microcode update on the cores must be serialized until this
+ * requirement can be relaxed in the future. Right now, this is conservative
+ * and good.

Eek! If I read that right, this effectively halts the entire box until every core is updated, with one core entering deep-coma at a time (the rest are left either spinning or cpu_relax()ing depending on whether they have already updated or not)?

If this is correct, I shudder at what it would do on a server with dozens, or hundreds of cores... According to Ben Hawkes' paper, Intel's on-die microcode update loader takes linear time relative to the update size to do the crypto dance.

On my single-xeon X5550 workstation, which should be relatively fast since its microcode update is small, the whole thing would take about 3,2 million cycles (circa 800k cycles per core, 4 cores, skipping hyperthreads) to do a sync late update. I don't believe this has changed much, but I *did not* test, e.g., a Skylake Xeon, or anything newer than that Xeon X5550.

Anyway, maybe there is a safe way to do it in a more parallel fashion based on cpu topology?

AFAIK, it is not like there is any way to make OS microcode updates (early or late) safe against SMIs and NMIs hitting the sibling hyperthread while updating the other, so we don't have to care about *that* nasty corner case simply because we can't avoid it in the first place.

Hopefully AMD has none of those pitfalls, and could just trigger an update on half the cores at a time, easily bounding it to approximately twice the time it takes to update a single core :-(

Signature · 1 line
  Henrique Holschuh
author maintainer
#20 lore raw

Re: [PATCH 7/7] x86/microcode: Synchronize late microcode loading

Quoted 3 lines · Henrique de Moraes Holschuh

Eek! If I read that right, this effectively halts the entire box until every core is updated, with one core entering deep-coma at a time (the rest are left either spinning or cpu_relax()ing

I think *you* should relax. :)

Late microcode loading on a long running box is not something you do more than 2-3 times a year. And if the box needs to restart, it'll get the early microcode.

And yes, this is addressing *late* loading, if you haven't noticed yet. I added a big fat note *twice*:

"Before you read any further: the early loading method is still the preferred one and you should always do that. This patchset is improving the late loading mechanism for long running jobs and cloud use cases - i.e., use cases where early loading is, hm, a bit problematic."

So keep doing the early method and you'll be fine.

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
#21 lore raw

Re: [PATCH 7/7] x86/microcode: Synchronize late microcode loading

Quoted 6 lines · Borislav Petkov
Quoted 3 lines · Henrique de Moraes Holschuh

Eek! If I read that right, this effectively halts the entire box until every core is updated, with one core entering deep-coma at a time (the rest are left either spinning or cpu_relax()ing

I think *you* should relax. :)

Well, I don't expect any general-use distro to unleash late loading on the users, certainly :-) Least of all, Debian... It is, nowadays, "use it only if you know what you're doing" land.

But it is not yet sufficiently documented as such, I fear.

Quoted 3 lines

Late microcode loading on a long running box is not something you do more than 2-3 times a year. And if the box needs to restart, it'll get the early microcode.

Sure, but the thing is so damn expensive (and the time it takes is directly proportional to the number of cores, thus likely to hurt worse exactly those who would want to use it), that I was left wondering if it should not be optimized further to do the work in parallel (if that can be made safe enough).

Besides, we likely don't want to have early microcode updates end up being the reason AP bringup has to be serialized during boot either (and it *is* likely to dominate the time taken for AP bringup, too!), so it would be nice to have a way to make parallel microcode updates possible in general... but I don't think we're there, yet.

No matter. I am not opposing the patch in the first place. And any paralell microcode update work would be best done in an incremental fashion, on top of working serial updates, anyway.

Quoted 1 line

And yes, this is addressing *late* loading, if you haven't noticed yet.

I did get that message, yes :)

Quoted 1 line

So keep doing the early method and you'll be fine.

We need that in the documentation :-P Microcode updates have always been somewhat slow, but now they are potentially going to be *much* more painful and noticeable in the late-update case...

Signature · 1 line
  Henrique Holschuh
Reviewed-by on v1 7/7
#22 lore raw

Re: [PATCH 7/7] x86/microcode: Synchronize late microcode loading

In reply to Borislav Petkov
Quoted 13 lines · Borislav Petkov

From: Ashok Raj <[email protected]>

Original idea by Ashok, completely rewritten by Borislav.

Before you read any further: the early loading method is still the preferred one and you should always do that. The following patch is improving the late loading mechanism for long running jobs and cloud use cases.

Gather all cores and serialize the microcode update on them by doing it one-by-one to make the late update process as reliable as possible and avoid potential issues caused by the microcode update.

Quoted 8 lines
---
 arch/x86/kernel/cpu/microcode/core.c | 118 +++++++++++++++++++++++++++--------
 1 file changed, 92 insertions(+), 26 deletions(-)
#23 lore raw

[tip:x86/pti] x86/microcode: Synchronize late microcode loading

In reply to Borislav Petkov
x86/microcode: Synchronize late microcode loading
Commit-ID:  a5321aec6412b20b5ad15db2d6b916c05349dbff
Gitweb:     https://git.kernel.org/tip/a5321aec6412b20b5ad15db2d6b916c05349dbff
Author:     Ashok Raj <[email protected]>
AuthorDate: Wed, 28 Feb 2018 11:28:46 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Thu, 8 Mar 2018 10:19:26 +0100

x86/microcode: Synchronize late microcode loading

Original idea by Ashok, completely rewritten by Borislav.

Before you read any further: the early loading method is still the preferred one and you should always do that. The following patch is improving the late loading mechanism for long running jobs and cloud use cases.

Gather all cores and serialize the microcode update on them by doing it one-by-one to make the late update process as reliable as possible and avoid potential issues caused by the microcode update.

[ Borislav: Rewrite completely. ]

---
 arch/x86/kernel/cpu/microcode/core.c | 118 +++++++++++++++++++++++++++--------
 1 file changed, 92 insertions(+), 26 deletions(-)
arch/x86/kernel/cpu/microcode/core.c +92 -26
index 5dd157d48606..70ecbc8099c9 100644
@@ -22,13 +22,16 @@
 #define pr_fmt(fmt) "microcode: " fmt
 
 #include <linux/platform_device.h>
+#include <linux/stop_machine.h>
 #include <linux/syscore_ops.h>
 #include <linux/miscdevice.h>
 #include <linux/capability.h>
 #include <linux/firmware.h>
 #include <linux/kernel.h>
+#include <linux/delay.h>
 #include <linux/mutex.h>
 #include <linux/cpu.h>
+#include <linux/nmi.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
 
@@ -64,6 +67,11 @@ LIST_HEAD(microcode_cache);
  */
 static DEFINE_MUTEX(microcode_mutex);
 
+/*
+ * Serialize late loading so that CPUs get updated one-by-one.
+ */
+static DEFINE_SPINLOCK(update_lock);
+
 struct ucode_cpu_info		ucode_cpu_info[NR_CPUS];
 
 struct cpu_info_ctx {
@@ -486,6 +494,19 @@ static void __exit microcode_dev_exit(void)
 /* fake device for request_firmware */
 static struct platform_device	*microcode_pdev;
 
+/*
+ * Late loading dance. Why the heavy-handed stomp_machine effort?
+ *
+ * - HT siblings must be idle and not execute other code while the other sibling
+ *   is loading microcode in order to avoid any negative interactions caused by
+ *   the loading.
+ *
+ * - In addition, microcode update on the cores must be serialized until this
+ *   requirement can be relaxed in the future. Right now, this is conservative
+ *   and good.
+ */
+#define SPINUNIT 100 /* 100 nsec */
+
 static int check_online_cpus(void)
 {
 	if (num_online_cpus() == num_present_cpus())
@@ -496,23 +517,85 @@ static int check_online_cpus(void)
 	return -EINVAL;
 }
 
-static enum ucode_state reload_for_cpu(int cpu)
+static atomic_t late_cpus;
+
+/*
+ * Returns:
+ * < 0 - on error
+ *   0 - no update done
+ *   1 - microcode was updated
+ */
+static int __reload_late(void *info)
 {
-	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
+	unsigned int timeout = NSEC_PER_SEC;
+	int all_cpus = num_online_cpus();
+	int cpu = smp_processor_id();
+	enum ucode_state err;
+	int ret = 0;
 
-	if (!uci->valid)
-		return UCODE_OK;
+	atomic_dec(&late_cpus);
+
+	/*
+	 * Wait for all CPUs to arrive. A load will not be attempted unless all
+	 * CPUs show up.
+	 * */
+	while (atomic_read(&late_cpus)) {
+		if (timeout < SPINUNIT) {
+			pr_err("Timeout while waiting for CPUs rendezvous, remaining: %d\n",
+				atomic_read(&late_cpus));
+			return -1;
+		}
+
+		ndelay(SPINUNIT);
+		timeout -= SPINUNIT;
+
+		touch_nmi_watchdog();
+	}
+
+	spin_lock(&update_lock);
+	apply_microcode_local(&err);
+	spin_unlock(&update_lock);
+
+	if (err > UCODE_NFOUND) {
+		pr_warn("Error reloading microcode on CPU %d\n", cpu);
+		ret = -1;
+	} else if (err == UCODE_UPDATED) {
+		ret = 1;
+	}
 
-	return apply_microcode_on_target(cpu);
+	atomic_inc(&late_cpus);
+
+	while (atomic_read(&late_cpus) != all_cpus)
+		cpu_relax();
+
+	return ret;
+}
+
+/*
+ * Reload microcode late on all CPUs. Wait for a sec until they
+ * all gather together.
+ */
+static int microcode_reload_late(void)
+{
+	int ret;
+
+	atomic_set(&late_cpus, num_online_cpus());
+
+	ret = stop_machine_cpuslocked(__reload_late, NULL, cpu_online_mask);
+	if (ret < 0)
+		return ret;
+	else if (ret > 0)
+		microcode_check();
+
+	return ret;
 }
 
 static ssize_t reload_store(struct device *dev,
 			    struct device_attribute *attr,
 			    const char *buf, size_t size)
 {
-	int cpu, bsp = boot_cpu_data.cpu_index;
 	enum ucode_state tmp_ret = UCODE_OK;
-	bool do_callback = false;
+	int bsp = boot_cpu_data.cpu_index;
 	unsigned long val;
 	ssize_t ret = 0;
 
@@ -534,30 +617,13 @@ static ssize_t reload_store(struct device *dev,
 		goto put;
 
 	mutex_lock(&microcode_mutex);
-
-	for_each_online_cpu(cpu) {
-		tmp_ret = reload_for_cpu(cpu);
-		if (tmp_ret > UCODE_NFOUND) {
-			pr_warn("Error reloading microcode on CPU %d\n", cpu);
-
-			/* set retval for the first encountered reload error */
-			if (!ret)
-				ret = -EINVAL;
-		}
-
-		if (tmp_ret == UCODE_UPDATED)
-			do_callback = true;
-	}
-
-	if (!ret && do_callback)
-		microcode_check();
-
+	ret = microcode_reload_late();
 	mutex_unlock(&microcode_mutex);
 
 put:
 	put_online_cpus();
 
-	if (!ret)
+	if (ret >= 0)
 		ret = size;
 
 	return ret;
Tested-by on the v1 series
#24 lore raw
In reply to Borislav Petkov
Quoted 19 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

Hi,

here are a bunch of patches which improve microcode late loading.

Before you read any further: the early loading method is still the preferred one and you should always do that. This patchset is improving the late loading mechanism for long running jobs and cloud use cases - i.e., use cases where early loading is, hm, a bit problematic.

Ashok Raj (4):
x86/microcode/intel: Check microcode revision before updating sibling
threads
x86/microcode/intel: Writeback and invalidate caches before updating
microcode
x86/microcode: Do not upload microcode if CPUs are offline x86/microcode: Synchronize late microcode loading

Quoted 9 lines

Borislav Petkov (3):
x86/microcode: Get rid of struct apply_microcode_ctx x86/microcode/intel: Look into the patch cache first x86/microcode: Request microcode on the BSP

 arch/x86/kernel/cpu/microcode/core.c  | 158 +++++++++++++++++++++++++---------
 arch/x86/kernel/cpu/microcode/intel.c |  48 +++++++++--
 2 files changed, 159 insertions(+), 47 deletions(-)

Tested on my Family 17h (EPYC) system (including taking CPUs offline and back on before and after reloads). I'll try and find some additional systems to test on, but for now:

Tested-by
#25 lore raw

Thanks Tom

Quoted 11 lines · Tom Lendacky

Borislav Petkov (3):

Quoted 9 lines

x86/microcode: Get rid of struct apply_microcode_ctx x86/microcode/intel: Look into the patch cache first x86/microcode: Request microcode on the BSP

 arch/x86/kernel/cpu/microcode/core.c  | 158
+++++++++++++++++++++++++---------
 arch/x86/kernel/cpu/microcode/intel.c |  48 +++++++++--
 2 files changed, 159 insertions(+), 47 deletions(-)
Quoted 6 lines

Tested on my Family 17h (EPYC) system (including taking CPUs offline and
back on before and after reloads). I'll try and find some additional systems to test on, but for now:

Tested on variety of Intel systems.