Changeset 20746 in vbox

Timestamp:

Jun 21, 2009 7:53:12 PM (15 years ago)

Author:

vboxsync

Message:

REMR3ReplayHandlerNotification,remNotifyHandlerInsert: paranoia.

Location:

trunk/src

Files:

• VBox/VMM/REMInternal.h (modified) (1 diff)
• VBox/VMM/VMMAll/REMAll.cpp (modified) (4 diffs)
• recompiler/VBoxRecompiler.c (modified) (4 diffs)

trunk/src/VBox/VMM/REMInternal.h

@@ -109 +109 @@
     } u;
     uint32_t                    idxSelf;
-    uint32_t                    idxNext;
+    uint32_t volatile           idxNext;
 } REMHANDLERNOTIFICATION, *PREMHANDLERNOTIFICATION;
 

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

@@ -100 +100 @@
     do
     {
-        idxFree = pVM->rem.s.idxFreeList;
+        idxFree = ASMAtomicUoReadU32(&pVM->rem.s.idxFreeList);
         if (idxFree == (uint32_t)-1)
         {
@@ -106 +106 @@
             break;
         }
-        pFree = &pVM->rem.s.aHandlerNotifications[idxFree];  
+        pFree = &pVM->rem.s.aHandlerNotifications[idxFree];
     } while (!ASMAtomicCmpXchgU32(&pVM->rem.s.idxFreeList, pFree->idxNext, idxFree));
 
@@ -112 +112 @@
     {
         remFlushHandlerNotifications(pVM);
+        /** @todo why are we dropping the pReq here without a fight? If we can drop
+         *        one, we can drop all... */
         return;
     }
@@ -122 +124 @@
     do
     {
-        idxNext = pVM->rem.s.idxPendingList;
-        pFree->idxNext = idxNext;
+        idxNext = ASMAtomicUoReadU32(&pVM->rem.s.idxPendingList);
+        ASMAtomicWriteU32(&pFree->idxNext, idxNext);
+        ASMCompilerBarrier();
     } while (!ASMAtomicCmpXchgU32(&pVM->rem.s.idxPendingList, idxFree, idxNext));
 
     VM_FF_SET(pVM, VM_FF_REM_HANDLER_NOTIFY);
 }
+
 
 /**

trunk/src/recompiler/VBoxRecompiler.c

@@ -279 +279 @@
      * Initialize the REM critical section.
      *
-     * Note: This is not a 100% safe solution as updating the internal memory state while another VCPU 
+     * Note: This is not a 100% safe solution as updating the internal memory state while another VCPU
      *       is executing code could be dangerous. Taking the REM lock is not an option due to the danger of
      *       deadlocks. (mostly pgm vs rem locking)
@@ -2725 +2725 @@
     VM_ASSERT_EMT(pVM);
 
+    /** @todo this isn't ensuring correct replay order. */
     if (VM_FF_TESTANDCLEAR(pVM, VM_FF_REM_HANDLER_NOTIFY_BIT))
     {
@@ -2764 +2765 @@
                 case REMHANDLERNOTIFICATIONKIND_PHYSICAL_REGISTER:
                     remR3NotifyHandlerPhysicalRegister(pVM,
-                                                    pRec->u.PhysicalRegister.enmType,
-                                                    pRec->u.PhysicalRegister.GCPhys,
-                                                    pRec->u.PhysicalRegister.cb,
-                                                    pRec->u.PhysicalRegister.fHasHCHandler);
+                                                       pRec->u.PhysicalRegister.enmType,
+                                                       pRec->u.PhysicalRegister.GCPhys,
+                                                       pRec->u.PhysicalRegister.cb,
+                                                       pRec->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);
+                                                         pRec->u.PhysicalDeregister.enmType,
+                                                         pRec->u.PhysicalDeregister.GCPhys,
+                                                         pRec->u.PhysicalDeregister.cb,
+                                                         pRec->u.PhysicalDeregister.fHasHCHandler,
+                                                         pRec->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);
+                                                     pRec->u.PhysicalModify.enmType,
+                                                     pRec->u.PhysicalModify.GCPhysOld,
+                                                     pRec->u.PhysicalModify.GCPhysNew,
+                                                     pRec->u.PhysicalModify.cb,
+                                                     pRec->u.PhysicalModify.fHasHCHandler,
+                                                     pRec->u.PhysicalModify.fRestoreAsRAM);
                     break;
 
@@ -2806 +2807 @@
             do
             {
-                idxNext = pVM->rem.s.idxFreeList;
-                pRec->idxNext = idxNext;
+                idxNext = ASMAtomicUoReadU32(&pVM->rem.s.idxFreeList);
+                ASMAtomicWriteU32(&pRec->idxNext, idxNext);
+                ASMCompilerBarrier();
             } while (!ASMAtomicCmpXchgU32(&pVM->rem.s.idxFreeList, pRec->idxSelf, idxNext));
         }