Changeset 36060 in vbox

Timestamp:

Feb 23, 2011 11:20:04 AM (14 years ago)

Author:

vboxsync

Message:

VBoxGuest-win: PNP cleanup: process only required PNP IRPs according to MSDN.

Location:

trunk/src/VBox/Additions/common/VBoxGuest

Files:

• Makefile.kmk (modified) (1 diff)
• VBoxGuest-win-pnp.cpp (modified) (7 diffs)
• VBoxGuest-win.cpp (modified) (6 diffs)
• VBoxGuest-win.h (modified) (2 diffs)

trunk/src/VBox/Additions/common/VBoxGuest/Makefile.kmk

@@ -44 +44 @@
   VBoxGuest_DEFS.win    += VBOX_WITH_GUEST_BUGCHECK_DETECTION
  endif
+ #VBoxGuest_DEFS.win     += LOG_ENABLED LOG_TO_BACKDOOR
  VBoxGuest_DEPS.solaris += $(VBOX_SVN_REV_KMK)
  VBoxGuest_DEPS.linux   += $(VBOX_SVN_REV_HEADER)

trunk/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win-pnp.cpp

@@ -154 +154 @@
         {
             Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: START_DEVICE\n"));
+
+            /* This must be handled first by the lower driver. */
             rc = vboxguestwinSendIrpSynchronously(pDevExt->win.s.pNextLowerDriver, pIrp, TRUE);
 
@@ -184 +186 @@
         }
 
-        case IRP_MN_QUERY_PNP_DEVICE_STATE:
-        {
-            Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: QUERY_PNP_DEVICE_STATE\n"));
-            break;
-        }
-
         case IRP_MN_CANCEL_REMOVE_DEVICE:
         {
             Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: CANCEL_REMOVE_DEVICE\n"));
-            break;
+
+            /* This must be handled first by the lower driver. */
+            rc = vboxguestwinSendIrpSynchronously(pDevExt->win.s.pNextLowerDriver, pIrp, TRUE);
+
+            if (NT_SUCCESS(rc) && pDevExt->win.s.devState == PENDINGREMOVE)
+            {
+                /* Return to the state prior to receiving the IRP_MN_QUERY_REMOVE_DEVICE request. */
+                pDevExt->win.s.devState = pDevExt->win.s.prevDevState;
+            }
+
+            /* Complete the IRP. */
+            break;
+        }
+
+        case IRP_MN_SURPRISE_REMOVAL:
+        {
+            Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: IRP_MN_SURPRISE_REMOVAL\n"));
+
+            VBOXGUEST_UPDATE_DEVSTATE(pDevExt, SURPRISEREMOVED);
+
+            /* Do nothing here actually. Cleanup is done in IRP_MN_REMOVE_DEVICE. 
+             * This request is not expected for VBoxGuest.
+             */
+            LogRel(("VBoxGuest: unexpected device removal\n"));
+
+            /* Pass to the lower driver. */
+            pIrp->IoStatus.Status = STATUS_SUCCESS;
+
+            IoSkipCurrentIrpStackLocation(pIrp);
+
+            rc = IoCallDriver(pDevExt->win.s.pNextLowerDriver, pIrp);
+
+            /* Do not complete the IRP. */
+            return rc;
         }
 
@@ -201 +230 @@
 
 #ifdef VBOX_REBOOT_ON_UNINSTALL
-            /* The device can not be removed without a reboot. */
-            if (pDevExt->win.s.devState == WORKING)
-            {
-                Log(("VBoxGuest::vboxguestwinGuestPnp: QUERY_REMOVE_DEVICE: Device cannot be removed without a reboot!\n"));
-                pDevExt->win.s.devState = PENDINGREMOVE;
-            }
+            Log(("VBoxGuest::vboxguestwinGuestPnp: QUERY_REMOVE_DEVICE: Device cannot be removed without a reboot.\n"));
             rc = STATUS_UNSUCCESSFUL;
-            Log(("VBoxGuest::vboxguestwinGuestPnp: QUERY_REMOVE_DEVICE: Refuse with rc = %Rrc\n", rc));
-#else
-            if (pDevExt->win.s.devState == WORKING)
-                pDevExt->win.s.devState = PENDINGREMOVE;
 #endif /* VBOX_REBOOT_ON_UNINSTALL */
+
+            if (NT_SUCCESS(rc))
+            {
+                VBOXGUEST_UPDATE_DEVSTATE(pDevExt, PENDINGREMOVE);
+
+                /* This IRP passed down to lower driver. */
+                pIrp->IoStatus.Status = STATUS_SUCCESS;
+
+                rc = vboxguestwinSendIrpSynchronously(pDevExt->win.s.pNextLowerDriver, pIrp, TRUE);
+
+                /* Do not complete the IRP. */
+                return rc;
+            }
+
+            /* Complete the IRP on failure. */
             break;
         }
@@ -219 +254 @@
         {
             Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: REMOVE_DEVICE\n"));
-            if (pDevExt->win.s.devState == PENDINGREMOVE)
-            {
-                rc = vboxguestwinCleanup(pDevObj);
-                if (NT_SUCCESS(rc))
-                {
-                    /*
-                     * We need to send the remove down the stack before we detach,
-                     * but we don't need to wait for the completion of this operation
-                     * (and to register a completion routine).
-                     */
-                    pIrp->IoStatus.Status = STATUS_SUCCESS;
-                    IoSkipCurrentIrpStackLocation(pIrp);
-
-                    if (pDevExt->win.s.pNextLowerDriver != NULL)
-                    {
-                        rc = IoCallDriver(pDevExt->win.s.pNextLowerDriver, pIrp);
-                        IoDetachDevice(pDevExt->win.s.pNextLowerDriver);
-
-                        Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Next lower driver replied rc = 0x%x\n", rc));
-                    }
-
-                    Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Removing device ...\n"));
-
-                    /* Remove DOS device + symbolic link. */
-                    UNICODE_STRING win32Name;
-                    RtlInitUnicodeString(&win32Name, VBOXGUEST_DEVICE_NAME_DOS);
-                    IoDeleteSymbolicLink(&win32Name);
-
-                    pDevExt->win.s.devState = REMOVED;
-
-                    Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Deleting device ...\n"));
-
-                    /* Last action: Delete our device! pDevObj is *not* failed
-                     * anymore after this call! */
-                    IoDeleteDevice(pDevObj);
-
-                    Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Device removed!\n"));
-                    return rc; /* Make sure that we don't do anything below here anymore! */
-                }
-                else
-                    Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Error while cleaning up, rc = 0x%x\n", rc));
-            }
-            else
-                Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Devices state is not PENDINGREMOVE but %d\n",
-                     pDevExt->win.s.devState));
-            break;
+
+            VBOXGUEST_UPDATE_DEVSTATE(pDevExt, REMOVED);
+
+            /* Free hardware resources. */
+            /* @todo this should actually free I/O ports, interrupts, etc. */
+            rc = vboxguestwinCleanup(pDevObj);
+            Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: vboxguestwinCleanup rc = 0x%08X\n", rc));
+
+            /*
+             * We need to send the remove down the stack before we detach,
+             * but we don't need to wait for the completion of this operation
+             * (and to register a completion routine).
+             */
+            pIrp->IoStatus.Status = STATUS_SUCCESS;
+
+            IoSkipCurrentIrpStackLocation(pIrp);
+
+            rc = IoCallDriver(pDevExt->win.s.pNextLowerDriver, pIrp);
+            Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Next lower driver replied rc = 0x%x\n", rc));
+
+            IoDetachDevice(pDevExt->win.s.pNextLowerDriver);
+
+            Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Removing device ...\n"));
+
+            /* Destroy device extension and clean up everything else. */
+            VBoxGuestDeleteDevExt(pDevExt);
+
+            /* Remove DOS device + symbolic link. */
+            UNICODE_STRING win32Name;
+            RtlInitUnicodeString(&win32Name, VBOXGUEST_DEVICE_NAME_DOS);
+            IoDeleteSymbolicLink(&win32Name);
+
+            Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Deleting device ...\n"));
+
+            /* Last action: Delete our device! pDevObj is *not* failed
+             * anymore after this call! */
+            IoDeleteDevice(pDevObj);
+
+            Log(("VBoxGuest::vboxguestwinGuestPnp: REMOVE_DEVICE: Device removed!\n"));
+
+            /* Propagating rc from IoCallDriver. */
+            return rc; /* Make sure that we don't do anything below here anymore! */
         }
 
@@ -270 +301 @@
         {
             Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: CANCEL_STOP_DEVICE\n"));
+
+            /* This must be handled first by the lower driver. */
+            rc = vboxguestwinSendIrpSynchronously(pDevExt->win.s.pNextLowerDriver, pIrp, TRUE);
+
+            if (NT_SUCCESS(rc) && pDevExt->win.s.devState == PENDINGSTOP)
+            {
+                /* Return to the state prior to receiving the IRP_MN_QUERY_STOP_DEVICE request. */
+                pDevExt->win.s.devState = pDevExt->win.s.prevDevState;
+            }
+
+            /* Complete the IRP. */
             break;
         }
@@ -276 +318 @@
         {
             Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: QUERY_STOP_DEVICE\n"));
-            if (pDevExt->win.s.devState == WORKING)
-                pDevExt->win.s.devState = PENDINGSTOP;
+
 #ifdef VBOX_REBOOT_ON_UNINSTALL
-            Log(("VBoxGuest::vboxguestwinGuestPnp: QUERY_STOP_DEVICE: Device cannot be stopped!\n"));
-
-            /* The device can not be stopped without a reboot. */
+            Log(("VBoxGuest::vboxguestwinGuestPnp: QUERY_STOP_DEVICE: Device cannot be stopped without a reboot!\n"));
             pIrp->IoStatus.Status = STATUS_UNSUCCESSFUL;
-            Log(("VBoxGuest::vboxguestwinGuestPnp: QUERY_STOP_DEVICE: Refuse with rc = 0x%x\n", rc));
 #endif /* VBOX_REBOOT_ON_UNINSTALL */
+
+            if (NT_SUCCESS(rc))
+            {
+                VBOXGUEST_UPDATE_DEVSTATE(pDevExt, PENDINGSTOP);
+
+                /* This IRP passed down to lower driver. */
+                pIrp->IoStatus.Status = STATUS_SUCCESS;
+
+                rc = vboxguestwinSendIrpSynchronously(pDevExt->win.s.pNextLowerDriver, pIrp, TRUE);
+
+                /* Do not complete the IRP. */
+                return rc;
+            }
+
+            /* Complete the IRP on failure. */
             break;
         }
@@ -292 +345 @@
             Log(("VBoxGuest::vboxguestwinVBoxGuestPnP: STOP_DEVICE\n"));
 
-            if (pDevExt->win.s.devState == PENDINGSTOP)
-            {
-                rc = vboxguestwinCleanup(pDevObj);
-                if (NT_SUCCESS(rc))
-                {
-                    pDevExt->win.s.devState = STOPPED;
-                    IoInvalidateDeviceState(pDevObj);
-                    Log(("VBoxGuest::vboxguestwinGuestPnp: STOP_DEVICE: Device has been disabled\n"));
-                }
-                else
-                    Log(("VBoxGuest::vboxguestwinGuestPnp: STOP_DEVICE: Error while cleaning up, rc = 0x%x\n", rc));
-            }
-            else
-                Log(("VBoxGuest::vboxguestwinGuestPnp: STOP_DEVICE: Devices state is not PENDINGSTOP but %d\n",
-                     pDevExt->win.s.devState));
-            break;
+            VBOXGUEST_UPDATE_DEVSTATE(pDevExt, STOPPED);
+
+            /* Free hardware resources. */
+            /* @todo this should actually free I/O ports, interrupts, etc. */
+            rc = vboxguestwinCleanup(pDevObj);
+            Log(("VBoxGuest::vboxguestwinGuestPnp: STOP_DEVICE: cleaning up, rc = 0x%x\n", rc));
+
+            /* Pass to the lower driver. */
+            pIrp->IoStatus.Status = STATUS_SUCCESS;
+
+            IoSkipCurrentIrpStackLocation(pIrp);
+
+            rc = IoCallDriver(pDevExt->win.s.pNextLowerDriver, pIrp);
+            Log(("VBoxGuest::vboxguestwinGuestPnp: STOP_DEVICE: Next lower driver replied rc = 0x%x\n", rc));
+
+            return rc;
         }
 

trunk/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.cpp

@@ -223 +223 @@
 
             pDevExt->win.s.pDeviceObject = pDeviceObject;
+            pDevExt->win.s.prevDevState = STOPPED;
             pDevExt->win.s.devState = STOPPED;
 
@@ -501 +502 @@
         /* Ready to rumble! */
         Log(("VBoxGuest::vboxguestwinInit: Device is ready!\n"));
-        pDevExt->win.s.devState = WORKING;
+        VBOXGUEST_UPDATE_DEVSTATE(pDevExt, WORKING);
     }
     else
@@ -514 +515 @@
 
 /**
- * Cleans up all data (like device extension and guest mapping).
+ * Cleans up hardware resources.
+ * Do not delete DevExt here.
  *
  * @param   pDrvObj     Driver object.
@@ -530 +532 @@
         /* According to MSDN we have to unmap previously mapped memory. */
         vboxguestwinUnmapVMMDevMemory(pDevExt);
-
-        /* Destroy device extension and clean up everything else. */
-        VBoxGuestDeleteDevExt(pDevExt);
     }
     return STATUS_SUCCESS;
@@ -548 +547 @@
 #ifdef TARGET_NT4
     vboxguestwinCleanup(pDrvObj->DeviceObject);
+
+    /* Destroy device extension and clean up everything else. */
+    if (pDrvObj->DeviceObject && pDrvObj->DeviceObject->DeviceExtension)
+        VBoxGuestDeleteDevExt((PVBOXGUESTDEVEXT)pDrvObj->DeviceObject->DeviceExtension);
+
     /*
      * I don't think it's possible to unload a driver which processes have
@@ -582 +586 @@
     NTSTATUS           rc       = STATUS_SUCCESS;
 
-    /*
-     * We are not remotely similar to a directory...
-     * (But this is possible.)
-     */
-    if (pStack->Parameters.Create.Options & FILE_DIRECTORY_FILE)
-    {
+    if (pDevExt->win.s.devState != WORKING)
+    {
+        Log(("VBoxGuest::vboxguestwinGuestCreate: device is not working currently: %d!\n",
+             pDevExt->win.s.devState));
+        rc = STATUS_UNSUCCESSFUL;
+    }
+    else if (pStack->Parameters.Create.Options & FILE_DIRECTORY_FILE)
+    {
+        /*
+         * We are not remotely similar to a directory...
+         * (But this is possible.)
+         */
         Log(("VBoxGuest::vboxguestwinGuestCreate: Uhm, we're not a directory!\n"));
         rc = STATUS_NOT_A_DIRECTORY;

trunk/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.h

@@ -104 +104 @@
     /** Device state. */
     DEVSTATE devState;
+    DEVSTATE prevDevState;
 
     /** Last system power action set (see VBoxGuestPower). */
@@ -116 +117 @@
     PVBOXGUESTSESSION pKernelSession;
 } VBOXGUESTDEVEXTWIN, *PVBOXGUESTDEVEXTWIN;
+
+#define VBOXGUEST_UPDATE_DEVSTATE(_pDevExt, _newDevState) do {    \
+    (_pDevExt)->win.s.prevDevState =  (_pDevExt)->win.s.devState; \
+    (_pDevExt)->win.s.devState = (_newDevState);                  \
+} while (0)