Changeset 98122 in vbox

Timestamp:

Jan 19, 2023 12:03:23 AM (2 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

155291

Message:

VMM/PDM: Fixed assertion in PDMR3ResumeUsb after attaching a virtual MSD.

Location:

trunk

Files:

• include/VBox/vmm/pdmusb.h (modified) (2 diffs)
• include/VBox/vmm/vm.h (modified) (2 diffs)
• src/VBox/VMM/VMMR3/PDM.cpp (modified) (1 diff)
• src/VBox/VMM/VMMR3/PDMUsb.cpp (modified) (5 diffs)
• src/VBox/VMM/include/PDMInternal.h (modified) (1 diff)

trunk/include/VBox/vmm/pdmusb.h

@@ -240 +240 @@
      * VM Resume notification.
      *
+     * This is not called when the device is hotplugged device, instead
+     * pfnHotPlugged will be called.
+     *
      * @returns VBox status.
      * @param   pUsbIns     The USB device instance data.
@@ -260 +263 @@
      * Called after the constructor when attaching a device at run time.
      *
-     * This can be used to do tasks normally assigned to pfnInitComplete and/or pfnVMPowerOn.
+     * This can be used to do tasks normally assigned to pfnInitComplete and/or
+     * pfnVMPowerOn.  There will not be a call to pfnVMResume following this.
      *
      * @returns VBox status.

trunk/include/VBox/vmm/vm.h

@@ -989 +989 @@
  */
 #define VMSTATE_IS_RUNNING(a_enmVMState) \
-    (   (enmVMState) == VMSTATE_RUNNING \
-     || (enmVMState) == VMSTATE_RUNNING_LS )
+    (   (a_enmVMState) == VMSTATE_RUNNING \
+     || (a_enmVMState) == VMSTATE_RUNNING_LS )
 
 /** @def VM_IS_RUNNING_FOR_ASSERTIONS_ONLY
@@ -1000 +1000 @@
     (   (pVM)->enmVMState == VMSTATE_RUNNING \
      || (pVM)->enmVMState == VMSTATE_RUNNING_LS )
+
+
+/** @def VMSTATE_IS_POWERED_ON
+ * Checks if the given state indicates the VM is powered on.
+ *
+ * @note Excludes all error states, so a powered on VM that hit a fatal error,
+ *       guru meditation, state load failure or similar will not be considered
+ *       powered on by this test.
+ */
+#define VMSTATE_IS_POWERED_ON(a_enmVMState) \
+    ( (a_enmVMState) >= VMSTATE_RESUMING && (a_enmVMState) < VMSTATE_POWERING_OFF )
 
 /** @def VM_ASSERT_IS_NOT_RUNNING

trunk/src/VBox/VMM/VMMR3/PDM.cpp

@@ -2121 +2121 @@
 DECLINLINE(int) pdmR3ResumeUsb(PPDMUSBINS pUsbIns)
 {
-    Assert(pUsbIns->Internal.s.fVMSuspended);
-    if (pUsbIns->pReg->pfnVMResume)
-    {
-        LogFlow(("PDMR3Resume: Notifying - device '%s'/%d\n", pUsbIns->pReg->szName, pUsbIns->iInstance));
-        int rc = VINF_SUCCESS; pUsbIns->pReg->pfnVMResume(pUsbIns);
-        if (RT_FAILURE(rc))
-        {
-            LogRel(("PDMR3Resume: Device '%s'/%d -> %Rrc\n", pUsbIns->pReg->szName, pUsbIns->iInstance, rc));
-            return rc;
-        }
-    }
-    pUsbIns->Internal.s.fVMSuspended = false;
+    if (pUsbIns->Internal.s.fVMSuspended)
+    {
+        if (pUsbIns->pReg->pfnVMResume)
+        {
+            LogFlow(("PDMR3Resume: Notifying - device '%s'/%d\n", pUsbIns->pReg->szName, pUsbIns->iInstance));
+            int rc = VINF_SUCCESS; pUsbIns->pReg->pfnVMResume(pUsbIns);
+            if (RT_FAILURE(rc))
+            {
+                LogRel(("PDMR3Resume: Device '%s'/%d -> %Rrc\n", pUsbIns->pReg->szName, pUsbIns->iInstance, rc));
+                return rc;
+            }
+        }
+        pUsbIns->Internal.s.fVMSuspended = false;
+    }
     return VINF_SUCCESS;
 }

trunk/src/VBox/VMM/VMMR3/PDMUsb.cpp

@@ -547 +547 @@
                                 PCFGMNODE *ppInstanceNode, VUSBSPEED enmSpeed, const char *pszCaptureFilename)
 {
-    const bool fAtRuntime = iInstance == -1;
     int rc;
 
@@ -568 +567 @@
     PCFGMNODE pInstanceToDelete = NULL;
     PCFGMNODE pInstanceNode = NULL;
-    if (fAtRuntime)
+    if (iInstance == -1)
     {
         /** @todo r=bird: This code is bogus as it ASSUMES that all USB devices are
@@ -646 +645 @@
     //pUsbIns->Internal.s.pHub                = NULL;
     pUsbIns->Internal.s.iPort               = UINT32_MAX; /* to be determined. */
-    /* Set the flag accordingly.
-     * Otherwise VMPowerOff, VMSuspend will not be called for devices attached at runtime.
-     */
-    pUsbIns->Internal.s.fVMSuspended        = !fAtRuntime;
+    VMSTATE const enmVMState = VMR3GetState(pVM);
+    pUsbIns->Internal.s.fVMSuspended        = !VMSTATE_IS_POWERED_ON(enmVMState);
     //pUsbIns->Internal.s.pfnAsyncNotify      = NULL;
     pUsbIns->pHlpR3                         = &g_pdmR3UsbHlp;
@@ -712 +709 @@
 
             /* Send the hot-plugged notification if applicable. */
-            if (fAtRuntime && pUsbIns->pReg->pfnHotPlugged)
+            if (VMSTATE_IS_POWERED_ON(enmVMState) && pUsbIns->pReg->pfnHotPlugged)
                 pUsbIns->pReg->pfnHotPlugged(pUsbIns);
 
@@ -729 +726 @@
             rc = VERR_PDM_USBDEV_VERSION_MISMATCH;
     }
-    if (fAtRuntime)
+    if (VMSTATE_IS_POWERED_ON(enmVMState))
         pdmR3UsbDestroyDevice(pVM, pUsbIns);
     /* else: destructors are invoked later. */

trunk/src/VBox/VMM/include/PDMInternal.h

@@ -367 +367 @@
     uint32_t                        iPort;
     /** Indicates that the USB device hasn't been powered on or resumed.
-     * See PDMDEVINSINT_FLAGS_SUSPENDED. */
+     * See PDMDEVINSINT_FLAGS_SUSPENDED.
+     * @note Runtime attached USB devices gets a pfnHotPlugged callback rather than
+     *       a pfnVMResume one. */
     bool                            fVMSuspended;
     /** Indicates that the USB device has been reset. */