From 26e5d89c7316340b67e08b8341d95d14dfa1463c Mon Sep 17 00:00:00 2001 From: Nexoniarz Date: Wed, 23 Sep 2026 19:37:35 +0200 Subject: [PATCH] smp: every stack on every AP was misaligned by eight `jmp mpenter`. x86-64 requires rsp to be 16-byte aligned immediately before a `call`, so a function starts with rsp ending in 8 -- the return address -- and its `push %rbp; mov %rsp,%rbp` prologue leaves %rbp 16-byte aligned. Compilers build on that: an aligned vector store to a frame slot is `movaps`, and `movaps` to a misaligned address is #GP. entry64mp loaded the AP's stack pointer, which is page-aligned, and *jumped*. Nothing pushed a return address, so %rbp came out eight bytes off -- on every frame, on every application processor, for the life of the machine. It cost nothing for as long as no function an AP ran needed an aligned vector store. Then the console's glyph path started returning a small struct by value (the cell-as-a-surface change), clang spilled it with movaps, and CPU 1 took a #GP the instant it announced itself on a verbose boot: unexpected trap 13 from cpu 1 rip ffff800000120a3b 120a37: 0f 10 45 90 movups -0x70(%rbp), %xmm0 120a3b: 0f 29 45 b0 movaps %xmm0, -0x50(%rbp) <- #GP movups on the line above loads fine; movaps stores and faults. The displacement is a multiple of 16, so what is misaligned is %rbp itself, which is a fact about how the function was entered. `call mpenter`, not `jmp`. Eight bytes for an ABI the whole toolchain assumes. **It was hiding behind a second bug.** The first report was not a #GP at all: Stop code ACQUIRE #2 cprintf+0xae #3 trap+0x2c4 trap() reported the fault with cprintf(), cprintf() takes the console lock, and the faulting code was already holding it -- on a verbose boot every character is drawn under that lock. acquire() panicked about a lock its own call chain owned, and the trap number, the faulting address and the registers were all lost. A kernel-mode trap ends in panic() a dozen lines later anyway, so the console goes into panic mode *before* the report rather than during it. **And a third, in the same boot.** The console reported "JetBrains Mono, 8x16 cells" -- a name from one font and metrics from another. The built-in bitmap became a provider like any other in the previous commit, so draw_glyph_px() installs it lazily on the first glyph; on a verbose boot that happens before gfx_font_init() runs, which then returned early because a font was already installed. The console spent the whole boot in the fallback while claiming not to be. "A font is installed" and "a typeface was chosen" had been the same test and quietly stopped being; they are two flags now. Verified: verbose boot reaches a shell twice running with no traps and no panics, and reports 10x21 cells -- the real typeface -- where it reported 8x16 before. Both supported configurations quiet: one boot each, no traps, no panics, socktest and memfdtest green. Written up as section 22 of docs/history/kernel.md, because the interesting part is not the missing eight bytes but that three separate faults had to be peeled off in order before the first one could be seen. Co-Authored-By: Claude Opus 5 --- docs/history/kernel.md | 70 ++++++++++++++++++++++++++++++++++++++ kernel/arch/x86_64/entry.S | 21 +++++++++++- kernel/arch/x86_64/trap.c | 12 +++++++ kernel/dev/gfx.c | 20 +++++++++-- 4 files changed, 120 insertions(+), 3 deletions(-) diff --git a/docs/history/kernel.md b/docs/history/kernel.md index f1c8967..6b52cdb 100644 --- a/docs/history/kernel.md +++ b/docs/history/kernel.md @@ -1125,3 +1125,73 @@ and startup is 0.14 s. mksh keeps `/bin/sh`, so schinit's no-jobs fallback is a different program from the one that might have failed. Completion is the piece that waits on the filesystem. + +## 22. Every stack on every application processor was misaligned by eight + +`jmp mpenter`. + +x86-64 requires `rsp` to be 16-byte aligned immediately before a `call`, so a +function begins with `rsp` ending in 8 — the return address — and its +`push %rbp; mov %rsp,%rbp` prologue leaves `%rbp` 16-byte aligned. Compilers +build on that: an aligned vector store to a frame slot is `movaps`, and +`movaps` to a misaligned address is `#GP`. + +`entry64mp` loaded the AP's stack pointer, which is page-aligned, and *jumped*. +No return address was pushed, so `%rbp` came out eight bytes off — on every +frame, on every application processor, for the life of the machine. + +It cost nothing for as long as no function an AP ran needed an aligned vector +store. Then the console's glyph path started returning a small struct by value, +clang spilled it with `movaps`, and CPU 1 took a `#GP` the instant it announced +itself: + +``` +unexpected trap 13 from cpu 1 rip ffff800000120a3b +``` + +``` +ffff800000120a37: 0f 10 45 90 movups -0x70(%rbp), %xmm0 +ffff800000120a3b: 0f 29 45 b0 movaps %xmm0, -0x50(%rbp) <- #GP +``` + +`movups` — *unaligned* — loads fine on the line above; `movaps` stores and +faults. The displacement is a multiple of 16, so the misalignment is `%rbp` +itself, which is a fact about how the function was entered rather than +anything it did. + +`call mpenter` instead of `jmp`. Eight bytes for an ABI the whole toolchain +assumes. + +### It hid behind a second bug + +The first report of this was not a `#GP`, it was: + +``` +Stop code ACQUIRE + #2 cprintf+0xae + #3 trap+0x2c4 +``` + +`trap()` reported the fault with `cprintf()`, `cprintf()` takes the console +lock, and the faulting code was already holding it — every character drawn on +a verbose boot is drawn under that lock. So `acquire()` panicked about a lock +its own call chain owned, and the trap number, the faulting address and the +register state were all lost. + +A kernel-mode trap ends in `panic()` a dozen lines later regardless, so the +console is put into panic mode *before* the report rather than during it. The +same fault then prints what it actually was, which is the entire purpose of +the report. + +### And a third, in the same boot + +The console reported `JetBrains Mono, 8x16 cells` — a name from one font and +metrics from another. The built-in bitmap had become a provider like any other, +so `draw_glyph_px()` installed it lazily on the first glyph; on a verbose boot +that happens before `gfx_font_init()` runs, and `gfx_font_init()` returned early +because a font was already installed. The console spent the whole boot in the +fallback while claiming not to be. + +"A font is installed" and "a typeface was chosen" had been the same test +(`cfont == 0`) and quietly stopped being. They are two flags now, and +`gfx_font_ready()` answers the second one. diff --git a/kernel/arch/x86_64/entry.S b/kernel/arch/x86_64/entry.S index 64eabc3..c1e564f 100644 --- a/kernel/arch/x86_64/entry.S +++ b/kernel/arch/x86_64/entry.S @@ -233,7 +233,26 @@ __deadloop: entry64mp: mov $0x7000, %rax mov -16(%rax), %rsp - jmp mpenter + + # `call`, not `jmp`, and the difference is an ABI requirement rather than a + # style preference. + # + # x86-64 says rsp is 16-byte aligned immediately before a `call`, so a + # function starts with rsp ending in 8 -- the return address the call just + # pushed -- and its `push %rbp; mov %rsp,%rbp` prologue therefore leaves + # %rbp 16-byte aligned. Compilers rely on that: an aligned SSE store to a + # frame slot is `movaps`, and `movaps` to a misaligned address is #GP. + # + # This was a `jmp` into a 16-byte-aligned stack, so every frame on every + # application processor was off by eight, for the life of the machine. It + # cost nothing for as long as no function the APs ran happened to need an + # aligned vector store -- and then one did, deep in the console, and CPU 1 + # took a #GP the moment it announced itself on a verbose boot. + # + # mpenter never returns, so the pushed address is only ever a backtrace + # entry. Eight bytes for an ABI the whole toolchain assumes is a bargain. + call mpenter + jmp __deadloop .global wrmsr wrmsr: diff --git a/kernel/arch/x86_64/trap.c b/kernel/arch/x86_64/trap.c index 6622b49..1ade375 100644 --- a/kernel/arch/x86_64/trap.c +++ b/kernel/arch/x86_64/trap.c @@ -148,6 +148,18 @@ trap(struct trapframe *tf) unexpected: if(myproc() == 0 || (tf->cs&3) == 0){ + // A kernel-mode trap ends in panic() a dozen lines down, so the console + // can be put into its panic mode *now* -- and it has to be, because the + // fault may have happened inside the console itself. + // + // cprintf() takes cons.lock. If the faulting code was already holding + // it -- anything under console output, which on a verbose boot is most + // of the kernel -- then this report deadlocks on a lock its own call + // chain owns, acquire() panics with "acquire", and the trap that + // actually happened is never printed. That is a bad trade at the best + // of times and a worse one here: the whole point of the report is to + // name the fault. + cons_panic_mode(); // The rip is named where it can be. This is a kernel address, so // ksym_lookup() has something to say about it -- and the whole reason // docs/architecture/debugging.md puts D0 first is that "rip ffff8000001294c2" diff --git a/kernel/dev/gfx.c b/kernel/dev/gfx.c index 53cefa4..d9bc3a8 100644 --- a/kernel/dev/gfx.c +++ b/kernel/dev/gfx.c @@ -61,6 +61,17 @@ static int cell_h = BITMAP_H; // The typeface. Null only until the first glyph is drawn, which fills it in // with the built-in bitmap; gfx_font_init() replaces that with a real face // when there is one. +// Whether the font in `cfont` is the built-in bitmap standing in, or a real +// typeface that was chosen. +// +// The distinction was implicit in `cfont == 0` until the built-in became a +// provider like any other, and losing it cost a boot: draw_glyph_px() installs +// the fallback lazily, a verbose boot draws before gfx_font_init() runs, and +// gfx_font_init() then saw a font already installed and returned without +// loading anything. The console stayed 8x16 for the whole boot while reporting +// itself as JetBrains Mono. +static int cfont_is_builtin; + static struct ttf consttf; static struct font *cfont; static int cfont_baseline; @@ -318,6 +329,7 @@ draw_glyph_px(int x, int y, uint32 cp, uint32 fg, uint32 bg) if (!cfont) { cfont = font_builtin(); cfont_baseline = cfont->ops->baseline(cfont); + cfont_is_builtin = 1; } // Background first. A terminal cell owns its rectangle, and leaving the old @@ -366,7 +378,10 @@ gfx_font_init(void) uint32 len = (uint32)(_binary_consfont_end - _binary_consfont_start); struct glyph gl; - if (cfont) + // A built-in font standing in is not a font having been chosen. Returning + // here on that basis is what kept the bitmap on screen for an entire + // verbose boot. + if (cfont && !cfont_is_builtin) return; // A typeface off the boot volume, if the loader found one. @@ -430,6 +445,7 @@ gfx_font_init(void) cell_h = h; cfont_baseline = f->ops->baseline(f); cfont = f; + cfont_is_builtin = 0; } // **Nothing here prints.** gfx_font_init() runs before vt_init(), because @@ -443,7 +459,7 @@ gfx_font_init(void) int gfx_font_ready(void) { - return cfont != 0; + return cfont != 0 && !cfont_is_builtin; } void