Re: [PATCH -v1 2/2] x86/microcode: Add microcode loader debugging functionality

[Borislav Petkov <bp@alien8.de>]

On Wed, Aug 20, 2025 at 10:19:11PM -0700, Sohil Mehta wrote:
> Does this need to be (IS_ENABLED(CONFIG_MICROCODE_DBG) && dbg)?

Both you and Nikolay have a point - we don't need both. So actually, dbg can
go and can be added when really needed.

Right now, the debugging stuff is for in a guest only and will be build-time
enabled.

If we decide we want to have runtime controllable and *baremetal* debugging,
then we can extend that and add the cmdline switch.

I think...

---

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e7badf2aba63..2c142e5f9f06 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3774,8 +3774,6 @@
 			Set the base microcode revision of each thread when in
 			debug mode.
 
-			dbg: enable debugging mode when run in a guest
-
 			dis_ucode_ldr: disable the microcode loader
 
 			force_minrev:
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index f045670a1fae..f75c140906d0 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -53,8 +53,6 @@ bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
  * instead, leave them in. When microcode debugging is not enabled,
  * those are meaningless anyway.
  */
-/* enable loader debugging */
-bool dbg;
 /* base microcode revision for debugging */
 u32 base_rev;
 u32 microcode_rev[NR_CPUS] = {};
@@ -130,7 +128,8 @@ bool __init microcode_loader_disabled(void)
 	 *    overwritten.
 	 */
 	if (!cpuid_feature() ||
-	    ((native_cpuid_ecx(1) & BIT(31)) && !dbg) ||
+	    ((native_cpuid_ecx(1) & BIT(31)) &&
+	      !IS_ENABLED(CONFIG_MICROCODE_DBG)) ||
 	    amd_check_current_patch_level())
 		dis_ucode_ldr = true;
 
@@ -145,9 +144,6 @@ static void early_parse_cmdline(void)
 	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"))
-					dbg = true;
-
 				if (strstr(s, "base_rev=")) {
 					/* advance to the option arg */
 					strsep(&s, "=");
diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index bca806dd1aac..ae8dbc2b908d 100644
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -46,7 +46,6 @@ extern struct early_load_data early_data;
 extern struct ucode_cpu_info ucode_cpu_info[];
 extern u32 microcode_rev[NR_CPUS];
 extern u32 base_rev;
-extern bool dbg;
 
 struct cpio_data find_microcode_in_initrd(const char *path);
 
@@ -128,7 +127,7 @@ static inline struct microcode_ops *init_intel_microcode(void) { return NULL; }
 
 #define ucode_dbg(fmt, ...)					\
 ({								\
-	if (dbg)						\
+	if (IS_ENABLED(CONFIG_MICROCODE_DBG))			\
 		pr_info(fmt, ##__VA_ARGS__);			\
 })
 
-- 
Regards/Gruss,
    Boris.

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