[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
Builtin/initrd microcode will not be used the ucode loader is disabled. But currently, save_microcode_in_initrd is always performed and it accesses MSR_IA32_UCODE_REV even if dis_ucode_ldr is true, and in particular even if X86_FEATURE_HYPERVISOR is set; the TDX module does not implement the MSR and the result is a call trace at boot for TDX guests.
Mainline Linux fixed this as part of a more complex rework of microcode caching that went into 6.7 (see in particular commits dd5e3e3ca6, "x86/microcode/intel: Simplify early loading"; and a7939f0167203, "x86/microcode/amd: Cache builtin/initrd microcode early"). Do the bare minimum in stable kernels, setting initrd_gone just like mainline Linux does in mark_initrd_gone().
Note that save_microcode_in_initrd() is not in the microcode application path, which runs with paging disabled on 32-bit systems, so it can (and has to) use dis_ucode_ldr instead of check_loader_disabled_ap().
Cc: stable@vger.kernel.org # v6.6+ Cc: x86@kernel.org # v6.6+ Cc: Borislav Petkov bp@suse.de Cc: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- arch/x86/kernel/cpu/microcode/core.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 35d39a13dc90..503b5da56685 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -214,6 +214,11 @@ static int __init save_microcode_in_initrd(void) struct cpuinfo_x86 *c = &boot_cpu_data; int ret = -EINVAL;
+ if (dis_ucode_ldr) { + ret = 0; + goto out; + } + switch (c->x86_vendor) { case X86_VENDOR_INTEL: if (c->x86 >= 6) @@ -227,6 +230,7 @@ static int __init save_microcode_in_initrd(void) break; }
+out: initrd_gone = true;
return ret;
On Mon, Jan 15, 2024 at 11:22:02AM +0100, Paolo Bonzini wrote:
[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
Builtin/initrd microcode will not be used the ucode loader is disabled. But currently, save_microcode_in_initrd is always performed and it accesses MSR_IA32_UCODE_REV even if dis_ucode_ldr is true, and in particular even if X86_FEATURE_HYPERVISOR is set; the TDX module does not implement the MSR and the result is a call trace at boot for TDX guests.
Mainline Linux fixed this as part of a more complex rework of microcode caching that went into 6.7 (see in particular commits dd5e3e3ca6, "x86/microcode/intel: Simplify early loading"; and a7939f0167203, "x86/microcode/amd: Cache builtin/initrd microcode early"). Do the bare minimum in stable kernels, setting initrd_gone just like mainline Linux does in mark_initrd_gone().
Note that save_microcode_in_initrd() is not in the microcode application path, which runs with paging disabled on 32-bit systems, so it can (and has to) use dis_ucode_ldr instead of check_loader_disabled_ap().
... simply because save_microcode_in_initrd() is a fs_initcall and by then 32-bit has managed to enable paging.
Cc: stable@vger.kernel.org # v6.6+ Cc: x86@kernel.org # v6.6+ Cc: Borislav Petkov bp@suse.de Cc: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com
arch/x86/kernel/cpu/microcode/core.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index 35d39a13dc90..503b5da56685 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -214,6 +214,11 @@ static int __init save_microcode_in_initrd(void) struct cpuinfo_x86 *c = &boot_cpu_data; int ret = -EINVAL;
- if (dis_ucode_ldr) {
ret = 0;goto out;- }
- switch (c->x86_vendor) { case X86_VENDOR_INTEL: if (c->x86 >= 6)
@@ -227,6 +230,7 @@ static int __init save_microcode_in_initrd(void) break; } +out: initrd_gone = true; return ret; --
Makes sense to me.
Acked-by: Borislav Petkov (AMD) bp@alien8.de
On Mon, Jan 15, 2024 at 11:22:02AM +0100, Paolo Bonzini wrote:
[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
This really isn't this commit id, sorry.
Builtin/initrd microcode will not be used the ucode loader is disabled. But currently, save_microcode_in_initrd is always performed and it accesses MSR_IA32_UCODE_REV even if dis_ucode_ldr is true, and in particular even if X86_FEATURE_HYPERVISOR is set; the TDX module does not implement the MSR and the result is a call trace at boot for TDX guests.
Mainline Linux fixed this as part of a more complex rework of microcode caching that went into 6.7 (see in particular commits dd5e3e3ca6, "x86/microcode/intel: Simplify early loading"; and a7939f0167203, "x86/microcode/amd: Cache builtin/initrd microcode early"). Do the bare minimum in stable kernels, setting initrd_gone just like mainline Linux does in mark_initrd_gone().
Why can't we take the changes in 6.7? Doing a one-off almost always causes problems :(
What exact commits are needed?
thanks,
greg
On Mon, Jan 15, 2024 at 7:35 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 15, 2024 at 11:22:02AM +0100, Paolo Bonzini wrote:
[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
This really isn't this commit id, sorry.
True, that's the point of the mainline kernel where the logic most closely resembles the patch. stable-kernel-rules.rst does not quite say what to do in this case.
Builtin/initrd microcode will not be used the ucode loader is disabled. But currently, save_microcode_in_initrd is always performed and it accesses MSR_IA32_UCODE_REV even if dis_ucode_ldr is true, and in particular even if X86_FEATURE_HYPERVISOR is set; the TDX module does not implement the MSR and the result is a call trace at boot for TDX guests.
Mainline Linux fixed this as part of a more complex rework of microcode caching that went into 6.7 (see in particular commits dd5e3e3ca6, "x86/microcode/intel: Simplify early loading"; and a7939f0167203, "x86/microcode/amd: Cache builtin/initrd microcode early"). Do the bare minimum in stable kernels, setting initrd_gone just like mainline Linux does in mark_initrd_gone().
Why can't we take the changes in 6.7? Doing a one-off almost always causes problems :(
The series is https://lore.kernel.org/all/20231002115506.217091296@linutronix.de/
+ fixes at https://lore.kernel.org/lkml/20231010150702.495139089@linutronix.de/T/
for a total of 35 patches and 800 lines changed.
There is no individual patch that fixes the bug, because it wasn't really intentional in those patches; it just came out naturally as a consequence of cleaning up unnecessary code. It is fixed as of patch 7, so one possibility would be to apply patches 1-7 in the series above but, especially for older kernels, that would be way more scary than this three line patch. It literally says "if microcode won't be loaded in the processor don't keep it in memory either".
Paolo
What exact commits are needed?
thanks,
greg
On Mon, Jan 15, 2024 at 07:54:59PM +0100, Paolo Bonzini wrote:
On Mon, Jan 15, 2024 at 7:35 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 15, 2024 at 11:22:02AM +0100, Paolo Bonzini wrote:
[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
This really isn't this commit id, sorry.
True, that's the point of the mainline kernel where the logic most closely resembles the patch. stable-kernel-rules.rst does not quite say what to do in this case.
Ok, then just say, "this is not upstream" and the rest of your changelog is good. I'll edit it up tomorrow and apply it, thanks.
greg k-h
On Mon, Jan 15, 2024 at 09:59:23PM +0100, Greg KH wrote:
On Mon, Jan 15, 2024 at 07:54:59PM +0100, Paolo Bonzini wrote:
On Mon, Jan 15, 2024 at 7:35 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 15, 2024 at 11:22:02AM +0100, Paolo Bonzini wrote:
[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
This really isn't this commit id, sorry.
True, that's the point of the mainline kernel where the logic most closely resembles the patch. stable-kernel-rules.rst does not quite say what to do in this case.
Ok, then just say, "this is not upstream" and the rest of your changelog is good. I'll edit it up tomorrow and apply it, thanks.
Ok, now queued up for 6.6.y, but what about older kernel versions? Is this relevant for them as well? I will note that it does seem to apply to them properly and build, is it not relevant?
thanks,
greg k-h
On Wed, Jan 17, 2024 at 12:00 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 15, 2024 at 09:59:23PM +0100, Greg KH wrote:
On Mon, Jan 15, 2024 at 07:54:59PM +0100, Paolo Bonzini wrote:
On Mon, Jan 15, 2024 at 7:35 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 15, 2024 at 11:22:02AM +0100, Paolo Bonzini wrote:
[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
This really isn't this commit id, sorry.
True, that's the point of the mainline kernel where the logic most closely resembles the patch. stable-kernel-rules.rst does not quite say what to do in this case.
Ok, then just say, "this is not upstream" and the rest of your changelog is good. I'll edit it up tomorrow and apply it, thanks.
Ok, now queued up for 6.6.y, but what about older kernel versions?
6.6 is where I tested that it works, and I didn't want to put an old kernel version in the "Cc" line, without even testing that a non-upstream patch applies there.
The benefit would be absolutely marginal. People playing with Intel TDX are not going to use old kernels (6.1 counts as old) anyway, for example support for lazy acceptance of memory went into 6.5.
Paolo
On Wed, Jan 17, 2024 at 12:57:40PM +0100, Paolo Bonzini wrote:
On Wed, Jan 17, 2024 at 12:00 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 15, 2024 at 09:59:23PM +0100, Greg KH wrote:
On Mon, Jan 15, 2024 at 07:54:59PM +0100, Paolo Bonzini wrote:
On Mon, Jan 15, 2024 at 7:35 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 15, 2024 at 11:22:02AM +0100, Paolo Bonzini wrote:
[ Upstream commit a7939f01672034a58ad3fdbce69bb6c665ce0024 ]
This really isn't this commit id, sorry.
True, that's the point of the mainline kernel where the logic most closely resembles the patch. stable-kernel-rules.rst does not quite say what to do in this case.
Ok, then just say, "this is not upstream" and the rest of your changelog is good. I'll edit it up tomorrow and apply it, thanks.
Ok, now queued up for 6.6.y, but what about older kernel versions?
6.6 is where I tested that it works, and I didn't want to put an old kernel version in the "Cc" line, without even testing that a non-upstream patch applies there.
The benefit would be absolutely marginal. People playing with Intel TDX are not going to use old kernels (6.1 counts as old) anyway, for example support for lazy acceptance of memory went into 6.5.
Fair enough, thanks for the explaination, I'll leave it alone then.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Borislav Petkov -
Greg KH -
Paolo Bonzini