[VIDEOPRT] Fix INT10h support initialization (#8717)

This problem was noticed by Zombiedeth.

Addendum to commit 89d5a3dbb8 (PR #8451).

There exist buggy 3rd-party miniport drivers, like those for the NVIDIA GeForce
8400 GS card (`nv4_mini.sys` from `181.22_geforce_winxp_32bit_english_whql.exe`)
that invoke the VideoPort Int10 routines (either `VideoPortInt10()`, or the
INT10 interface via `VideoPortQueryServices()`) from their `HwFindAdapter()`
callback (invoked via `VideoPortInitialize()`) or from their `DriverEntry()`.

However, at this point, the VideoPort Int10 services are not fully initialized:
on the x86 port, the VDM memory isn't mapped until later, when the `\Device\VideoX`
is opened by Win32k as part of its initialization in the context of the CSRSS process.

Additionally on the x86 ReactOS port since commit 89d5a3dbb8 (PR #8451), the
X86 emulator is used by default, unless either a registry setting is set to
disable the emulator, or the HAL (e.g. the NT5.x HAL) doesn't export the X86
emulator routines; in these cases the VDM is employed instead.
In the NT5.x builds VideoPort imports the HAL X86 emulator routines at runtime,
since it has to handle their possible absence. However, this importing was
previously delayed up until VideoPort initialized the Int10 services (when
`\Device\VideoX` is opened the first time by Win32k from CSRSS), meaning,
these services and the X86 emulator imports weren't initialized until then.
When booting ReactOS in these conditions with the NVIDIA driver installed,
its too-early `VideoPortInt10()` call was invoking the X86 emulator with no
imports set up, which resulted in the invocation of a NULL function pointer,
leading to a BSOD.

## Proposed changes

- Make the existing `IntInitializeInt10()` function support two invocations:
  * When invoked the first time, it initializes the X86 emulator support, by
    importing the HAL emulator routines, if applicable.
  * When invoked the second+ time, it maps the VDM memory space (done when
    `Device\VideoX` is opened by Win32k/CSRSS initialization).

- Actually perform the first `IntInitializeInt10()` invocation in the
  `if (!FirstInitialization)` block of `VideoPortInitialize()`.

- Set a `VDMAddressSpaceInitialized` flag to TRUE only when the VDM memory
  space is mapped (during the second `IntInitializeInt10()` invocation).
  Check this flag in both `IntInt10CallBiosV86()` and `IntInt10CallBiosEmu()`
  (x86 build only).

- Adjust two returned status values in `IntInitializeVideoAddressSpace()`.
This commit is contained in:
Hermès Bélusca-Maïto
2025-12-13 23:17:32 +01:00
parent 3f9f0e2a96
commit f4786c1b91
3 changed files with 67 additions and 20 deletions

View File

@@ -388,7 +388,7 @@ IntVideoPortDispatchOpen(
Status = IntInitializeInt10();
if (!NT_SUCCESS(Status))
{
ERR_(VIDEOPRT, "IntInitializeInt10() failed: 0x%lx\n", Status);
ERR_(VIDEOPRT, "IntInitializeInt10(CSR) failed: 0x%lx\n", Status);
ObDereferenceObject(CsrProcess);
CsrProcess = NULL;
return Status;

View File

@@ -33,6 +33,9 @@
/* Use the 32-bit x86 emulator by default, on NT 6.x (Vista+), or on NT 5.x
* if the HAL has the necessary exports. Otherwise fall back to V86 mode. */
BOOLEAN VideoPortDisableX86Emulator = FALSE;
/* Tells whether the video address space has been initialized for VDM calls */
static BOOLEAN VDMAddressSpaceInitialized = FALSE;
#endif
KMUTEX VideoPortInt10Mutex;
@@ -171,7 +174,8 @@ IntInitializeVideoAddressSpace(VOID)
/* We should only do that for CSRSS */
ASSERT(PsGetCurrentProcess() == (PEPROCESS)CsrProcess);
/* Free the 1MB pre-reserved region. In reality, ReactOS should simply support us mapping the view into the reserved area, but it doesn't. */
/* Free the 1MB pre-reserved region. In reality, ReactOS should simply
* support us mapping the view into the reserved area, but it doesn't. */
BaseAddress = 0;
ViewSize = 1024 * 1024;
Status = ZwFreeVirtualMemory(NtCurrentProcess(),
@@ -181,7 +185,7 @@ IntInitializeVideoAddressSpace(VOID)
if (!NT_SUCCESS(Status))
{
DPRINT1("Couldn't unmap reserved memory (%x)\n", Status);
return 0;
return Status;
}
/* Open the physical memory section */
@@ -250,7 +254,7 @@ IntInitializeVideoAddressSpace(VOID)
{
DPRINT1("Failed to allocate virtual memory at right address (was %x)\n",
BaseAddress);
return 0;
return STATUS_UNSUCCESSFUL;
}
/* Get the real mode IVT and BDA from the kernel */
@@ -265,6 +269,7 @@ IntInitializeVideoAddressSpace(VOID)
ProtectLowV86Mem();
/* Return success */
VDMAddressSpaceInitialized = TRUE;
return STATUS_SUCCESS;
}
@@ -594,6 +599,19 @@ IntInt10CallBiosEmu(
UNREFERENCED_PARAMETER(Context);
#ifdef _M_IX86
if (!VDMAddressSpaceInitialized)
{
/* There are buggy 3rd-party miniport drivers that invoke the Int10 service
* in their initialization routine, before the VDM address space has been
* mapped and Ke386CallBios() can be called. They would therefore hit this
* ASSERT. Instead, disable it, log an error and bail out. */
//ASSERT(FALSE);
ERR_(VIDEOPRT, "Warning: Attempt to call %s before Int10 support is initialized.\n", __FUNCTION__);
return ERROR_INVALID_PARAMETER;
}
#endif
/* Clear the context and fill out the BIOS arguments */
RtlZeroMemory(&BiosContext, sizeof(BiosContext));
BiosContext.Eax = BiosArguments->Eax;
@@ -645,6 +663,17 @@ IntInt10CallBiosV86(
UNREFERENCED_PARAMETER(Context);
if (!VDMAddressSpaceInitialized)
{
/* There are buggy 3rd-party miniport drivers that invoke the Int10 service
* in their initialization routine, before the VDM address space has been
* mapped and Ke386CallBios() can be called. They would therefore hit this
* ASSERT. Instead, disable it, log an error and bail out. */
//ASSERT(FALSE);
ERR_(VIDEOPRT, "Warning: Attempt to call %s before Int10 support is initialized.\n", __FUNCTION__);
return ERROR_INVALID_PARAMETER;
}
/* Clear the context and fill out the BIOS arguments */
RtlZeroMemory(&BiosContext, sizeof(BiosContext));
BiosContext.Eax = BiosArguments->Eax;
@@ -736,27 +765,38 @@ static const INT10_INTERFACE *Int10Vtbl = &Int10IFace[0];
NTSTATUS
IntInitializeInt10(VOID)
{
#ifdef _M_IX86
/* We should only do that for CSRSS */
ASSERT(PsGetCurrentProcess() == (PEPROCESS)CsrProcess);
static BOOLEAN FirstInitialization = FALSE;
/* Initialize the x86 emulator if necessary, otherwise fall back to V86 mode */
if (!VideoPortDisableX86Emulator)
if (!FirstInitialization)
{
/* Use the emulation routines */
//Int10Vtbl = &Int10IFace[0];
if (IntInitializeX86Emu())
return STATUS_SUCCESS;
DPRINT1("Could not initialize the x86 emulator; falling back to V86 mode\n");
VideoPortDisableX86Emulator = TRUE;
FirstInitialization = TRUE;
#ifdef _M_IX86
/* Initialize the x86 emulator if necessary, otherwise fall back to V86 mode */
if (!VideoPortDisableX86Emulator)
{
/* Use the emulation routines */
//Int10Vtbl = &Int10IFace[0];
if (IntInitializeX86Emu())
return STATUS_SUCCESS;
DPRINT1("Could not initialize the x86 emulator; falling back to V86 mode\n");
VideoPortDisableX86Emulator = TRUE;
}
/* Fall back to the V86 routines */
Int10Vtbl = &Int10IFace[1];
return STATUS_SUCCESS;
#else
/* Initialize the x86 emulator */
return (IntInitializeX86Emu() ? STATUS_SUCCESS : STATUS_NOT_SUPPORTED);
#endif
}
/* Fall back to the V86 routines */
Int10Vtbl = &Int10IFace[1];
/* We should only do that for CSRSS */
ASSERT(PsGetCurrentProcess() == (PEPROCESS)CsrProcess);
#ifdef _M_IX86
return IntInitializeVideoAddressSpace();
#else
/* Initialize the x86 emulator */
return (IntInitializeX86Emu() ? STATUS_SUCCESS : STATUS_NOT_SUPPORTED);
return STATUS_SUCCESS;
#endif
}

View File

@@ -940,7 +940,7 @@ VideoPortInitialize(
NTSTATUS Status;
PVIDEO_PORT_DRIVER_EXTENSION DriverExtension;
BOOLEAN PnpDriver = FALSE, LegacyDetection = FALSE;
static BOOLEAN FirstInitialization;
static BOOLEAN FirstInitialization = FALSE;
TRACE_(VIDEOPRT, "VideoPortInitialize\n");
@@ -951,6 +951,13 @@ VideoPortInitialize(
KeInitializeMutex(&VgaSyncLock, 0);
KeInitializeSpinLock(&HwResetAdaptersLock);
IntLoadRegistryParameters();
Status = IntInitializeInt10();
if (!NT_SUCCESS(Status))
{
ERR_(VIDEOPRT, "IntInitializeInt10(FirstInit) failed: 0x%lx\n", Status);
// return Status; // Let's continue and hope for the best...
}
}
/* As a first thing do parameter checks. */