From 91f2cfa17bab393af2816d40b5bb67592213149d Mon Sep 17 00:00:00 2001
From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com>
Date: Wed, 8 Jul 2026 13:41:14 +0100
Subject: [PATCH] add smp-retry test for the AP wake retry path

Test hook forces the first wake to fail; the case asserts every core still comes online. Verified it fails when retry is disabled.
---
 src/kernel/arch/x86_64/cpu.zig |  6 ++++++
 src/kernel/arch/x86_64/smp.zig | 13 +++++++++++++
 src/kernel/main.zig            |  6 ++++++
 src/kernel/tests.zig           | 16 ++++++++++++++++
 test/qemu_test.py              |  5 +++++
 5 files changed, 46 insertions(+)

diff --git a/src/kernel/arch/x86_64/cpu.zig b/src/kernel/arch/x86_64/cpu.zig
index a86d5b5..2e1ef31 100644
--- a/src/kernel/arch/x86_64/cpu.zig
+++ b/src/kernel/arch/x86_64/cpu.zig
@@ -124,6 +124,12 @@ pub fn setSecondaryEntry(entry: *const fn () callconv(.c) noreturn) void {
     smp.setSecondaryEntry(entry);
 }
 
+/// Test hook: force the next `n` AP wake attempts to fail, so the retry path can be
+/// exercised deterministically (see the smp-retry test). No effect when `n` is 0.
+pub fn testFailNextWakes(n: u32) void {
+    smp.testFailNextWakes(n);
+}
+
 /// Kernel tick rate: 1000 Hz (1 ms), the scheduler's time quantum.
 pub const timer_hz = 1000;
 
diff --git a/src/kernel/arch/x86_64/smp.zig b/src/kernel/arch/x86_64/smp.zig
index f342814..e1f6ad4 100644
--- a/src/kernel/arch/x86_64/smp.zig
+++ b/src/kernel/arch/x86_64/smp.zig
@@ -51,6 +51,14 @@ pub fn setSecondaryEntry(entry: *const fn () callconv(.c) noreturn) void {
     secondary_entry = entry;
 }
 
+/// Test hook: force the next `n` wake attempts to fail (skipping the actual
+/// INIT-SIPI-SIPI), so the retry path can be exercised deterministically. Zero in
+/// normal operation — the smp-retry test arms it via `arch.testFailNextWakes`.
+var fail_next_wakes: u32 = 0;
+pub fn testFailNextWakes(n: u32) void {
+    fail_next_wakes = n;
+}
+
 /// Record the reserved low frame the trampoline uses. Call once at boot. The frame
 /// starts inert (identity-mapped RW+NX like all RAM); each wake arms it and disarms
 /// it again, so it's only ever executable while a core is climbing.
@@ -101,6 +109,11 @@ pub fn startAp(apic_id: u32, stack_top: usize, percpu: usize, index: usize, cr3:
     arm();
     defer disarm();
 
+    if (fail_next_wakes > 0) { // test hook: simulate a core missing this attempt
+        fail_next_wakes -= 1;
+        return false;
+    }
+
     boot_index = index;
     param("ap_tramp_cr3").* = cr3;
     param("ap_tramp_stack").* = stack_top;
diff --git a/src/kernel/main.zig b/src/kernel/main.zig
index ddd3c7d..bdf89fe 100644
--- a/src/kernel/main.zig
+++ b/src/kernel/main.zig
@@ -269,6 +269,12 @@ fn bringUpSecondaries() void {
     arch.setTrampolinePage(ap_trampoline_page);
     arch.setSecondaryEntry(scheduler.secondaryMain); // where a woken core joins the run loop
 
+    // Test hook: the smp-retry case forces the first wake to fail, so the retry below
+    // must still bring every core online. Inert in a normal build (test_case is null).
+    if (build_options.test_case) |tc| {
+        if (std.mem.eql(u8, tc, "smp-retry")) arch.testFailNextWakes(1);
+    }
+
     log.print("\ndanos: bringing up {d} application processor(s)\n", .{cores.len - 1});
     const max_wake_attempts = 3; // a core that misses the first INIT-SIPI-SIPI gets retried
     for (cores[1..], 1..) |core, index| {
diff --git a/src/kernel/tests.zig b/src/kernel/tests.zig
index 3a198ab..d42f7bf 100644
--- a/src/kernel/tests.zig
+++ b/src/kernel/tests.zig
@@ -72,6 +72,8 @@ pub fn run(case: []const u8, boot_info: *const BootInfo) void {
         smpTest();
     } else if (eql(case, "smp-stress")) {
         stressTest();
+    } else if (eql(case, "smp-retry")) {
+        smpRetryTest();
     } else if (eql(case, "fault-ud")) {
         faultInvalidOpcode();
     } else if (eql(case, "fault-pf")) {
@@ -570,6 +572,20 @@ fn stressTest() void {
     result();
 }
 
+/// Retry: `main` forced the first AP wake attempt to fail (arch.testFailNextWakes),
+/// so a core missed its first INIT-SIPI-SIPI. The boot retry must have brought it back
+/// anyway — every enumerated core should be online. If retry were broken, that core
+/// would be parked and the count would fall short.
+fn smpRetryTest() void {
+    log("DANOS-TEST-BEGIN: smp-retry\n", .{});
+    const total = platform.cpus().len;
+    const online = sched.onlineCount();
+    log("DANOS-RETRY: {d}/{d} cores online after a forced first-wake failure\n", .{ online, total });
+    check("multiple cores enumerated (run with -smp)", total >= 2);
+    check("retry brought every core online despite a failed first wake", online == total);
+    result();
+}
+
 fn faultInvalidOpcode() void {
     log("DANOS-TEST-BEGIN: fault-ud\n", .{});
     asm volatile ("ud2");
diff --git a/test/qemu_test.py b/test/qemu_test.py
index b19eaf6..7dda43e 100644
--- a/test/qemu_test.py
+++ b/test/qemu_test.py
@@ -119,6 +119,11 @@ CASES = [
      "timeout": 90,
      "expect": r"DANOS-TEST-RESULT: PASS",
      "fail":   r"DANOS-TEST-RESULT: FAIL"},
+    # Retry: a forced first-wake failure must still bring every core online.
+    {"name": "smp-retry",
+     "smp":    4,
+     "expect": r"DANOS-TEST-RESULT: PASS",
+     "fail":   r"DANOS-TEST-RESULT: FAIL"},
     {"name": "fault-ud", "expect": r"invalid opcode \(vector 6\)"},
     {"name": "fault-pf", "expect": r"page fault \(vector 14\)"},
     {"name": "fault-df", "expect": r"double fault \(vector 8\)"},