[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

@@ -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) */
+    /* This will load the ACPI driver (IO initialization will wait for this operation to finish) */
-    IoSynchronousInvalidateDeviceRelations(FdoExtension->PhysicalDeviceObject, BusRelations);
+    IoInvalidateDeviceRelations(FdoExtension->PhysicalDeviceObject, BusRelations);
 }
 
 NTSTATUS

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

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
 //

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

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();

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
     //

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(
+PipEnumerateDevice(
-    IN PDEVICE_OBJECT DeviceObject)
+    _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
+                status = PipEnumerateDevice(deviceNode);
-                IoSynchronousInvalidateDeviceRelations(Request->DeviceObject, BusRelations);
                 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;
+}

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);
 

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;
 }

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;