Changeset 98065 in vbox for trunk

Timestamp:

Jan 12, 2023 3:57:23 PM (2 years ago)

Author:

vboxsync

Message:

DevVGA: Simplified the tracking of remapped VGA pages, we don't need two variables for that. Corrected comments about the range, since it's technically the 128KB range A0000 thru BFFFF, not just 64KB thru AFFFF. bugref:10251

Location:

trunk/src/VBox/Devices

Files:

• Graphics/DevVGA.cpp (modified) (17 diffs)
• Graphics/DevVGA.h (modified) (2 diffs)
• testcase/tstDeviceStructSize.cpp (modified) (1 diff)

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

@@ -311 +311 @@
 
 /**
- * Mark a page in VGA A0000-AFFFF range as remapped.
+ * Mark a page in VGA A0000-BFFFF range as remapped.
  *
  * @param   pThis       VGA instance data.
@@ -318 +318 @@
 DECLINLINE(void) vgaMarkRemapped(PVGASTATE pThis, RTGCPHYS offVGAMem)
 {
-    AssertMsg(offVGAMem < _64K, ("offVGAMem = %p > 64K\n", offVGAMem));
-    ASMBitSet(&pThis->bmPageMapBitmap, offVGAMem >> GUEST_PAGE_SHIFT);
-    pThis->fRemappedVGA = true;
-}
-
-/**
- * Checks if a page in VGA A0000-AFFFF range is remapped.
+    AssertMsg(offVGAMem < _128K, ("offVGAMem = %RGp\n", offVGAMem));
+    pThis->bmPageRemappedVGA |= RT_BIT_32((uint32_t)offVGAMem >> GUEST_PAGE_SHIFT);
+}
+
+
+/**
+ * Checks if a page in VGA A0000-BFFFF range is remapped.
  *
  * @returns true if remapped.
@@ -333 +333 @@
 DECLINLINE(bool) vgaIsRemapped(PVGASTATE pThis, RTGCPHYS offVGAMem)
 {
-    AssertMsg(offVGAMem < _64K, ("offVGAMem = %p > 64K\n", offVGAMem));
-    return ASMBitTest(&pThis->bmPageMapBitmap, offVGAMem >> GUEST_PAGE_SHIFT);
-}
+    AssertMsg(offVGAMem < _128K, ("offVGAMem = %RGp\n", offVGAMem));
+    return pThis->bmPageRemappedVGA & RT_BIT_32((uint32_t)offVGAMem >> GUEST_PAGE_SHIFT);
+}
+
 
 /**
@@ -344 +345 @@
 DECLINLINE(void) vgaResetRemapped(PVGASTATE pThis)
 {
-    pThis->fRemappedVGA = false;
-    ASMBitClearRange(&pThis->bmPageMapBitmap, 0, _64K >> GUEST_PAGE_SHIFT);
-}
+    pThis->bmPageRemappedVGA = 0;
+}
+
 
 /**
@@ -745 +746 @@
             ||  pThis->sr_index == 2 /* plane mask */)
         {
-            if (pThis->fRemappedVGA)
+            if (pThis->bmPageRemappedVGA != 0)
             {
                 PDMDevHlpMmioResetRegion(pDevIns, pThis->hMmioLegacy);
@@ -785 +786 @@
         if (pThis->gr_index == 6 /* memory map mode */)
         {
-            if (pThis->fRemappedVGA)
+            if (pThis->bmPageRemappedVGA != 0)
             {
                 PDMDevHlpMmioResetRegion(pDevIns, pThis->hMmioLegacy);
@@ -1044 +1045 @@
 # ifndef IN_RC
             /* The VGA region is (could be) affected by this change; reset all aliases we've created. */
-            if (pThis->fRemappedVGA)
+            if (pThis->bmPageRemappedVGA != 0)
             {
                 PDMDevHlpMmioResetRegion(pDevIns, pThis->hMmioLegacy);
@@ -1154 +1155 @@
 
             /* The VGA region is (could be) affected by this change; reset all aliases we've created. */
-            if (pThis->fRemappedVGA)
+            if (pThis->bmPageRemappedVGA != 0)
             {
                 PDMDevHlpMmioResetRegion(pDevIns, pThis->hMmioLegacy);
@@ -1221 +1222 @@
 
     /* convert to VGA memory offset */
+    addr &= 0x1ffff;
 #ifndef IN_RC
-    RTGCPHYS GCPhys = addr; /* save original address */
+    RTGCPHYS const offMmio = addr; /* save original MMIO range offset  */
 #endif
-    addr &= 0x1ffff;
 
     int const memory_map_mode = (pThis->gr[6] >> 2) & 3;
@@ -1253 +1254 @@
         /* If all planes are accessible, then map the page to the frame buffer and make it writable. */
         if (   (pThis->sr[2] & 3) == 3
-            && !vgaIsRemapped(pThis, GCPhys - 0xa0000)
+            && !vgaIsRemapped(pThis, offMmio)
             && pThis->GCPhysVRAM)
         {
             /** @todo only allow read access (doesn't work now) */
             STAM_COUNTER_INC(&pThis->StatMapPage);
-            PDMDevHlpMmioMapMmio2Page(pDevIns, pThis->hMmioLegacy, GCPhys - 0xa0000,
-                                      pThis->hMmio2VRam, addr, X86_PTE_RW | X86_PTE_P);
+            PDMDevHlpMmioMapMmio2Page(pDevIns, pThis->hMmioLegacy, offMmio, pThis->hMmio2VRam, addr, X86_PTE_RW | X86_PTE_P);
             /* Set as dirty as write accesses won't be noticed now. */
             vgaR3MarkDirty(pThis, addr);
-            vgaMarkRemapped(pThis, GCPhys - 0xa0000);
+            vgaMarkRemapped(pThis, offMmio);
         }
 #endif /* !IN_RC */
@@ -1327 +1327 @@
 
     /* convert to VGA memory offset */
+    addr &= 0x1ffff;
 #ifndef IN_RC
-    RTGCPHYS const GCPhys = addr; /* save original address */
+    RTGCPHYS const offMmio = addr; /* save original MMIO range offset  */
 #endif
-    addr &= 0x1ffff;
 
     int const memory_map_mode = (pThis->gr[6] >> 2) & 3;
@@ -1362 +1362 @@
             /* If all planes are accessible, then map the page to the frame buffer and make it writable. */
             if (   (pThis->sr[2] & 3) == 3
-                && !vgaIsRemapped(pThis, GCPhys - 0xa0000)
+                && !vgaIsRemapped(pThis, offMmio)
                 && pThis->GCPhysVRAM)
             {
                 STAM_COUNTER_INC(&pThis->StatMapPage);
-                PDMDevHlpMmioMapMmio2Page(pDevIns, pThis->hMmioLegacy, GCPhys - 0xa0000,
-                                          pThis->hMmio2VRam, addr, X86_PTE_RW | X86_PTE_P);
-                vgaMarkRemapped(pThis, GCPhys - 0xa0000);
+                PDMDevHlpMmioMapMmio2Page(pDevIns, pThis->hMmioLegacy, offMmio, pThis->hMmio2VRam, addr, X86_PTE_RW | X86_PTE_P);
+                vgaMarkRemapped(pThis, offMmio);
             }
 #endif /* !IN_RC */
@@ -3659 +3658 @@
  * @callback_method_impl{FNIOMMMIONEWREAD,
  * Legacy VGA memory (0xa0000 - 0xbffff) read hook\, to be called from IOM.}
+ *
+ * @note The @a off is an absolute address in the 0xa0000 - 0xbffff range, not
+ *       an offset.
  */
 static DECLCALLBACK(VBOXSTRICTRC) vgaMmioRead(PPDMDEVINS pDevIns, void *pvUser, RTGCPHYS off, void *pv, unsigned cb)
@@ -3718 +3720 @@
  * @callback_method_impl{FNIOMMMIONEWWRITE,
  * Legacy VGA memory (0xa0000 - 0xbffff) write hook\, to be called from IOM.}
+ *
+ * @note The @a off is an absolute address in the 0xa0000 - 0xbffff range, not
+ *       an offset.
  */
 static DECLCALLBACK(VBOXSTRICTRC) vgaMmioWrite(PPDMDEVINS pDevIns, void *pvUser, RTGCPHYS off, void const *pv, unsigned cb)
@@ -4875 +4880 @@
         vgaR3UpdateDirtyBitsAndResetMonitoring(pDevIns, pThis);
 
-    if (pThis->fRemappedVGA)
+    if (pThis->bmPageRemappedVGA != 0)
     {
         PDMDevHlpMmioResetRegion(pDevIns, pThis->hMmioLegacy);
@@ -4904 +4909 @@
     }
 
-    if (pThis->fRemappedVGA)
+    if (pThis->bmPageRemappedVGA != 0)
     {
         PDMDevHlpMmioResetRegion(pDevIns, pThis->hMmioLegacy);
@@ -6126 +6131 @@
         AssertRC(rc);
     }
-    if (pThis->fRemappedVGA)
+    if (pThis->bmPageRemappedVGA != 0)
     {
         PDMDevHlpMmioResetRegion(pDevIns, pThis->hMmioLegacy);

trunk/src/VBox/Devices/Graphics/DevVGA.h

@@ -362 +362 @@
     /** Bitmap tracking dirty pages. */
     uint64_t                    bmDirtyBitmap[VGA_VRAM_MAX / GUEST_PAGE_SIZE / 64];
-    /** Bitmap tracking remapped pages (only needs 16 bits). */
-    uint64_t                    bmPageMapBitmap;
+    /** Bitmap tracking which VGA memory pages in the 0xa0000-0xbffff region has
+     * been remapped to allow direct access.
+     * @note It's quite possible that mapping in the 0xb0000-0xbffff isn't possible,
+     *       but we're playing safe and cover the whole VGA MMIO region here. */
+    uint32_t                    bmPageRemappedVGA;
 
     /** Flag indicating that there are dirty bits. This is used to optimize the handler resetting. */
     bool                        fHasDirtyBits;
-    /** Flag indicating that the VGA memory in the 0xa0000-0xbffff region has been remapped to allow direct access. */
+    /** Flag indicating that the VGA memory in the 0xa0000-0xbffff region has been remapped to allow direct access.
+     * @todo This is just an unnecessary summmary of bmPageMapBitmap.  */
     bool                        fRemappedVGA;
     /** Whether to render the guest VRAM to the framebuffer memory. False only for some LFB modes. */
@@ -381 +385 @@
     bool                        fVMSVGAPciId;
     bool                        fVMSVGAPciBarLayout;
-    bool                        Padding4[3];
 #else
-    bool                        Padding4[4+3];
+    bool                        afPadding4[4];
 #endif
 

trunk/src/VBox/Devices/testcase/tstDeviceStructSize.cpp

@@ -409 +409 @@
     CHECK_MEMBER_ALIGNMENT(VGASTATE, StatRZMemoryRead, 8);
     CHECK_MEMBER_ALIGNMENT(VGASTATE, CritSectIRQ, 8);
+    CHECK_MEMBER_ALIGNMENT(VGASTATE, bmDirtyBitmap, 8);
+    CHECK_MEMBER_ALIGNMENT(VGASTATE, pciRegions, 8);
     CHECK_MEMBER_ALIGNMENT(VMMDEV, CritSect, 8);
 #ifdef VBOX_WITH_PCI_PASSTHROUGH_IMPL