[ros-diffs] [sir_richard] 55894: [NTOS]: A few key changes to the page fault path: 1) MiCheckVirtualAddress should be called *after* determining if the PTE is a Demand Zero PTE. This is because when memory is ...

Author: sir_richard
Date: Mon Feb 27 23:42:22 2012
New Revision: 55894

URL: http://svn.reactos.org/svn/reactos?rev=55894&view=rev
Log:
[NTOS]: A few key changes to the page fault path:
1) MiCheckVirtualAddress should be called *after* determining if the PTE is a Demand Zero
PTE. This is because when memory is allocated with MEM_RESERVE, and then MEM_COMMIT is
called later, the VAD does not have the MemCommit flag set to TRUE. As such,
MiCheckVirtualAddress returns MM_NOACCESS for the VAD (even though one is found) and the
demand zero fault results in an access violation. Double-checked with Windows and this is
the right behavior.
2) MiCheckVirtualAddress now supports non-commited reserve VADs (ie: trying to access
MEM_RESERVE memory). It used to ASSERT, now it returns MM_NOACCESS so an access violation
is raised. Before change #1, this would also happen if MEM_COMMIT was later performed on
the ranges, but this is now fixed.
3) When calling MiResolveDemandZeroFault, we should not make the PDE a demand zero PDE.
This is senseless. The whole point is that the PDE does exist, and MiInitializePfn needs
it to keep track of the page table allocation. Removed the nonsensical line of code which
performed cleard the PDE during a demand-zero fault.
I am able to boot to 3rd stage with these changes, so I have seen no regressions.
Additionally, with these changes, the as-of-yet-uncommitted VAD-based Virtual Memory code
completes 1st stage setup successfully, instead of instantly crashing on boot.

Modified:
    trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c

Modified: trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pagfault.…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] Mon Feb 27 23:42:22 2012
@@ -76,11 +76,19 @@
     ASSERT(Vad->u.VadFlags.VadType == VadNone);
 
     /* Check if it's a section, or just an allocation */
-    if (Vad->u.VadFlags.PrivateMemory == TRUE)
+    if (Vad->u.VadFlags.PrivateMemory)
     {
         /* This must be a TEB/PEB VAD */
-        ASSERT(Vad->u.VadFlags.MemCommit == TRUE);
-        *ProtectCode = (ULONG)Vad->u.VadFlags.Protection;
+        if (Vad->u.VadFlags.MemCommit)
+        {
+            /* It's committed, so return the VAD protection */
+            *ProtectCode = (ULONG)Vad->u.VadFlags.Protection;
+        }
+        else
+        {
+            /* It has not yet been committed, so return no access */
+            *ProtectCode = MM_NOACCESS;
+        }
         return NULL;
     }
     else
@@ -349,7 +357,7 @@
     //
     // It's all good now
     //
-    DPRINT("Paged pool page has now been paged in\n");
+    DPRINT("Demand zero page has now been paged in\n");
     return STATUS_PAGE_FAULT_DEMAND_ZERO;
 }
 
@@ -679,7 +687,6 @@
     PMMVAD Vad;
     PFN_NUMBER PageFrameIndex;
     ULONG Color;
-
     DPRINT("ARM3 FAULT AT: %p\n", Address);
 
     /* Check for page fault on high IRQL */
@@ -887,15 +894,6 @@
     ASSERT(PointerPde->u.Hard.LargePage == 0);
 #endif
 
-    /* Check if this address range belongs to a valid allocation (VAD) */
-    ProtoPte = MiCheckVirtualAddress(Address, &ProtectionCode, &Vad);
-    if (ProtectionCode == MM_NOACCESS)
-    {
-        /* This is a bogus VA */
-        MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread);
-        return STATUS_ACCESS_VIOLATION;
-    }
-
 #if (_MI_PAGING_LEVELS == 4)
     /* Check if the PXE is valid */
     if (PointerPxe->u.Hard.Valid == 0)
@@ -966,7 +964,6 @@
     if (TempPte.u.Long == (MM_READWRITE << MM_PTE_SOFTWARE_PROTECTION_BITS))
     {
         /* Resolve the fault */
-        MI_WRITE_INVALID_PDE(PointerPde, DemandZeroPde);
         MiResolveDemandZeroFault(Address,
                                  (ULONG)PointerPte->u.Soft.Protection,
                                  CurrentProcess,
@@ -979,6 +976,15 @@
 
     /* Make sure it's not a prototype PTE */
     ASSERT(TempPte.u.Soft.Prototype == 0);
+    
+    /* Check if this address range belongs to a valid allocation (VAD) */
+    ProtoPte = MiCheckVirtualAddress(Address, &ProtectionCode, &Vad);
+    if (ProtectionCode == MM_NOACCESS)
+    {
+        /* This is a bogus VA */
+        MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread);
+        return STATUS_ACCESS_VIOLATION;
+    }
 
     /* Check for non-demand zero PTE */
     if (TempPte.u.Long != 0)