Merge pull request #2416 from ergo720/dpc_recursion_fix

Dpc recursion fix

src/core/kernel/exports/EmuKrnl.cpp

@@ -158,7 +158,10 @@ void CallSoftwareInterrupt(const xbox::KIRQL SoftwareIrql)
 		xbox::KiExecuteKernelApc();
 		break;
 	case DISPATCH_LEVEL: // = 2
-		ExecuteDpcQueue();
+		// This can be recursively called by KiUnlockDispatcherDatabase and KfLowerIrql, so avoid calling DPCs again if the current one has queued yet another one
+		if (!IsDpcActive()) { // Avoid KeIsExecutingDpc(), as that logs
+			ExecuteDpcQueue();
+		}
 		break;
 	case APC_LEVEL | DISPATCH_LEVEL: // = 3
 		KiUnexpectedInterrupt();
@@ -450,7 +453,8 @@ XBSYSAPI EXPORTNUM(163) xbox::void_xt FASTCALL xbox::KiUnlockDispatcherDatabase
 	LOG_FUNC_ONE_ARG_TYPE(KIRQL_TYPE, OldIrql);
 
 	// Wrong, this should only happen when OldIrql >= DISPATCH_LEVEL
-	if (!(KeGetCurrentPrcb()->DpcRoutineActive)) { // Avoid KeIsExecutingDpc(), as that logs
+	// Checking DpcRoutineActive doesn't work because our Prcb is per-thread instead of being per-processor
+	if (!IsDpcActive()) { // Avoid KeIsExecutingDpc(), as that logs
 		HalRequestSoftwareInterrupt(DISPATCH_LEVEL);
 	}
 

src/core/kernel/exports/EmuKrnlKe.cpp

@@ -96,11 +96,11 @@ namespace NtDll
 // TODO : Move towards thread-simulation based Dpc emulation
 typedef struct _DpcData {
 	CRITICAL_SECTION Lock;
-	HANDLE DpcEvent;
+	std::atomic_flag IsDpcActive;
 	xbox::LIST_ENTRY DpcQueue; // TODO : Use KeGetCurrentPrcb()->DpcListHead instead
 } DpcData;
 
-DpcData g_DpcData = { 0 }; // Note : g_DpcData is initialized in InitDpcThread()
+DpcData g_DpcData = { 0 }; // Note : g_DpcData is initialized in InitDpcData()
 
 xbox::ulonglong_xt LARGE_INTEGER2ULONGLONG(xbox::LARGE_INTEGER value)
 {
@@ -460,8 +460,10 @@ void ExecuteDpcQueue()
 		// Mark it as no longer linked into the DpcQueue
 		pkdpc->Inserted = FALSE;
 		// Set DpcRoutineActive to support KeIsExecutingDpc:
+		g_DpcData.IsDpcActive.test_and_set();
 		KeGetCurrentPrcb()->DpcRoutineActive = TRUE; // Experimental
-		EmuLog(LOG_LEVEL::DEBUG, "Global DpcQueue, calling DPC at 0x%.8X", pkdpc->DeferredRoutine);
+		LeaveCriticalSection(&(g_DpcData.Lock));
+		EmuLog(LOG_LEVEL::DEBUG, "Global DpcQueue, calling DPC object 0x%.8X at 0x%.8X", pkdpc, pkdpc->DeferredRoutine);
 
 		// Call the Deferred Procedure  :
 		pkdpc->DeferredRoutine(
@@ -470,7 +472,9 @@ void ExecuteDpcQueue()
 			pkdpc->SystemArgument1,
 			pkdpc->SystemArgument2);
 
+		EnterCriticalSection(&(g_DpcData.Lock));
 		KeGetCurrentPrcb()->DpcRoutineActive = FALSE; // Experimental
+		g_DpcData.IsDpcActive.clear();
 	}
 
 //    Assert(g_DpcData._dwThreadId == GetCurrentThreadId());
@@ -479,14 +483,17 @@ void ExecuteDpcQueue()
 	LeaveCriticalSection(&(g_DpcData.Lock));
 }
 
-void InitDpcThread()
+void InitDpcData()
 {
-	DWORD dwThreadId = 0;
-
+	// Let's initialize the Dpc handling thread too,
+	// here for now (should be called by our caller)
 	InitializeCriticalSection(&(g_DpcData.Lock));
 	InitializeListHead(&(g_DpcData.DpcQueue));
-	EmuLogEx(CXBXR_MODULE::INIT, LOG_LEVEL::DEBUG, "Creating DPC event\n");
-	g_DpcData.DpcEvent = CreateEvent(/*lpEventAttributes=*/nullptr, /*bManualReset=*/FALSE, /*bInitialState=*/FALSE, /*lpName=*/nullptr);
+}
+
+bool IsDpcActive()
+{
+	return g_DpcData.IsDpcActive.test();
 }
 
 static constexpr uint32_t XBOX_TSC_FREQUENCY = 733333333; // Xbox Time Stamp Counter Frequency = 733333333 (CPU Clock)
@@ -498,13 +505,6 @@ ULONGLONG CxbxGetPerformanceCounter(bool acpi)
 	return Timer_GetScaledPerformanceCounter(period);
 }
 
-void CxbxInitPerformanceCounters()
-{
-	// Let's initialize the Dpc handling thread too,
-	// here for now (should be called by our caller)
-	InitDpcThread();
-}
-
 // ******************************************************************
 // * 0x005C - KeAlertResumeThread()
 // ******************************************************************
@@ -1268,7 +1268,9 @@ XBSYSAPI EXPORTNUM(119) xbox::boolean_xt NTAPI xbox::KeInsertQueueDpc
 		InsertTailList(&(g_DpcData.DpcQueue), &(Dpc->DpcListEntry));
 		// TODO : Instead of DpcQueue, add the DPC to KeGetCurrentPrcb()->DpcListHead
 		// Signal the Dpc handling code there's work to do
-		HalRequestSoftwareInterrupt(DISPATCH_LEVEL);
+		if (!IsDpcActive()) {
+			HalRequestSoftwareInterrupt(DISPATCH_LEVEL);
+		}
 		// OpenXbox has this instead:
 		// if (!pKPRCB->DpcRoutineActive && !pKPRCB->DpcInterruptRequested) {
 		//	pKPRCB->DpcInterruptRequested = TRUE;
@@ -1289,7 +1291,12 @@ XBSYSAPI EXPORTNUM(121) xbox::boolean_xt NTAPI xbox::KeIsExecutingDpc
 {
 	LOG_FUNC();
 
+#if 0
+	// This is the correct implementation, but it doesn't work because our Prcb is per-thread instead of being per-processor
 	BOOLEAN ret = (BOOLEAN)KeGetCurrentPrcb()->DpcRoutineActive;
+#else
+	BOOLEAN ret = (BOOLEAN)IsDpcActive();
+#endif
 
 	RETURN(ret);
 }

src/core/kernel/init/CxbxKrnl.cpp

@@ -1205,8 +1205,8 @@ static void CxbxrKrnlInitHacks()
 	Timer_Init();
 	// for unicode conversions
 	setlocale(LC_ALL, "English");
-	// Initialize time-related variables for the kernel and the timers
-	CxbxInitPerformanceCounters();
+	// Initialize DPC global
+	InitDpcData();
 #ifdef _DEBUG
 //	PopupCustom(LOG_LEVEL::INFO, "Attach a Debugger");
 //  Debug child processes using https://marketplace.visualstudio.com/items?itemName=GreggMiskelly.MicrosoftChildProcessDebuggingPowerTool

src/core/kernel/init/CxbxKrnl.h

@@ -155,7 +155,8 @@ void CxbxKrnlPanic();
 /*! empty function */
 void CxbxKrnlNoFunc();
 
-void CxbxInitPerformanceCounters(); // Implemented in EmuKrnlKe.cpp
+void InitDpcData(); // Implemented in EmuKrnlKe.cpp
+bool IsDpcActive();
 
 /*! kernel thunk table */
 extern uint32_t CxbxKrnl_KernelThunkTable[379];