| 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
| ||
|
Message-ID: <ZWjVt5dNRjbcvlzR@a4bf019067fa.jf.intel.com>
Date: Thu, 30 Nov 2023 10:34:31 -0800
From: Ashok Raj <ashok.raj@...el.com>
To: Borislav Petkov <bp@...en8.de>
CC: X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
"Linus Torvalds" <torvalds@...ux-foundation.org>,
Ashok Raj <ashok.raj@...el.com>
Subject: Re: [PATCH 2/2] x86/microcode: Rework early revisions reporting
Hi Boris,
On Wed, Nov 15, 2023 at 10:02:12PM +0100, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@...en8.de>
>
> The AMD side of the loader issues the microcode revision for each
> logical thread on the system, which can become really noisy on huge
> machines. And doing that doesn't make a whole lot of sense - the
> microcode revision is already in /proc/cpuinfo.
>
> So in case one is interested in the theoretical support of mixed silicon
> steppings on AMD, one can check there.
>
> What is also missing on the AMD side - something which people have
> requested before - is showing the microcode revision the CPU had
> *before* the early update.
>
> So abstract that up in the main code and have the BSP on each vendor
> provide those revision numbers.
>
> Then, dump them only once on driver init.
>
> On Intel, do not dump the patch date - it is not needed.
>
> Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Borislav Petkov (AMD) <bp@...en8.de>
> Link: https://lore.kernel.org/r/CAHk-=wg=%2B8rceshMkB4VnKxmRccVLtBLPBawnewZuuqyx5U=3A@mail.gmail.com
> ---
> arch/x86/kernel/cpu/microcode/amd.c | 39 +++++++-----------------
> arch/x86/kernel/cpu/microcode/core.c | 12 ++++++--
> arch/x86/kernel/cpu/microcode/intel.c | 17 +++++------
> arch/x86/kernel/cpu/microcode/internal.h | 14 ++++++---
> 4 files changed, 38 insertions(+), 44 deletions(-)
>
[snip]
> void load_ucode_ap(void)
> @@ -826,6 +828,12 @@ static int __init microcode_init(void)
> if (!microcode_ops)
> return -ENODEV;
>
> + pr_info_once("Current revision: 0x%08x\n", (early_data.new_rev ?: early_data.old_rev));
> +
> + if (early_data.new_rev)
> + pr_info_once("Updated early from: 0x%08x\n",
> + early_data.old_rev);
See below, new_rev is always assigned. The above message appears even when
no new microcode was applied.
> +
> microcode_pdev = platform_device_register_simple("microcode", -1, NULL, 0);
> if (IS_ERR(microcode_pdev))
> return PTR_ERR(microcode_pdev);
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 6024feb98d29..070426b9895f 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -339,16 +339,9 @@ static enum ucode_state __apply_microcode(struct ucode_cpu_info *uci,
> static enum ucode_state apply_microcode_early(struct ucode_cpu_info *uci)
> {
> struct microcode_intel *mc = uci->mc;
> - enum ucode_state ret;
> - u32 cur_rev, date;
> + u32 cur_rev;
>
> - ret = __apply_microcode(uci, mc, &cur_rev);
> - if (ret == UCODE_UPDATED) {
> - date = mc->hdr.date;
> - pr_info_once("updated early: 0x%x -> 0x%x, date = %04x-%02x-%02x\n",
> - cur_rev, mc->hdr.rev, date & 0xffff, date >> 24, (date >> 16) & 0xff);
> - }
> - return ret;
> + return __apply_microcode(uci, mc, &cur_rev);
> }
>
> static __init bool load_builtin_intel_microcode(struct cpio_data *cp)
> @@ -413,13 +406,17 @@ static int __init save_builtin_microcode(void)
> early_initcall(save_builtin_microcode);
>
> /* Load microcode on BSP from initrd or builtin blobs */
> -void __init load_ucode_intel_bsp(void)
> +void __init load_ucode_intel_bsp(struct early_load_data *ed)
> {
> struct ucode_cpu_info uci;
>
> + ed->old_rev = intel_get_microcode_revision();
> +
> uci.mc = get_microcode_blob(&uci, false);
> if (uci.mc && apply_microcode_early(&uci) == UCODE_UPDATED)
> ucode_patch_va = UCODE_BSP_LOADED;
> +
> + ed->new_rev = uci.cpu_sig.rev;
new_rev is always assigned even if there was no microcode to apply.
Feel free to squash this patch if it makes sense.
From: Ashok Raj <ashok.raj@...el.com>
Date: Tue, 28 Nov 2023 15:30:31 -0800
Subject: [PATCH] x86/microcode: Suppress early load message when the revision
is unchanged
After early loading, early_data.old_rev is always updated, that results in
printing a message even if the revision is unchanged.
Currently, it's displayed as below:
[ 113.395868] microcode: Current revision: 0x21000170
[ 113.404244] microcode: Updated early from: 0x21000170
This should happen on both AMD and Intel. Although for different reasons.
- On AMD, the ucode is loaded even if the current revision matches what is
being loaded.
- On Intel, load_ucode_intel_bsp() assigns new_rev unconditionally. So it's
never 0.
Suppress the "Updated early" message when revision is unchanged.
Fixes: 080990aa3344 ("x86/microcode: Rework early revisions reporting")
Signed-off-by: Ashok Raj <ashok.raj@...el.com>
---
arch/x86/kernel/cpu/microcode/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 232026a239a6..18e61ecab005 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -830,7 +830,7 @@ static int __init microcode_init(void)
pr_info_once("Current revision: 0x%08x\n", (early_data.new_rev ?: early_data.old_rev));
- if (early_data.new_rev)
+ if (early_data.new_rev && early_data.new_rev != early_data.old_rev)
pr_info_once("Updated early from: 0x%08x\n", early_data.old_rev);
microcode_pdev = platform_device_register_simple("microcode", -1, NULL, 0);
--
2.39.2
Powered by blists - more mailing lists