diff mbox series

[13/13] x86/microcode/intel: Add ability to update microcode even if rev is unchanged

Message ID 20221014200913.14644-14-ashok.raj@intel.com
State New
Headers
Series Make microcode loading more robust |

Commit Message

Ashok Raj Oct. 14, 2022, 8:09 p.m. UTC 
  This comes in handy for testing without the need for a new microcode file.
It introduces a new boot parameter ucode_load_same, or it can be switched
dynamically at run time via debugfs file
/sys/kernel/debug/microcode/load_same.

NOT_FOR_INCLUSION:
Leave it to the discretion of Boris if its suitable for inclusion. It will
at least serve to validate some parts of the series without the need for a
new microcode.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---
 arch/x86/include/asm/microcode.h      |  2 ++
 arch/x86/kernel/cpu/microcode/amd.c   |  2 +-
 arch/x86/kernel/cpu/microcode/core.c  | 22 +++++++++++++++++-----
 arch/x86/kernel/cpu/microcode/intel.c |  4 ++--
 4 files changed, 22 insertions(+), 8 deletions(-)

Comments

Ashok Raj Oct. 19, 2022, 3:51 p.m. UTC | #1 
Hi Boris,

There is a minor bug in this force reload option I added for testing.

See below:

On Fri, Oct 14, 2022 at 01:09:13PM -0700, Ashok Raj wrote:

[snip]

> diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
> index eb2caa74de01..632c7a1fcffb 100644
> --- a/arch/x86/kernel/cpu/microcode/core.c
> +++ b/arch/x86/kernel/cpu/microcode/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/miscdevice.h>
>  #include <linux/capability.h>
>  #include <linux/firmware.h>
> +#include <linux/debugfs.h>
>  #include <linux/kernel.h>
>  #include <linux/delay.h>
>  #include <linux/mutex.h>
> @@ -46,6 +47,7 @@
>  
>  static struct microcode_ops	*microcode_ops;
>  static bool dis_ucode_ldr = true;
> +bool ucode_load_same;
>  
>  bool initrd_gone;
>  
> @@ -542,11 +544,12 @@ static int __reload_late(void *info)
>  		goto wait_for_siblings;
>  	}
>  
> -	if (err >= UCODE_NFOUND) {
> -		if (err == UCODE_ERROR)
> +	if (ret || err >= UCODE_NFOUND) {
> +		if (err == UCODE_ERROR ||
> +		    (err == UCODE_NFOUND && !ucode_load_same)) {
>  			pr_warn("Error reloading microcode on CPU %d\n", cpu);
> -
> -		ret = -1;
> +			ret = -1;
> +		}
>  	}
>  
>  wait_for_siblings:
> @@ -636,9 +639,12 @@ static ssize_t reload_store(struct device *dev,
>  	}
>  
>  	tmp_ret = microcode_ops->request_microcode_fw(bsp, &microcode_pdev->dev, true);
> -	if (tmp_ret != UCODE_NEW)
> +	if (tmp_ret != UCODE_NEW && !ucode_load_same)
>  		goto put;
>  
> +	if (tmp_ret != UCODE_NEW)
> +		pr_info("Force loading ucode\n");
> +

The above needs additional check for UCODE_ERROR. If you are testing with
old microcode with minrev=0, noticed this went and tried to perform an
update without the UCODE_ERROR check. 

I've queued for the next update.


-       if (tmp_ret != UCODE_NEW)
+       if (tmp_ret == UCODE_ERROR ||
+           (tmp_ret != UCODE_NEW && !ucode_load_same))
                goto put;

+       if (tmp_ret != UCODE_NEW)
+               pr_info("Force loading ucode\n");
+

>  	mutex_lock(&microcode_mutex);
>  	ret = microcode_reload_late();
>  	mutex_unlock(&microcode_mutex);
> @@ -841,6 +847,7 @@ static const struct attribute_group cpu_root_microcode_group = {
>  static int __init microcode_init(void)
>  {
>  	struct cpuinfo_x86 *c = &boot_cpu_data;
> +	static struct dentry *dentry_ucode;
>  	int error;
>  
>  	if (dis_ucode_ldr)
> @@ -884,7 +891,12 @@ static int __init microcode_init(void)
>  	cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "x86/microcode:online",
>  				  mc_cpu_online, mc_cpu_down_prep);
>  
> +	dentry_ucode = debugfs_create_dir("microcode", NULL);
> +	debugfs_create_bool("load_same", 0644, dentry_ucode, &ucode_load_same);
> +
>  	pr_info("Microcode Update Driver: v%s.", DRIVER_VERSION);
> +	pr_info("ucode_load_same is %s\n",
> +		ucode_load_same ? "enabled" : "disabled");
>  
>  	return 0;
>  
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index c61aa661ac2f..c9f1e6f5e53b 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -763,7 +763,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
>  	 * already.
>  	 */
>  	rev = intel_get_microcode_revision();
> -	if (rev >= mc->hdr.rev) {
> +	if (rev >= mc->hdr.rev && !ucode_load_same) {
>  		ret = UCODE_OK;
>  		goto out;
>  	}
> @@ -779,7 +779,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
>  		return UCODE_ERROR;
>  	}
>  
> -	if (bsp && rev != prev_rev) {
> +	if (bsp && (rev != prev_rev || ucode_load_same)) {
>  		pr_info("updated to revision 0x%x, date = %04x-%02x-%02x\n",
>  			rev,
>  			mc->hdr.date & 0xffff,
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 38b501d842de..4baca634c2f7 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -43,6 +43,8 @@  struct ucode_patch {
 extern struct list_head microcode_cache;
 extern int ucode_updating;
 
+extern bool ucode_load_same;
+
 struct cpu_signature {
 	unsigned int sig;
 	unsigned int pf;
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index c18d3f01a452..124461e2d7a1 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -694,7 +694,7 @@  static enum ucode_state apply_microcode_amd(int cpu)
 	rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
 
 	/* need to apply patch? */
-	if (rev >= mc_amd->hdr.patch_id) {
+	if (rev >= mc_amd->hdr.patch_id && !ucode_load_same) {
 		ret = UCODE_OK;
 		goto out;
 	}
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index eb2caa74de01..632c7a1fcffb 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -23,6 +23,7 @@ 
 #include <linux/miscdevice.h>
 #include <linux/capability.h>
 #include <linux/firmware.h>
+#include <linux/debugfs.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
 #include <linux/mutex.h>
@@ -46,6 +47,7 @@ 
 
 static struct microcode_ops	*microcode_ops;
 static bool dis_ucode_ldr = true;
+bool ucode_load_same;
 
 bool initrd_gone;
 
@@ -542,11 +544,12 @@  static int __reload_late(void *info)
 		goto wait_for_siblings;
 	}
 
-	if (err >= UCODE_NFOUND) {
-		if (err == UCODE_ERROR)
+	if (ret || err >= UCODE_NFOUND) {
+		if (err == UCODE_ERROR ||
+		    (err == UCODE_NFOUND && !ucode_load_same)) {
 			pr_warn("Error reloading microcode on CPU %d\n", cpu);
-
-		ret = -1;
+			ret = -1;
+		}
 	}
 
 wait_for_siblings:
@@ -636,9 +639,12 @@  static ssize_t reload_store(struct device *dev,
 	}
 
 	tmp_ret = microcode_ops->request_microcode_fw(bsp, &microcode_pdev->dev, true);
-	if (tmp_ret != UCODE_NEW)
+	if (tmp_ret != UCODE_NEW && !ucode_load_same)
 		goto put;
 
+	if (tmp_ret != UCODE_NEW)
+		pr_info("Force loading ucode\n");
+
 	mutex_lock(&microcode_mutex);
 	ret = microcode_reload_late();
 	mutex_unlock(&microcode_mutex);
@@ -841,6 +847,7 @@  static const struct attribute_group cpu_root_microcode_group = {
 static int __init microcode_init(void)
 {
 	struct cpuinfo_x86 *c = &boot_cpu_data;
+	static struct dentry *dentry_ucode;
 	int error;
 
 	if (dis_ucode_ldr)
@@ -884,7 +891,12 @@  static int __init microcode_init(void)
 	cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "x86/microcode:online",
 				  mc_cpu_online, mc_cpu_down_prep);
 
+	dentry_ucode = debugfs_create_dir("microcode", NULL);
+	debugfs_create_bool("load_same", 0644, dentry_ucode, &ucode_load_same);
+
 	pr_info("Microcode Update Driver: v%s.", DRIVER_VERSION);
+	pr_info("ucode_load_same is %s\n",
+		ucode_load_same ? "enabled" : "disabled");
 
 	return 0;
 
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index c61aa661ac2f..c9f1e6f5e53b 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -763,7 +763,7 @@  static enum ucode_state apply_microcode_intel(int cpu)
 	 * already.
 	 */
 	rev = intel_get_microcode_revision();
-	if (rev >= mc->hdr.rev) {
+	if (rev >= mc->hdr.rev && !ucode_load_same) {
 		ret = UCODE_OK;
 		goto out;
 	}
@@ -779,7 +779,7 @@  static enum ucode_state apply_microcode_intel(int cpu)
 		return UCODE_ERROR;
 	}
 
-	if (bsp && rev != prev_rev) {
+	if (bsp && (rev != prev_rev || ucode_load_same)) {
 		pr_info("updated to revision 0x%x, date = %04x-%02x-%02x\n",
 			rev,
 			mc->hdr.date & 0xffff,