[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241019232138.GDZxQ_AtkqA9iAR2td@fat_crate.local>
Date: Sun, 20 Oct 2024 01:21:38 +0200
From: Borislav Petkov <bp@...en8.de>
To: Jens Axboe <axboe@...nel.dk>
Cc: Thomas Gleixner <tglx@...utronix.de>,
the arch/x86 maintainers <x86@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
"regressions@...ts.linux.dev" <regressions@...ts.linux.dev>
Subject: Re: AMD zen microcode updates breaks boot
On Sat, Oct 19, 2024 at 07:54:07AM -0600, Jens Axboe wrote:
> Added that, and here's the full boot output until it crashes. Sent you
> the full thing as there's some microcode debug prints initially, and
> then some later on. Didn't want to miss any.
Thanks, I think I see it. It is that weird node-per-L3 setting which
apparently doesn't set up the ACPI tables properly or the loader runs too
early but load_microcode_amd() sees a very funky node maps. Node 0's first CPU
in the map is CPU 0, which is correct but then cpu_data(0) is a funky pointer
which causes the splat.
All the other "nodes" up to 31 have the first CPU in the mask as 512 which is
WTF?!
So the below is the same conglomerate patch but with code to dump those node
ids in load_microcode_amd() so that I can see what your system says.
It should boot ok now - fingers crossed - but let's see what it really does on
your machine. Just replace it with the previous one, pls, and send me full
dmesg again.
Thx.
---
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index f63b051f25a0..0d840a43b915 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -158,6 +158,9 @@ static union cpuid_1_eax ucode_rev_to_cpuid(unsigned int val)
c.family = 0xf;
c.ext_fam = p.ext_fam;
+ pr_info("%s: val: 0x%x, p.stepping: 0x%x, c.stepping: 0x%x\n",
+ __func__, val, p.stepping, c.stepping);
+
return c;
}
@@ -613,16 +616,22 @@ static int __init save_microcode_in_initrd(void)
}
early_initcall(save_microcode_in_initrd);
-static inline bool patch_cpus_equivalent(struct ucode_patch *p, struct ucode_patch *n)
+static inline bool patch_cpus_equivalent(struct ucode_patch *p,
+ struct ucode_patch *n,
+ bool ignore_stepping)
{
/* Zen and newer hardcode the f/m/s in the patch ID */
if (x86_family(bsp_cpuid_1_eax) >= 0x17) {
union cpuid_1_eax p_cid = ucode_rev_to_cpuid(p->patch_id);
union cpuid_1_eax n_cid = ucode_rev_to_cpuid(n->patch_id);
- /* Zap stepping */
- p_cid.stepping = 0;
- n_cid.stepping = 0;
+ if (ignore_stepping) {
+ p_cid.stepping = 0;
+ n_cid.stepping = 0;
+ }
+
+ pr_info("%s: p_cid.full: 0x%x, n_cid.full: 0x%x\n",
+ __func__, p_cid.full, n_cid.full);
return p_cid.full == n_cid.full;
} else {
@@ -641,16 +650,22 @@ static struct ucode_patch *cache_find_patch(struct ucode_cpu_info *uci, u16 equi
n.equiv_cpu = equiv_cpu;
n.patch_id = uci->cpu_sig.rev;
+ pr_info("%s: equiv_cpu: 0x%x, patch_id: 0x%x\n",
+ __func__, equiv_cpu, uci->cpu_sig.rev);
+
WARN_ON_ONCE(!n.patch_id);
- list_for_each_entry(p, µcode_cache, plist)
- if (patch_cpus_equivalent(p, &n))
+ list_for_each_entry(p, µcode_cache, plist) {
+ if (patch_cpus_equivalent(p, &n, false)) {
+ pr_info("%s: using 0x%x\n", __func__, p->patch_id);
return p;
+ }
+ }
return NULL;
}
-static inline bool patch_newer(struct ucode_patch *p, struct ucode_patch *n)
+static inline int patch_newer(struct ucode_patch *p, struct ucode_patch *n)
{
/* Zen and newer hardcode the f/m/s in the patch ID */
if (x86_family(bsp_cpuid_1_eax) >= 0x17) {
@@ -659,6 +674,9 @@ static inline bool patch_newer(struct ucode_patch *p, struct ucode_patch *n)
zp.ucode_rev = p->patch_id;
zn.ucode_rev = n->patch_id;
+ if (zn.stepping != zp.stepping)
+ return -1;
+
return zn.rev > zp.rev;
} else {
return n->patch_id > p->patch_id;
@@ -668,22 +686,32 @@ static inline bool patch_newer(struct ucode_patch *p, struct ucode_patch *n)
static void update_cache(struct ucode_patch *new_patch)
{
struct ucode_patch *p;
+ int ret;
list_for_each_entry(p, µcode_cache, plist) {
- if (patch_cpus_equivalent(p, new_patch)) {
- if (!patch_newer(p, new_patch)) {
+ if (patch_cpus_equivalent(p, new_patch, true)) {
+ ret = patch_newer(p, new_patch);
+ if (ret < 0)
+ continue;
+ else if (!ret) {
/* we already have the latest patch */
kfree(new_patch->data);
kfree(new_patch);
return;
}
+ pr_info("%s: replace 0x%x with 0x%x\n",
+ __func__, p->patch_id, new_patch->patch_id);
+
list_replace(&p->plist, &new_patch->plist);
kfree(p->data);
kfree(p);
return;
}
}
+
+ pr_info("%s: add patch: 0x%x\n", __func__, new_patch->patch_id);
+
/* no patch found, add it */
list_add_tail(&new_patch->plist, µcode_cache);
}
@@ -964,11 +992,14 @@ static enum ucode_state load_microcode_amd(u8 family, const u8 *data, size_t siz
cpu = cpumask_first(cpumask_of_node(nid));
c = &cpu_data(cpu);
+ pr_info("%s: nid: %d, cpu: %d, c ptr: 0x%px\n",
+ __func__, nid, cpu, c);
+
p = find_patch(cpu);
if (!p)
continue;
- if (c->microcode >= p->patch_id)
+ if (boot_cpu_data.microcode >= p->patch_id)
continue;
ret = UCODE_NEW;
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists