[reactos] 02/04: [NTOS:IO] Enumerate devices only inside the PipDeviceActionWorker Introduce the PiPerformSyncDeviceAction routine for queuing synchronous device actions Change all kernel code to use PiPerformSyncDeviceAction and PiQueueDeviceAction for device enumeration
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2839c850927ce52e59673d...
commit 2839c850927ce52e59673dd9b04ffd19c96d44c5 Author: Victor Perevertkin victor.perevertkin@reactos.org AuthorDate: Wed Jun 24 19:34:44 2020 +0300 Commit: Victor Perevertkin victor.perevertkin@reactos.org CommitDate: Fri Aug 21 08:02:26 2020 +0300
[NTOS:IO] Enumerate devices only inside the PipDeviceActionWorker Introduce the PiPerformSyncDeviceAction routine for queuing synchronous device actions Change all kernel code to use PiPerformSyncDeviceAction and PiQueueDeviceAction for device enumeration
CORE-10456 --- hal/halx86/acpi/halpnpdd.c | 4 +- hal/halx86/legacy/halpnpdd.c | 2 +- ntoskrnl/include/internal/io.h | 10 ++-- ntoskrnl/io/iomgr/driver.c | 12 +++++ ntoskrnl/io/iomgr/iomgr.c | 3 -- ntoskrnl/io/iomgr/ramdisk.c | 10 ++++ ntoskrnl/io/pnpmgr/devaction.c | 102 +++++++++++++++++++++++++++++++++++------ ntoskrnl/io/pnpmgr/plugplay.c | 2 +- ntoskrnl/io/pnpmgr/pnpinit.c | 7 +++ ntoskrnl/io/pnpmgr/pnpmgr.c | 13 +++++- 10 files changed, 139 insertions(+), 26 deletions(-)
diff --git a/hal/halx86/acpi/halpnpdd.c b/hal/halx86/acpi/halpnpdd.c index cc30284d513..0719e7ebcb0 100644 --- a/hal/halx86/acpi/halpnpdd.c +++ b/hal/halx86/acpi/halpnpdd.c @@ -99,8 +99,8 @@ HalpReportDetectedDevices(IN PDRIVER_OBJECT DriverObject, DPRINT1("You have an ACPI Watchdog. That's great! You should be proud ;-)\n"); }
- /* This will synchronously load the ACPI driver (needed because we're critical for boot) */ - IoSynchronousInvalidateDeviceRelations(FdoExtension->PhysicalDeviceObject, BusRelations); + /* This will load the ACPI driver (IO initialization will wait for this operation to finish) */ + IoInvalidateDeviceRelations(FdoExtension->PhysicalDeviceObject, BusRelations); }
NTSTATUS diff --git a/hal/halx86/legacy/halpnpdd.c b/hal/halx86/legacy/halpnpdd.c index 084ae866add..5f84e03a684 100644 --- a/hal/halx86/legacy/halpnpdd.c +++ b/hal/halx86/legacy/halpnpdd.c @@ -91,7 +91,7 @@ HalpReportDetectedDevices(IN PDRIVER_OBJECT DriverObject, PdoDeviceObject->Flags &= ~DO_DEVICE_INITIALIZING;
/* Invalidate device relations since we added a new device */ - IoSynchronousInvalidateDeviceRelations(FdoExtension->PhysicalDeviceObject, BusRelations); + IoInvalidateDeviceRelations(FdoExtension->PhysicalDeviceObject, BusRelations); }
NTSTATUS diff --git a/ntoskrnl/include/internal/io.h b/ntoskrnl/include/internal/io.h index 2d3c53fec24..4e810bc34fe 100644 --- a/ntoskrnl/include/internal/io.h +++ b/ntoskrnl/include/internal/io.h @@ -687,11 +687,6 @@ IopActionInitChildServices( IN PVOID Context );
-NTSTATUS -IopEnumerateDevice( - IN PDEVICE_OBJECT DeviceObject -); - NTSTATUS IoCreateDriverList( VOID @@ -1424,6 +1419,11 @@ PiQueueDeviceAction( _In_opt_ PKEVENT CompletionEvent, _Out_opt_ NTSTATUS *CompletionStatus);
+NTSTATUS +PiPerformSyncDeviceAction( + _In_ PDEVICE_OBJECT DeviceObject, + _In_ DEVICE_ACTION Action); + // // Global I/O Data // diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c index 8e0bea4092f..8727c26330f 100644 --- a/ntoskrnl/io/iomgr/driver.c +++ b/ntoskrnl/io/iomgr/driver.c @@ -35,6 +35,8 @@ POBJECT_TYPE IoDriverObjectType = NULL;
extern BOOLEAN ExpInTextModeSetup; extern BOOLEAN PnpSystemInit; +extern BOOLEAN PnPBootDriversLoaded; +extern KEVENT PiEnumerationFinished;
USHORT IopGroupIndex; PLIST_ENTRY IopGroupTable; @@ -1125,6 +1127,13 @@ IopInitializeBootDrivers(VOID) } }
+ /* HAL Root Bus is being initialized before loading the boot drivers so this may cause issues + * when some devices are not being initialized with their drivers. This flag is used to delay + * all actions with devices (except PnP root device) until boot drivers are loaded. + * See PiQueueDeviceAction function + */ + PnPBootDriversLoaded = TRUE; + /* In old ROS, the loader list became empty after this point. Simulate. */ InitializeListHead(&KeLoaderBlock->LoadOrderListHead); } @@ -1478,6 +1487,9 @@ IopReinitializeBootDrivers(VOID) Entry = ExInterlockedRemoveHeadList(&DriverBootReinitListHead, &DriverBootReinitListLock); } + + /* Wait for all device actions being finished*/ + KeWaitForSingleObject(&PiEnumerationFinished, Executive, KernelMode, FALSE, NULL); }
NTSTATUS diff --git a/ntoskrnl/io/iomgr/iomgr.c b/ntoskrnl/io/iomgr/iomgr.c index 2aadf7a7502..d815dda0c3b 100644 --- a/ntoskrnl/io/iomgr/iomgr.c +++ b/ntoskrnl/io/iomgr/iomgr.c @@ -581,9 +581,6 @@ IoInitSystem(IN PLOADER_PARAMETER_BLOCK LoaderBlock) return FALSE; }
- /* Initialize PnP root relations */ - IopEnumerateDevice(IopRootDeviceNode->PhysicalDeviceObject); - #if !defined(_WINKD_) && defined(KDBG) /* Read KDB Data */ KdbInit(); diff --git a/ntoskrnl/io/iomgr/ramdisk.c b/ntoskrnl/io/iomgr/ramdisk.c index 67ec971982a..fd78ddf4601 100644 --- a/ntoskrnl/io/iomgr/ramdisk.c +++ b/ntoskrnl/io/iomgr/ramdisk.c @@ -14,6 +14,10 @@ #define NDEBUG #include <debug.h>
+/* GLOBALS *******************************************************************/ + +extern KEVENT PiEnumerationFinished; + /* DATA ***********************************************************************/
#if defined (ALLOC_PRAGMA) @@ -268,6 +272,12 @@ IopStartRamdisk(IN PLOADER_PARAMETER_BLOCK LoaderBlock) IoCreateSymbolicLink(&DriveLetter, &DeviceString); }
+ // + // Wait for ramdisk relations being initialized + // + + KeWaitForSingleObject(&PiEnumerationFinished, Executive, KernelMode, FALSE, NULL); + // // We made it // diff --git a/ntoskrnl/io/pnpmgr/devaction.c b/ntoskrnl/io/pnpmgr/devaction.c index 75390c47cbd..77b9990f35b 100644 --- a/ntoskrnl/io/pnpmgr/devaction.c +++ b/ntoskrnl/io/pnpmgr/devaction.c @@ -5,6 +5,22 @@ * COPYRIGHT: Casper S. Hornstrup (chorns@users.sourceforge.net) * 2007 Hervé Poussineau (hpoussin@reactos.org) * 2014-2017 Thomas Faber (thomas.faber@reactos.org) + * 2020 Victor Perevertkin (victor.perevertkin@reactos.org) + */ + +/* Device tree is a resource shared among all system services: hal, kernel, drivers etc. + * Thus all code which interacts with the tree needs to be synchronized. + * Here it's done via a list of DEVICE_ACTION_REQUEST structures, which represents + * the device action queue. It is being processed exclusively by the PipDeviceActionWorker. + * + * Operation queuing can be done with the PiQueueDeviceAction function or with + * the PiPerfomSyncDeviceAction for synchronous operations. + * All device manipulation like starting, removing, enumeration (see DEVICE_ACTION enum) + * have to be done with the PiQueueDeviceAction in order to avoid race conditions. + * + * Note: there is one special operation here - PiActionEnumRootDevices. It is meant to be done + * during initialization process (and be the first device tree operation executed) and + * is always executed synchronously. */
/* INCLUDES ******************************************************************/ @@ -18,6 +34,7 @@ extern ERESOURCE IopDriverLoadResource; extern BOOLEAN PnpSystemInit; extern PDEVICE_NODE IopRootDeviceNode; +extern BOOLEAN PnPBootDriversLoaded;
#define MAX_DEVICE_ID_LEN 200 #define MAX_SEPARATORS_INSTANCEID 0 @@ -29,6 +46,7 @@ LIST_ENTRY IopDeviceActionRequestList; WORK_QUEUE_ITEM IopDeviceActionWorkItem; BOOLEAN IopDeviceActionInProgress; KSPIN_LOCK IopDeviceActionLock; +KEVENT PiEnumerationFinished;
/* TYPES *********************************************************************/
@@ -1349,7 +1367,7 @@ IopStartAndEnumerateDevice(IN PDEVICE_NODE DeviceNode) (DeviceNode->Flags & DNF_NEED_ENUMERATION_ONLY)) { /* Enumerate us */ - IoSynchronousInvalidateDeviceRelations(DeviceObject, BusRelations); + IoInvalidateDeviceRelations(DeviceObject, BusRelations); Status = STATUS_SUCCESS; } else @@ -2060,12 +2078,11 @@ IopInitializePnpServices(IN PDEVICE_NODE DeviceNode) return IopTraverseDeviceTree(&Context); }
- +static NTSTATUS -IopEnumerateDevice( - IN PDEVICE_OBJECT DeviceObject) +PipEnumerateDevice( + _In_ PDEVICE_NODE DeviceNode) { - PDEVICE_NODE DeviceNode = IopGetDeviceNode(DeviceObject); DEVICETREE_TRAVERSE_CONTEXT Context; PDEVICE_RELATIONS DeviceRelations; PDEVICE_OBJECT ChildDeviceObject; @@ -2075,13 +2092,11 @@ IopEnumerateDevice( NTSTATUS Status; ULONG i;
- DPRINT("DeviceObject 0x%p\n", DeviceObject); - if (DeviceNode->Flags & DNF_NEED_ENUMERATION_ONLY) { DeviceNode->Flags &= ~DNF_NEED_ENUMERATION_ONLY;
- DPRINT("Sending GUID_DEVICE_ARRIVAL\n"); + DPRINT("Sending GUID_DEVICE_ARRIVAL %wZ\n", &DeviceNode->InstancePath); IopQueueTargetDeviceEvent(&GUID_DEVICE_ARRIVAL, &DeviceNode->InstancePath); } @@ -2091,11 +2106,11 @@ IopEnumerateDevice( Stack.Parameters.QueryDeviceRelations.Type = BusRelations;
Status = IopInitiatePnpIrp( - DeviceObject, + DeviceNode->PhysicalDeviceObject, &IoStatusBlock, IRP_MN_QUERY_DEVICE_RELATIONS, &Stack); - if (!NT_SUCCESS(Status) || Status == STATUS_PENDING) + if (!NT_SUCCESS(Status)) { DPRINT("IopInitiatePnpIrp() failed with status 0x%08lx\n", Status); return Status; @@ -2344,23 +2359,46 @@ PipDeviceActionWorker( KeReleaseSpinLock(&IopDeviceActionLock, OldIrql); Request = CONTAINING_RECORD(ListEntry, DEVICE_ACTION_REQUEST, RequestListEntry);
+ ASSERT(Request->DeviceObject); + + PDEVICE_NODE deviceNode = IopGetDeviceNode(Request->DeviceObject); + ASSERT(deviceNode); + + NTSTATUS status = STATUS_SUCCESS; + + DPRINT("Processing PnP request %p: DeviceObject - %p, Action - %s\n", + Request, Request->DeviceObject, ActionToStr(Request->Action)); + switch (Request->Action) { + case PiActionEnumRootDevices: case PiActionEnumDeviceTree: - // this will be reworked in next commits - IoSynchronousInvalidateDeviceRelations(Request->DeviceObject, BusRelations); + status = PipEnumerateDevice(deviceNode); break;
default: DPRINT1("Unimplemented device action %u\n", Request->Action); + status = STATUS_NOT_IMPLEMENTED; break; }
+ if (Request->CompletionStatus) + { + *Request->CompletionStatus = status; + } + + if (Request->CompletionEvent) + { + KeSetEvent(Request->CompletionEvent, IO_NO_INCREMENT, FALSE); + } + + DPRINT("Finished processing PnP request %p\n", Request); ObDereferenceObject(Request->DeviceObject); ExFreePoolWithTag(Request, TAG_IO); KeAcquireSpinLock(&IopDeviceActionLock, &OldIrql); } IopDeviceActionInProgress = FALSE; + KeSetEvent(&PiEnumerationFinished, IO_NO_INCREMENT, FALSE); KeReleaseSpinLock(&IopDeviceActionLock, OldIrql); }
@@ -2402,14 +2440,52 @@ PiQueueDeviceAction(
KeAcquireSpinLock(&IopDeviceActionLock, &OldIrql); InsertTailList(&IopDeviceActionRequestList, &Request->RequestListEntry); - if (IopDeviceActionInProgress) + + if (Action == PiActionEnumRootDevices) + { + ASSERT(!IopDeviceActionInProgress); + + IopDeviceActionInProgress = TRUE; + KeClearEvent(&PiEnumerationFinished); + KeReleaseSpinLock(&IopDeviceActionLock, OldIrql); + + PipDeviceActionWorker(NULL); + return; + } + + if (IopDeviceActionInProgress || !PnPBootDriversLoaded) { KeReleaseSpinLock(&IopDeviceActionLock, OldIrql); return; } IopDeviceActionInProgress = TRUE; + KeClearEvent(&PiEnumerationFinished); KeReleaseSpinLock(&IopDeviceActionLock, OldIrql);
ExInitializeWorkItem(&IopDeviceActionWorkItem, PipDeviceActionWorker, NULL); ExQueueWorkItem(&IopDeviceActionWorkItem, DelayedWorkQueue); } + +/** + * @brief Perfom a device operation synchronously via PiQueueDeviceAction + * + * @param[in] DeviceObject The device object + * @param[in] Action The action + * + * @return Status of the operation + */ + +NTSTATUS +PiPerformSyncDeviceAction( + _In_ PDEVICE_OBJECT DeviceObject, + _In_ DEVICE_ACTION Action) +{ + KEVENT opFinished; + NTSTATUS status; + + KeInitializeEvent(&opFinished, SynchronizationEvent, FALSE); + PiQueueDeviceAction(DeviceObject, Action, &opFinished, &status); + KeWaitForSingleObject(&opFinished, Executive, KernelMode, FALSE, NULL); + + return status; +} diff --git a/ntoskrnl/io/pnpmgr/plugplay.c b/ntoskrnl/io/pnpmgr/plugplay.c index 96bac114d27..968c6804b86 100644 --- a/ntoskrnl/io/pnpmgr/plugplay.c +++ b/ntoskrnl/io/pnpmgr/plugplay.c @@ -215,7 +215,7 @@ IopPnpEnumerateDevice(PPLUGPLAY_CONTROL_ENUMERATE_DEVICE_DATA DeviceData) return STATUS_NO_SUCH_DEVICE; }
- Status = IopEnumerateDevice(DeviceObject); + Status = PiPerformSyncDeviceAction(DeviceObject, PiActionEnumDeviceTree);
ObDereferenceObject(DeviceObject);
diff --git a/ntoskrnl/io/pnpmgr/pnpinit.c b/ntoskrnl/io/pnpmgr/pnpinit.c index a32192bcc39..b4c99683609 100644 --- a/ntoskrnl/io/pnpmgr/pnpinit.c +++ b/ntoskrnl/io/pnpmgr/pnpinit.c @@ -23,6 +23,7 @@ typedef struct _IOPNP_DEVICE_EXTENSION PUNICODE_STRING PiInitGroupOrderTable; USHORT PiInitGroupOrderTableCount; INTERFACE_TYPE PnpDefaultInterfaceType; +BOOLEAN PnPBootDriversLoaded = FALSE;
ARBITER_INSTANCE IopRootBusNumberArbiter; ARBITER_INSTANCE IopRootIrqArbiter; @@ -30,6 +31,8 @@ ARBITER_INSTANCE IopRootDmaArbiter; ARBITER_INSTANCE IopRootMemArbiter; ARBITER_INSTANCE IopRootPortArbiter;
+extern KEVENT PiEnumerationFinished; + NTSTATUS NTAPI IopPortInitialize(VOID); NTSTATUS NTAPI IopMemInitialize(VOID); NTSTATUS NTAPI IopDmaInitialize(VOID); @@ -438,6 +441,7 @@ IopInitializePlugPlayServices(VOID) KeInitializeSpinLock(&IopDeviceTreeLock); KeInitializeSpinLock(&IopDeviceActionLock); InitializeListHead(&IopDeviceActionRequestList); + KeInitializeEvent(&PiEnumerationFinished, NotificationEvent, TRUE);
/* Get the default interface */ PnpDefaultInterfaceType = IopDetermineDefaultInterfaceType(); @@ -597,6 +601,9 @@ IopInitializePlugPlayServices(VOID) /* Close the handle to the control set */ NtClose(KeyHandle);
+ /* Initialize PnP root relations (this is a syncronous operation) */ + PiQueueDeviceAction(IopRootDeviceNode->PhysicalDeviceObject, PiActionEnumRootDevices, NULL, NULL); + /* We made it */ return STATUS_SUCCESS; } diff --git a/ntoskrnl/io/pnpmgr/pnpmgr.c b/ntoskrnl/io/pnpmgr/pnpmgr.c index 443c3ab7b35..7ed3eee62a0 100644 --- a/ntoskrnl/io/pnpmgr/pnpmgr.c +++ b/ntoskrnl/io/pnpmgr/pnpmgr.c @@ -18,6 +18,7 @@ ERESOURCE PpRegistryDeviceResource; KGUARDED_MUTEX PpDeviceReferenceTableLock; RTL_AVL_TABLE PpDeviceReferenceTable; +BOOLEAN PnPBootDriversLoaded;
extern ULONG ExpInitializationPhase;
@@ -2474,6 +2475,11 @@ IoInvalidateDeviceRelations( IN PDEVICE_OBJECT DeviceObject, IN DEVICE_RELATION_TYPE Type) { + if (!IopIsValidPhysicalDeviceObject(DeviceObject)) + { + KeBugCheckEx(PNP_DETECTED_FATAL_ERROR, 0x2, (ULONG_PTR)DeviceObject, 0, 0); + } + switch (Type) { case BusRelations: @@ -2497,11 +2503,16 @@ IoSynchronousInvalidateDeviceRelations( { PAGED_CODE();
+ if (!IopIsValidPhysicalDeviceObject(DeviceObject)) + { + KeBugCheckEx(PNP_DETECTED_FATAL_ERROR, 0x2, (ULONG_PTR)DeviceObject, 0, 0); + } + switch (Type) { case BusRelations: /* Enumerate the device */ - return IopEnumerateDevice(DeviceObject); + return PiPerformSyncDeviceAction(DeviceObject, PiActionEnumDeviceTree); case PowerRelations: /* Not handled yet */ return STATUS_NOT_IMPLEMENTED;
-
Victor Perevertkin