[PATCH v2 0/2] MR5238: krnl386, mmsystem: Simulate a hardware interrupt in MMSYSTDRV_Callback3216.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56260 -- v2: mmsystem: Simulate a hardware interrupt in MMSYSTDRV_Callback3216. krnl386: Add functions to simulate a hardware interrupt. https://gitlab.winehq.org/wine/wine/-/merge_requests/5238
From: Alex Henrie <alexhenrie24(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56260 --- dlls/krnl386.exe16/kernel16_private.h | 2 + dlls/krnl386.exe16/krnl386.exe16.spec | 3 + dlls/krnl386.exe16/task.c | 87 +++++++++++++++++++++++++-- dlls/krnl386.exe16/wowthunk.c | 37 +++++++++--- include/wine/winbase16.h | 2 + 5 files changed, 117 insertions(+), 14 deletions(-) diff --git a/dlls/krnl386.exe16/kernel16_private.h b/dlls/krnl386.exe16/kernel16_private.h index 9cf60d068a7..1ccbaf68122 100644 --- a/dlls/krnl386.exe16/kernel16_private.h +++ b/dlls/krnl386.exe16/kernel16_private.h @@ -283,6 +283,8 @@ extern void TASK_ExitTask(void); extern HTASK16 TASK_GetTaskFromThread( DWORD thread ); extern TDB *TASK_GetCurrent(void); extern void TASK_InstallTHHook( THHOOK *pNewThook ); +extern void TASK_SuspendAll(void); +extern void TASK_ResumeAll(void); extern BOOL WOWTHUNK_Init(void); diff --git a/dlls/krnl386.exe16/krnl386.exe16.spec b/dlls/krnl386.exe16/krnl386.exe16.spec index d98e136c77e..34f9c9b3846 100644 --- a/dlls/krnl386.exe16/krnl386.exe16.spec +++ b/dlls/krnl386.exe16/krnl386.exe16.spec @@ -744,6 +744,9 @@ 2000 pascal -register __wine_call_int_handler(word) __wine_call_int_handler16 @ stdcall -arch=win32 __wine_call_int_handler16(long ptr) +# Hardware interrupt simulation +@ cdecl -arch=win32 __wine_wow_interrupt16(long long long ptr ptr) + # VxDs @ cdecl -arch=win32 -private __wine_vxd_open(wstr long ptr) @ cdecl -arch=win32 -private __wine_vxd_get_proc(long) diff --git a/dlls/krnl386.exe16/task.c b/dlls/krnl386.exe16/task.c index 534b912e884..303196f0467 100644 --- a/dlls/krnl386.exe16/task.c +++ b/dlls/krnl386.exe16/task.c @@ -77,6 +77,15 @@ static UINT16 nTaskCount = 0; static HTASK16 initial_task, main_task; +static CRITICAL_SECTION task_list_cs; +static CRITICAL_SECTION_DEBUG task_list_debug = +{ + 0, 0, &task_list_cs, + { &task_list_debug.ProcessLocksList, &task_list_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": task_list_cs") } +}; +static CRITICAL_SECTION task_list_cs = { &task_list_debug, -1, 0, 0, 0, 0 }; + /*********************************************************************** * TASK_InstallTHHook */ @@ -109,6 +118,8 @@ TDB *TASK_GetCurrent(void) /*********************************************************************** * TASK_LinkTask + * + * Be sure to lock task_list_cs before calling this function. */ static void TASK_LinkTask( HTASK16 hTask ) { @@ -131,6 +142,8 @@ static void TASK_LinkTask( HTASK16 hTask ) /*********************************************************************** * TASK_UnlinkTask + * + * Be sure to lock task_list_cs before calling this function. */ static void TASK_UnlinkTask( HTASK16 hTask ) { @@ -239,11 +252,6 @@ static BOOL TASK_FreeThunk( SEGPTR thunk ) /*********************************************************************** * TASK_Create - * - * NOTE: This routine might be called by a Win32 thread. Thus, we need - * to be careful to protect global data structures. We do this - * by entering the Win16Lock while linking the task into the - * global task list. */ static TDB *TASK_Create( NE_MODULE *pModule, UINT16 cmdShow, LPCSTR cmdline, BYTE len ) { @@ -396,6 +404,8 @@ void TASK_CreateMainTask(void) STARTUPINFOA startup_info; UINT cmdShow = 1; /* SW_SHOWNORMAL but we don't want to include winuser.h here */ + RtlEnterCriticalSection( &task_list_cs ); + GetStartupInfoA( &startup_info ); if (startup_info.dwFlags & STARTF_USESHOWWINDOW) cmdShow = startup_info.wShowWindow; pTask = TASK_Create( NULL, cmdShow, NULL, 0 ); @@ -414,6 +424,8 @@ void TASK_CreateMainTask(void) /* (no need to get the win16 lock, we are the only thread at this point) */ TASK_LinkTask( pTask->hSelf ); main_task = pTask->hSelf; + + LeaveCriticalSection( &task_list_cs ); } @@ -470,10 +482,16 @@ static DWORD CALLBACK task_start( LPVOID p ) HeapFree( GetProcessHeap(), 0, data ); _EnterWin16Lock(); + + RtlEnterCriticalSection( &task_list_cs ); TASK_LinkTask( pTask->hSelf ); pTask->teb = NtCurrentTeb(); + LeaveCriticalSection( &task_list_cs ); + ret = NE_StartTask(); + _LeaveWin16Lock(); + return ret; } @@ -521,6 +539,59 @@ HTASK16 TASK_GetTaskFromThread( DWORD thread ) } +/*********************************************************************** + * TASK_SuspendAll + */ +void TASK_SuspendAll(void) +{ + TDB *p; + + RtlEnterCriticalSection( &task_list_cs ); + + p = TASK_GetPtr( hFirstTask ); + + while (p) + { + DWORD thread_id = HandleToULong( p->teb->ClientId.UniqueThread ); + if (thread_id != GetCurrentThreadId()) + { + HANDLE thread = OpenThread( THREAD_SUSPEND_RESUME, FALSE, thread_id ); + SuspendThread( thread ); + CloseHandle( thread ); + } + p = TASK_GetPtr( p->hNext ); + } + + RtlLeaveCriticalSection( &task_list_cs ); +} + + +/*********************************************************************** + * TASK_ResumeAll + */ +void TASK_ResumeAll(void) +{ + TDB *p; + + RtlEnterCriticalSection( &task_list_cs ); + + p = TASK_GetPtr( hFirstTask ); + while (p) + { + DWORD thread_id = HandleToULong( p->teb->ClientId.UniqueThread ); + if (thread_id != GetCurrentThreadId()) + { + HANDLE thread = OpenThread( THREAD_SUSPEND_RESUME, FALSE, thread_id ); + ResumeThread( thread ); + CloseHandle( thread ); + } + p = TASK_GetPtr( p->hNext ); + } + + RtlLeaveCriticalSection( &task_list_cs ); +} + + /*********************************************************************** * TASK_CallTaskSignalProc */ @@ -559,7 +630,9 @@ void TASK_ExitTask(void) TASK_CallTaskSignalProc( USIG16_TERMINATION, pTask->hSelf ); /* Remove the task from the list to be sure we never switch back to it */ + RtlEnterCriticalSection( &task_list_cs ); TASK_UnlinkTask( pTask->hSelf ); + RtlLeaveCriticalSection( &task_list_cs ); if (!nTaskCount || (nTaskCount == 1 && hFirstTask == initial_task)) { @@ -737,8 +810,12 @@ void WINAPI SetPriority16( HTASK16 hTask, INT16 delta ) else if (newpriority > 15) newpriority = 15; pTask->priority = newpriority + 1; + + EnterCriticalSection( &task_list_cs ); TASK_UnlinkTask( pTask->hSelf ); TASK_LinkTask( pTask->hSelf ); + LeaveCriticalSection( &task_list_cs ); + pTask->priority--; } diff --git a/dlls/krnl386.exe16/wowthunk.c b/dlls/krnl386.exe16/wowthunk.c index b729c38e8f1..39657d66c5e 100644 --- a/dlls/krnl386.exe16/wowthunk.c +++ b/dlls/krnl386.exe16/wowthunk.c @@ -421,11 +421,7 @@ WORD WINAPI K32WOWHandle16( HANDLE handle, WOW_HANDLE_TYPE type ) } } -/********************************************************************** - * K32WOWCallback16Ex (KERNEL32.55) - */ -BOOL WINAPI K32WOWCallback16Ex( DWORD vpfn16, DWORD dwFlags, - DWORD cbArgs, LPVOID pArgs, LPDWORD pdwRetCode ) +static BOOL wow_callback16( DWORD vpfn16, DWORD dwFlags, DWORD cbArgs, void *pArgs, DWORD *pdwRetCode, BOOL interrupt ) { /* * Arguments must be prepared in the correct order by the caller @@ -460,9 +456,9 @@ BOOL WINAPI K32WOWCallback16Ex( DWORD vpfn16, DWORD dwFlags, *((SEGPTR *)stack) = call16_ret_addr; cbArgs += sizeof(SEGPTR); - _EnterWin16Lock(); + interrupt ? TASK_SuspendAll() : _EnterWin16Lock(); wine_call_to_16_regs( context, cbArgs, call16_handler ); - _LeaveWin16Lock(); + interrupt ? TASK_ResumeAll() : _LeaveWin16Lock(); if (TRACE_ON(relay)) { @@ -500,10 +496,10 @@ BOOL WINAPI K32WOWCallback16Ex( DWORD vpfn16, DWORD dwFlags, * the callee to do so; after the routine has returned, the 16-bit * stack pointer is always reset to the position it had before. */ - _EnterWin16Lock(); + interrupt ? TASK_SuspendAll() : _EnterWin16Lock(); ret = wine_call_to_16( (FARPROC16)vpfn16, cbArgs, call16_handler ); if (pdwRetCode) *pdwRetCode = ret; - _LeaveWin16Lock(); + interrupt ? TASK_ResumeAll() : _LeaveWin16Lock(); if (TRACE_ON(relay)) { @@ -515,6 +511,29 @@ BOOL WINAPI K32WOWCallback16Ex( DWORD vpfn16, DWORD dwFlags, return TRUE; /* success */ } +/********************************************************************** + * __wine_wow_interrupt16 + * + * Simulates a hardware interrupt by pausing all other tasks. Only one task can + * run at a time, so pausing the other tasks would typically have no effect. + * However, some programs depend on hardware interrupts interrupting the active + * task in order to call a callback. We don't have real hardware interrupts, but + * there can be background threads that fulfill the same role, processing data + * and calling callbacks when finished. + */ +BOOL __wine_wow_interrupt16( DWORD func, DWORD flags, DWORD arg_count, void *args, DWORD *ret_code ) +{ + return wow_callback16( func, flags, arg_count, args, ret_code, TRUE ); +} + +/********************************************************************** + * K32WOWCallback16Ex (KERNEL32.55) + */ +BOOL WINAPI K32WOWCallback16Ex( DWORD func, DWORD flags, DWORD arg_count, void *args, DWORD *ret_code ) +{ + return wow_callback16( func, flags, arg_count, args, ret_code, FALSE ); +} + /********************************************************************** * K32WOWCallback16 (KERNEL32.54) */ diff --git a/include/wine/winbase16.h b/include/wine/winbase16.h index 28cde59f9b3..b7ba1bb9a7e 100644 --- a/include/wine/winbase16.h +++ b/include/wine/winbase16.h @@ -562,6 +562,8 @@ BOOL16 WINAPI WritePrivateProfileSection16(LPCSTR,LPCSTR,LPCSTR); BOOL16 WINAPI WritePrivateProfileStruct16(LPCSTR,LPCSTR,LPVOID,UINT16,LPCSTR); BOOL16 WINAPI WriteProfileSection16(LPCSTR,LPCSTR); +BOOL __wine_wow_interrupt16(DWORD,DWORD,DWORD,void*,DWORD*); + #define CURRENT_STACK16 ((STACK16FRAME *)MapSL((SEGPTR)NtCurrentTeb()->SystemReserved1[0])) #define CURRENT_DS (CURRENT_STACK16->ds) #define CURRENT_SP (((WORD *)NtCurrentTeb()->SystemReserved1)[0]) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5238
From: Alex Henrie <alexhenrie24(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56260 --- dlls/mmsystem.dll16/message16.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/mmsystem.dll16/message16.c b/dlls/mmsystem.dll16/message16.c index b12b9b94327..bf3f9a71f6b 100644 --- a/dlls/mmsystem.dll16/message16.c +++ b/dlls/mmsystem.dll16/message16.c @@ -838,7 +838,7 @@ static LRESULT CALLBACK MMSYSTDRV_Callback3216(struct mmsystdrv_thunk* thunk, HD args[2] = LOWORD(dwParam1); args[1] = HIWORD(dwParam2); args[0] = LOWORD(dwParam2); - return WOWCallback16Ex(thunk->callback, WCB16_PASCAL, sizeof(args), args, NULL); + return __wine_wow_interrupt16(thunk->callback, WCB16_PASCAL, sizeof(args), args, NULL); case CALLBACK_EVENT: TRACE("Event(%08lx) !\n", thunk->callback); SetEvent((HANDLE)thunk->callback); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5238
The bigger question is, do we actually need to do this? What bug is going to be fixed by suspending the tasks?
That would be [Bug 56260](https://bugs.winehq.org/show_bug.cgi?id=56260). Fabian wrote a [nice test program](https://bugs.winehq.org/attachment.cgi?id=76137) to demonstrate the essence of the problem. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5238#note_64328
That would be [Bug 56260](https://bugs.winehq.org/show_bug.cgi?id=56260). Fabian wrote a [nice test program](https://bugs.winehq.org/attachment.cgi?id=76137) to demonstrate the essence of the problem.
I don't see anything in there that would require tasks to be suspended. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5238#note_64341
On Tue Mar 12 07:03:12 2024 +0000, Alexandre Julliard wrote:
That would be [Bug 56260](https://bugs.winehq.org/show_bug.cgi?id=56260). Fabian wrote a [nice test program](https://bugs.winehq.org/attachment.cgi?id=76137) to demonstrate the essence of the problem. I don't see anything in there that would require tasks to be suspended. Would you rather run the hardware interrupt callback in parallel with the current task, even though that would not have been possible on Windows? That would work fine for Myst, I was just concerned that it might break something else.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5238#note_64424
participants (3)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Alexandre Julliard (＠julliard)