Re: [PATCH 1/2] x86/microcode: Do not access MSR_IA32_PLATFORM_ID when running as a guest

[Binbin Wu]

On 5/14/2026 4:06 AM, Borislav Petkov wrote:
> Patch in Fixes: causes the usual:
> 
>   unchecked MSR access error: RDMSR from 0x17 at ... (intel_get_platform_id)
>   Call Trace:
>    early_init_intel
>    early_cpu_init
>    setup_arch
>    _printk
>    start_kernel
>    x86_64_start_reservations
>    x86_64_start_kernel
>    common_startup_64
> 
> because the kernel is booted in a guest.
> 
> In order to avoid it, this MSR access needs to be prevented when running
> virtualized. That is usually done by checking X86_FEATURE_HYPERVISOR but
> for this particular case it is too early yet.
> 
> The platform ID needs to be read as early as when microcode is loaded on
> the BSP:
> 
>   load_ucode_bsp ... -> get_microcode_blob ... -> intel_find_matching_signature
> 
> and by that time, CPUID leafs haven't been parsed yet.
> 
> The microcode loader already has logic to check early whether the kernel
> is running virtualized so make that globally available to arch/x86/. The
> query whether running virtualized is getting more and more prominent in
> recent times so might as well make it an arch-global var which the rest
> of the code can use.
> 
> Fixes: d8630b67ca1ed ("x86/cpu: Add platform ID to CPU info structure")
> Reported-by: Vishal Verma <vishal.l.verma@xxxxxxxxx>
> Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
> Tested-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/20260430020953.1405535-1-binbin.wu@xxxxxxxxxxxxxxx

Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>

One nit below.

[...]

> @@ -118,14 +118,9 @@ 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;
> -	}
> -
> -	/*
> +	 * 1) The CPU does not support CPUID, detected below in
                                                         ^
Nit: 
"below" is a bit ambiguous, maybe "earlier"?

> +	 *    load_ucode_bsp().
> +	 *
>  	 * 2) Bit 31 in CPUID[1]:ECX is set
>  	 *    The bit is reserved for hypervisor use. This is still not
>  	 *    completely accurate as XEN PV guests don't see that CPUID bit
> @@ -135,9 +130,7 @@ bool __init microcode_loader_disabled(void)
>  	 * 3) Certain AMD patch levels are not allowed to be
>  	 *    overwritten.
>  	 */
> -	hypervisor_present = native_cpuid_ecx(1) & BIT(31);
> -
> -	if ((hypervisor_present && !IS_ENABLED(CONFIG_MICROCODE_DBG)) ||
> +	if ((x86_hypervisor_present && !IS_ENABLED(CONFIG_MICROCODE_DBG)) ||
>  	    amd_check_current_patch_level())
>  		dis_ucode_ldr = true;
>  
> @@ -179,6 +172,11 @@ void __init load_ucode_bsp(void)
>  
>  	early_parse_cmdline();
>  
> +	if (!cpuid_feature())
> +		dis_ucode_ldr = true;
> +	else
> +		x86_hypervisor_present = native_cpuid_ecx(1) & BIT(31);
> +
>  	if (microcode_loader_disabled())
>  		return;
>  
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 37ac4afe0972..a4c0a0cf928b 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -138,6 +138,9 @@ u32 intel_get_platform_id(void)
>  {
>  	unsigned int val[2];
>  
> +	if (x86_hypervisor_present)
> +		return 0;
> +
>  	/*
>  	 * This can be called early. Use CPUID directly instead of
>  	 * relying on cpuinfo_x86 which may not be fully initialized.
> diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
> index 3b93c0676b4f..a10b547eda1e 100644
> --- a/arch/x86/kernel/cpu/microcode/internal.h
> +++ b/arch/x86/kernel/cpu/microcode/internal.h
> @@ -48,7 +48,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 hypervisor_present;
>  
>  struct cpio_data find_microcode_in_initrd(const char *path);
>