Changeset 81947 in vbox for trunk/src/VBox/VMM

Timestamp:

Nov 18, 2019 4:14:43 PM (5 years ago)

Author:

vboxsync

Message:

IOMAllMmioNew: Fixed bogus lock assertions with iomMmioHandlerNew on the stack - didn't check the status code. Duh. bugref:9218

File:

• trunk/src/VBox/VMM/VMMAll/IOMAllMmioNew.cpp (modified) (6 diffs)

trunk/src/VBox/VMM/VMMAll/IOMAllMmioNew.cpp

@@ -858 +858 @@
      * for SMP VMs if we're unlucky and the guest isn't well behaved.
      */
-    IOM_LOCK_SHARED(pVM);  /** @todo Need lookup that doesn't require locking... */
-    RTGCPHYS offIgn;
-    uint16_t idxIgn = UINT16_MAX;
 # ifdef IN_RING0
-    Assert(pRegEntry == iomMmioGetEntry(pVM, GCPhysFault, &offIgn, &idxIgn) || !pRegEntryR3->fMapped);
+    Assert(pRegEntry && (GCPhysFault - pRegEntryR3->GCPhysMapping < pRegEntryR3->cbRegion || !pRegEntryR3->fMapped));
 # else
-    Assert(pRegEntry == iomMmioGetEntry(pVM, GCPhysFault, &offIgn, &idxIgn) || !pRegEntry->fMapped);
+    Assert(pRegEntry && (GCPhysFault - pRegEntry->GCPhysMapping   < pRegEntry->cbRegion   || !pRegEntry->fMapped));
 # endif
-    IOM_UNLOCK_SHARED(pVM);
 #endif
 
@@ -1046 +1042 @@
  *
  * @returns VBox status code.  This API may return VINF_SUCCESS even if no
- *          remapping is made,.
+ *          remapping is made.
  *
  * @param   pVM             The cross context VM structure.
@@ -1102 +1098 @@
      * to prevent remapping.  Shared suffices as we change nothing.
      */
-    IOM_LOCK_SHARED(pVM);
-
-    RTGCPHYS const GCPhys = pRegEntry->fMapped ? pRegEntry->GCPhysMapping : NIL_RTGCPHYS;
-    AssertReturnStmt(GCPhys != NIL_RTGCPHYS, IOM_UNLOCK_SHARED(pVM), VERR_IOM_MMIO_REGION_NOT_MAPPED);
-    Assert(!(GCPhys & PAGE_OFFSET_MASK));
-
-    /*
-     * Do the aliasing; page align the addresses since PGM is picky.
-     */
+    int rc = IOM_LOCK_SHARED(pVM);
+    if (rc == VINF_SUCCESS)
+    {
+        RTGCPHYS const GCPhys = pRegEntry->fMapped ? pRegEntry->GCPhysMapping : NIL_RTGCPHYS;
+        if (GCPhys != NIL_RTGCPHYS)
+        {
+            Assert(!(GCPhys & PAGE_OFFSET_MASK));
+
+            /*
+             * Do the aliasing; page align the addresses since PGM is picky.
+             */
 #if 0 /** @todo fix when DevVGA is converted to new model.  */
-    int rc = PGMHandlerPhysicalPageAlias(pVM, GCPhys, GCPhys + (offRange & ~(RTGCPHYS)PAGE_OFFSET_MASK),
-                                         pDevIns, hMmio2, offMmio2);
+            rc = PGMHandlerPhysicalPageAlias(pVM, GCPhys, GCPhys + (offRange & ~(RTGCPHYS)PAGE_OFFSET_MASK),
+                                             pDevIns, hMmio2, offMmio2);
 #else
-    AssertFailed();
-    int rc = VERR_NOT_IMPLEMENTED;
-    RT_NOREF(offMmio2, hMmio2);
-#endif
-
-    IOM_UNLOCK_SHARED(pVM);
-
-    AssertRCReturn(rc, rc);
+            AssertFailed();
+            rc = VERR_NOT_IMPLEMENTED;
+            RT_NOREF(offMmio2, hMmio2);
+#endif
+        }
+        else
+            AssertFailedStmt(rc = VERR_IOM_MMIO_REGION_NOT_MAPPED);
+
+        IOM_UNLOCK_SHARED(pVM);
+    }
 
 /** @todo either ditch this or replace it with something that works in the
@@ -1144 +1144 @@
     Assert(rc == VINF_SUCCESS || rc == VERR_PAGE_NOT_PRESENT || rc == VERR_PAGE_TABLE_NOT_PRESENT);
 #endif
-    return VINF_SUCCESS;
+    return rc;
 }
 
@@ -1180 +1180 @@
      * Check input address (it's HM calling, not the device, so no region handle).
      */
-    IOM_LOCK_SHARED(pVM);
-    RTGCPHYS offIgn;
-    uint16_t idxIgn = UINT16_MAX;
-    PIOMMMIOENTRYR0 pRegEntry = iomMmioGetEntry(pVM, GCPhys, &offIgn, &idxIgn);
-    IOM_UNLOCK_SHARED(pVM);
-    Assert(pRegEntry);
-    Assert(pRegEntry && !(pRegEntry->cbRegion & PAGE_OFFSET_MASK));
+    int rcSem = IOM_LOCK_SHARED(pVM);
+    if (rcSem == VINF_SUCCESS)
+    {
+        RTGCPHYS offIgn;
+        uint16_t idxIgn = UINT16_MAX;
+        PIOMMMIOENTRYR0 pRegEntry = iomMmioGetEntry(pVM, GCPhys, &offIgn, &idxIgn);
+        IOM_UNLOCK_SHARED(pVM);
+        Assert(pRegEntry);
+        Assert(pRegEntry && !(pRegEntry->cbRegion & PAGE_OFFSET_MASK));
+    }
 # endif
 
@@ -1263 +1266 @@
     Assert((pRegEntry->cbRegion & PAGE_OFFSET_MASK) == 0);
 
-    IOM_LOCK_SHARED(pVM);
+    int rcSem = IOM_LOCK_SHARED(pVM);
     RTGCPHYS GCPhys = pRegEntry->fMapped ? pRegEntry->GCPhysMapping : NIL_RTGCPHYS;
-    IOM_UNLOCK_SHARED(pVM);
+    if (rcSem == VINF_SUCCESS)
+        IOM_UNLOCK_SHARED(pVM);
 
     Assert(!(GCPhys              & PAGE_OFFSET_MASK));