Changeset 20747 in vbox

Timestamp:

Jun 21, 2009 8:23:22 PM (16 years ago)

Author:

vboxsync

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

48863

Message:

REM: Don't drop handler reply records. More handler replay paranoia and massaging.

Location:

trunk/src

Files:

• VBox/VMM/REMInternal.h (modified) (1 diff)
• VBox/VMM/VMMAll/REMAll.cpp (modified) (2 diffs)
• VBox/VMM/testcase/tstVMStructGC.cpp (modified) (1 diff)
• VBox/VMM/testcase/tstVMStructSize.cpp (modified) (1 diff)
• recompiler/VBoxRecompiler.c (modified) (1 diff)

trunk/src/VBox/VMM/REMInternal.h

@@ -110 +110 @@
     uint32_t                    idxSelf;
     uint32_t volatile           idxNext;
-} REMHANDLERNOTIFICATION, *PREMHANDLERNOTIFICATION;
+} REMHANDLERNOTIFICATION;
+/** Pointer to a handler notification record. */
+typedef REMHANDLERNOTIFICATION *PREMHANDLERNOTIFICATION;
 
 /**

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

@@ -93 +93 @@
 static void remNotifyHandlerInsert(PVM pVM, PREMHANDLERNOTIFICATION pRec)
 {
-    uint32_t idxFree;
-    uint32_t idxNext;
+    /*
+     * Fetch a free record.
+     */
+    uint32_t                cFlushes = 0;
+    uint32_t                idxFree;
     PREMHANDLERNOTIFICATION pFree;
-
-    /* Fetch a free record. */
     do
     {
@@ -103 +104 @@
         if (idxFree == (uint32_t)-1)
         {
-            pFree = NULL;
-            break;
+            do
+            {
+                Assert(cFlushes++ != 128);
+                AssertFatal(cFlushes < _1M);
+                remFlushHandlerNotifications(pVM);
+                idxFree = ASMAtomicUoReadU32(&pVM->rem.s.idxFreeList);
+            } while (idxFree == (uint32_t)-1);
         }
         pFree = &pVM->rem.s.aHandlerNotifications[idxFree];
     } while (!ASMAtomicCmpXchgU32(&pVM->rem.s.idxFreeList, pFree->idxNext, idxFree));
 
-    if (!pFree)
-    {
-        remFlushHandlerNotifications(pVM);
-        /** @todo why are we dropping the pReq here without a fight? If we can drop
-         *        one, we can drop all... */
-        return;
-    }
-
-    /* Copy the record. */
-    *pFree = *pRec;
-    pFree->idxSelf = idxFree; /* was trashed */
-
-    /* Insert it into the pending list. */
+    /*
+     * Copy the record.
+     */
+    pFree->enmKind = pRec->enmKind;
+    pFree->u = pRec->u;
+
+    /*
+     * Insert it into the pending list.
+     */
+    uint32_t idxNext;
     do
     {

trunk/src/VBox/VMM/testcase/tstVMStructGC.cpp

@@ -739 +739 @@
     GEN_CHECK_OFF(REM, Env);
 
+    GEN_CHECK_SIZE(REMHANDLERNOTIFICATION);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, enmKind);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalRegister.GCPhys);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalRegister.cb);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalRegister.enmType);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalRegister.fHasHCHandler);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalDeregister.GCPhys);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalDeregister.cb);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalDeregister.enmType);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalDeregister.fHasHCHandler);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalDeregister.fRestoreAsRAM);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalModify.GCPhysOld);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalModify.GCPhysNew);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalModify.cb);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalModify.enmType);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalModify.fHasHCHandler);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, u.PhysicalModify.fRestoreAsRAM);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, idxSelf);
+    GEN_CHECK_OFF(REMHANDLERNOTIFICATION, idxNext);
+
     GEN_CHECK_SIZE(SELM);
     GEN_CHECK_OFF(SELM, offVM);

trunk/src/VBox/VMM/testcase/tstVMStructSize.cpp

@@ -278 +278 @@
     CHECK_MEMBER_ALIGNMENT(PGMMMIO2RANGE, RamRange, 16);
 
-    /* misc */
+    /* rem */
     CHECK_MEMBER_ALIGNMENT(REM, aGCPtrInvalidatedPages, 8);
-    CHECK_PADDING3(EMCPU, u.FatalLongJump, u.achPaddingFatalLongJump);
     CHECK_PADDING3(REMHANDLERNOTIFICATION, u.PhysicalRegister, u.padding);
     CHECK_PADDING3(REMHANDLERNOTIFICATION, u.PhysicalDeregister, u.padding);
     CHECK_PADDING3(REMHANDLERNOTIFICATION, u.PhysicalModify, u.padding);
+    CHECK_SIZE_ALIGNMENT(REMHANDLERNOTIFICATION, 8);
+    CHECK_MEMBER_ALIGNMENT(REMHANDLERNOTIFICATION, u.PhysicalDeregister.GCPhys, 8);
+
+    /* misc */
+    CHECK_PADDING3(EMCPU, u.FatalLongJump, u.achPaddingFatalLongJump);
     CHECK_SIZE_ALIGNMENT(VMMR0JMPBUF, 8);
     CHECK_SIZE_ALIGNMENT(PATCHINFO, 8);

trunk/src/recompiler/VBoxRecompiler.c

@@ -2728 +2728 @@
     if (VM_FF_TESTANDCLEAR(pVM, VM_FF_REM_HANDLER_NOTIFY_BIT))
     {
+        PREMHANDLERNOTIFICATION pReqsRev;
+        PREMHANDLERNOTIFICATION pReqs;
+        uint32_t                idxNext;
+        uint32_t                idxReqs;
+
         /* Lockless purging of pending notifications. */
-        uint32_t idxReqs = ASMAtomicXchgU32(&pVM->rem.s.idxPendingList, -1);
-        if (idxReqs == -1)
+        idxReqs = ASMAtomicXchgU32(&pVM->rem.s.idxPendingList, UINT32_MAX);
+        if (idxReqs == UINT32_MAX)
             return;
-
         Assert(idxReqs < RT_ELEMENTS(pVM->rem.s.aHandlerNotifications));
-        PREMHANDLERNOTIFICATION pReqs = &pVM->rem.s.aHandlerNotifications[idxReqs];
 
         /*
          * Reverse the list to process it in FIFO order.
          */
-        PREMHANDLERNOTIFICATION pReq = pReqs;
-        pReqs = NULL;
-        while (pReq)
+        pReqsRev = &pVM->rem.s.aHandlerNotifications[idxReqs];
+        pReqs    = NULL;
+        while (pReqsRev)
         {
-            PREMHANDLERNOTIFICATION pCur = pReq;
-
-            if (pReq->idxNext != -1)
+            PREMHANDLERNOTIFICATION pCur = pReqsRev;
+            idxNext = pReqsRev->idxNext;
+            if (idxNext != UINT32_MAX)
             {
-                Assert(pReq->idxNext < RT_ELEMENTS(pVM->rem.s.aHandlerNotifications));
-                pReq = &pVM->rem.s.aHandlerNotifications[pReq->idxNext];
+                Assert(idxNext < RT_ELEMENTS(pVM->rem.s.aHandlerNotifications));
+                pReqsRev = &pVM->rem.s.aHandlerNotifications[idxNext];
             }
             else
-                pReq = NULL;
-
-            pCur->idxNext = (pReqs) ? pReqs->idxSelf : -1;
+                pReqsRev = NULL;
+            pCur->idxNext = idxNext;
             pReqs = pCur;
         }
 
+        /*
+         * Loop thru the list, reinserting the record into the free list as they are
+         * processed to avoid having other EMTs running out of entries while we're flushing.
+         */
         while (pReqs)
         {
-            PREMHANDLERNOTIFICATION pRec = pReqs;
-
-            switch (pRec->enmKind)
+            PREMHANDLERNOTIFICATION pCur = pReqs;
+
+            switch (pCur->enmKind)
             {
                 case REMHANDLERNOTIFICATIONKIND_PHYSICAL_REGISTER:
                     remR3NotifyHandlerPhysicalRegister(pVM,
-                                                       pRec->u.PhysicalRegister.enmType,
-                                                       pRec->u.PhysicalRegister.GCPhys,
-                                                       pRec->u.PhysicalRegister.cb,
-                                                       pRec->u.PhysicalRegister.fHasHCHandler);
+                                                       pCur->u.PhysicalRegister.enmType,
+                                                       pCur->u.PhysicalRegister.GCPhys,
+                                                       pCur->u.PhysicalRegister.cb,
+                                                       pCur->u.PhysicalRegister.fHasHCHandler);
                     break;
 
                 case REMHANDLERNOTIFICATIONKIND_PHYSICAL_DEREGISTER:
                     remR3NotifyHandlerPhysicalDeregister(pVM,
-                                                         pRec->u.PhysicalDeregister.enmType,
-                                                         pRec->u.PhysicalDeregister.GCPhys,
-                                                         pRec->u.PhysicalDeregister.cb,
-                                                         pRec->u.PhysicalDeregister.fHasHCHandler,
-                                                         pRec->u.PhysicalDeregister.fRestoreAsRAM);
+                                                         pCur->u.PhysicalDeregister.enmType,
+                                                         pCur->u.PhysicalDeregister.GCPhys,
+                                                         pCur->u.PhysicalDeregister.cb,
+                                                         pCur->u.PhysicalDeregister.fHasHCHandler,
+                                                         pCur->u.PhysicalDeregister.fRestoreAsRAM);
                     break;
 
                 case REMHANDLERNOTIFICATIONKIND_PHYSICAL_MODIFY:
                     remR3NotifyHandlerPhysicalModify(pVM,
-                                                     pRec->u.PhysicalModify.enmType,
-                                                     pRec->u.PhysicalModify.GCPhysOld,
-                                                     pRec->u.PhysicalModify.GCPhysNew,
-                                                     pRec->u.PhysicalModify.cb,
-                                                     pRec->u.PhysicalModify.fHasHCHandler,
-                                                     pRec->u.PhysicalModify.fRestoreAsRAM);
+                                                     pCur->u.PhysicalModify.enmType,
+                                                     pCur->u.PhysicalModify.GCPhysOld,
+                                                     pCur->u.PhysicalModify.GCPhysNew,
+                                                     pCur->u.PhysicalModify.cb,
+                                                     pCur->u.PhysicalModify.fHasHCHandler,
+                                                     pCur->u.PhysicalModify.fRestoreAsRAM);
                     break;
 
                 default:
-                    AssertReleaseMsgFailed(("enmKind=%d\n", pRec->enmKind));
+                    AssertReleaseMsgFailed(("enmKind=%d\n", pCur->enmKind));
                     break;
             }
-            if (pReqs->idxNext != -1)
+
+            /*
+             * Advance pReqs.
+             */
+            idxNext = pCur->idxNext;
+            if (idxNext != UINT32_MAX)
             {
-                AssertMsg(pReqs->idxNext < RT_ELEMENTS(pVM->rem.s.aHandlerNotifications), ("pReqs->idxNext=%d\n", pReqs->idxNext));
-                pReqs = &pVM->rem.s.aHandlerNotifications[pReqs->idxNext];
+                AssertMsg(idxNext < RT_ELEMENTS(pVM->rem.s.aHandlerNotifications), ("idxNext=%d\n", idxNext));
+                pReqs = &pVM->rem.s.aHandlerNotifications[idxNext];
             }
             else
                 pReqs = NULL;
 
-            /* Put the record back into the free list */
-            uint32_t idxNext;
-
+            /*
+             * Put the record back into the free list.
+             */
             do
             {
                 idxNext = ASMAtomicUoReadU32(&pVM->rem.s.idxFreeList);
-                ASMAtomicWriteU32(&pRec->idxNext, idxNext);
+                ASMAtomicWriteU32(&pCur->idxNext, idxNext);
                 ASMCompilerBarrier();
-            } while (!ASMAtomicCmpXchgU32(&pVM->rem.s.idxFreeList, pRec->idxSelf, idxNext));
+            } while (!ASMAtomicCmpXchgU32(&pVM->rem.s.idxFreeList, pCur->idxSelf, idxNext));
         }
     }