diff --git a/kernel/src/ACPI/AcpiEvents.cpp b/kernel/src/ACPI/AcpiEvents.cpp index bd69f3c..edd3cf8 100644 --- a/kernel/src/ACPI/AcpiEvents.cpp +++ b/kernel/src/ACPI/AcpiEvents.cpp @@ -47,19 +47,15 @@ namespace Hal { Io::Out16(bits, (uint16_t)g_pm1bEventBlock); } - static void EnablePM1Events(uint16_t mask) { + static void SetPM1Events(uint16_t mask) { uint16_t enableOffset = g_pm1EventLength / 2; if (enableOffset == 0) return; if (g_pm1aEventBlock != 0) { - uint16_t en = Io::In16((uint16_t)(g_pm1aEventBlock + enableOffset)); - en |= mask; - Io::Out16(en, (uint16_t)(g_pm1aEventBlock + enableOffset)); + Io::Out16(mask, (uint16_t)(g_pm1aEventBlock + enableOffset)); } if (g_pm1bEventBlock != 0) { - uint16_t en = Io::In16((uint16_t)(g_pm1bEventBlock + enableOffset)); - en |= mask; - Io::Out16(en, (uint16_t)(g_pm1bEventBlock + enableOffset)); + Io::Out16(mask, (uint16_t)(g_pm1bEventBlock + enableOffset)); } } @@ -112,7 +108,7 @@ namespace Hal { AcpiSleep::PM1_BM_STS); // Enable power button event - EnablePM1Events(AcpiSleep::PM1_PWRBTN_EN); + SetPM1Events(AcpiSleep::PM1_PWRBTN_EN); // Route SCI to an IRQ vector. The SCI is level-triggered, // active-low (ACPI spec requirement). The MADT may have an @@ -137,10 +133,18 @@ namespace Hal { void Reinitialize() { if (!g_initialized) return; - // Clear pending status and re-enable power button event + // Resume may leave WAK/RTC/GBL bits pending. Because the SCI is + // level-triggered, re-enabling interrupts without clearing them can + // immediately retrigger the SCI in a tight loop. ClearPM1StatusBits( - AcpiSleep::PM1_PWRBTN_STS | AcpiSleep::PM1_SLPBTN_STS); - EnablePM1Events(AcpiSleep::PM1_PWRBTN_EN); + AcpiSleep::PM1_PWRBTN_STS | AcpiSleep::PM1_SLPBTN_STS | + AcpiSleep::PM1_WAK_STS | AcpiSleep::PM1_TMR_STS | + AcpiSleep::PM1_GBL_STS | AcpiSleep::PM1_RTC_STS | + AcpiSleep::PM1_BM_STS); + + // Drop the suspend-time RTC wake enable and restore only the + // fixed events we actually want during normal runtime. + SetPM1Events(AcpiSleep::PM1_PWRBTN_EN); } }; diff --git a/kernel/src/ACPI/S3Wake.asm b/kernel/src/ACPI/S3Wake.asm index 1917800..865487c 100644 --- a/kernel/src/ACPI/S3Wake.asm +++ b/kernel/src/ACPI/S3Wake.asm @@ -15,11 +15,12 @@ section .text ; ; Saves all CPU registers into the CpuState structure, then returns 1. ; The caller is expected to enter S3 after this returns. -; When the system resumes, AcpiResumeLongMode jumps to the saved RIP -; with RAX=0, so Suspend() knows it's a resume. +; On resume we rebuild the caller-visible stack frame and return to the +; original Suspend() continuation via AcpiResumeEntry(). ; ; Returns: 1 on initial call (proceed to enter S3) -; 0 on resume from S3 +; Resume does not re-enter this function directly; control is +; reconstructed by AcpiResumeLongMode/AcpiResumeEntry. ; ; CpuState layout: ; 0x00 RAX 0x40 R8 0x80 RFLAGS @@ -41,7 +42,12 @@ AcpiSaveAndSuspend: mov [rdi + 0x20], rsi mov [rdi + 0x28], rdi ; save rdi (pointer to state area) mov [rdi + 0x30], rbp - mov [rdi + 0x38], rsp + ; Save the caller's post-return RSP, not our own entry RSP. The raw + ; entry RSP points at this call's temporary return-address slot, which + ; the suspend path will later reuse for other calls before the machine + ; actually enters S3. + lea rax, [rsp + 8] + mov [rdi + 0x38], rax mov [rdi + 0x40], r8 mov [rdi + 0x48], r9 mov [rdi + 0x50], r10 @@ -122,8 +128,8 @@ g_wakeStatePtr: dq 0 ; extern "C" void AcpiResumeLongMode(CpuState* stateArea) ; rdi = pointer to CpuState structure ; -; Restores all saved registers and returns to the original Suspend() caller -; with RAX=0 to indicate "resumed from S3". +; Restores the saved machine state, reconstructs the original Suspend() +; continuation on the stack, and then jumps into the C resume path. ; global AcpiResumeLongMode AcpiResumeLongMode: @@ -139,7 +145,10 @@ AcpiResumeLongMode: ; We must restore the kernel stack FIRST so push/pop work, then reload ; GDT/CS/IDT before any interrupt can fire. - ; Restore kernel RSP immediately so we have a valid stack + ; Restore the caller-visible kernel RSP immediately so we have a valid + ; stack. AcpiSaveAndSuspend saved the post-return value (RSP after the + ; original call had completed), so we will reconstruct the return address + ; explicitly below before entering C. mov rsp, [rdi + 0x38] ; Restore GDT @@ -208,8 +217,13 @@ AcpiResumeLongMode: mov r14, [rdi + 0x70] mov r15, [rdi + 0x78] - ; Restore RFLAGS + ; Restore RFLAGS but keep IF masked until AcpiResumeEntry finishes + ; rebuilding GS base, TSS, APIC, and the rest of the interrupt state. + ; The saved flags usually have IF=1 because Suspend() is entered from a + ; live syscall path; restoring that too early lets an IRQ hit a + ; half-restored kernel. mov rax, [rdi + 0x80] + and rax, ~(1 << 9) push rax popfq @@ -219,11 +233,14 @@ AcpiResumeLongMode: mov al, 0xC4 out 0x71, al - ; Restore rdi last - mov rdi, [rdi + 0x28] + ; Reconstruct the original return address. The suspend path ran many more + ; calls after AcpiSaveAndSuspend returned, so the old call-frame slot on + ; the kernel stack can no longer be trusted. + push qword [rdi + 0x88] - ; Instead of returning to Suspend() via ret (which is fragile due to - ; compiler stack frame assumptions), jump directly to a dedicated C - ; resume function. This is how Linux and other kernels handle S3 resume. + ; Instead of re-entering Suspend() directly from assembly, jump to a + ; dedicated C resume function which rebuilds the rest of the runtime + ; environment and then returns to the original caller via the RIP we just + ; pushed. extern AcpiResumeEntry jmp AcpiResumeEntry diff --git a/kernel/src/Api/Power.hpp b/kernel/src/Api/Power.hpp index de9d3e3..56dad0d 100644 --- a/kernel/src/Api/Power.hpp +++ b/kernel/src/Api/Power.hpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include diff --git a/kernel/src/Hal/GDT.cpp b/kernel/src/Hal/GDT.cpp index 326ed51..a40aba4 100644 --- a/kernel/src/Hal/GDT.cpp +++ b/kernel/src/Hal/GDT.cpp @@ -70,6 +70,11 @@ namespace Hal { } void LoadTSS() { + // LTR marks the TSS descriptor busy in the GDT (type 0xB). S3 loses + // the CPU's loaded task register state, but the busy bit in RAM + // persists, so a plain second LTR on resume can #GP. Re-mark it as an + // available 64-bit TSS before reloading TR. + kernelGDT.TSS.AccessByte = 0x89; LoadTR(); KernelLogStream(OK, "Hal") << "Loaded TSS (selector 0x28)"; }