linux-kernel - Re: [PATCH tip] x86/microcode/AMD: Read from MSR_AMD64_PATCH_LEVEL to get base

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251117211101.GGaRuPZQQ2InlyRbk9@fat_crate.local>
Date: Mon, 17 Nov 2025 22:11:01 +0100
From: Borislav Petkov <bp@...en8.de>
To: Waiman Long <llong@...hat.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH tip] x86/microcode/AMD: Read from MSR_AMD64_PATCH_LEVEL
 to get base_rev if not defined

On Mon, Nov 17, 2025 at 02:58:30PM -0500, Waiman Long wrote:
> when CONFIG_MICROCODE_DBG is on.

Again, CONFIG_MICROCODE_DBG is only to be used in a guest. Like the help text
says. For now at least.

I have tried to extend it to debugging on baremetal - see below - but this is
unfinished.

---
Author: Borislav Petkov (AMD) <bp@...en8.de>
Date:   Mon Oct 6 17:50:10 2025 +0200

    Host debugging
    
    Signed-off-by: Borislav Petkov (AMD) <bp@...en8.de>

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fa3b616af03a..c213e00ea963 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1362,10 +1362,12 @@ config MICROCODE_DBG
 	default n
 	depends on MICROCODE
 	help
-	  Enable code which allows for debugging the microcode loader in
-	  a guest. Meaning the patch loading is simulated but everything else
+	  Enable code which allows to debug the microcode loader. When running
+	  in a guest the patch loading is simulated but everything else
 	  related to patch parsing and handling is done as on baremetal with
-	  the purpose of debugging solely the software side of things.
+	  the purpose of debugging solely the software side of things. On
+	  baremetal, it simply dumps additional debugging information as it
+	  goes.
 
 	  You almost certainly want to say n here.
 
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index a584f9cbf9a3..c25db0d40629 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -301,7 +301,7 @@ static u32 get_patch_level(void)
 {
 	u32 rev, dummy __always_unused;
 
-	if (IS_ENABLED(CONFIG_MICROCODE_DBG)) {
+	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && hypervisor_present) {
 		int cpu = smp_processor_id();
 
 		if (!microcode_rev[cpu]) {
@@ -694,7 +694,7 @@ static bool __apply_microcode_amd(struct microcode_amd *mc, u32 *cur_rev,
 			invlpg(p_addr_end);
 	}
 
-	if (IS_ENABLED(CONFIG_MICROCODE_DBG))
+	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && hypervisor_present)
 		microcode_rev[smp_processor_id()] = mc->hdr.patch_id;
 
 	/* verify patch application was successful */
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index f75c140906d0..ae0ba9df501b 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -57,6 +57,8 @@ bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
 u32 base_rev;
 u32 microcode_rev[NR_CPUS] = {};
 
+bool hypervisor_present;
+
 /*
  * Synchronization.
  *
@@ -117,6 +119,13 @@ bool __init microcode_loader_disabled(void)
 	 * Disable when:
 	 *
 	 * 1) The CPU does not support CPUID.
+	 */
+	if (!cpuid_feature()) {
+		dis_ucode_ldr = true;
+		return dis_ucode_ldr;
+	}
+
+	/*
 	 *
 	 * 2) Bit 31 in CPUID[1]:ECX is clear
 	 *    The bit is reserved for hypervisor use. This is still not
@@ -127,9 +136,9 @@ bool __init microcode_loader_disabled(void)
 	 * 3) Certain AMD patch levels are not allowed to be
 	 *    overwritten.
 	 */
-	if (!cpuid_feature() ||
-	    ((native_cpuid_ecx(1) & BIT(31)) &&
-	      !IS_ENABLED(CONFIG_MICROCODE_DBG)) ||
+	hypervisor_present = native_cpuid_ecx(1) & BIT(31);
+
+	if ((hypervisor_present && !IS_ENABLED(CONFIG_MICROCODE_DBG)) ||
 	    amd_check_current_patch_level())
 		dis_ucode_ldr = true;
 
diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index ae8dbc2b908d..f084aac6c839 100644
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -46,6 +46,7 @@ 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 hypervisor_present;
 
 struct cpio_data find_microcode_in_initrd(const char *path);
 

-- 
Regards/Gruss,
    Boris.

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