From b43e51a95dce93991a08c862eaa862a2786d20a9 Mon Sep 17 00:00:00 2001 From: Robert Allan James Date: Tue, 11 Aug 2026 12:16:21 -0400 Subject: [PATCH] starkernel: fix muldiv64() inline-asm register-allocation hazard FABRIC.md item 4.5d: root-caused the -O2 boot stall via QEMU '-d int' tracing -- not a hang. It's a genuine divide-error (#DE) cascading through a double fault into a triple fault, which -no-reboot converts into a silent, clean QEMU exit (indistinguishable from a hang without tracing). muldiv64()'s inline asm declared RDX as a plain output ("=d"(hi)), which only tells GCC "I want RDX's value after this block" -- nothing told it that mulq writes RDX *before* divq needs to read a *different* value (the divisor c) out of it. Nothing stopped the register allocator from placing c itself in RDX, which mulq then overwrites with the product's high 64 bits before divq ever reads it. Confirmed via the fault's register state: RAX=0xe8d4a51000 (=1000*1e9 exactly, product fits in the low 64 bits, so mulq's high-word output is 0) -- if c got allocated to RDX, divq then divides by that corrupted 0, exactly matching #DE. Worked by accident at -O0 (different, more conservative allocation); -O2 actually hit it. A unsigned __int128 rewrite was tried first but needs libgcc's __udivti3 for the general 128-bit case, undefined in this freestanding build -- not viable, same class of problem as the earlier putc/getc finding. Fixed instead by declaring rdx a pure clobber rather than an output, the same pattern the Linux kernel's own mul_u64_u64_div_u64 uses -- a clobber tells GCC the register is used internally for the whole block and must never be allocated to any operand, which is the guarantee the previous constraint list was missing. Also added a defensive end_tsc --- src/starkernel/arch/amd64/timer.c | 50 +++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/src/starkernel/arch/amd64/timer.c b/src/starkernel/arch/amd64/timer.c index aca21a9..52cd2f9 100644 --- a/src/starkernel/arch/amd64/timer.c +++ b/src/starkernel/arch/amd64/timer.c @@ -134,6 +134,28 @@ static uint64_t vm_ns_base = 0; * @p b are both large (e.g., nanosecond conversion of multi-GHz tick counts). * No libgcc dependency — pure inline assembly. * + * FABRIC.md item 4.5d, 2026-08-11: this asm previously declared @c RDX as a + * plain output (@c "=d"(hi)), which tells GCC only "I want to read RDX's + * value after this block" — nothing told it that @c mulq writes RDX *before* + * @c divq needs to read a *different* value (the divisor @c c) out of it. + * Nothing stopped the register allocator from placing @c c itself in RDX, + * which @c mulq then overwrote with the product's high 64 bits before + * @c divq ever read it. At @c -O0 this happened not to manifest; at @c -O2, + * GCC's more aggressive allocation actually did it, and for a product like + * `1000 * 1000000000` (fits entirely in the low 64 bits, so mulq's + * high-word output is 0), that corrupted the intended divisor to 0 -- a + * genuine #DE (divide error) confirmed via QEMU `-d int` tracing, cascading + * to a double fault and then a triple fault (silently exited by + * `-no-reboot`, which is why this looked like an infinite hang rather than + * a crash at first). A `unsigned __int128` rewrite was tried first but + * needs libgcc's `__udivti3` for the general 128÷64 case, undefined in this + * freestanding, `-nostdlib` build — not viable. Fixed instead the way the + * Linux kernel's own `mul_u64_u64_div_u64` does it: declare `rdx` as a pure + * *clobber*, not an output. A clobber tells GCC the register is used + * internally by the whole asm block and must never be allocated to any + * input or output operand for its duration — exactly the guarantee the + * previous constraint list was missing. + * * Undefined behaviour if @c a × b / c overflows @c uint64_t (i.e., the * true quotient exceeds @c UINT64_MAX). Callers must ensure this cannot * happen with their inputs. @@ -145,17 +167,14 @@ static uint64_t vm_ns_base = 0; */ static inline uint64_t muldiv64(uint64_t a, uint64_t b, uint64_t c) { - uint64_t lo, hi, result; - /* mul: RDX:RAX = RAX * operand */ + uint64_t result; __asm__ volatile ( - "mulq %3\n\t" /* RDX:RAX = a * b */ - "divq %4\n\t" /* RAX = RDX:RAX / c, RDX = remainder */ - : "=a"(result), "=d"(hi), "=r"(lo) - : "r"(b), "r"(c), "0"(a) - : "cc" + "mulq %2\n\t" /* RDX:RAX = a * b */ + "divq %3\n\t" /* RAX = RDX:RAX / c, RDX = remainder (discarded) */ + : "=a"(result) + : "a"(a), "rm"(b), "rm"(c) + : "rdx", "cc" ); - (void)hi; - (void)lo; return result; } @@ -578,6 +597,19 @@ static uint64_t calibrate_tsc_with_pmtimer(void) uint64_t elapsed_ns = muldiv64(elapsed_ticks, 1000000000ull, PMTIMER_FREQ_HZ); if (elapsed_ns == 0) return 0; + /* FABRIC.md item 4.5d, 2026-08-11: this file's own comments already + * flag TSC non-monotonicity as a real risk under TCG ("invariant + * TSC not present under hypervisor... no determinism guarantees"). + * If end_tsc < start_tsc, this subtraction wraps to a huge unsigned + * value; muldiv64()'s own contract is explicit that an overflowing + * quotient is caller-UB, and DIVQ raises #DE on overflow -- exactly + * the divide-error / double-fault / triple-fault cascade found by + * QEMU '-d int' tracing an -O2 boot stall that turned out not to be + * a hang at all. Treat non-monotonic TSC as a calibration failure, + * the same as the elapsed_ns==0 case just above, instead of feeding + * an underflowed value into the division. */ + if (end_tsc < start_tsc) return 0; + uint64_t delta_tsc = end_tsc - start_tsc; return muldiv64(delta_tsc, 1000000000ull, elapsed_ns); }