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] [day] [month] [year] [list]

Message-ID: <68106c72-20a1-4ded-89f7-4d804d774abf@redhat.com>
Date: Mon, 17 Nov 2025 19:28:59 -0500
From: Waiman Long <llong@...hat.com>
To: Borislav Petkov <bp@...en8.de>, 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 11/17/25 4:11 PM, Borislav Petkov wrote:
> 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.

I see. In that case, I am going to wait for your patch then.

Thanks for the info.

Cheers,
Longman

>
> ---
> 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);
>   
>