[PATCH 0/6] MR7861: krnl386: Fixes for DPMI functions
* Set carry flag for every failed DPMI call * Fix return value for DPMI calls 0102h, 0202h and 0900h-0902h * Fix comments and FIXME -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861
From: Pali Rohár <pali(a)kernel.org> According to the DPMI specification, DPMI functions 0900h-0902h related to Virtual Interrupt State has to always return state in AL register. As wine does not support disabling virtual interrupt flag, report that interrupts are always enabled (AL=1). Signed-off-by: Pali Rohár <pali(a)kernel.org> --- dlls/krnl386.exe16/int31.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dlls/krnl386.exe16/int31.c b/dlls/krnl386.exe16/int31.c index 0d13763451c..2414fb7e55e 100644 --- a/dlls/krnl386.exe16/int31.c +++ b/dlls/krnl386.exe16/int31.c @@ -628,15 +628,18 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) break; case 0x0900: /* Get and Disable Virtual Interrupt State */ - TRACE( "Get and Disable Virtual Interrupt State - not supported\n" ); + FIXME( "Get and Disable Virtual Interrupt State - unimplemented\n" ); + SET_AL( context, 1 ); /* report that interrupts are always enabled */ break; case 0x0901: /* Get and Enable Virtual Interrupt State */ - TRACE( "Get and Enable Virtual Interrupt State - not supported\n" ); + FIXME( "Get and Enable Virtual Interrupt State - unimplemented\n" ); + SET_AL( context, 1 ); /* report that interrupts are always enabled */ break; case 0x0902: /* Get Virtual Interrupt State */ - TRACE( "Get Virtual Interrupt State - not supported\n" ); + FIXME( "Get Virtual Interrupt State - unimplemented\n" ); + SET_AL( context, 1 ); /* report that interrupts are always enabled */ break; case 0x0e00: /* Get Coprocessor Status (1.0) */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7861
From: Pali Rohár <pali(a)kernel.org> DPMI functions 0202h and 0203h related to Processor Exception Handler Vector are not supported by wine. So update FIXME comments about this fact. Signed-off-by: Pali Rohár <pali(a)kernel.org> --- dlls/krnl386.exe16/int31.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/krnl386.exe16/int31.c b/dlls/krnl386.exe16/int31.c index 2414fb7e55e..8dd13e3786c 100644 --- a/dlls/krnl386.exe16/int31.c +++ b/dlls/krnl386.exe16/int31.c @@ -394,14 +394,14 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) break; case 0x0202: /* Get Processor Exception Handler Vector */ - FIXME( "Get Processor Exception Handler Vector (0x%02x)\n", + FIXME( "Get Processor Exception Handler Vector (0x%02x) - not supported\n", BL_reg(context) ); SET_CX( context, 0 ); SET_DX( context, 0 ); break; case 0x0203: /* Set Processor Exception Handler Vector */ - FIXME( "Set Processor Exception Handler Vector (0x%02x)\n", + FIXME( "Set Processor Exception Handler Vector (0x%02x) - not supported\n", BL_reg(context) ); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7861
From: Pali Rohár <pali(a)kernel.org> According to the DPMI specification, when DPMI function fails the Carry flag has to be set. Wine does not implement more DPMI function calls, so it is needed to properly signal this fact to the called application. For example if the application request for physical memory mapping via the DPMI 0800h call (which is unsupported by both wine and Windows NT), it is required to set carry flag to indicate failure. Otherwise it would be treated as successful operation in which case in BX:CX registers is stored linear address of the mapped memory to which application can access. Obviously on successful path if the BX:CX registers are not set then application would try to access random memory and crashes. Signed-off-by: Pali Rohár <pali(a)kernel.org> --- dlls/krnl386.exe16/int31.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/dlls/krnl386.exe16/int31.c b/dlls/krnl386.exe16/int31.c index 8dd13e3786c..45c9fd92c34 100644 --- a/dlls/krnl386.exe16/int31.c +++ b/dlls/krnl386.exe16/int31.c @@ -59,16 +59,17 @@ typedef struct static void *real_mode_ptr( WORD seg, WORD off ) { return (void *)((int)seg * 16 + off); } -static void simulate_real_mode_interrupt( REALMODECALL *ctx, int num ) +static BOOL simulate_real_mode_interrupt( REALMODECALL *ctx, int num ) { if (num == 0x21 && AX_reg(ctx) == 0x440d && CX_reg(ctx) == 0x0866) { /* int 21/440d block ioctl, used by Myst */ memset( real_mode_ptr( ctx->ds, DX_reg(ctx) ), 0, 25 ); - return; + return TRUE; } WARN( "%02x ax %04x bx %04x cx %04x dx %04x si %04x di %04x ds %04x es %04x - unsupported\n", num, AX_reg(ctx), BX_reg(ctx), CX_reg(ctx), DX_reg(ctx), SI_reg(ctx), DI_reg(ctx), ctx->ds, ctx->es ); + return FALSE; } /********************************************************************** @@ -344,10 +345,12 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) case 0x000e: /* Get Multiple Descriptors (1.0) */ FIXME( "get multiple descriptors - unimplemented\n" ); + SET_CFLAG( context ); break; case 0x000f: /* Set Multiple Descriptors (1.0) */ FIXME( "set multiple descriptors - unimplemented\n" ); + SET_CFLAG( context ); break; case 0x0100: /* Allocate DOS memory block */ @@ -379,6 +382,7 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) case 0x0102: /* Resize DOS Memory Block */ FIXME( "resize DOS memory block (0x%04x, 0x%x paragraphs) - unimplemented\n", DX_reg(context), BX_reg(context) ); + SET_CFLAG( context ); break; case 0x0200: /* get real mode interrupt vector */ @@ -386,11 +390,13 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) BL_reg(context) ); SET_CX( context, 0 ); SET_DX( context, 0 ); + SET_CFLAG( context ); break; case 0x0201: /* set real mode interrupt vector */ TRACE( "set realmode interrupt vector (0x%02x, 0x%04x:0x%04x) - not supported\n", BL_reg(context), CX_reg(context), DX_reg(context) ); + SET_CFLAG( context ); break; case 0x0202: /* Get Processor Exception Handler Vector */ @@ -398,11 +404,13 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) BL_reg(context) ); SET_CX( context, 0 ); SET_DX( context, 0 ); + SET_CFLAG( context ); break; case 0x0203: /* Set Processor Exception Handler Vector */ FIXME( "Set Processor Exception Handler Vector (0x%02x) - not supported\n", BL_reg(context) ); + SET_CFLAG( context ); break; case 0x0204: /* Get protected mode interrupt vector */ @@ -427,23 +435,30 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) case 0x0300: /* Simulate real mode interrupt */ TRACE( "Simulate real mode interrupt %02x\n", BL_reg(context) ); - simulate_real_mode_interrupt( ldt_get_ptr( context->SegEs, context->Edi ), BL_reg(context) ); + if (!simulate_real_mode_interrupt( ldt_get_ptr( context->SegEs, context->Edi ), BL_reg(context) )) + { + SET_CFLAG( context ); + } break; case 0x0301: /* Call real mode procedure with far return */ TRACE( "Call real mode procedure with far return - not supported\n" ); + SET_CFLAG( context ); break; case 0x0302: /* Call real mode procedure with interrupt return */ TRACE( "Call real mode procedure with interrupt return - not supported\n" ); + SET_CFLAG( context ); break; case 0x0303: /* Allocate Real Mode Callback Address */ TRACE( "Allocate real mode callback address - not supported\n" ); + SET_CFLAG( context ); break; case 0x0304: /* Free Real Mode Callback Address */ TRACE( "Free real mode callback address - not supported\n" ); + SET_CFLAG( context ); break; case 0x0305: /* Get State Save/Restore Addresses */ @@ -456,6 +471,7 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) /* protected mode */ SET_SI( context, 0 ); context->Edi = 0; + SET_CFLAG( context ); break; case 0x0306: /* Get Raw Mode Switch Addresses */ @@ -466,6 +482,7 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) /* protected mode */ SET_SI( context, 0 ); context->Edi = 0; + SET_CFLAG( context ); break; case 0x0400: /* Get DPMI version */ @@ -483,6 +500,7 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) case 0x0401: /* Get DPMI Capabilities (1.0) */ FIXME( "get dpmi capabilities - unimplemented\n"); + SET_CFLAG( context ); break; case 0x0500: /* Get free memory information */ @@ -579,6 +597,7 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) case 0x0507: /* Set page attributes (1.0) */ FIXME( "set page attributes - unimplemented\n" ); + SET_CFLAG( context ); break; /* Just ignore it */ case 0x0600: /* Lock linear region */ @@ -625,6 +644,7 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) case 0x0800: /* Physical address mapping */ FIXME( "physical address mapping (0x%08lx) - unimplemented\n", MAKELONG(CX_reg(context),BX_reg(context)) ); + SET_CFLAG( context ); break; case 0x0900: /* Get and Disable Virtual Interrupt State */ @@ -674,11 +694,12 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) * BX bit B0 is new value for MPv. * BX bit B1 is new value for EMv. */ + TRACE( "Set Coprocessor Emulation to %d\n", BX_reg(context) ); if (BX_reg(context) != 1) - FIXME( "Set Coprocessor Emulation to %d - unimplemented\n", - BX_reg(context) ); - else - TRACE( "Set Coprocessor Emulation - ignored\n" ); + { + TRACE( "invalid request\n" ); + SET_CFLAG( context ); + } break; default: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7861
From: Pali Rohár <pali(a)kernel.org> According to the DPMI specification, when DPMI function 0102h (Resize DOS Memory Block) fails then AX has to be set to error code and BX to maximum possible block size. As the wine does not implement the DPMI function 0102h (Resize DOS Memory Block), set the error code to 0x0008 (insufficient memory) and maximum possible block size to 0 (no memory available). Signed-off-by: Pali Rohár <pali(a)kernel.org> --- dlls/krnl386.exe16/int31.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/krnl386.exe16/int31.c b/dlls/krnl386.exe16/int31.c index 45c9fd92c34..404a04a16b7 100644 --- a/dlls/krnl386.exe16/int31.c +++ b/dlls/krnl386.exe16/int31.c @@ -382,6 +382,8 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) case 0x0102: /* Resize DOS Memory Block */ FIXME( "resize DOS memory block (0x%04x, 0x%x paragraphs) - unimplemented\n", DX_reg(context), BX_reg(context) ); + SET_AX( context, 0x0008 ); /* insufficient memory */ + SET_BX( context, 0 ); /* maximum possible block size */ SET_CFLAG( context ); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7861
From: Pali Rohár <pali(a)kernel.org> According to the DPMI specification, DPMI function 0202h call (Get Processor Exception Handler Vector) sets the whole EDX register and not only its low DX part. Signed-off-by: Pali Rohár <pali(a)kernel.org> --- dlls/krnl386.exe16/int31.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/krnl386.exe16/int31.c b/dlls/krnl386.exe16/int31.c index 404a04a16b7..b0fb001ec7e 100644 --- a/dlls/krnl386.exe16/int31.c +++ b/dlls/krnl386.exe16/int31.c @@ -405,7 +405,7 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) FIXME( "Get Processor Exception Handler Vector (0x%02x) - not supported\n", BL_reg(context) ); SET_CX( context, 0 ); - SET_DX( context, 0 ); + context->Edx = 0; SET_CFLAG( context ); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7861
From: Pali Rohár <pali(a)kernel.org> Windows provides DPMI host according to DPMI version 0.9. Add cases for all missing DPMI 0.9 function calls into wine code, set Carry flag which indicates DPMI failure and mark them with appropriate FIXME comments. Signed-off-by: Pali Rohár <pali(a)kernel.org> --- dlls/krnl386.exe16/int31.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/dlls/krnl386.exe16/int31.c b/dlls/krnl386.exe16/int31.c index b0fb001ec7e..07f11b86d63 100644 --- a/dlls/krnl386.exe16/int31.c +++ b/dlls/krnl386.exe16/int31.c @@ -664,6 +664,31 @@ void WINAPI DOSVM_Int31Handler( CONTEXT *context ) SET_AL( context, 1 ); /* report that interrupts are always enabled */ break; + case 0x0a00: /* Get Vendor-Specific API Entry Point */ + FIXME( "Get Vendor-Specific API Entry Point - unimplemented\n" ); + SET_CFLAG( context ); + break; + + case 0x0b00: /* Set Debug Watchpoint */ + FIXME( "Set Debug Watchpoint - unimplemented\n" ); + SET_CFLAG( context ); + break; + + case 0x0b01: /* Clear Debug Watchpoint */ + FIXME( "Clear Debug Watchpoint - unimplemented\n" ); + SET_CFLAG( context ); + break; + + case 0x0b02: /* Get State of Debug Watchpoint */ + FIXME( "Get State of Debug Watchpoint - unimplemented\n" ); + SET_CFLAG( context ); + break; + + case 0x0b03: /* Reset Debug Watchpoint */ + FIXME( "Reset Debug Watchpoint - unimplemented\n" ); + SET_CFLAG( context ); + break; + case 0x0e00: /* Get Coprocessor Status (1.0) */ /* * Return status in AX bits: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7861
Are there actual apps that depend on these things? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101501
On Sun Apr 20 20:42:19 2025 +0000, Alexandre Julliard wrote:
Are there actual apps that depend on these things? Yes, this is known technique described in the Andrew Schulman book: Unauthorized Windows 95. kernel32.dll VxDCall function allows to call DPMI functions directly from native 32-bit applications. Code samples of VxDCall usage are part of the book. The DPMI physical address mapping function (as described in the commit message) is used for example by SysToolsLib or by WinIO or by lspci. All of them are checking for carry flag to figure out if the DPMI function call failed or not.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101524
On Sun Apr 20 20:42:19 2025 +0000, Pali Rohár wrote:
Yes, this is known technique described in the Andrew Schulman book: Unauthorized Windows 95. kernel32.dll VxDCall function allows to call DPMI functions directly from native 32-bit applications. Code samples of VxDCall usage are part of the book. The DPMI physical address mapping function (as described in the commit message) is used for example by SysToolsLib or by WinIO or by lspci. All of them are checking for carry flag to figure out if the DPMI function call failed or not. The problem is that it's not unlikely that some app currently works because we don't report failure. So we shouldn't change everything to report errors just because the spec says so. Only cases that demonstrably fix an actual app should be changed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101525
On Sun Apr 20 21:04:37 2025 +0000, Alexandre Julliard wrote:
The problem is that it's not unlikely that some app currently works because we don't report failure. So we shouldn't change everything to report errors just because the spec says so. Only cases that demonstrably fix an actual app should be changed. WinIO is expecting the carry flag to be set on DPMI failure, otherwise it will segfault on pseudo-random address dereference. SysToolsLib is also expecting the carry flag to be set on the failure. And lspci contains workaround for this "buggy" wine DPMI behavior. The workaround for windows version of lspci was added there by me because of this bug which was causing segfault. Note that I have also checked the Windows 7 behavior and it is also setting the carry flag for physical address mapping request (Windows 7 does not support DPMI physical address mapping functionality).
I highly doubt that some application can work when is asking for physical address mapping and wine returns bogus address instead of failure. If application is asking for physical address mapping then application would want to access that mapped address. And accessing the returned bogus address returned by wine will cause segfault of application. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101526
On Sun Apr 20 22:02:46 2025 +0000, Pali Rohár wrote:
WinIO is expecting the carry flag to be set on DPMI failure, otherwise it will segfault on pseudo-random address dereference. SysToolsLib is also expecting the carry flag to be set on the failure. And lspci contains workaround for this "buggy" wine DPMI behavior. The workaround for windows version of lspci was added there by me because of this bug which was causing segfault. Note that I have also checked the Windows 7 behavior and it is also setting the carry flag for physical address mapping request (Windows 7 does not support DPMI physical address mapping functionality). I highly doubt that some application can work when is asking for physical address mapping and wine returns bogus address instead of failure. If application is asking for physical address mapping then application would want to access that mapped address. And accessing the returned bogus address returned by wine will cause segfault of application. I understand your concern that a broad CF fixes might be required to run more apps.
However, you should try to fix *only* the specific function(s) that yoir application (game), as broad changes tend to cause regressions in all other apps. The broad fix could pass if there are (conformance) test cases for those functions, but we lack them, so we try to err on the safe side. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101527
On Sun Apr 20 22:43:56 2025 +0000, Jinoh Kang wrote:
I understand your concern that a broad CF fixes might be required to run more apps. However, you should try to fix *only* the specific function(s) that yoir application (game), as broad changes tend to cause regressions in all other apps. The broad fix could pass if there are (conformance) test cases for those functions, but we lack them, so we try to err on the safe side. Ok, could you help me which commit and how should I modify it to make it acceptable?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101644
On Tue Apr 22 20:08:09 2025 +0000, Pali Rohár wrote: > Ok, could you help me which commit and how should I modify it to make it acceptable? A few suggestions, take them with a grain of salt: * Open a bug report at https://bugs.winehq.org/ that describes one specific broken program and add `Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=<bug-number>` to each commit that is necessary to fix that program. * Replace the FIXMEs in the last commit with a single `ERR` in the `default` case of the switch. * Move some of the commits to separate MRs. It looks like the first, second, and last commits could be applied independently, so they could be submitted separately instead of asking for an all-or-nothing approval on all six commits. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101646
On Tue Apr 22 20:35:55 2025 +0000, Alex Henrie wrote:
A few suggestions, take them with a grain of salt: * Open a bug report at https://bugs.winehq.org/ that describes one specific broken program and add `Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=<bug-number>` to each commit that is necessary to fix that program. * Replace the FIXMEs in the last commit with a single `ERR` in the `default` case of the switch. * Move some of the commits to separate MRs. It looks like the first, second, and last commits could be applied independently, so they could be submitted separately instead of asking for an all-or-nothing approval on all six commits. Ok, I have split it into separate changes: !7902 !7903 !7904 !7905
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7861#note_101913
participants (4)
-
Alex Henrie (＠alexhenrie) -
Alexandre Julliard (＠julliard)
-
Jinoh Kang (＠iamahuman)
-
Pali Rohár