wip: S3 sleep debugging

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));
+                Io::Out16(mask, (uint16_t)(g_pm1aEventBlock + enableOffset));
-                en |= mask;
-                Io::Out16(en, (uint16_t)(g_pm1aEventBlock + enableOffset));
             }
             if (g_pm1bEventBlock != 0) {
-                uint16_t en = Io::In16((uint16_t)(g_pm1bEventBlock + enableOffset));
+                Io::Out16(mask, (uint16_t)(g_pm1bEventBlock + enableOffset));
-                en |= mask;
-                Io::Out16(en, (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);
+                AcpiSleep::PM1_PWRBTN_STS | AcpiSleep::PM1_SLPBTN_STS |
-            EnablePM1Events(AcpiSleep::PM1_PWRBTN_EN);
+                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);
         }
 
     };

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
+; On resume we rebuild the caller-visible stack frame and return to the
-; with RAX=0, so Suspend() knows it's a resume.
+; 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
+; Restores the saved machine state, reconstructs the original Suspend()
-; with RAX=0 to indicate "resumed from S3".
+; 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
+    ; Reconstruct the original return address. The suspend path ran many more
-    mov rdi, [rdi + 0x28]
+    ; 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
+    ; Instead of re-entering Suspend() directly from assembly, jump to a
-    ; compiler stack frame assumptions), jump directly to a dedicated C
+    ; dedicated C resume function which rebuilds the rest of the runtime
-    ; resume function. This is how Linux and other kernels handle S3 resume.
+    ; environment and then returns to the original caller via the RIP we just
+    ; pushed.
     extern AcpiResumeEntry
     jmp AcpiResumeEntry

kernel/src/Api/Power.hpp

@@ -8,6 +8,8 @@
 #include <cstdint>
 #include <Efi/UEFI.hpp>
 #include <Memory/Paging.hpp>
+#include <Io/IoPort.hpp>
+#include <Graphics/Cursor.hpp>
 #include <ACPI/AcpiShutdown.hpp>
 #include <ACPI/AcpiSleep.hpp>
 

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)";
     }