Changeset 39887 in vbox

Timestamp:

Jan 26, 2012 5:56:17 PM (13 years ago)

Author:

vboxsync

Message:

DevVGA_VBVA: fixed guest SMP unsafe code. Fixed a case when loaded saved state data was incorrectly discarded.

File:

• trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp (modified) (9 diffs)

trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp

@@ -173 +173 @@
     }
 
-    VBVARECORD *pRecord = &pVBVA->aRecords[indexRecordFirst];
-
-    LOGVBVABUFFER(("cbRecord = 0x%08X\n", pRecord->cbRecord));
-
-    uint32_t cbRecord = pRecord->cbRecord & ~VBVA_F_RECORD_PARTIAL;
+    uint32_t cbRecordCurrent = ASMAtomicReadU32(&pVBVA->aRecords[indexRecordFirst].cbRecord);
+
+    LOGVBVABUFFER(("cbRecord = 0x%08X, pPartialRecord->cb = 0x%08X\n", cbRecordCurrent, pPartialRecord->cb));
+
+    uint32_t cbRecord = cbRecordCurrent & ~VBVA_F_RECORD_PARTIAL;
 
     if (pPartialRecord->cb)
@@ -185 +185 @@
 
         LOGVBVABUFFER(("continue partial record cb = %d cbRecord 0x%08X, first = %d, free = %d\n",
-                      pPartialRecord->cb, pRecord->cbRecord, indexRecordFirst, indexRecordFree));
+                      pPartialRecord->cb, cbRecordCurrent, indexRecordFirst, indexRecordFree));
 
         if (cbRecord > pPartialRecord->cb)
@@ -196 +196 @@
         }
 
-        if (!(pRecord->cbRecord & VBVA_F_RECORD_PARTIAL))
+        if (!(cbRecordCurrent & VBVA_F_RECORD_PARTIAL))
         {
             /* The record is completed by guest. Return it to the caller. */
@@ -216 +216 @@
 
     /* A new record need to be processed. */
-    if (pRecord->cbRecord & VBVA_F_RECORD_PARTIAL)
+    if (cbRecordCurrent & VBVA_F_RECORD_PARTIAL)
     {
         /* Current record is being written by guest. '=' is important here,
@@ -232 +232 @@
 
             LOGVBVABUFFER(("started partial record cb = 0x%08X cbRecord 0x%08X, first = %d, free = %d\n",
-                          pPartialRecord->cb, pRecord->cbRecord, indexRecordFirst, indexRecordFree));
+                          pPartialRecord->cb, cbRecordCurrent, indexRecordFirst, indexRecordFree));
         }
 
@@ -468 +468 @@
 }
 
-static int vbvaEnable (unsigned uScreenId, PVGASTATE pVGAState, VBVACONTEXT *pCtx, VBVABUFFER *pVBVA, uint32_t u32Offset)
+static int vbvaEnable (unsigned uScreenId, PVGASTATE pVGAState, VBVACONTEXT *pCtx, VBVABUFFER *pVBVA, uint32_t u32Offset, bool fRestored)
 {
     /* @todo old code did a UpdateDisplayAll at this place. */
@@ -492 +492 @@
                      pVBVA->hostFlags.u32HostEvents, pVBVA->hostFlags.u32SupportedOrders));
 
-        pCtx->aViews[uScreenId].partialRecord.pu8 = NULL;
-        pCtx->aViews[uScreenId].partialRecord.cb = 0;
+        if (!fRestored)
+        {
+            /* @todo Actually this function must not touch the partialRecord structure at all,
+             * because initially it is a zero and when VBVA is disabled this should be set to zero.
+             * But I'm not sure that no code depends on zeroing partialRecord here.
+             * So for now (a quick fix for 4.1) just do not do this if the VM was restored,
+             * when partialRecord might be loaded already from the saved state.
+             */
+            pCtx->aViews[uScreenId].partialRecord.pu8 = NULL;
+            pCtx->aViews[uScreenId].partialRecord.cb = 0;
+        }
 
         pCtx->aViews[uScreenId].pVBVA = pVBVA;
@@ -1600 +1609 @@
             if (pView->pVBVA)
             {
-                vbvaEnable (iView, pVGAState, pCtx, pView->pVBVA, pView->u32VBVAOffset);
+                vbvaEnable (iView, pVGAState, pCtx, pView->pVBVA, pView->u32VBVAOffset, true /* fRestored */);
                 vbvaResize (pVGAState, pView, &pView->screen);
             }
@@ -1892 +1901 @@
                     vbvaFlush (pVGAState, pCtx);
 
-                    rc = vbvaEnable (uScreenId, pVGAState, pCtx, pVBVA, u32Offset);
+                    rc = vbvaEnable (uScreenId, pVGAState, pCtx, pVBVA, u32Offset, false /* fRestored */);
                 }
                 else