clang-tidy bug-class cleanup + VirtualBox bring-up & firmware-robustness hardening

kernel/acpi/acpi.cpp

@@ -33,6 +33,7 @@
 #include "core/panic.h"
 #include "mm/multiboot2.h"
 #include "mm/page.h"
+#include "mm/paging.h"
 #include "acpi/aml.h"
 #include "acpi/srat.h"
 #include "acpi/acpi_rust/include/acpi_rust.h"
@@ -365,13 +366,61 @@ const Rsdp* FindRsdpInMultiboot(uptr info_phys)
     return new_rsdp != nullptr ? new_rsdp : old_rsdp;
 }
 
+// Map `len` bytes of ACPI physical memory and return a readable virtual
+// pointer. ACPI tables can live anywhere in physical RAM: QEMU/OVMF parks
+// them low (inside the 1 GiB direct map) so the fast PhysToVirt path is
+// used; VirtualBox places the XSDT near the top of 2 GiB RAM, outside the
+// direct map, so we fall back to an MMIO mapping. Mappings are cached by
+// physical base (a handful of distinct ACPI pages) so the repeated XSDT
+// walks across the ~6 FindTable calls don't exhaust the MMIO arena, and
+// kept for the kernel's lifetime (matching the prior PhysToVirt-forever
+// assumption — the DSDT/SSDT scanners reuse these addresses post-boot).
+struct AcpiMapEntry
+{
+    u64 phys;
+    u64 len;
+    void* virt;
+};
+constinit AcpiMapEntry g_acpi_maps[24] = {};
+constinit u64 g_acpi_map_count = 0;
+
+void* AcpiMapPhys(u64 phys, u64 len)
+{
+    if (len == 0)
+    {
+        len = 1;
+    }
+    if (phys + len <= mm::kDirectMapBytes)
+    {
+        return mm::PhysToVirt(phys);
+    }
+    for (u64 i = 0; i < g_acpi_map_count; ++i)
+    {
+        if (g_acpi_maps[i].phys == phys && g_acpi_maps[i].len >= len)
+        {
+            return g_acpi_maps[i].virt;
+        }
+    }
+    void* v = mm::MapMmio(phys, len);
+    if (v == nullptr)
+    {
+        PanicAcpi("ACPI table mapping failed (MMIO arena exhausted)");
+    }
+    if (g_acpi_map_count < 24)
+    {
+        g_acpi_maps[g_acpi_map_count++] = AcpiMapEntry{phys, len, v};
+    }
+    return v;
+}
+
 const SdtHeader* PhysToHeader(u64 phys)
 {
-    // All ACPI tables live below 1 GiB on the machines we target today
-    // (see scope note in acpi.h). PhysToVirt panics if that assumption
-    // breaks, which is the diagnostic we want — silent corruption is
-    // worse than a clear "ACPI table out of direct-map range".
-    return static_cast<const SdtHeader*>(mm::PhysToVirt(phys));
+    // Read the fixed 36-byte header first to learn the table length,
+    // then ensure the whole table is mapped. AcpiMapPhys picks the
+    // direct map or an MMIO fallback depending on where the firmware
+    // placed the table.
+    const auto* probe = static_cast<const SdtHeader*>(AcpiMapPhys(phys, sizeof(SdtHeader)));
+    return static_cast<const SdtHeader*>(AcpiMapPhys(phys, probe->length));
 }
 
 // XSDT entries are 8-byte physical pointers stored right after the
@@ -395,8 +444,13 @@ inline u64 XsdtEntryAt(const SdtHeader* xsdt, u64 i)
 
 const SdtHeader* FindTable(const Rsdp& rsdp, const char* sig4)
 {
-    // Prefer XSDT (64-bit entry pointers) on ACPI 2.0+ firmware. Fall back
-    // to RSDT (32-bit pointers) on ACPI 1.0 or when no XSDT is present.
+    // Prefer the XSDT (64-bit entry pointers) on ACPI 2.0+ firmware,
+    // then fall back to the RSDT (32-bit pointers) — used on ACPI 1.0,
+    // when no XSDT is present, OR when the XSDT is present but does not
+    // list the requested table. The last case is real: VirtualBox ships
+    // an incomplete XSDT (only FADT + SSDT) and lists the MADT and the
+    // rest only in the legacy RSDT. The spec says the two tables should
+    // agree; firmware in the wild does not always honour that.
     if (rsdp.revision >= 2 && rsdp.xsdt_address != 0)
     {
         const auto* xsdt = PhysToHeader(rsdp.xsdt_address);
@@ -423,9 +477,16 @@ const SdtHeader* FindTable(const Rsdp& rsdp, const char* sig4)
                 return h;
             }
         }
-        return nullptr;
+        // Not found in the XSDT. Do NOT give up here — fall through to
+        // the RSDT scan below (incomplete-XSDT firmware, see header
+        // comment). A genuinely-absent table is reported by returning
+        // nullptr only after both roots have been searched.
     }
 
+    if (rsdp.rsdt_address == 0)
+    {
+        return nullptr;
+    }
     const auto* rsdt = PhysToHeader(rsdp.rsdt_address);
     if (!BytesEqual(rsdt->signature, "RSDT", 4))
     {
@@ -450,6 +511,62 @@ const SdtHeader* FindTable(const Rsdp& rsdp, const char* sig4)
     return nullptr;
 }
 
+// One-shot boot diagnostic: dump the RSDP + root system table + every
+// entry's physical address and 4-char signature. WARN-level so it lands
+// in a serial capture by default. Kept (gated by the once-at-boot call
+// site) because non-QEMU firmware — VirtualBox, real UEFI — lays the
+// ACPI tables out differently than the QEMU/OVMF path the parser was
+// written against, and this is the cheapest way to see that layout when
+// a table lookup fails on a machine we can't introspect any other way.
+void AcpiDiagDumpRoot(const char* tag, u64 root_phys, bool entries_are_64bit)
+{
+    if (root_phys == 0)
+    {
+        KLOG_WARN_S("acpi", "diag root absent", "which", tag);
+        return;
+    }
+    const auto* root = PhysToHeader(root_phys);
+    char rsig[5] = {root->signature[0], root->signature[1], root->signature[2], root->signature[3], 0};
+    KLOG_WARN_S("acpi", "diag root which", "which", tag);
+    KLOG_WARN_S("acpi", "diag root signature", "sig", rsig);
+    KLOG_WARN_2V("acpi", "diag root phys/length", "phys", root_phys, "length", root->length);
+
+    const u64 esz = entries_are_64bit ? sizeof(u64) : sizeof(u32);
+    const u64 count = (root->length >= sizeof(SdtHeader)) ? (root->length - sizeof(SdtHeader)) / esz : 0;
+    KLOG_WARN_V("acpi", "diag root entry count", count);
+    for (u64 i = 0; i < count; ++i)
+    {
+        u64 ep = 0;
+        if (entries_are_64bit)
+        {
+            ep = XsdtEntryAt(root, i);
+        }
+        else
+        {
+            const auto* e32 = reinterpret_cast<const u32*>(reinterpret_cast<uptr>(root) + sizeof(SdtHeader));
+            ep = e32[i];
+        }
+        const auto* th = PhysToHeader(ep);
+        char s[5] = {th->signature[0], th->signature[1], th->signature[2], th->signature[3], 0};
+        KLOG_WARN_2V("acpi", "diag entry", "idx", i, "phys", ep);
+        KLOG_WARN_S("acpi", "diag entry signature", "sig", s);
+    }
+}
+
+void AcpiDiagDumpTables(const Rsdp& rsdp)
+{
+    KLOG_WARN_2V("acpi", "diag RSDP", "revision", rsdp.revision, "rsdt_address", rsdp.rsdt_address);
+    KLOG_WARN_V("acpi", "diag RSDP xsdt_address", rsdp.xsdt_address);
+    // Dump BOTH roots — VirtualBox ships an incomplete XSDT and the
+    // MADT may live only in the RSDT (or vice versa), so we need to
+    // see exactly what each one lists.
+    if (rsdp.revision >= 2 && rsdp.xsdt_address != 0)
+    {
+        AcpiDiagDumpRoot("XSDT", rsdp.xsdt_address, /*entries_are_64bit=*/true);
+    }
+    AcpiDiagDumpRoot("RSDT", static_cast<u64>(rsdp.rsdt_address), /*entries_are_64bit=*/false);
+}
+
 void ParseMadt(const Madt& madt)
 {
     g_lapic_address = madt.local_apic_addr;
@@ -582,7 +699,7 @@ void ParseFadt(const Fadt& fadt)
     if (fadt.dsdt != 0)
     {
         g_dsdt_address = fadt.dsdt;
-        const auto* dsdt_hdr = static_cast<const SdtHeader*>(mm::PhysToVirt(fadt.dsdt));
+        const auto* dsdt_hdr = PhysToHeader(fadt.dsdt);
         if (dsdt_hdr != nullptr)
         {
             g_dsdt_length = dsdt_hdr->length;
@@ -707,6 +824,17 @@ void ParseMcfg(const McfgTable& mcfg)
 
 } // namespace
 
+// Shared ACPI physical→virtual mapper. Thin named wrapper around the
+// file-local AcpiMapPhys so other ACPI TUs (aml.cpp) resolve table
+// addresses through the same direct-map / MapMmio fallback + cache
+// instead of calling mm::PhysToVirt directly (which panics for the
+// >1 GiB tables VirtualBox/real-UEFI firmware hands us). One source of
+// truth for ACPI table mapping.
+const void* AcpiMapTable(u64 phys, u64 len)
+{
+    return AcpiMapPhys(phys, len);
+}
+
 void AcpiInit(uptr multiboot_info_phys)
 {
     KLOG_TRACE_SCOPE("acpi", "AcpiInit");
@@ -730,6 +858,8 @@ void AcpiInit(uptr multiboot_info_phys)
         }
     }
 
+    AcpiDiagDumpTables(*rsdp);
+
     const SdtHeader* madt_hdr = FindTable(*rsdp, "APIC");
     if (madt_hdr == nullptr)
     {
@@ -1104,13 +1234,13 @@ bool AmlContainsName(const char* name4)
         return false;
     if (g_dsdt_address != 0 && g_dsdt_length > 0)
     {
-        const auto* buf = static_cast<const u8*>(mm::PhysToVirt(g_dsdt_address));
+        const auto* buf = static_cast<const u8*>(AcpiMapPhys(g_dsdt_address, g_dsdt_length));
         if (buf != nullptr && ContainsName4(buf, g_dsdt_length, name4))
             return true;
     }
     for (u64 i = 0; i < g_ssdt_count; ++i)
     {
-        const auto* buf = static_cast<const u8*>(mm::PhysToVirt(g_ssdt_address[i]));
+        const auto* buf = static_cast<const u8*>(AcpiMapPhys(g_ssdt_address[i], g_ssdt_length[i]));
         if (buf != nullptr && ContainsName4(buf, g_ssdt_length[i], name4))
             return true;
     }

kernel/acpi/acpi.h

@@ -16,10 +16,11 @@
  *     14 or 15. No EBDA / low-1MiB fallback scan — GRUB always hands it
  *     over, and anything booted via a loader that doesn't is a config
  *     bug, not a runtime recoverable one.
- *   - Assumes every ACPI table lives in the first 1 GiB of physical
- *     RAM (reachable via the boot direct map). Panics otherwise. The
- *     fix is to MapMmio the out-of-range range; deferred until a real
- *     machine makes us care.
+ *   - ACPI tables below the 1 GiB direct map (QEMU/OVMF) are read
+ *     directly via PhysToVirt; tables the firmware parks higher
+ *     (VirtualBox puts the XSDT near the top of 2 GiB RAM) are reached
+ *     through a cached MapMmio fallback in AcpiMapPhys(). Mappings are
+ *     kept for the kernel lifetime (the DSDT/SSDT scanners reuse them).
  *   - FADT parsing is minimal — only RESET_REG + RESET_VALUE + SCI_INT
  *     are cached. The rest (PM1a/PM1b event/control blocks, PM timer,
  *     GPE blocks, preferred CPU C-state hints) lands when a consumer
@@ -70,6 +71,14 @@ struct InterruptOverride
 /// lying about something critical.
 void AcpiInit(uptr multiboot_info_phys);
 
+/// Map `len` bytes of an ACPI table's physical memory and return a
+/// readable virtual pointer. Uses the kernel direct map when the table
+/// is below it, an MMIO mapping (cached, kept for kernel lifetime)
+/// otherwise — firmware (VirtualBox, real UEFI) frequently parks ACPI
+/// tables above the 1 GiB direct map. Every ACPI TU must resolve table
+/// addresses through this, never mm::PhysToVirt directly.
+const void* AcpiMapTable(u64 phys, u64 len);
+
 /// LAPIC base physical address from the MADT header. Typically
 /// 0xFEE00000 but firmware can relocate it. Callers should prefer this
 /// over the IA32_APIC_BASE MSR when the two disagree — the MADT is

kernel/acpi/aml.cpp

@@ -705,7 +705,7 @@ void AmlNamespaceBuild()
     const u32 dsdt_len = DsdtLength();
     if (dsdt_phys != 0 && dsdt_len >= 36)
     {
-        const auto* sdt = static_cast<const u8*>(mm::PhysToVirt(dsdt_phys));
+        const auto* sdt = static_cast<const u8*>(AcpiMapTable(dsdt_phys, dsdt_len));
         if (sdt != nullptr)
             WalkTable(sdt, dsdt_len, /*source_idx=*/0);
     }
@@ -716,7 +716,7 @@ void AmlNamespaceBuild()
         const u32 len = SsdtLength(i);
         if (phys == 0 || len < 36)
             continue;
-        const auto* sdt = static_cast<const u8*>(mm::PhysToVirt(phys));
+        const auto* sdt = static_cast<const u8*>(AcpiMapTable(phys, len));
         if (sdt != nullptr)
             WalkTable(sdt, len, u8(i + 1));
     }
@@ -842,7 +842,7 @@ bool AmlReadS5(u8* slp_typa, u8* slp_typb)
         // table rather than underflowing the subtraction below.
         if (dsdt_len < 36)
             return false;
-        const auto* hdr = static_cast<const u8*>(mm::PhysToVirt(dsdt_phys));
+        const auto* hdr = static_cast<const u8*>(AcpiMapTable(dsdt_phys, dsdt_len));
         aml = hdr + 36; // skip SdtHeader
         aml_len = dsdt_len - 36;
     }
@@ -857,7 +857,7 @@ bool AmlReadS5(u8* slp_typa, u8* slp_typb)
         const u32 ssdt_len = SsdtLength(idx);
         if (ssdt_len < 36)
             return false;
-        const auto* hdr = static_cast<const u8*>(mm::PhysToVirt(ssdt_phys));
+        const auto* hdr = static_cast<const u8*>(AcpiMapTable(ssdt_phys, ssdt_len));
         aml = hdr + 36;
         aml_len = ssdt_len - 36;
     }

kernel/arch/x86_64/boot.S

@@ -48,20 +48,22 @@ multiboot2_header_start:
     .long   multiboot2_header_end - multiboot2_header_start
     .long   -(MULTIBOOT2_MAGIC + MULTIBOOT2_ARCH_I386 + (multiboot2_header_end - multiboot2_header_start))
 
-    /* Framebuffer request tag (type 5). width=height=depth=0 means
-     * "give me any linear framebuffer the firmware has" — GRUB picks
-     * a sensible default (1024x768x32 on QEMU std-vga, 800x600 on
-     * VESA fallback). Flags bit 0 = 0 → optional: if no mode is
-     * available, GRUB still boots us with EGA text. The kernel-side
-     * framebuffer driver treats "no tag 8" as "no graphics; keep
-     * using serial" rather than a hard failure. */
+    /* Framebuffer request tag (type 5). Request a concrete, universally
+     * supported mode (1024x768x32) rather than 0/0/0 "any": with "any",
+     * GRUB lands at 1024x768 on QEMU std-vga but at the lowest VBE mode
+     * (640x480) under VirtualBox, which renders the desktop unusably
+     * oversized. 1024x768x32 is in every VBE/VBoxVGA mode list and fits
+     * default VBox VRAM (3 MiB). Flags bit 0 = 1 → optional: if the mode
+     * is unavailable GRUB still boots us (EGA text / serial); the
+     * kernel-side framebuffer driver treats "no tag 8" as "no graphics,
+     * keep using serial" rather than a hard failure. */
     .align 8
     .word   5                   /* type = MULTIBOOT_HEADER_TAG_FRAMEBUFFER */
     .word   1                   /* flags = 1 → optional */
     .long   20                  /* size */
-    .long   0                   /* width  — any  */
-    .long   0                   /* height — any  */
-    .long   0                   /* depth  — any  */
+    .long   1024                /* width  */
+    .long   768                 /* height */
+    .long   32                  /* depth  */
 
     /* End tag */
     .align 8

kernel/arch/x86_64/ioapic.cpp

@@ -201,6 +201,18 @@ void IoApicRoute(u32 gsi, u8 vector, u8 lapic_id, u8 isa_irq)
     const u32 entry = gsi - io->gsi_base;
     WriteRedir(*io, entry, static_cast<u64>(kRedirLowMask));
     WriteRedir(*io, entry, value);
+
+    // Diagnostic (Debug-gated, kept per the keep-it/gate-it discipline):
+    // read the entry back so a debug-build serial capture proves whether
+    // the IOAPIC actually latched the route — some emulated IOAPICs
+    // silently drop writes to certain entries. gsi/vector identify the
+    // device (GSI1 = PS/2 kbd, GSI12 = PS/2 mouse); bit 16 of the low
+    // dword set in the read-back means still masked = no IRQ will fire.
+    const u64 readback = ReadRedir(*io, entry);
+    core::LogWithValue(core::LogLevel::Debug, "arch/ioapic", "route gsi", gsi);
+    core::LogWithValue(core::LogLevel::Debug, "arch/ioapic", "  vector", vector);
+    core::LogWithValue(core::LogLevel::Debug, "arch/ioapic", "  wrote", value);
+    core::LogWithValue(core::LogLevel::Debug, "arch/ioapic", "  readback", readback);
 }
 
 void IoApicMask(u32 gsi)

kernel/core/main.cpp

@@ -393,8 +393,10 @@ extern "C" void kernel_main(duetos::u32 multiboot_magic, duetos::uptr multiboot_
     const char* cmdline = duetos::core::FindBootCmdline(multiboot_info);
 
     duetos::core::BootBringupKernelServices(cmdline, multiboot_info);
+    SerialWrite("[bringup-tail] kernel-services done\n");
 
     duetos::core::BootBringupDevices(CmdlineMatches(cmdline, "netsmoke", "force"));
+    SerialWrite("[bringup-tail] devices done\n");
 
     // Keyboard reader thread: consumes KeyEvents and writes the
     // printable ones into the framebuffer console. Backspace and

kernel/drivers/input/ps2kbd.cpp

@@ -77,6 +77,11 @@ constexpr u8 kResponseTestPort1Pass = 0x00;
 constexpr u8 kConfigPort1IrqEnable = 1U << 0;
 constexpr u8 kConfigPort2IrqEnable = 1U << 1;
 constexpr u8 kConfigPort1ClockDisable = 1U << 4;
+// Bit 6: first-port scancode translation (8042 rewrites the keyboard's
+// Set 2 codes into Set 1 for the host). We put the keyboard explicitly
+// into Set 1 (step 8), so this MUST be off — with it on, the controller
+// runs Set-1 codes through the Set-2→Set-1 table and mangles every key.
+constexpr u8 kConfigPort1Translation = 1U << 6;
 
 // Bounded spin count for controller-response polling. 1M reads is
 // ~tens of milliseconds on a modern CPU — well past any legitimate
@@ -488,11 +493,14 @@ void ControllerInit()
     Drain();
 
     // Step 3: pull the current config byte, turn OFF both IRQ enables
-    // (we'll re-enable port 1 last), and leave translation whatever
-    // firmware set it to — our scan-code translator expects set 1 +
-    // translation on, which is the PC-AT default every BIOS honours.
+    // (we'll re-enable port 1 last), and explicitly turn OFF first-port
+    // translation. Step 8 forces the keyboard into Set 1, so the 8042
+    // must NOT also run a Set-2→Set-1 translation pass or every key is
+    // mangled. SeaBIOS/OVMF happen to leave translation off; VirtualBox
+    // firmware leaves it on, which previously wrecked the keymap — so
+    // make it deterministic instead of inheriting the firmware default.
     u8 config = ReadConfigByte();
-    config = static_cast<u8>(config & ~(kConfigPort1IrqEnable | kConfigPort2IrqEnable));
+    config = static_cast<u8>(config & ~(kConfigPort1IrqEnable | kConfigPort2IrqEnable | kConfigPort1Translation));
     WriteConfigByte(config);
 
     // Step 4: controller self-test. Some buggy firmware resets the