[sir_richard] 47183: [NTOS]: Add assertions to the paged pool and demand zero page faults, to catch possible errors and corruptions. These paths are not yet taken in today's builds, so they do not affect any runtime code. [NTOS]: Add assertions regarding the portability of certain code, which will need changes on ARM/x64. These should probably be C_ASSERT's but I don't want to break Timo's build. [NTOS]: Define MM_NOIRQL (found in assertions) instead of magical -1. [NTOS]: Add MI_IS_SESSION_PTE

Author: sir_richard
Date: Wed May 12 22:48:15 2010
New Revision: 47183

URL: http://svn.reactos.org/svn/reactos?rev=47183&view=rev
Log:
[NTOS]: Add assertions to the paged pool and demand zero page faults, to catch possible
errors and corruptions. These paths are not yet taken in today's builds, so they do
not affect any runtime code.
[NTOS]: Add assertions regarding the portability of certain code, which will need changes
on ARM/x64. These should probably be C_ASSERT's but I don't want to break
Timo's build.
[NTOS]: Define MM_NOIRQL (found in assertions) instead of magical -1.
[NTOS]: Add MI_IS_SESSION_PTE macro.
[NTOS]: Export the MiXxxPte variables.
[NTOS]: Fix some typos in comments.


Modified:
    trunk/reactos/ntoskrnl/mm/ARM3/miarm.h
    trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c
    trunk/reactos/ntoskrnl/mm/ARM3/pool.c

Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/miarm.h?r…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] Wed May 12 22:48:15 2010
@@ -42,7 +42,7 @@
 #define _1KB (1024)
 #define _1MB (1024 * _1KB)
 
-/* Are mapped by a PDE */
+/* Area mapped by a PDE */
 #define PDE_MAPPED_VA   (PTE_COUNT * PAGE_SIZE)
 
 /* Size of a PDE directory, and size of a page table */
@@ -90,6 +90,18 @@
 #define MM_NOACCESS            (MM_DECOMMIT | MM_NOCACHE)
 
 //
+// Assertions for session images, addresses, and PTEs
+//
+#define MI_IS_SESSION_IMAGE_ADDRESS(Address) \
+    (((Address) >= MiSessionImageStart) && ((Address) <
MiSessionImageEnd))
+    
+#define MI_IS_SESSION_ADDRESS(Address) \
+    (((Address) >= MmSessionBase) && ((Address) < MiSessionSpaceEnd))
+        
+#define MI_IS_SESSION_PTE(Pte) \
+    ((((PMMPTE)Pte) >= MiSessionBasePte) && (((PMMPTE)Pte) <
MiSessionLastPte))
+
+//
 // Corresponds to MMPTE_SOFTWARE.Protection
 //
 #ifdef _M_IX86
@@ -119,6 +131,11 @@
 //
 #define LIST_HEAD 0xFFFFFFFF
 
+//
+// Special IRQL value (found in assertions)
+//
+#define MM_NOIRQL (KIRQL)0xFFFFFFFF
+    
 //
 // FIXFIX: These should go in ex.h after the pool merge
 //
@@ -284,6 +301,10 @@
 extern ULONG MmSystemViewSize;
 extern PVOID MmSessionBase;
 extern PVOID MiSessionSpaceEnd;
+extern PMMPTE MiSessionImagePteStart;
+extern PMMPTE MiSessionImagePteEnd;
+extern PMMPTE MiSessionBasePte;
+extern PMMPTE MiSessionLastPte;
 extern ULONG MmSizeOfPagedPoolInBytes;
 extern PMMPTE MmSystemPagePtes;
 extern PVOID MmSystemCacheStart;
@@ -327,16 +348,17 @@
 extern PFN_NUMBER MmTotalSystemDriverPages;
 extern PVOID MiSessionImageStart;
 extern PVOID MiSessionImageEnd;
+extern PMMPTE MiHighestUserPte;
+extern PMMPDE MiHighestUserPde;
+extern PFN_NUMBER MmSystemPageDirectory[PD_COUNT];
 
 #define MI_PFN_TO_PFNENTRY(x)     (&MmPfnDatabase[1][x])
 #define MI_PFNENTRY_TO_PFN(x)     (x - MmPfnDatabase[1])
 
-#define MI_IS_SESSION_IMAGE_ADDRESS(Address) \
-    (((Address) >= MiSessionImageStart) && ((Address) <
MiSessionImageEnd))
-
-#define MI_IS_SESSION_ADDRESS(Address) \
-    (((Address) >= MmSessionBase) && ((Address) < MiSessionSpaceEnd))
-        
+//
+// Returns if the page is physically resident (ie: a large page)
+// FIXFIX: CISC/x86 only?
+//
 FORCEINLINE
 BOOLEAN
 MI_IS_PHYSICAL_ADDRESS(IN PVOID Address)

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] Wed May 12 22:48:15 2010
@@ -27,6 +27,10 @@
     PMMPDE PointerPde;
     NTSTATUS Status = STATUS_SUCCESS;
     
+    /* No session support in ReactOS yet */
+    ASSERT(MI_IS_SESSION_ADDRESS(Address) == FALSE);
+    ASSERT(MI_IS_SESSION_PTE(Address) == FALSE);
+    
     //
     // Check if this is a fault while trying to access the page table itself
     //
@@ -60,6 +64,9 @@
     //
     if (PointerPde->u.Hard.Valid == 0)
     {
+        /* This seems to be making the assumption that one PDE is one page long */
+        ASSERT(PAGE_SIZE == (PD_COUNT * (sizeof(MMPTE) * PDE_COUNT)));
+        
         //
         // Copy it from our double-mapped system page directory
         //
@@ -88,6 +95,10 @@
             Address,
             Process);
     
+    /* Must currently only be called by paging path, for system addresses only */
+    ASSERT(OldIrql == MM_NOIRQL);
+    ASSERT(Process == NULL);
+        
     //
     // Lock the PFN database
     //
@@ -110,11 +121,16 @@
     //
     InterlockedIncrement(&KeGetCurrentPrcb()->MmDemandZeroCount);
     
+    /* Shouldn't see faults for user PTEs yet */
+    ASSERT(PointerPte > MiHighestUserPte);
+    
     //
     // Build the PTE
     //
     TempPte = ValidKernelPte;
     TempPte.u.Hard.PageFrameNumber = PageFrameNumber;
+    ASSERT(TempPte.u.Hard.Valid == 1);
+    ASSERT(PointerPte->u.Hard.Valid == 0);
     *PointerPte = TempPte;
     ASSERT(PointerPte->u.Hard.Valid == 1);
     
@@ -155,6 +171,9 @@
     //
     TempPte = *PointerPte;
     
+    /* No prototype */
+    ASSERT(PrototypePte == NULL);
+    
     //
     // The PTE must be invalid, but not totally blank
     //
@@ -175,7 +194,8 @@
     Status = MiResolveDemandZeroFault(Address,
                                       PointerPte,
                                       Process,
-                                      -1);
+                                      MM_NOIRQL);
+    ASSERT(KeAreAllApcsDisabled () == TRUE);
     if (NT_SUCCESS(Status))
     {
         //

Modified: trunk/reactos/ntoskrnl/mm/ARM3/pool.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pool.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/pool.c [iso-8859-1] Wed May 12 22:48:15 2010
@@ -334,12 +334,13 @@
                 //
                 // Save it into our double-buffered system page directory
                 //
+                /* This seems to be making the assumption that one PDE is one page long
*/
+                ASSERT(PAGE_SIZE == (PD_COUNT * (sizeof(MMPTE) * PDE_COUNT)));
                 MmSystemPagePtes[(ULONG_PTR)PointerPte & (PAGE_SIZE - 1) /
                                  sizeof(MMPTE)] = TempPte;
                             
-                //
-                // Write the actual PTE now
-                //
+                /* Write the actual PTE now */
+                ASSERT(TempPte.u.Hard.Valid == 1);
                 *PointerPte++ = TempPte;
                 
                 //
@@ -432,6 +433,7 @@
             //
             // Write the demand zero PTE and keep going
             //
+            ASSERT(PointerPte->u.Hard.Valid == 0);
             *PointerPte++ = TempPte;
         } while (PointerPte < StartPte);