Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
@ 2026-04-30  2:09 Binbin Wu
  2026-05-11  9:38 ` Kiryl Shutsemau
  2026-05-11 10:04 ` Borislav Petkov
  0 siblings, 2 replies; 10+ messages in thread
From: Binbin Wu @ 2026-04-30  2:09 UTC (permalink / raw)
  To: linux-kernel, x86, kvm
  Cc: dave.hansen, seanjc, pbonzini, kas, rick.p.edgecombe,
	vishal.l.verma, xiaoyao.li, chao.gao, binbin.wu

The kernel now reads MSR_IA32_PLATFORM_ID during CPU init.  When
running as a guest, if the underlying hypervisor does not emulate the
MSR, the RDMSR from MSR_IA32_PLATFORM_ID can trigger an unchecked MSR
access during early boot.  This MSR is not emulated in the case for KVM
TDX, where the following is observed in the TD guest:

    unchecked MSR access error: RDMSR from 0x17 at rIP: 0xffffffffba38d6fc (intel_get_platform_id+0x7c/0xb0)
    Call Trace:
     <TASK>
     ? early_init_intel+0x28/0x2c0
     ? early_cpu_init+0x9b/0x930
     ? setup_arch+0xbf/0xbb0
     ? _printk+0x6b/0x90
     ? start_kernel+0x7f/0xaa0
     ? x86_64_start_reservations+0x24/0x30
     ? x86_64_start_kernel+0xda/0xe0
     ? common_startup_64+0x13e/0x141
     </TASK>

The platform ID is used for one thing and one thing only: microcode
updates.  Those updates are solely the domain of the bare-metal OS.  The
guest kernel code should not even try to touch the MSR.  Skip reading
the MSR when the kernel is running in a virtualized environment.  0 is a
valid platform ID, however, microcode related logic is skipped in a
virtualized environment.

Since intel_get_platform_id() could be called early before cpuinfo_x86
is fully initialized in the case of CONFIG_MICROCODE_DBG, check whether
the kernel is running in a virtualized environment from CPUID.  Use
cpuid_ecx() instead of native_cpuid_ecx() so that Xen PV guest will see
the virtualized bit.

Fixes: d8630b67ca1ed ("x86/cpu: Add platform ID to CPU info structure")
Reported-by: Vishal Verma <vishal.l.verma@intel.com>
Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v2:
- Drop the patch on KVM side. (Sean, Dave)
- Use X86_FEATURE_HYPERVISOR for better readability. (Dave)
- Use cpuid_ecx() instead of native_cpuid_ecx() to check the hypervisor bit.
- Add RB from Rick.
---
 arch/x86/kernel/cpu/microcode/intel.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 37ac4afe0972..1bc0c350726c 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -147,6 +147,10 @@ u32 intel_get_platform_id(void)
 	if (intel_cpuid_vfm() <= INTEL_PENTIUM_II_KLAMATH)
 		return 0;
 
+	/* Don't try to read microcode bits when virtualized. */
+	if (cpuid_ecx(1) & BIT(X86_FEATURE_HYPERVISOR & 0x1f))
+		return 0;
+
 	/* get processor flags from MSR 0x17 */
 	native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]);
 

base-commit: 9974969c14031a097d6b45bcb7a06bb4aa525c40
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-04-30  2:09 [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment Binbin Wu
@ 2026-05-11  9:38 ` Kiryl Shutsemau
  2026-05-11 10:04 ` Borislav Petkov
  1 sibling, 0 replies; 10+ messages in thread
From: Kiryl Shutsemau @ 2026-05-11  9:38 UTC (permalink / raw)
  To: Binbin Wu
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao

On Thu, Apr 30, 2026 at 10:09:53AM +0800, Binbin Wu wrote:
> The kernel now reads MSR_IA32_PLATFORM_ID during CPU init.  When
> running as a guest, if the underlying hypervisor does not emulate the
> MSR, the RDMSR from MSR_IA32_PLATFORM_ID can trigger an unchecked MSR
> access during early boot.  This MSR is not emulated in the case for KVM
> TDX, where the following is observed in the TD guest:
> 
>     unchecked MSR access error: RDMSR from 0x17 at rIP: 0xffffffffba38d6fc (intel_get_platform_id+0x7c/0xb0)
>     Call Trace:
>      <TASK>
>      ? early_init_intel+0x28/0x2c0
>      ? early_cpu_init+0x9b/0x930
>      ? setup_arch+0xbf/0xbb0
>      ? _printk+0x6b/0x90
>      ? start_kernel+0x7f/0xaa0
>      ? x86_64_start_reservations+0x24/0x30
>      ? x86_64_start_kernel+0xda/0xe0
>      ? common_startup_64+0x13e/0x141
>      </TASK>
> 
> The platform ID is used for one thing and one thing only: microcode
> updates.  Those updates are solely the domain of the bare-metal OS.  The
> guest kernel code should not even try to touch the MSR.  Skip reading
> the MSR when the kernel is running in a virtualized environment.  0 is a
> valid platform ID, however, microcode related logic is skipped in a
> virtualized environment.
> 
> Since intel_get_platform_id() could be called early before cpuinfo_x86
> is fully initialized in the case of CONFIG_MICROCODE_DBG, check whether
> the kernel is running in a virtualized environment from CPUID.  Use
> cpuid_ecx() instead of native_cpuid_ecx() so that Xen PV guest will see
> the virtualized bit.
> 
> Fixes: d8630b67ca1ed ("x86/cpu: Add platform ID to CPU info structure")
> Reported-by: Vishal Verma <vishal.l.verma@intel.com>
> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Acked-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-04-30  2:09 [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment Binbin Wu
  2026-05-11  9:38 ` Kiryl Shutsemau
@ 2026-05-11 10:04 ` Borislav Petkov
  2026-05-12  1:57   ` Binbin Wu
  1 sibling, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2026-05-11 10:04 UTC (permalink / raw)
  To: Binbin Wu
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao

On Thu, Apr 30, 2026 at 10:09:53AM +0800, Binbin Wu wrote:
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 37ac4afe0972..1bc0c350726c 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -147,6 +147,10 @@ u32 intel_get_platform_id(void)
>  	if (intel_cpuid_vfm() <= INTEL_PENTIUM_II_KLAMATH)
>  		return 0;
>  
> +	/* Don't try to read microcode bits when virtualized. */
> +	if (cpuid_ecx(1) & BIT(X86_FEATURE_HYPERVISOR & 0x1f))
> +		return 0;
> +
>  	/* get processor flags from MSR 0x17 */
>  	native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]);
>  
> 
> base-commit: 9974969c14031a097d6b45bcb7a06bb4aa525c40
> -- 

Does that work too?

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 18d2eff7a4b7..1b24de94bdd5 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -139,6 +139,9 @@ u32 intel_get_platform_id(void)
 {
 	unsigned int val[2];
 
+	if (hypervisor_present)
+		return 0;
+
 	/*
 	 * This can be called early. Use CPUID directly instead of
 	 * relying on cpuinfo_x86 which may not be fully initialized.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-05-11 10:04 ` Borislav Petkov
@ 2026-05-12  1:57   ` Binbin Wu
  2026-05-13 10:14     ` Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: Binbin Wu @ 2026-05-12  1:57 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao



On 5/11/2026 6:04 PM, Borislav Petkov wrote:
> On Thu, Apr 30, 2026 at 10:09:53AM +0800, Binbin Wu wrote:
>> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
>> index 37ac4afe0972..1bc0c350726c 100644
>> --- a/arch/x86/kernel/cpu/microcode/intel.c
>> +++ b/arch/x86/kernel/cpu/microcode/intel.c
>> @@ -147,6 +147,10 @@ u32 intel_get_platform_id(void)
>>  	if (intel_cpuid_vfm() <= INTEL_PENTIUM_II_KLAMATH)
>>  		return 0;
>>  
>> +	/* Don't try to read microcode bits when virtualized. */
>> +	if (cpuid_ecx(1) & BIT(X86_FEATURE_HYPERVISOR & 0x1f))
>> +		return 0;
>> +
>>  	/* get processor flags from MSR 0x17 */
>>  	native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]);
>>  
>>
>> base-commit: 9974969c14031a097d6b45bcb7a06bb4aa525c40
>> -- 
> 
> Does that work too?
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 18d2eff7a4b7..1b24de94bdd5 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -139,6 +139,9 @@ u32 intel_get_platform_id(void)
>  {
>  	unsigned int val[2];
>  
> +	if (hypervisor_present)

hypervisor_present could be uninitialized if dis_ucode_ldr is true.
intel_get_platform_id() is also called during the normal cpu initialization.


> +		return 0;
> +
>  	/*
>  	 * This can be called early. Use CPUID directly instead of
>  	 * relying on cpuinfo_x86 which may not be fully initialized.
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-05-12  1:57   ` Binbin Wu
@ 2026-05-13 10:14     ` Borislav Petkov
  2026-05-13 11:02       ` Binbin Wu
  2026-05-13 14:41       ` Binbin Wu
  0 siblings, 2 replies; 10+ messages in thread
From: Borislav Petkov @ 2026-05-13 10:14 UTC (permalink / raw)
  To: Binbin Wu
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao

On Tue, May 12, 2026 at 09:57:58AM +0800, Binbin Wu wrote:
> hypervisor_present could be uninitialized if dis_ucode_ldr is true.
> intel_get_platform_id() is also called during the normal cpu initialization.

Right, that needs more surgery. See if the below works instead pls.

We might as well do it - the question whether we run on a HV comes very often
recenly - might as well make it an "official" variable.

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 10b5355b323e..67dd932305db 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -733,6 +733,7 @@ bool xen_set_default_idle(void);
 #endif
 
 void __noreturn stop_this_cpu(void *dummy);
+extern bool x86_hypervisor_present;
 void microcode_check(struct cpuinfo_x86 *prev_info);
 void store_cpu_caps(struct cpuinfo_x86 *info);
 
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index e533881284a1..5c0afae75e9f 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -322,7 +322,7 @@ static u32 get_patch_level(void)
 {
 	u32 rev, dummy __always_unused;
 
-	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && hypervisor_present) {
+	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && x86_hypervisor_present) {
 		int cpu = smp_processor_id();
 
 		if (!microcode_rev[cpu]) {
@@ -714,7 +714,7 @@ static bool __apply_microcode_amd(struct microcode_amd *mc, u32 *cur_rev,
 			invlpg(p_addr_end);
 	}
 
-	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && hypervisor_present)
+	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && x86_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 68a1a893246c..f6722c9618e0 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -57,7 +57,7 @@ bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
 u32 base_rev;
 u32 microcode_rev[NR_CPUS] = {};
 
-bool hypervisor_present;
+bool __ro_after_init x86_hypervisor_present;
 
 /*
  * Synchronization.
@@ -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
+	 *    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);
 
-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-05-13 10:14     ` Borislav Petkov
@ 2026-05-13 11:02       ` Binbin Wu
  2026-05-13 11:08         ` Borislav Petkov
  2026-05-13 14:41       ` Binbin Wu
  1 sibling, 1 reply; 10+ messages in thread
From: Binbin Wu @ 2026-05-13 11:02 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao



On 5/13/2026 6:14 PM, Borislav Petkov wrote:
> On Tue, May 12, 2026 at 09:57:58AM +0800, Binbin Wu wrote:
>> hypervisor_present could be uninitialized if dis_ucode_ldr is true.
>> intel_get_platform_id() is also called during the normal cpu initialization.
> 
> Right, that needs more surgery. See if the below works instead pls.
> 
> We might as well do it - the question whether we run on a HV comes very often
> recenly - might as well make it an "official" variable.

Make sense.
My version treated it as a bug fix, so I tried to minimize the code change. 

[...]

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

There is one case not covered by setting the global x86_hypervisor_present at this point.
Xen PV guest will not go this path, so Xen PV guest will not see the hypervisor bit.

But maybe we can just ignore Xen PV guest case today?


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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-05-13 11:02       ` Binbin Wu
@ 2026-05-13 11:08         ` Borislav Petkov
  2026-05-13 12:20           ` Binbin Wu
  0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2026-05-13 11:08 UTC (permalink / raw)
  To: Binbin Wu
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao

On Wed, May 13, 2026 at 07:02:53PM +0800, Binbin Wu wrote:
> My version treated it as a bug fix, so I tried to minimize the code change.

I didn't like

	BIT(X86_FEATURE_HYPERVISOR & 0x1f))

in your patch. I'd use BIT(31) as it does.

But, more importantly, it added yet another place where we test whether we run
on a HV. And it is high time we unified those because we run more and more on
HVs nowadays.

> There is one case not covered by setting the global x86_hypervisor_present at this point.
> Xen PV guest will not go this path, so Xen PV guest will not see the hypervisor bit.
> 
> But maybe we can just ignore Xen PV guest case today?

Nothing changes before or after this fix. So Xen PV can be handled
completely separated from this.

Thx.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-05-13 11:08         ` Borislav Petkov
@ 2026-05-13 12:20           ` Binbin Wu
  2026-05-13 13:11             ` Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: Binbin Wu @ 2026-05-13 12:20 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao



On 5/13/2026 7:08 PM, Borislav Petkov wrote:
> On Wed, May 13, 2026 at 07:02:53PM +0800, Binbin Wu wrote:
>> My version treated it as a bug fix, so I tried to minimize the code change.
> 
> I didn't like
> 
> 	BIT(X86_FEATURE_HYPERVISOR & 0x1f))
> 
> in your patch. I'd use BIT(31) as it does.

It was suggested by Dave in the v1 comment.
https://lore.kernel.org/kvm/1a1b0f7c-aa3f-4758-8e17-bc2176c52952@intel.com/

> 
> But, more importantly, it added yet another place where we test whether we run
> on a HV. And it is high time we unified those because we run more and more on
> HVs nowadays.

Agree.

> 
>> There is one case not covered by setting the global x86_hypervisor_present at this point.
>> Xen PV guest will not go this path, so Xen PV guest will not see the hypervisor bit.
>>
>> But maybe we can just ignore Xen PV guest case today?
> 
> Nothing changes before or after this fix. So Xen PV can be handled
> completely separated from this.

intel_get_platform_id() can be called in Xen PV guest during normal boot.
Using x86_hypervisor_present just doesn't cover the case.
 
My fix was intended to skip reading MSR_IA32_PLATFORM_ID if hypervisor is
detected, I used cpuid_ecx() instead of cpuid_ecx_native() so that Xen PV
guest can also read the hypervisor bit using xen_cpuid() (Xen PV guest has
overwritten the pv ops) to make the coverage complete.


> 
> Thx.
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-05-13 12:20           ` Binbin Wu
@ 2026-05-13 13:11             ` Borislav Petkov
  0 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2026-05-13 13:11 UTC (permalink / raw)
  To: Binbin Wu
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao

On May 13, 2026 12:20:01 PM UTC, Binbin Wu <binbin.wu@linux.intel.com> wrote:
>My fix was intended to skip reading MSR_IA32_PLATFORM_ID if hypervisor is
>detected, I used cpuid_ecx() instead of cpuid_ecx_native() so that Xen PV
>guest can also read the hypervisor bit using xen_cpuid() (Xen PV guest has
>overwritten the pv ops) to make the coverage 


Which part of "can be handled completely separated from this" wasn't clear?

The current code uses the native cpuid variant. If you want to fix Xen, send a patch ontop please.


-- 
Small device. Typos and formatting crap

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment
  2026-05-13 10:14     ` Borislav Petkov
  2026-05-13 11:02       ` Binbin Wu
@ 2026-05-13 14:41       ` Binbin Wu
  1 sibling, 0 replies; 10+ messages in thread
From: Binbin Wu @ 2026-05-13 14:41 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, x86, kvm, dave.hansen, seanjc, pbonzini, kas,
	rick.p.edgecombe, vishal.l.verma, xiaoyao.li, chao.gao



On 5/13/2026 6:14 PM, Borislav Petkov wrote:
> On Tue, May 12, 2026 at 09:57:58AM +0800, Binbin Wu wrote:
>> hypervisor_present could be uninitialized if dis_ucode_ldr is true.
>> intel_get_platform_id() is also called during the normal cpu initialization.
> 
> Right, that needs more surgery. See if the below works instead pls.
> 
> We might as well do it - the question whether we run on a HV comes very often
> recenly - might as well make it an "official" variable.

Tested the diff by running as a TDX guest and it fixed the unchecked MSR access error.

Tested-by: Binbin Wu <binbin.wu@linux.intel.com>

> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 10b5355b323e..67dd932305db 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -733,6 +733,7 @@ bool xen_set_default_idle(void);
>  #endif
>  
>  void __noreturn stop_this_cpu(void *dummy);
> +extern bool x86_hypervisor_present;
>  void microcode_check(struct cpuinfo_x86 *prev_info);
>  void store_cpu_caps(struct cpuinfo_x86 *info);
>  
> diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
> index e533881284a1..5c0afae75e9f 100644
> --- a/arch/x86/kernel/cpu/microcode/amd.c
> +++ b/arch/x86/kernel/cpu/microcode/amd.c
> @@ -322,7 +322,7 @@ static u32 get_patch_level(void)
>  {
>  	u32 rev, dummy __always_unused;
>  
> -	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && hypervisor_present) {
> +	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && x86_hypervisor_present) {
>  		int cpu = smp_processor_id();
>  
>  		if (!microcode_rev[cpu]) {
> @@ -714,7 +714,7 @@ static bool __apply_microcode_amd(struct microcode_amd *mc, u32 *cur_rev,
>  			invlpg(p_addr_end);
>  	}
>  
> -	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && hypervisor_present)
> +	if (IS_ENABLED(CONFIG_MICROCODE_DBG) && x86_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 68a1a893246c..f6722c9618e0 100644
> --- a/arch/x86/kernel/cpu/microcode/core.c
> +++ b/arch/x86/kernel/cpu/microcode/core.c
> @@ -57,7 +57,7 @@ bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
>  u32 base_rev;
>  u32 microcode_rev[NR_CPUS] = {};
>  
> -bool hypervisor_present;
> +bool __ro_after_init x86_hypervisor_present;
>  
>  /*
>   * Synchronization.
> @@ -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
> +	 *    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);
>  


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-05-13 14:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30  2:09 [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment Binbin Wu
2026-05-11  9:38 ` Kiryl Shutsemau
2026-05-11 10:04 ` Borislav Petkov
2026-05-12  1:57   ` Binbin Wu
2026-05-13 10:14     ` Borislav Petkov
2026-05-13 11:02       ` Binbin Wu
2026-05-13 11:08         ` Borislav Petkov
2026-05-13 12:20           ` Binbin Wu
2026-05-13 13:11             ` Borislav Petkov
2026-05-13 14:41       ` Binbin Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox