From 5b8ae8393b732f9fd545849c501baab391685578 Mon Sep 17 00:00:00 2001 From: ergo720 <45463469+ergo720@users.noreply.github.com> Date: Tue, 8 Jan 2019 16:44:56 +0100 Subject: [PATCH] Reworked kernel timer lock --- src/core/kernel/exports/EmuKrnlKe.cpp | 28 +++++++++++---- src/core/kernel/exports/EmuKrnlKi.cpp | 50 +++++++++++++++------------ src/core/kernel/exports/EmuKrnlKi.h | 17 +++++++-- 3 files changed, 63 insertions(+), 32 deletions(-) diff --git a/src/core/kernel/exports/EmuKrnlKe.cpp b/src/core/kernel/exports/EmuKrnlKe.cpp index 52786a1c2..b1b2c1cf4 100644 --- a/src/core/kernel/exports/EmuKrnlKe.cpp +++ b/src/core/kernel/exports/EmuKrnlKe.cpp @@ -449,6 +449,8 @@ XBSYSAPI EXPORTNUM(96) xboxkrnl::BOOLEAN NTAPI xboxkrnl::KeCancelTimer assert(Timer); + KiTimerLock(); + /* Lock the Database and Raise IRQL */ KiLockDispatcherDatabase(&OldIrql); @@ -461,6 +463,8 @@ XBSYSAPI EXPORTNUM(96) xboxkrnl::BOOLEAN NTAPI xboxkrnl::KeCancelTimer /* Release Dispatcher Lock */ KiUnlockDispatcherDatabase(OldIrql); + KiTimerUnlock(); + /* Return the old state */ RETURN(Inserted); } @@ -1686,6 +1690,8 @@ XBSYSAPI EXPORTNUM(150) xboxkrnl::BOOLEAN NTAPI xboxkrnl::KeSetTimerEx assert(Timer); assert(Timer->Header.Type == TimerNotificationObject || Timer->Header.Type == TimerSynchronizationObject); + KiTimerLock(); + KiLockDispatcherDatabase(&OldIrql); // Same as KeCancelTimer(Timer) : @@ -1716,7 +1722,9 @@ XBSYSAPI EXPORTNUM(150) xboxkrnl::BOOLEAN NTAPI xboxkrnl::KeSetTimerEx } /* Exit the dispatcher */ - KiUnlockDispatcherDatabase(OldIrql); + KiUnlockDispatcherDatabase(OldIrql); + + KiTimerUnlock(); RETURN(Inserted); } @@ -1945,7 +1953,8 @@ XBSYSAPI EXPORTNUM(158) xboxkrnl::NTSTATUS NTAPI xboxkrnl::KeWaitForMultipleObje goto NoWait; } - // Setup a timer for the thread + // Setup a timer for the thread + KiTimerLock(); PKTIMER Timer = &Thread->Timer; PKWAIT_BLOCK WaitTimer = &Thread->TimerWaitBlock; WaitBlock->NextWaitBlock = WaitTimer; @@ -1953,11 +1962,13 @@ XBSYSAPI EXPORTNUM(158) xboxkrnl::NTSTATUS NTAPI xboxkrnl::KeWaitForMultipleObje Timer->Header.WaitListHead.Blink = &WaitTimer->WaitListEntry; WaitTimer->NextWaitBlock = WaitBlock; if (KiInsertTreeTimer(Timer, *Timeout) == FALSE) { - WaitStatus = (NTSTATUS)STATUS_TIMEOUT; + WaitStatus = (NTSTATUS)STATUS_TIMEOUT; + KiTimerUnlock(); goto NoWait; } - DueTime.QuadPart = Timer->DueTime.QuadPart; + DueTime.QuadPart = Timer->DueTime.QuadPart; + KiTimerUnlock(); } else { WaitBlock->NextWaitBlock = WaitBlock; @@ -2127,7 +2138,8 @@ XBSYSAPI EXPORTNUM(159) xboxkrnl::NTSTATUS NTAPI xboxkrnl::KeWaitForSingleObject goto NoWait; } - // Setup a timer for the thread + // Setup a timer for the thread + KiTimerLock(); PKTIMER Timer = &Thread->Timer; PKWAIT_BLOCK WaitTimer = &Thread->TimerWaitBlock; WaitBlock->NextWaitBlock = WaitTimer; @@ -2135,11 +2147,13 @@ XBSYSAPI EXPORTNUM(159) xboxkrnl::NTSTATUS NTAPI xboxkrnl::KeWaitForSingleObject Timer->Header.WaitListHead.Blink = &WaitTimer->WaitListEntry; WaitTimer->NextWaitBlock = WaitBlock; if (KiInsertTreeTimer(Timer, *Timeout) == FALSE) { - WaitStatus = (NTSTATUS)STATUS_TIMEOUT; + WaitStatus = (NTSTATUS)STATUS_TIMEOUT; + KiTimerUnlock(); goto NoWait; } - DueTime.QuadPart = Timer->DueTime.QuadPart; + DueTime.QuadPart = Timer->DueTime.QuadPart; + KiTimerUnlock(); } else { WaitBlock->NextWaitBlock = WaitBlock; diff --git a/src/core/kernel/exports/EmuKrnlKi.cpp b/src/core/kernel/exports/EmuKrnlKi.cpp index 7c3a77f7c..da876fe5b 100644 --- a/src/core/kernel/exports/EmuKrnlKi.cpp +++ b/src/core/kernel/exports/EmuKrnlKi.cpp @@ -36,7 +36,7 @@ // * // ****************************************************************** -// Acknowledgment: ReactOS (GPLv2) +// Acknowledgment (timer functions): ReactOS (GPLv2) // https://github.com/reactos/reactos // Changed from ReactOS: slight changes to the Hand parameter usage @@ -58,6 +58,8 @@ namespace xboxkrnl // ReactOS uses a size of 512, but disassembling the kernel reveals it to be 32 instead #define TIMER_TABLE_SIZE 32 +#define ASSERT_TIMER_LOCKED assert(xboxkrnl::KiTimerMtx.Acquired == true) + xboxkrnl::KTIMER_TABLE_ENTRY KiTimerTableListHead[TIMER_TABLE_SIZE]; xboxkrnl::KDPC KiTimerExpireDpc; @@ -68,6 +70,7 @@ VOID xboxkrnl::KiInitSystem() InitializeListHead(&KiWaitInListHead); + KiTimerMtx.Acquired = false; KeInitializeDpc(&KiTimerExpireDpc, KiTimerExpiration, NULL); for (i = 0; i < TIMER_TABLE_SIZE; i++) { InitializeListHead(&KiTimerTableListHead[i].Entry); @@ -76,6 +79,18 @@ VOID xboxkrnl::KiInitSystem() } } +VOID xboxkrnl::KiTimerLock() +{ + KiTimerMtx.Mtx.lock(); + KiTimerMtx.Acquired = true; +} + +VOID xboxkrnl::KiTimerUnlock() +{ + KiTimerMtx.Mtx.unlock(); + KiTimerMtx.Acquired = false; +} + VOID xboxkrnl::KiClockIsr(unsigned int ScalingFactor) { KIRQL OldIrql; @@ -107,14 +122,16 @@ VOID xboxkrnl::KiClockIsr(unsigned int ScalingFactor) // Because this function must be fast to continuously update the kernel clocks, if somebody else is currently // holding the lock, we won't wait and instead skip the check of the timers for this cycle - if (TimerMtx.try_lock()) { + if (KiTimerMtx.Mtx.try_lock()) { + KiTimerMtx.Acquired = true; // Check if a timer has expired Hand = KeTickCount & (TIMER_TABLE_SIZE - 1); if (KiTimerTableListHead[Hand].Entry.Flink != &KiTimerTableListHead[Hand].Entry && InterruptTime.QuadPart >= KiTimerTableListHead[Hand].Time.QuadPart) { KeInsertQueueDpc(&KiTimerExpireDpc, (PVOID)&KeTickCount, 0); } - TimerMtx.unlock(); + KiTimerMtx.Mtx.unlock(); + KiTimerMtx.Acquired = false; } KfLowerIrql(OldIrql); @@ -125,7 +142,7 @@ VOID xboxkrnl::KxInsertTimer( IN xboxkrnl::ULONG Hand ) { - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Try to insert the timer */ if (KiInsertTimerTable(Timer, Hand)) @@ -133,7 +150,6 @@ VOID xboxkrnl::KxInsertTimer( /* Complete it */ KiCompleteTimer(Timer, Hand); } - TimerMtx.unlock(); } VOID FASTCALL xboxkrnl::KiCompleteTimer( @@ -144,7 +160,7 @@ VOID FASTCALL xboxkrnl::KiCompleteTimer( LIST_ENTRY ListHead; BOOLEAN RequestInterrupt = FALSE; - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Remove it from the timer list */ KiRemoveEntryTimer(Timer, Hand); @@ -164,7 +180,6 @@ VOID FASTCALL xboxkrnl::KiCompleteTimer( if (RequestInterrupt) { HalRequestSoftwareInterrupt(DISPATCH_LEVEL); } - TimerMtx.unlock(); } VOID xboxkrnl::KiRemoveEntryTimer( @@ -175,7 +190,7 @@ VOID xboxkrnl::KiRemoveEntryTimer( ULONG Hand; PKTIMER_TABLE_ENTRY TableEntry; - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Remove the timer from the timer list and check if it's empty */ if (RemoveEntryList(&Timer->TimerListEntry)) @@ -192,7 +207,6 @@ VOID xboxkrnl::KiRemoveEntryTimer( /* Clear the list entries so we can tell the timer is gone */ Timer->TimerListEntry.Flink = NULL; Timer->TimerListEntry.Blink = NULL; - TimerMtx.unlock(); } VOID xboxkrnl::KxRemoveTreeTimer( @@ -202,7 +216,7 @@ VOID xboxkrnl::KxRemoveTreeTimer( ULONG Hand = KiComputeTimerTableIndex(Timer->DueTime.QuadPart); PKTIMER_TABLE_ENTRY TimerEntry; - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Set the timer as non-inserted */ Timer->Header.Inserted = FALSE; @@ -218,7 +232,6 @@ VOID xboxkrnl::KxRemoveTreeTimer( TimerEntry->Time.u.HighPart = 0xFFFFFFFF; } } - TimerMtx.unlock(); } xboxkrnl::BOOLEAN FASTCALL xboxkrnl::KiInsertTimerTable( @@ -234,7 +247,7 @@ xboxkrnl::BOOLEAN FASTCALL xboxkrnl::KiInsertTimerTable( DBG_PRINTF("%s: inserting Timer %p, Hand: %lu\n", __func__, Timer, Hand); - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Check if the period is zero */ if (!Timer->Period) { @@ -272,7 +285,6 @@ xboxkrnl::BOOLEAN FASTCALL xboxkrnl::KiInsertTimerTable( Expired = TRUE; } } - TimerMtx.unlock(); /* Return expired state */ return Expired; @@ -286,7 +298,7 @@ xboxkrnl::BOOLEAN FASTCALL xboxkrnl::KiInsertTreeTimer( BOOLEAN Inserted = FALSE; ULONG Hand = 0; - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Setup the timer's due time */ if (KiComputeDueTime(Timer, Interval, &Hand)) @@ -304,7 +316,6 @@ xboxkrnl::BOOLEAN FASTCALL xboxkrnl::KiInsertTreeTimer( Inserted = TRUE; } } - TimerMtx.unlock(); return Inserted; } @@ -323,7 +334,7 @@ xboxkrnl::BOOLEAN xboxkrnl::KiComputeDueTime( { LARGE_INTEGER InterruptTime, SystemTime, DifferenceTime; - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Convert to relative time if needed */ Timer->Header.Absolute = FALSE; @@ -344,8 +355,6 @@ xboxkrnl::BOOLEAN xboxkrnl::KiComputeDueTime( Timer->Header.SignalState = TRUE; Timer->DueTime.QuadPart = 0; *Hand = 0; - TimerMtx.unlock(); - return FALSE; } @@ -362,7 +371,6 @@ xboxkrnl::BOOLEAN xboxkrnl::KiComputeDueTime( /* Get the handle */ *Hand = KiComputeTimerTableIndex(Timer->DueTime.QuadPart); Timer->Header.Inserted = TRUE; - TimerMtx.unlock(); return TRUE; } @@ -376,7 +384,7 @@ xboxkrnl::BOOLEAN FASTCALL xboxkrnl::KiSignalTimer( ULONG Period = Timer->Period; LARGE_INTEGER Interval, SystemTime; - TimerMtx.lock(); + ASSERT_TIMER_LOCKED; /* Set default values */ Timer->Header.Inserted = FALSE; @@ -417,8 +425,6 @@ xboxkrnl::BOOLEAN FASTCALL xboxkrnl::KiSignalTimer( RequestInterrupt = TRUE; } - TimerMtx.unlock(); - /* Return whether we need to request a DPC interrupt or not */ return RequestInterrupt; } diff --git a/src/core/kernel/exports/EmuKrnlKi.h b/src/core/kernel/exports/EmuKrnlKi.h index 6495eb937..5476a86ee 100644 --- a/src/core/kernel/exports/EmuKrnlKi.h +++ b/src/core/kernel/exports/EmuKrnlKi.h @@ -43,14 +43,25 @@ namespace xboxkrnl ULARGE_INTEGER Time; } KTIMER_TABLE_ENTRY, *PKTIMER_TABLE_ENTRY; - const ULONG CLOCK_TIME_INCREMENT = 0x2710; - LIST_ENTRY KiWaitInListHead; // Actually, this lock isn't required, but because raising the irql to dpc level doesn't really prevent thread switching at the // moment, we need it for now to prevent concurrent access to the timer table - std::recursive_mutex TimerMtx; + typedef struct _KI_TIMER_LOCK + { + std::recursive_mutex Mtx; + bool Acquired; + } KI_TIMER_LOCK; + + const ULONG CLOCK_TIME_INCREMENT = 0x2710; + LIST_ENTRY KiWaitInListHead; + KI_TIMER_LOCK KiTimerMtx; + VOID KiInitSystem(); + VOID KiTimerLock(); + + VOID KiTimerUnlock(); + VOID KiClockIsr( IN unsigned int ScalingFactor );