儲存庫 vbox 的更動 86477

時間撮記:

2020-10-7 下午08:10:05 (4 年 以前)

作者:

vboxsync

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

140787

訊息:

VMM/PGM: Use atomics when updating accessed and dirty flags from the Trap0e handler code, also take the PGM lock first to reduce races. bugref:9841 bugref:9746

位置:

trunk/src/VBox/VMM

檔案:

• VMMAll/PGMAllBth.h (修改) (4 筆差異)
• include/PGMGstDefs.h (修改) (6 筆差異)

trunk/src/VBox/VMM/VMMAll/PGMAllBth.h

@@ -464 +464 @@
     }
 
+    /* Take the big lock now before we update flags. */
+    *pfLockTaken = true;
+    pgmLock(pVM);
+
     /*
      * Set the accessed and dirty flags.
      */
-/** @todo Use atomics here as we don't own the lock and stuff: */
+    /** @todo Should probably use cmpxchg logic here as we're potentially racing
+     *        other CPUs in SMP configs. (the lock isn't enough, since we take it
+     *        after walking and the page tables could be stale already) */
 #   if PGM_GST_TYPE == PGM_TYPE_AMD64
-    GstWalk.Pml4e.u     |= X86_PML4E_A;
-    GstWalk.pPml4e->u   |= X86_PML4E_A;
-    GstWalk.Pdpe.u      |= X86_PDPE_A;
-    GstWalk.pPdpe->u    |= X86_PDPE_A;
+    if (!(GstWalk.Pml4e.u & X86_PML4E_A))
+    {
+        GstWalk.Pml4e.u |= X86_PML4E_A;
+        GST_ATOMIC_OR(&GstWalk.pPml4e->u, X86_PML4E_A);
+    }
+    if (!(GstWalk.Pdpe.u & X86_PDPE_A))
+    {
+        GstWalk.Pdpe.u |= X86_PDPE_A;
+        GST_ATOMIC_OR(&GstWalk.pPdpe->u, X86_PDPE_A);
+    }
 #   endif
     if (GstWalk.Core.fBigPage)
@@ -479 +491 @@
         if (uErr & X86_TRAP_PF_RW)
         {
-            GstWalk.Pde.u   |= X86_PDE4M_A | X86_PDE4M_D;
-            GstWalk.pPde->u |= X86_PDE4M_A | X86_PDE4M_D;
+            if ((GstWalk.Pde.u & (X86_PDE4M_A | X86_PDE4M_D)) != (X86_PDE4M_A | X86_PDE4M_D))
+            {
+                GstWalk.Pde.u |= X86_PDE4M_A | X86_PDE4M_D;
+                GST_ATOMIC_OR(&GstWalk.pPde->u, X86_PDE4M_A | X86_PDE4M_D);
+            }
         }
         else
         {
-            GstWalk.Pde.u   |= X86_PDE4M_A;
-            GstWalk.pPde->u |= X86_PDE4M_A;
+            if (!(GstWalk.Pde.u & X86_PDE4M_A))
+            {
+                GstWalk.Pde.u |= X86_PDE4M_A;
+                GST_ATOMIC_OR(&GstWalk.pPde->u, X86_PDE4M_A);
+            }
         }
     }
@@ -491 +509 @@
     {
         Assert(!(GstWalk.Pde.u & X86_PDE_PS));
-        GstWalk.Pde.u   |= X86_PDE_A;
-        GstWalk.pPde->u |= X86_PDE_A;
+        if (!(GstWalk.Pde.u & X86_PDE_A))
+        {
+            GstWalk.Pde.u |= X86_PDE_A;
+            GST_ATOMIC_OR(&GstWalk.pPde->u, X86_PDE_A);
+        }
+
         if (uErr & X86_TRAP_PF_RW)
         {
 #   ifdef VBOX_WITH_STATISTICS
-            if (!GstWalk.Pte.n.u1Dirty)
+            if (GstWalk.Pte.u & X86_PTE_D)
                 STAM_COUNTER_INC(&pVCpu->pgm.s.CTX_SUFF(pStats)->CTX_MID_Z(Stat,DirtiedPage));
             else
                 STAM_COUNTER_INC(&pVCpu->pgm.s.CTX_SUFF(pStats)->CTX_MID_Z(Stat,PageAlreadyDirty));
 #   endif
-            GstWalk.Pte.u   |= X86_PTE_A | X86_PTE_D;
-            GstWalk.pPte->u |= X86_PTE_A | X86_PTE_D;
+            if ((GstWalk.Pte.u & (X86_PTE_A | X86_PTE_D)) != (X86_PTE_A | X86_PTE_D))
+            {
+                GstWalk.Pte.u |= X86_PTE_A | X86_PTE_D;
+                GST_ATOMIC_OR(&GstWalk.pPte->u, X86_PTE_A | X86_PTE_D);
+            }
         }
         else
         {
-            GstWalk.Pte.u   |= X86_PTE_A;
-            GstWalk.pPte->u |= X86_PTE_A;
+            if (!(GstWalk.Pte.u & X86_PTE_A))
+            {
+                GstWalk.Pte.u |= X86_PTE_A;
+                GST_ATOMIC_OR(&GstWalk.pPte->u, X86_PTE_A);
+            }
         }
         Assert(GstWalk.Pte.u == GstWalk.pPte->u);
@@ -515 +543 @@
 #  else  /* !PGM_WITH_PAGING(PGM_GST_TYPE, PGM_SHW_TYPE) */
     GSTPDE const PdeSrcDummy = { X86_PDE_P | X86_PDE_US | X86_PDE_RW | X86_PDE_A}; /** @todo eliminate this */
-#  endif /* !PGM_WITH_PAGING(PGM_GST_TYPE, PGM_SHW_TYPE) */
 
     /* Take the big lock now. */
     *pfLockTaken = true;
     pgmLock(pVM);
+#  endif /* !PGM_WITH_PAGING(PGM_GST_TYPE, PGM_SHW_TYPE) */
 
 #  ifdef PGM_WITH_MMIO_OPTIMIZATIONS

trunk/src/VBox/VMM/include/PGMGstDefs.h

@@ -20 +20 @@
 *   Defined Constants And Macros                                               *
 *******************************************************************************/
+#undef GSTUINT
+#undef GST_ATOMIC_OR
 #undef GSTPT
 #undef PGSTPT
@@ -67 +69 @@
 
 # if PGM_SHW_TYPE == PGM_TYPE_EPT
+#  define GSTUINT                               uint64_t
+#  define GST_ATOMIC_OR(a_pu, a_fFlags)         ASMAtomicOrU64((a_pu), (a_fFlags))
 #  define GSTPT                                 X86PTPAE
 #  define PGSTPT                                PX86PTPAE
@@ -80 +84 @@
 # else
 #  if PGM_SHW_TYPE == PGM_TYPE_32BIT /* Same as shadow paging, but no PGMSHWPTEPAE. */
+#   define GSTUINT                              uint32_t
+#   define GST_ATOMIC_OR(a_pu, a_fFlags)        ASMAtomicOrU32((a_pu), (a_fFlags))
 #   define GSTPT                                X86PT
 #   define PGSTPT                               PX86PT
@@ -90 +96 @@
 #   define GST_PTE_PG_MASK                      X86_PTE_PG_MASK
 #  else
+#   define GSTUINT                              uint64_t
+#   define GST_ATOMIC_OR(a_pu, a_fFlags)        ASMAtomicOrU64((a_pu), (a_fFlags))
 #   define GSTPT                                X86PTPAE
 #   define PGSTPT                               PX86PTPAE
@@ -123 +131 @@
 
 #elif PGM_GST_TYPE == PGM_TYPE_32BIT
+# define GSTUINT                                uint32_t
+# define GST_ATOMIC_OR(a_pu, a_fFlags)          ASMAtomicOrU32((a_pu), (a_fFlags))
 # define GSTPT                                  X86PT
 # define PGSTPT                                 PX86PT
@@ -166 +176 @@
 #elif   PGM_GST_TYPE == PGM_TYPE_PAE \
      || PGM_GST_TYPE == PGM_TYPE_AMD64
+# define GSTUINT                                uint64_t
+# define GST_ATOMIC_OR(a_pu, a_fFlags)          ASMAtomicOrU64((a_pu), (a_fFlags))
 # define GSTPT                                  X86PTPAE
 # define PGSTPT                                 PX86PTPAE