* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment [not found] <20260430020953.1405535-1-binbin.wu@linux.intel.com> @ 2026-05-11 9:38 ` Kiryl Shutsemau 2026-05-11 10:04 ` Borislav Petkov 1 sibling, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment [not found] <20260430020953.1405535-1-binbin.wu@linux.intel.com> 2026-05-11 9:38 ` [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment Kiryl Shutsemau @ 2026-05-11 10:04 ` Borislav Petkov 2026-05-12 1:57 ` Binbin Wu 1 sibling, 1 reply; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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 2026-05-13 20:00 ` Borislav Petkov 1 sibling, 1 reply; 12+ 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] 12+ messages in thread
* Re: [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment 2026-05-13 14:41 ` Binbin Wu @ 2026-05-13 20:00 ` Borislav Petkov 2026-05-13 20:06 ` [PATCH 1/2] x86/microcode: Do not access MSR_IA32_PLATFORM_ID when running as a guest Borislav Petkov 2026-05-13 20:07 ` [PATCH 2/2] x86/cpu: Move intel_get_platform_id() to cpu/intel.c Borislav Petkov 0 siblings, 2 replies; 12+ messages in thread From: Borislav Petkov @ 2026-05-13 20:00 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 10:41:18PM +0800, Binbin Wu wrote: > 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> Thanks. Patches coming up as a reply to this message. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] x86/microcode: Do not access MSR_IA32_PLATFORM_ID when running as a guest 2026-05-13 20:00 ` Borislav Petkov @ 2026-05-13 20:06 ` Borislav Petkov 2026-05-13 20:07 ` [PATCH 2/2] x86/cpu: Move intel_get_platform_id() to cpu/intel.c Borislav Petkov 1 sibling, 0 replies; 12+ messages in thread From: Borislav Petkov @ 2026-05-13 20:06 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 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@intel.com> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Tested-by: Binbin Wu <binbin.wu@linux.intel.com> Link: https://lore.kernel.org/r/20260430020953.1405535-1-binbin.wu@linux.intel.com --- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/microcode/amd.c | 4 ++-- arch/x86/kernel/cpu/microcode/core.c | 22 ++++++++++------------ arch/x86/kernel/cpu/microcode/intel.c | 3 +++ arch/x86/kernel/cpu/microcode/internal.h | 1 - 5 files changed, 16 insertions(+), 15 deletions(-) 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..dcc688f4d35f 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); -- 2.51.0 -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] x86/cpu: Move intel_get_platform_id() to cpu/intel.c 2026-05-13 20:00 ` Borislav Petkov 2026-05-13 20:06 ` [PATCH 1/2] x86/microcode: Do not access MSR_IA32_PLATFORM_ID when running as a guest Borislav Petkov @ 2026-05-13 20:07 ` Borislav Petkov 1 sibling, 0 replies; 12+ messages in thread From: Borislav Petkov @ 2026-05-13 20:07 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 It is not only used in the microcode loader anymore and the platform ID is cached in the cpuindo_x86 structure so move the getter to Intel CPU-specific code. No functional changes. Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> --- arch/x86/include/asm/microcode.h | 2 -- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/intel.c | 35 ++++++++++++++++++++++++++ arch/x86/kernel/cpu/microcode/intel.c | 36 --------------------------- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h index 3c317d155771..8b41f26f003b 100644 --- a/arch/x86/include/asm/microcode.h +++ b/arch/x86/include/asm/microcode.h @@ -61,8 +61,6 @@ static inline int intel_microcode_get_datasize(struct microcode_header_intel *hd return hdr->datasize ? : DEFAULT_UCODE_DATASIZE; } -extern u32 intel_get_platform_id(void); - static inline u32 intel_get_microcode_revision(void) { u32 rev, dummy; diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 67dd932305db..1a68a955863d 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -237,6 +237,7 @@ extern void early_cpu_init(void); extern void identify_secondary_cpu(unsigned int cpu); extern void print_cpu_info(struct cpuinfo_x86 *); void print_cpu_msr(struct cpuinfo_x86 *); +extern u32 intel_get_platform_id(void); /* * Friendlier CR3 helpers. diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index f28c0efb7c8f..3473b89e6907 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -199,6 +199,41 @@ void intel_unlock_cpuid_leafs(struct cpuinfo_x86 *c) c->cpuid_level = cpuid_eax(0); } +/* + * Use CPUID to generate a "vfm" value. Useful before cpuinfo_x86 + * structures are populated. + */ +static u32 intel_cpuid_vfm(void) +{ + u32 eax = cpuid_eax(1); + u32 fam = x86_family(eax); + u32 model = x86_model(eax); + + return IFM(fam, model); +} + +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. + * The PII does not have MSR_IA32_PLATFORM_ID. Everything + * before _it_ has no microcode (for Linux at least). + */ + if (intel_cpuid_vfm() <= INTEL_PENTIUM_II_KLAMATH) + return 0; + + /* get processor flags from MSR 0x17 */ + native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); + + return (val[1] >> 18) & 7; +} + static void early_init_intel(struct cpuinfo_x86 *c) { u64 misc_enable; diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index a4c0a0cf928b..f55ff81682e5 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -120,42 +120,6 @@ static inline unsigned int exttable_size(struct extended_sigtable *et) return et->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE; } - -/* - * Use CPUID to generate a "vfm" value. Useful before cpuinfo_x86 - * structures are populated. - */ -static u32 intel_cpuid_vfm(void) -{ - u32 eax = cpuid_eax(1); - u32 fam = x86_family(eax); - u32 model = x86_model(eax); - - return IFM(fam, model); -} - -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. - * The PII does not have MSR_IA32_PLATFORM_ID. Everything - * before _it_ has no microcode (for Linux at least). - */ - if (intel_cpuid_vfm() <= INTEL_PENTIUM_II_KLAMATH) - return 0; - - /* get processor flags from MSR 0x17 */ - native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); - - return (val[1] >> 18) & 7; -} - void intel_collect_cpu_info(struct cpu_signature *sig) { sig->sig = cpuid_eax(1); -- 2.51.0 -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-05-13 20:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260430020953.1405535-1-binbin.wu@linux.intel.com>
2026-05-11 9:38 ` [PATCH v2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment 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
2026-05-13 20:00 ` Borislav Petkov
2026-05-13 20:06 ` [PATCH 1/2] x86/microcode: Do not access MSR_IA32_PLATFORM_ID when running as a guest Borislav Petkov
2026-05-13 20:07 ` [PATCH 2/2] x86/cpu: Move intel_get_platform_id() to cpu/intel.c Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox