Skip to content

Commit

Permalink
Handle overflow on 32-bit timers correctly (or the system might hang …
Browse files Browse the repository at this point in the history
…after a while running)
  • Loading branch information
ilmmatias committed Jan 23, 2025
1 parent b4fbb9b commit b12a917
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 25 deletions.
19 changes: 12 additions & 7 deletions src/kernel/ev/clock.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ void EvpHandleClock(HalRegisterState *) {
while (ListHeader != &Processor->EventQueue) {
EvHeader *Header = CONTAINING_RECORD(ListHeader, EvHeader, ListHeader);
ListHeader = ListHeader->Next;
if ((Header->Deadline && CurrentTicks >= Header->Deadline) || Header->Finished) {
if ((Header->DeadlineTicks &&
HalCheckTimerExpiration(
CurrentTicks, Header->DeadlineReference, Header->DeadlineTicks)) ||
Header->Finished) {
TriggerEvent = 1;
}
}
Expand All @@ -34,12 +37,14 @@ void EvpHandleClock(HalRegisterState *) {
TriggerEvent = 1;
}

/* At last, check for anything that would swap out the thread (we probably only really need to
* heck quanta expiration though). */
if (Processor->InitialThread &&
(Processor->CurrentThread->Terminated ||
CurrentTicks >= Processor->CurrentThread->Expiration || Processor->ForceYield)) {
TriggerEvent = 1;
/* At last, check for anything that would swap out the thread. */
if (Processor->InitialThread) {
PsThread *CurrentThread = Processor->CurrentThread;
int QuantaExpired = HalCheckTimerExpiration(
CurrentTicks, CurrentThread->ExpirationReference, CurrentThread->ExpirationTicks);
if (CurrentThread->Terminated || QuantaExpired || Processor->ForceYield) {
TriggerEvent = 1;
}
}

/* Finally, trigger an event at dispatch IRQL if we need to do anything else. */
Expand Down
9 changes: 6 additions & 3 deletions src/kernel/ev/dispatch.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ extern "C" void EvpDispatchObject(void *Object, uint64_t Timeout, int Yield) {
/* Ignore the target timeout for already dispatched objects (we probably just want to yield
this thread out). */
if (!Header->Dispatched) {
Header->Deadline = 0;
Header->DeadlineTicks = 0;
if (Timeout) {
Header->Deadline = CurrentTicks + Timeout / HalGetTimerPeriod();
Header->DeadlineReference = CurrentTicks;
Header->DeadlineTicks = Timeout / HalGetTimerPeriod();
}
}

Expand Down Expand Up @@ -125,7 +126,9 @@ extern "C" void EvpHandleEvents(HalRegisterState *Context) {

/* Out of the deadline, for anything but timers, this will make WaitObject return an
error. */
if (Header->Deadline && CurrentTicks >= Header->Deadline) {
if (Header->DeadlineTicks &&
HalCheckTimerExpiration(
CurrentTicks, Header->DeadlineReference, Header->DeadlineTicks)) {
Header->Finished = 1;
}

Expand Down
4 changes: 2 additions & 2 deletions src/kernel/ev/types.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
*
* PARAMETERS:
* Timer - Pointer to the timer struct.
* Timeout - How many nanoseconds to sleep for; If this is set too low, EvWaitObject(s)
* Dpc - Optional DPC to be executed/enqueued when we sleep.
* Timeout - How many nanoseconds to sleep for.
* Dpc - Optional DPC to be executed/enqueued when we finish.
*
* RETURN VALUE:
* None.
Expand Down
23 changes: 21 additions & 2 deletions src/kernel/hal/amd64/hpet.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

static void *HpetAddress = NULL;
static uint64_t Period = 1;
static int Width = HAL_TIMER_WIDTH_64B;

/*-------------------------------------------------------------------------------------------------
* PURPOSE:
Expand Down Expand Up @@ -74,11 +75,14 @@ void HalpInitializeHpet(void) {
WriteHpetRegister(HPET_TIMER_CAP_REG(i), Reg & ~HPET_TIMER_MASK);
}

Period = (ReadHpetRegister(HPET_CAP_REG) >> 32) / 1000000;
Reg = ReadHpetRegister(HPET_CAP_REG);
Width = (Reg & HPET_CAP_64B) ? HAL_TIMER_WIDTH_64B : HAL_TIMER_WIDTH_32B;
Period = (Reg >> HPET_CAP_FREQ_START) / 1000000;
VidPrint(
VID_MESSAGE_DEBUG,
"Kernel HAL",
"using HPET as timer tick source (period = %llu ns)\n",
"using HPET as timer tick source (%u-bits counter, period = %llu ns)\n",
Width,
Period);

/* At last we can reenable the main counter (after zeroing it). */
Expand All @@ -87,6 +91,21 @@ void HalpInitializeHpet(void) {
WriteHpetRegister(HPET_CFG_REG, (Reg & ~HPET_CFG_MASK) | HPET_CFG_INT_ENABLE);
}

/*-------------------------------------------------------------------------------------------------
* PURPOSE:
* This function returns the width of the timer (useful for handling overflow!).
*
* PARAMETERS:
* None.
*
* RETURN VALUE:
* HAL_TIMER_WIDTH_32B if the timer is 32-bits long, or HAL_TIMER_WIDTH_64B if the timer is
* 64-bits long.
*-----------------------------------------------------------------------------------------------*/
int HalGetTimerWidth(void) {
return Width;
}

/*-------------------------------------------------------------------------------------------------
* PURPOSE:
* This function returns the period (in nanoseconds) of the HAL timer.
Expand Down
36 changes: 34 additions & 2 deletions src/kernel/hal/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,37 @@

#include <hal.h>

/*-------------------------------------------------------------------------------------------------
* PURPOSE:
* This function checks if we already reached or went past the given target timestamp.
* Use this instead of manually checking, as we also handle overflow on 32-bit timers.
*
* PARAMETERS:
* Current - Current value of HalGetTimerTicks().
* Reference - Value of HalGetTimerTicks() in the initial call time (for overflow checks).
* Time - How much timer ticks past the reference we want to wait until expiration.
*
* RETURN VALUE:
* 1 if we reached the target time, 0 otherwise.
*-----------------------------------------------------------------------------------------------*/
int HalCheckTimerExpiration(uint64_t Current, uint64_t Reference, uint64_t Ticks) {
uint64_t Target = Reference + Ticks;

if (HalGetTimerWidth() == HAL_TIMER_WIDTH_64B) {
/* 64-bit timers are easier to handle, we should be able to just not care about overflow. */
return Current >= Target;
}

/* For 32-bit timers, we need to handle overflow; If the target time is still a 32-bit
* number, then any overflow means we reached it; If it's 64-bits, then we need to wait til
* overflow, and then mask off the high bits (and wait again). */
if (Target <= UINT32_MAX) {
return Current < Reference || Current >= Target;
}

return Current < Reference && Current >= ((Target >> 32) << 32);
}

/*-------------------------------------------------------------------------------------------------
* PURPOSE:
* This function does a busy wait loop waiting until the specified amount of nanoseconds
Expand All @@ -15,7 +46,8 @@
* None.
*-----------------------------------------------------------------------------------------------*/
void HalWaitTimer(uint64_t Time) {
uint64_t End = HalGetTimerTicks() + Time / HalGetTimerPeriod();
while (HalGetTimerTicks() < End)
uint64_t Start = HalGetTimerTicks();
uint64_t Ticks = Time / HalGetTimerPeriod();
while (!HalCheckTimerExpiration(HalGetTimerTicks(), Start, Ticks))
;
}
3 changes: 3 additions & 0 deletions src/kernel/include/private/amd64/hpet.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
#define HPET_TIMER_CMP_REG(n) (0x108 + ((n) << 5))
#define HPET_TIMER_FSB_REG(n) (0x110 + ((n) << 5))

#define HPET_CAP_64B 0x2000
#define HPET_CAP_FREQ_START 32

#define HPET_CFG_INT_ENABLE 0x01
#define HPET_CFG_LEGACY_ENABLE 0x02
#define HPET_CFG_MASK (HPET_CFG_INT_ENABLE | HPET_CFG_LEGACY_ENABLE)
Expand Down
3 changes: 2 additions & 1 deletion src/kernel/include/public/ev.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ typedef struct {
int Finished;
struct PsThread *Source;
EvDpc *Dpc;
uint64_t Deadline;
uint64_t DeadlineReference;
uint64_t DeadlineTicks;
} EvHeader, EvTimer;

#ifdef __cplusplus
Expand Down
5 changes: 5 additions & 0 deletions src/kernel/include/public/hal.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@
#define HAL_NO_EVENT 0
#define HAL_PANIC_EVENT 1

#define HAL_TIMER_WIDTH_32B 32
#define HAL_TIMER_WIDTH_64B 64

#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */

KeProcessor *HalGetCurrentProcessor(void);

int HalGetTimerWidth(void);
uint64_t HalGetTimerPeriod(void);
uint64_t HalGetTimerTicks(void);
int HalCheckTimerExpiration(uint64_t Current, uint64_t Reference, uint64_t Ticks);
void HalWaitTimer(uint64_t Time);

#ifdef __cplusplus
Expand Down
3 changes: 2 additions & 1 deletion src/kernel/include/public/ps.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

typedef struct PsThread {
RtDList ListHeader;
uint64_t Expiration;
uint64_t ExpirationReference;
uint64_t ExpirationTicks;
int Terminated;
EvDpc TerminationDpc;
HalRegisterState Context;
Expand Down
2 changes: 1 addition & 1 deletion src/kernel/ke/entry.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ extern "C" [[noreturn]] void KiApEntry(KeProcessor *Processor) {
/* Stage 1 (AP): Create the idle thread, and spin up the scheduler (we'll be on the wait for
* threads to run). */
PspCreateIdleThread();
PspInitializeScheduler(1);
PspInitializeScheduler(0);

while (1) {
HalpStopProcessor();
Expand Down
2 changes: 2 additions & 0 deletions src/kernel/kernel.def
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ EXPORTS
EvInitializeTimer
EvWaitObject

HalCheckTimerExpiration
HalGetCurrentProcessor
HalGetTimerPeriod
HalGetTimerTicks
HalGetTimerWidth
HalWaitTimer

IoCreateDevice
Expand Down
18 changes: 12 additions & 6 deletions src/kernel/ps/scheduler.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ extern "C" void PsReadyThread(PsThread *Thread) {
RtAppendDList(&BestMatch->ThreadQueue, &Thread->ListHeader);
Guard.Release();

/* PspScheduleNext uses Expiration=0 as a sign that we need to switch asap (we were out of
/* PspScheduleNext uses ExpirationTicks=0 as a sign that we need to switch asap (we were out of
work). */
if (!BestMatch->CurrentThread->Expiration) {
if (!BestMatch->CurrentThread->ExpirationTicks) {
HalpNotifyProcessor(BestMatch, 0);
}
}
Expand Down Expand Up @@ -143,7 +143,8 @@ extern "C" void PspScheduleNext(HalRegisterState *Context) {
return;
} else if (!Processor->CurrentThread) {
Processor->CurrentThread = Processor->InitialThread;
Processor->CurrentThread->Expiration = 0;
Processor->CurrentThread->ExpirationReference = 0;
Processor->CurrentThread->ExpirationTicks = 0;
HalpRestoreThreadContext(Context, &Processor->CurrentThread->Context);
return;
}
Expand All @@ -155,7 +156,10 @@ extern "C" void PspScheduleNext(HalRegisterState *Context) {
/* On quantum expiry, we switch threads if possible;
* On force yield, we always switch threads (into the idle if nothing is left). */
uint64_t CurrentTicks = HalGetTimerTicks();
if (CurrentTicks < CurrentThread->Expiration && !Processor->ForceYield) {
if (CurrentThread->ExpirationTicks &&
!HalCheckTimerExpiration(
CurrentTicks, CurrentThread->ExpirationReference, CurrentThread->ExpirationTicks) &&
!Processor->ForceYield) {
return;
}

Expand All @@ -165,14 +169,16 @@ extern "C" void PspScheduleNext(HalRegisterState *Context) {
}

if (NewThread == Processor->IdleThread) {
NewThread->Expiration = 0;
NewThread->ExpirationReference = 0;
NewThread->ExpirationTicks = 0;
} else {
uint64_t ThreadQuantum = PSP_THREAD_QUANTUM / Processor->ThreadQueueSize;
if (ThreadQuantum < PSP_THREAD_MIN_QUANTUM) {
ThreadQuantum = PSP_THREAD_MIN_QUANTUM;
}

NewThread->Expiration = CurrentTicks + ThreadQuantum / HalGetTimerPeriod();
NewThread->ExpirationReference = CurrentTicks;
NewThread->ExpirationTicks = ThreadQuantum / HalGetTimerPeriod();
}

/* Thread cleanup cannot be done at the moment (we're still on the thread context!),
Expand Down

0 comments on commit b12a917

Please sign in to comment.