[PATCH 1/2] x86/microcode: Add microcode= cmdline parsing

Borislav Petkov posted 2 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Borislav Petkov 1 month, 3 weeks ago
From: "Borislav Petkov (AMD)" <bp@alien8.de>

Add a "microcode=" command line argument after which all options can be
passed in a comma-separated list.

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 .../admin-guide/kernel-parameters.txt         |  8 +++--
 arch/x86/kernel/cpu/microcode/core.c          | 30 +++++++++++++++----
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 747a55abf494..7c095177da85 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3767,10 +3767,12 @@
 
 	mga=		[HW,DRM]
 
-	microcode.force_minrev=	[X86]
-			Format: <bool>
+	microcode=      [X86] Control the behavior of the microcode loader.
+	                Available options, comma separated:
+
+			force_minrev
 			Enable or disable the microcode minimal revision
-			enforcement for the runtime microcode loader.
+ 			enforcement for the runtime microcode loader.
 
 	mini2440=	[ARM,HW,KNL]
 			Format:[0..2][b][c][t]
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index b92e09a87c69..351a51861f00 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -43,10 +43,9 @@
 #include "internal.h"
 
 static struct microcode_ops *microcode_ops;
-static bool dis_ucode_ldr = false;
+static bool dis_ucode_ldr;
 
-bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
-module_param(force_minrev, bool, S_IRUSR | S_IWUSR);
+bool force_minrev = false;
 
 /*
  * Synchronization.
@@ -126,13 +125,34 @@ bool __init microcode_loader_disabled(void)
 	return dis_ucode_ldr;
 }
 
+static void early_parse_cmdline(void)
+{
+	char cmd_buf[64] = {};
+	char *s, *p = cmd_buf;
+
+	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, 64) > 0) {
+		while ((s = strsep(&p, ","))) {
+			if (IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV)) {
+				if (!strcmp("force_minrev", s))
+					force_minrev = true;
+			}
+
+			if (!strcmp(s, "dis_ucode_ldr"))
+				dis_ucode_ldr = true;
+		}
+	}
+
+	/* old, compat option */
+	if (cmdline_find_option_bool(boot_command_line, "dis_ucode_ldr") > 0)
+		dis_ucode_ldr = true;
+}
+
 void __init load_ucode_bsp(void)
 {
 	unsigned int cpuid_1_eax;
 	bool intel = true;
 
-	if (cmdline_find_option_bool(boot_command_line, "dis_ucode_ldr") > 0)
-		dis_ucode_ldr = true;
+	early_parse_cmdline();
 
 	if (microcode_loader_disabled())
 		return;
-- 
2.43.0
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Chang S. Bae 1 month, 3 weeks ago
On 8/9/2025 2:42 AM, Borislav Petkov wrote:
>   
> -	microcode.force_minrev=	[X86]
> -			Format: <bool>
> +	microcode=      [X86] Control the behavior of the microcode loader.
> +	                Available options, comma separated:

It looks like microcode=dis_ucode_ldr is also supported.
This could be added here:
			ldr_ucode_ldr
			Disable the microcode loader.		

> +			force_minrev
>   			Enable or disable the microcode minimal revision
> -			enforcement for the runtime microcode loader.
> + 			enforcement for the runtime microcode loader.

I also noticed in arch/x86/Kconfig:

config MICROCODE_LATE_LOADING
	bool "Late microcode loading (DANGEROUS)"
	default n
	depends on MICROCODE && SMP
	help
	  ...
	  the kernel command line with "microcode.minrev=Y".

This outdated has been there already. Perhaps, it might be better to fix 
this typo with the new one while updating the option.
> -bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
> -module_param(force_minrev, bool, S_IRUSR | S_IWUSR);
> +bool force_minrev = false;

<snip>

> +static void early_parse_cmdline(void)
> +{
> +	char cmd_buf[64] = {};
> +	char *s, *p = cmd_buf;
> +
> +	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, 64) > 0) {

nit: s/64/sizeof(cmd_bug)/

> +		while ((s = strsep(&p, ","))) {
> +			if (IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV)) {
> +				if (!strcmp("force_minrev", s))
> +					force_minrev = true;
> +			}

I think the behavior here differs from before:

Previously, the minrev requirement could be enforced by either
   (a) Build with MICROCODE_LATE_FORCE_MINREV=y, or
   (b) microcode.force_minrev with MICROCODE_LATE=y.

Now, this requires both. I don't know this is intentional, but it’s like 
asking for more from the user.

Thanks,
Chang
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Borislav Petkov 1 month, 2 weeks ago
On Thu, Aug 14, 2025 at 04:08:57PM -0700, Chang S. Bae wrote:
> It looks like microcode=dis_ucode_ldr is also supported.
> This could be added here:
> 			ldr_ucode_ldr
> 			Disable the microcode loader.		

Done.

> I also noticed in arch/x86/Kconfig:
> 
> config MICROCODE_LATE_LOADING
> 	bool "Late microcode loading (DANGEROUS)"
> 	default n
> 	depends on MICROCODE && SMP
> 	help
> 	  ...
> 	  the kernel command line with "microcode.minrev=Y".
> 
> This outdated has been there already. Perhaps, it might be better to fix
> this typo with the new one while updating the option.

Done. Good catch.

> nit: s/64/sizeof(cmd_bug)/

Done.

> I think the behavior here differs from before:
> 
> Previously, the minrev requirement could be enforced by either
>   (a) Build with MICROCODE_LATE_FORCE_MINREV=y, or
>   (b) microcode.force_minrev with MICROCODE_LATE=y.
> 
> Now, this requires both. I don't know this is intentional, but it’s like
> asking for more from the user.

Yeah, you're right. FORCE_MINREV is not a CONFIG item which enables
force_minrev support. 

Ok, here's a diff ontop with all the changes I've caught up until now:

---

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fc002b1a9f57..e7badf2aba63 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3770,16 +3770,17 @@
 	microcode=      [X86] Control the behavior of the microcode loader.
 	                Available options, comma separated:
 
-			dbg - Format: <bool>
-			enable debugging mode when run in a guest
-
 			base_rev=X - with <X> with format: <u32>
 			Set the base microcode revision of each thread when in
 			debug mode.
 
-			force_minrev
+			dbg: enable debugging mode when run in a guest
+
+			dis_ucode_ldr: disable the microcode loader
+
+			force_minrev:
 			Enable or disable the microcode minimal revision
- 			enforcement for the runtime microcode loader.
+			enforcement for the runtime microcode loader.
 
 	mini2440=	[ARM,HW,KNL]
 			Format:[0..2][b][c][t]
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 462bf03aeda5..77f72f075d89 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1340,7 +1340,7 @@ config MICROCODE_LATE_LOADING
 	  use this at your own risk. Late loading taints the kernel unless the
 	  microcode header indicates that it is safe for late loading via the
 	  minimal revision check. This minimal revision check can be enforced on
-	  the kernel command line with "microcode.minrev=Y".
+	  the kernel command line with "microcode=force_minrev".
 
 config MICROCODE_LATE_FORCE_MINREV
 	bool "Enforce late microcode loading minimal revision check"
@@ -1356,7 +1356,7 @@ config MICROCODE_LATE_FORCE_MINREV
 	  revision check fails.
 
 	  This minimal revision check can also be controlled via the
-	  "microcode.minrev" parameter on the kernel command line.
+	  "microcode=force_minrev" parameter on the kernel command line.
 
 	  If unsure say Y.
 
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 3a4e210f6cf3..f045670a1fae 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -45,7 +45,7 @@
 static struct microcode_ops *microcode_ops;
 static bool dis_ucode_ldr;
 
-bool force_minrev = false;
+bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
 
 /*
  * Those below should be behind CONFIG_MICROCODE_DBG ifdeffery but in
@@ -142,7 +142,7 @@ static void early_parse_cmdline(void)
 	char cmd_buf[64] = {};
 	char *s, *p = cmd_buf;
 
-	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, 64) > 0) {
+	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, sizeof(cmd_buf)) > 0) {
 		while ((s = strsep(&p, ","))) {
 			if (IS_ENABLED(CONFIG_MICROCODE_DBG)) {
 				if (!strcmp(s, "dbg"))
@@ -155,10 +155,8 @@ static void early_parse_cmdline(void)
 				}
 			}
 
-			if (IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV)) {
-				if (!strcmp("force_minrev", s))
-					force_minrev = true;
-			}
+			if (!strcmp("force_minrev", s))
+				force_minrev = true;
 
 			if (!strcmp(s, "dis_ucode_ldr"))
 				dis_ucode_ldr = true;

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Sohil Mehta 1 month, 2 weeks ago
On 8/15/2025 2:20 AM, Borislav Petkov wrote:

> 
> Ok, here's a diff ontop with all the changes I've caught up until now:
> 

The updated patches look okay to me.

Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Chang S. Bae 1 month, 2 weeks ago
On 8/15/2025 2:20 AM, Borislav Petkov wrote:
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index fc002b1a9f57..e7badf2aba63 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3770,16 +3770,17 @@
>   	microcode=      [X86] Control the behavior of the microcode loader.
>   	                Available options, comma separated:
>   
> -			dbg - Format: <bool>
> -			enable debugging mode when run in a guest
> -
>   			base_rev=X - with <X> with format: <u32>
>   			Set the base microcode revision of each thread when in
>   			debug mode.
>   
> -			force_minrev
> +			dbg: enable debugging mode when run in a guest
> +
> +			dis_ucode_ldr: disable the microcode loader
> +
> +			force_minrev:
>   			Enable or disable the microcode minimal revision
> - 			enforcement for the runtime microcode loader.
> +			enforcement for the runtime microcode loader.
>   
>   	mini2440=	[ARM,HW,KNL]
>   			Format:[0..2][b][c][t]
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 462bf03aeda5..77f72f075d89 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1340,7 +1340,7 @@ config MICROCODE_LATE_LOADING
>   	  use this at your own risk. Late loading taints the kernel unless the
>   	  microcode header indicates that it is safe for late loading via the
>   	  minimal revision check. This minimal revision check can be enforced on
> -	  the kernel command line with "microcode.minrev=Y".
> +	  the kernel command line with "microcode=force_minrev".
>   
>   config MICROCODE_LATE_FORCE_MINREV
>   	bool "Enforce late microcode loading minimal revision check"
> @@ -1356,7 +1356,7 @@ config MICROCODE_LATE_FORCE_MINREV
>   	  revision check fails.
>   
>   	  This minimal revision check can also be controlled via the
> -	  "microcode.minrev" parameter on the kernel command line.
> +	  "microcode=force_minrev" parameter on the kernel command line.
>   
>   	  If unsure say Y.
>   
> diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
> index 3a4e210f6cf3..f045670a1fae 100644
> --- a/arch/x86/kernel/cpu/microcode/core.c
> +++ b/arch/x86/kernel/cpu/microcode/core.c
> @@ -45,7 +45,7 @@
>   static struct microcode_ops *microcode_ops;
>   static bool dis_ucode_ldr;
>   
> -bool force_minrev = false;
> +bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
>   
>   /*
>    * Those below should be behind CONFIG_MICROCODE_DBG ifdeffery but in
> @@ -142,7 +142,7 @@ static void early_parse_cmdline(void)
>   	char cmd_buf[64] = {};
>   	char *s, *p = cmd_buf;
>   
> -	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, 64) > 0) {
> +	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, sizeof(cmd_buf)) > 0) {
>   		while ((s = strsep(&p, ","))) {
>   			if (IS_ENABLED(CONFIG_MICROCODE_DBG)) {
>   				if (!strcmp(s, "dbg"))
> @@ -155,10 +155,8 @@ static void early_parse_cmdline(void)
>   				}
>   			}
>   
> -			if (IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV)) {
> -				if (!strcmp("force_minrev", s))
> -					force_minrev = true;
> -			}
> +			if (!strcmp("force_minrev", s))
> +				force_minrev = true;
>   
>   			if (!strcmp(s, "dis_ucode_ldr"))
>   				dis_ucode_ldr = true;
> 

Thanks for consolidating those loader options -- this makes them easier 
to find and configure in one place.

The overall changes look good to me, so:
   Reviewed-by: Chang S. Bae <chang.seok.bae@intel.com>
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Sohil Mehta 1 month, 3 weeks ago
On 8/9/2025 2:42 AM, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
> 
> Add a "microcode=" command line argument after which all options can be
> passed in a comma-separated list.
> 
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> ---
>  .../admin-guide/kernel-parameters.txt         |  8 +++--
>  arch/x86/kernel/cpu/microcode/core.c          | 30 +++++++++++++++----
>  2 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 747a55abf494..7c095177da85 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3767,10 +3767,12 @@
>  
>  	mga=		[HW,DRM]
>  
> -	microcode.force_minrev=	[X86]
> -			Format: <bool>
> +	microcode=      [X86] Control the behavior of the microcode loader.
> +	                Available options, comma separated:
> +


How about adding something like this to encourage using the new options:


@@ -1211,7 +1211,7 @@

-       dis_ucode_ldr   [X86] Disable the microcode loader.
+       dis_ucode_ldr   [X86] Same as microcode=disable_loader.


@@ -3770,6 +3770,9 @@

+                       disable_loader
+                               Disable the microcode loader.
+

> +			force_minrev
>  			Enable or disable the microcode minimal revision
> -			enforcement for the runtime microcode loader.
> + 			enforcement for the runtime microcode loader.


Unnecessary whitespace added before tab.


>  	mini2440=	[ARM,HW,KNL]
>  			Format:[0..2][b][c][t]
> diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
> index b92e09a87c69..351a51861f00 100644
> --- a/arch/x86/kernel/cpu/microcode/core.c
> +++ b/arch/x86/kernel/cpu/microcode/core.c
> @@ -43,10 +43,9 @@
>  #include "internal.h"
>  
>  static struct microcode_ops *microcode_ops;
> -static bool dis_ucode_ldr = false;
> +static bool dis_ucode_ldr;
>  
> -bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
> -module_param(force_minrev, bool, S_IRUSR | S_IWUSR);
> +bool force_minrev = false;
>  

Setting this to "false" is redundant.

>  /*
>   * Synchronization.
> @@ -126,13 +125,34 @@ bool __init microcode_loader_disabled(void)
>  	return dis_ucode_ldr;
>  }
>  
> +static void early_parse_cmdline(void)
> +{
> +	char cmd_buf[64] = {};
> +	char *s, *p = cmd_buf;
> +
> +	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, 64) > 0) {
> +		while ((s = strsep(&p, ","))) {
> +			if (IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV)) {
> +				if (!strcmp("force_minrev", s))
> +					force_minrev = true;
> +			}
> +
> +			if (!strcmp(s, "dis_ucode_ldr"))

If you agree with the above suggestion,
s/dis_ucode_ldr/disable_loader

> +				dis_ucode_ldr = true;
> +		}
> +	}
> +
> +	/* old, compat option */
> +	if (cmdline_find_option_bool(boot_command_line, "dis_ucode_ldr") > 0)
> +		dis_ucode_ldr = true;
> +}
> +
>  void __init load_ucode_bsp(void)
>  {
>  	unsigned int cpuid_1_eax;
>  	bool intel = true;
>  
> -	if (cmdline_find_option_bool(boot_command_line, "dis_ucode_ldr") > 0)
> -		dis_ucode_ldr = true;
> +	early_parse_cmdline();
>  
>  	if (microcode_loader_disabled())
>  		return;
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Borislav Petkov 1 month, 3 weeks ago
On Mon, Aug 11, 2025 at 07:26:59PM -0700, Sohil Mehta wrote:
> How about adding something like this to encourage using the new options:
> 
> 
> @@ -1211,7 +1211,7 @@
> 
> -       dis_ucode_ldr   [X86] Disable the microcode loader.
> +       dis_ucode_ldr   [X86] Same as microcode=disable_loader.
> 
> 
> @@ -3770,6 +3770,9 @@
> 
> +                       disable_loader
> +                               Disable the microcode loader.

Because adding a new alias of a cmdline parameter doesn't belong in the same
patch.

And if anything, that thing should be

	microcode=off

to save me some silly typing.

> Unnecessary whitespace added before tab.

Pff.

> Setting this to "false" is redundant.

Fixed.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Sohil Mehta 1 month, 3 weeks ago
On 8/12/2025 3:23 AM, Borislav Petkov wrote:
>> @@ -1211,7 +1211,7 @@
>>
>> -       dis_ucode_ldr   [X86] Disable the microcode loader.
>> +       dis_ucode_ldr   [X86] Same as microcode=disable_loader.
>>
>>
>> @@ -3770,6 +3770,9 @@
>>
>> +                       disable_loader
>> +                               Disable the microcode loader.
> 
> Because adding a new alias of a cmdline parameter doesn't belong in the same
> patch.
> 

But, this patch introduces code to support microcode=dis_ucode_ldr which
would then become ABI.

> And if anything, that thing should be
> 
> 	microcode=off
> 

Sure, a separate patch directly adding support for microcode=off works
as well.
Re: [PATCH 1/2] x86/microcode: Add microcode= cmdline parsing
Posted by Borislav Petkov 1 month, 3 weeks ago
On Tue, Aug 12, 2025 at 12:14:51PM -0700, Sohil Mehta wrote:
> But, this patch introduces code to support microcode=dis_ucode_ldr which
> would then become ABI.

... and supports the old cmdline argument as well.

> Sure, a separate patch directly adding support for microcode=off works
> as well.

I don't see the need for an alias atm.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette