From c731f718e93aaf7cf9f1d58c763625ab11ae8cef Mon Sep 17 00:00:00 2001 From: Daniel Hammer Date: Sun, 24 May 2026 14:25:13 +0200 Subject: [PATCH] fix: fix login screen hang when no USB input device plugged in --- kernel/src/Drivers/Input/InputEvents.cpp | 1 - kernel/src/Timekeeping/ApicTimer.cpp | 119 +++++------------------ kernel/src/Timekeeping/ApicTimer.hpp | 10 +- 3 files changed, 25 insertions(+), 105 deletions(-) diff --git a/kernel/src/Drivers/Input/InputEvents.cpp b/kernel/src/Drivers/Input/InputEvents.cpp index 97f91d1..0f7be14 100644 --- a/kernel/src/Drivers/Input/InputEvents.cpp +++ b/kernel/src/Drivers/Input/InputEvents.cpp @@ -29,7 +29,6 @@ namespace Drivers::InputEvents { } void NotifyActivity() { - Timekeeping::NoteInteractiveActivity(); g_serial.fetch_add(1, std::memory_order_release); Sched::WakeObjectWaiters(&g_waitObject); } diff --git a/kernel/src/Timekeeping/ApicTimer.cpp b/kernel/src/Timekeeping/ApicTimer.cpp index fc153c3..70562df 100644 --- a/kernel/src/Timekeeping/ApicTimer.cpp +++ b/kernel/src/Timekeeping/ApicTimer.cpp @@ -41,20 +41,12 @@ namespace Timekeeping { static constexpr uint32_t BSP_TICK_INTERVAL_MS = 1; static constexpr uint32_t BSP_TIMER_HZ = 1000 / BSP_TICK_INTERVAL_MS; static constexpr uint32_t AP_TICK_INTERVAL_MS = 10; - // Keep a bounded periodic wake while otherwise tickless so wall-clock - // accounting and typematic repeat still advance even on a fully idle BSP. - static constexpr uint32_t BSP_IDLE_MAX_INTERVAL_MS = 50; - static constexpr uint32_t BSP_INTERACTIVE_IDLE_MAX_INTERVAL_MS = 1; // Global state static std::atomic g_tickCount{0}; static uint32_t g_ticksPerMs = 0; static bool g_schedEnabled = false; - static volatile bool g_bspIdleOneShotArmed = false; - static uint32_t g_bspIdleOneShotMs = 0; - static uint32_t g_bspIdleInitialCount = 0; - static std::atomic g_interactiveUntilMs{0}; static uint32_t CountForIntervalMs(uint32_t intervalMs) { return g_ticksPerMs * intervalMs; @@ -76,30 +68,13 @@ namespace Timekeeping { ProgramTimer(true, BSP_TICK_INTERVAL_MS); } - static void ProgramBspIdleOneShotTimer(uint32_t intervalMs) { - g_bspIdleOneShotMs = intervalMs; - g_bspIdleInitialCount = CountForIntervalMs(intervalMs); - g_bspIdleOneShotArmed = true; - - Hal::LocalApic::WriteRegister(Hal::LocalApic::REG_TIMER_DIVIDE, DIVIDE_BY_16); - Hal::LocalApic::WriteRegister(Hal::LocalApic::REG_TIMER_LVT, - (Hal::IRQ_VECTOR_BASE + Hal::IRQ_TIMER)); - Hal::LocalApic::WriteRegister(Hal::LocalApic::REG_TIMER_INITIAL, g_bspIdleInitialCount); - } - - // Timer IRQ handler: BSP handles timekeeping and the few timer-driven - // fallbacks that are still required; APs only run scheduler accounting. + // Timer IRQ handler: BSP runs timekeeping + scheduler accounting; APs + // run scheduler accounting only. static void TimerHandler(uint8_t) { auto* cpu = Smp::GetCurrentCpuData(); uint32_t schedElapsedMs = (cpu->cpuIndex == 0) ? BSP_TICK_INTERVAL_MS : AP_TICK_INTERVAL_MS; if (cpu->cpuIndex == 0) { - if (g_bspIdleOneShotArmed) { - schedElapsedMs = g_bspIdleOneShotMs; - g_bspIdleOneShotArmed = false; - ProgramBspPeriodicTimer(); - } - g_tickCount.fetch_add(schedElapsedMs, std::memory_order_relaxed); if (Drivers::Net::E1000E::RequiresPolling()) { @@ -198,7 +173,6 @@ namespace Timekeeping { // Reprogram the APIC timer registers (they were lost during S3). // The calibrated g_ticksPerMs value is still valid (it's in RAM). // The IRQ handler registration also survives (it's a function pointer array in RAM). - g_bspIdleOneShotArmed = false; ProgramBspPeriodicTimer(); KernelLogStream(OK, "Timer") << "APIC timer restarted after S3 resume"; @@ -212,20 +186,6 @@ namespace Timekeeping { return g_tickCount.load(std::memory_order_relaxed); // 1 tick = 1 ms at 1000 Hz } - void NoteInteractiveActivity(uint32_t durationMs) { - uint64_t until = GetMilliseconds() + durationMs; - uint64_t current = g_interactiveUntilMs.load(std::memory_order_relaxed); - - while (until > current && - !g_interactiveUntilMs.compare_exchange_weak( - current, until, std::memory_order_relaxed, std::memory_order_relaxed)) { - } - } - - static bool IsInteractiveActivityActive() { - return GetMilliseconds() < g_interactiveUntilMs.load(std::memory_order_relaxed); - } - void EnableSchedulerTick() { g_schedEnabled = true; } @@ -242,18 +202,20 @@ namespace Timekeeping { } void IdleOnce(bool hasMwait, volatile uint64_t* monitorAddr) { + (void)hasMwait; + (void)monitorAddr; auto* cpu = Smp::GetCurrentCpuData(); // Drain USB hot-plug deferred work from any idle core, not just the BSP. - // ProcessDeferredWork uses an atomic CAS so only one core runs it at a - // time; the rest return immediately. This keeps hot-plug latency bounded - // by *any* core's idle gap instead of being pinned to BSP idle. if (Drivers::USB::Xhci::HasDeferredWork()) { Drivers::USB::Xhci::ProcessDeferredWork(); } if (cpu == nullptr || cpu->cpuIndex != 0 || !g_schedEnabled || g_ticksPerMs == 0) { - Hal::CpuIdle::Wait(BSP_TICK_INTERVAL_MS, hasMwait, monitorAddr); + // Non-BSP or pre-scheduler fallback: plain HLT keeps the LAPIC + // timer running in C1 on hardware where MWAIT promotes to deeper + // states that freeze the timer (no ARAT in practice). + Hal::HaltWithInterruptsDisabled(); return; } @@ -264,57 +226,20 @@ namespace Timekeeping { return; } - bool interactive = IsInteractiveActivityActive(); - uint32_t waitMs = interactive - ? BSP_INTERACTIVE_IDLE_MAX_INTERVAL_MS - : BSP_IDLE_MAX_INTERVAL_MS; - - uint64_t now = GetTicks(); - uint64_t nextDeadline = Sched::GetNextDeadlineTick(); - uint64_t keyboardDeadline = Drivers::USB::HidKeyboard::GetNextTickDeadline(); - if (keyboardDeadline != 0 && - (nextDeadline == 0 || keyboardDeadline < nextDeadline)) { - nextDeadline = keyboardDeadline; - } - - if (nextDeadline != 0) { - if (nextDeadline <= now) { - waitMs = BSP_TICK_INTERVAL_MS; - } else { - uint64_t untilDeadline = nextDeadline - now; - if (untilDeadline < waitMs) { - waitMs = (uint32_t)untilDeadline; - } - } - } - - if (waitMs == 0) { - waitMs = BSP_TICK_INTERVAL_MS; - } - - asm volatile("cli" ::: "memory"); - ProgramBspIdleOneShotTimer(waitMs); - if (interactive) { - Hal::CpuIdle::WaitShallowWithInterruptsDisabled(hasMwait, monitorAddr); - } else { - Hal::CpuIdle::WaitWithInterruptsDisabled(waitMs, hasMwait, monitorAddr); - } - - if (g_bspIdleOneShotArmed) { - uint32_t currentCount = Hal::LocalApic::ReadRegister(Hal::LocalApic::REG_TIMER_CURRENT); - uint32_t elapsedTicks = (g_bspIdleInitialCount > currentCount) - ? (g_bspIdleInitialCount - currentCount) - : 0; - uint32_t elapsedMs = (g_ticksPerMs > 0) ? (elapsedTicks / g_ticksPerMs) : 0; - if (elapsedMs > 0) { - g_tickCount.fetch_add(elapsedMs, std::memory_order_relaxed); - } - - g_bspIdleOneShotArmed = false; - Hal::LocalApic::WriteRegister(Hal::LocalApic::REG_TIMER_INITIAL, 0); - ProgramBspPeriodicTimer(); - } - asm volatile("sti" ::: "memory"); + // We deliberately DO NOT reprogram the LAPIC timer to a one-shot here. + // Some laptop CPUs let MWAIT/HLT drop into a deep C-state where the + // LAPIC timer stops firing, even though CPUID.06H:EAX[ARAT] claims + // it doesn't. Symptom: the BSP enters idle and never wakes from its + // timer; only external IRQs (PS/2, USB MSI) bring it out, and even + // those don't advance g_tickCount because the one-shot's current + // count is frozen. By leaving the boot-time periodic 1ms timer in + // place, TimerHandler runs every tick and g_tickCount keeps moving + // -- which is what RunBspMaintenance needs to wake sleeping procs. + // + // HLT (vs MWAIT) is the second half: MWAIT EAX=0 is a *hint* and + // the CPU may go deeper than C1; HLT generally stays in C1, which + // is the deepest state guaranteed to keep the LAPIC timer alive. + Hal::HaltWithInterruptsDisabled(); } void Sleep(uint64_t ms) { diff --git a/kernel/src/Timekeeping/ApicTimer.hpp b/kernel/src/Timekeeping/ApicTimer.hpp index 783984c..cbc246c 100644 --- a/kernel/src/Timekeeping/ApicTimer.hpp +++ b/kernel/src/Timekeeping/ApicTimer.hpp @@ -28,15 +28,11 @@ namespace Timekeeping { // Enable scheduler tick (called after scheduler is initialized) void EnableSchedulerTick(); - // Enter one idle wait cycle for the current CPU. The BSP uses a - // one-shot LAPIC timer while idle; APs keep their existing simple wait. + // Enter one idle wait cycle for the current CPU. Uses HLT and relies on + // the boot-time periodic LAPIC timer to wake -- one-shot wake doesn't + // survive deep C-states on some laptop CPUs. void IdleOnce(bool hasMwait, volatile uint64_t* monitorAddr = nullptr); - // Mark recent latency-sensitive input activity. Idle policy uses this to - // stay in shallow, high-responsiveness mode briefly while the user is - // moving the pointer or dragging windows. - void NoteInteractiveActivity(uint32_t durationMs = 250); - // Busy-wait sleep for the given number of milliseconds void Sleep(uint64_t ms); };