[ros-diffs] [jgardou] 54477: [NTOSKRNL/MM] - create a page op when changing page protection attribute - properly handle PAGE_NOACCESS protection change in NtProtectVirtualMemory - Round down the address we're ...

Author: jgardou
Date: Tue Nov 22 22:01:06 2011
New Revision: 54477

URL: http://svn.reactos.org/svn/reactos?rev=54477&view=rev
Log:
[NTOSKRNL/MM]
- create a page op when changing page protection attribute
- properly handle PAGE_NOACCESS protection change in NtProtectVirtualMemory
- Round down the address we're given when creating a page op, so we're sure there
is o,ly one page op for running for one page
- Add a small hack
Now ReatOS boots with DPH enabled on low memory conditions

Modified:
    trunk/reactos/ntoskrnl/mm/anonmem.c
    trunk/reactos/ntoskrnl/mm/i386/page.c
    trunk/reactos/ntoskrnl/mm/pageop.c

Modified: trunk/reactos/ntoskrnl/mm/anonmem.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/anonmem.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/anonmem.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/anonmem.c [iso-8859-1] Tue Nov 22 22:01:06 2011
@@ -420,16 +420,42 @@
             OldProtect != NewProtect)
     {
         ULONG i;
+        PMM_PAGEOP PageOp;
+        PMEMORY_AREA MArea;
+        char* addr = (char*)BaseAddress;
 
         for (i=0; i < PAGE_ROUND_UP(RegionSize)/PAGE_SIZE; i++)
         {
-            if (MmIsPagePresent(Process,
-                        (char*)BaseAddress + (i*PAGE_SIZE)))
+            MArea = MmLocateMemoryAreaByAddress(AddressSpace, addr);
+            do
             {
-                MmSetPageProtect(Process,
-                (char*)BaseAddress + (i*PAGE_SIZE),
-                NewProtect);
+                PageOp = MmGetPageOp(MArea, Process->UniqueProcessId, addr,
+                    NULL, 0, MM_PAGEOP_CHANGEPROTECT, TRUE);
+            } while(PageOp == NULL);
+            
+            /* Should we enable/disable virtual mapping? */
+            if((NewProtect & PAGE_NOACCESS) && 
+                !(OldProtect & PAGE_NOACCESS) && 
+                (MmIsPagePresent(Process, addr)))
+            {
+                /* Set other flags if any */
+                if(NewProtect != PAGE_NOACCESS)
+                    MmSetPageProtect(Process, addr, NewProtect & ~PAGE_NOACCESS);
+                MmDisableVirtualMapping(Process, addr, NULL, NULL);
             }
+            else if((OldProtect & PAGE_NOACCESS) && !(NewProtect &
PAGE_NOACCESS))
+            {
+                MmEnableVirtualMapping(Process, addr);
+            }
+            
+            /* Set new protection flags */
+            if(MmIsPagePresent(Process, addr))
+            {
+                MmSetPageProtect(Process, addr, NewProtect);
+            }
+            KeSetEvent(&PageOp->CompletionEvent, IO_NO_INCREMENT, FALSE);
+            MmReleasePageOp(PageOp);
+            addr += PAGE_SIZE;
         }
     }
 }

Modified: trunk/reactos/ntoskrnl/mm/i386/page.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/i386/page.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/i386/page.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/i386/page.c [iso-8859-1] Tue Nov 22 22:01:06 2011
@@ -304,21 +304,17 @@
                 MmReleasePageMemoryConsumer(MC_SYSTEM, Pfn);
             }
             InterlockedExchangePte(PageDir, MmGlobalKernelPageDirectory[PdeOffset]);
-            /* Flush it before accessing it */
-            KeInvalidateTlbEntry(MiPteToAddress(PageDir));
             RtlZeroMemory(MiPteToAddress(PageDir), PAGE_SIZE);
             return (PULONG)MiAddressToPte(Address);
         }
         InterlockedExchangePte(PageDir, MmGlobalKernelPageDirectory[PdeOffset]);
-        /* Flush it before accessing it */
-        KeInvalidateTlbEntry(MiPteToAddress(PageDir));
     }
     return (PULONG)MiAddressToPte(Address);
 }
 
 BOOLEAN MmUnmapPageTable(PULONG Pt)
 {
-    if (Pt >= (PULONG)PAGETABLE_MAP && Pt < (PULONG)PAGETABLE_MAP +
1024*1024)
+    if (!IS_HYPERSPACE(Pt))
     {
         return TRUE;
     }
@@ -383,7 +379,10 @@
         Pte = *Pt;
     } while (Pte != InterlockedCompareExchangePte(Pt, Pte & ~PA_PRESENT, Pte));
     
-    MiFlushTlb(Pt, Address);
+    if(Pte & PA_PRESENT)
+        MiFlushTlb(Pt, Address);
+    else
+        MmUnmapPageTable(Pt);
     
     WasValid = (PAGE_MASK(Pte) != 0);
     if (!WasValid)
@@ -457,16 +456,17 @@
      */
     Pte = InterlockedExchangePte(Pt, 0);
     
-    MiFlushTlb(Pt, Address);
     
     WasValid = (PAGE_MASK(Pte) != 0);
     if (WasValid)
     {
         Pfn = PTE_TO_PFN(Pte);
+        MiFlushTlb(Pt, Address);
     }
     else
     {
         Pfn = 0;
+        MmUnmapPageTable(Pt);
     }
     
     if (FreePage && WasValid)
@@ -523,7 +523,13 @@
      */
     Pte = InterlockedExchangePte(Pt, 0);
     
-    MiFlushTlb(Pt, Address);
+    //MiFlushTlb(Pt, Address);
+    MmUnmapPageTable(Pt);
+    
+    if(!(Pte & 0x800))
+    {
+        KeBugCheck(MEMORY_MANAGEMENT);
+    }
     
     /*
      * Return some information to the caller
@@ -632,8 +638,14 @@
     Pt = MmGetPageTableForProcess(Process, Address, FALSE);
     if (Pt == NULL)
     {
-        KeBugCheck(MEMORY_MANAGEMENT);
-    }
+        //HACK to get DPH working, waiting for MM rewrite :-/
+        //KeBugCheck(MEMORY_MANAGEMENT);
+        return;
+    }
+    
+    /* Do not mark a 0 page as present */
+    if(0 == InterlockedCompareExchangePte(Pt, 0, 0))
+        return;
     
     do
     {
@@ -695,9 +707,13 @@
     {
         KeBugCheck(MEMORY_MANAGEMENT);
     }
-    Pte = *Pt;
-    InterlockedExchangePte(Pt, SwapEntry << 1);
-    MiFlushTlb(Pt, Address);
+    Pte = InterlockedExchangePte(Pt, SwapEntry << 1);
+    if(PAGE_MASK(Pte))
+    {
+        KeBugCheck(MEMORY_MANAGEMENT);
+    }
+    //MiFlushTlb(Pt, Address);
+    MmUnmapPageTable(Pt);
         
     return(STATUS_SUCCESS);
 }
@@ -792,19 +808,19 @@
         }
         oldPdeOffset = PdeOffset;
         
-        Pte = *Pt;
+        Pte = InterlockedExchangePte(Pt, PFN_TO_PTE(Pages[i]) | Attributes);;
         /* There should not be anything valid here */
-        if ((Pte & PA_PRESENT) || (Pte & 0x800))
-        {
+        if (PAGE_MASK(Pte) != 0)
+        {
+            PMMPFN Pfn1 = MiGetPfnEntry(PTE_TO_PFN(Pte));
+            (void)Pfn1;
             DPRINT1("Bad PTE %lx\n", Pte);
             KeBugCheck(MEMORY_MANAGEMENT);
         }
-        InterlockedExchangePte(Pt, PFN_TO_PTE(Pages[i]) | Attributes);
         /* flush if currently mapped, just continue editing if hyperspace
-         * NOTE : Do not call MiFlushTlb, as it will unmap the page table,
-         * and we might need it afterwards */
-        if (Address >= MmSystemRangeStart ||
-            (Pt >= (PULONG)PAGETABLE_MAP && Pt < (PULONG)PAGETABLE_MAP +
1024*1024))
+         * NOTE : This check is similar to what is done in MiFlushTlb, but we 
+         * don't use it because it would unmap the page table */
+        if (Addr >= MmSystemRangeStart || (!IS_HYPERSPACE(Pt)))
         {
             KeInvalidateTlbEntry(Addr);
         }
@@ -889,6 +905,7 @@
 {
     ULONG Attributes = 0;
     PULONG Pt;
+    ULONG Pte;
     
     DPRINT("MmSetPageProtect(Process %x  Address %x  flProtect %x)\n",
            Process, Address, flProtect);
@@ -910,8 +927,17 @@
     {
         KeBugCheck(MEMORY_MANAGEMENT);
     }
-    InterlockedExchangePte(Pt, PAGE_MASK(*Pt) | Attributes | (*Pt &
(PA_ACCESSED|PA_DIRTY)));
-    MiFlushTlb(Pt, Address);
+    Pte = InterlockedExchangePte(Pt, PAGE_MASK(*Pt) | Attributes | (*Pt &
(PA_ACCESSED|PA_DIRTY)));
+    
+    if(!PAGE_MASK(Pte))
+    {
+        DPRINT1("Invalid Pte %lx\n", Pte);
+        __debugbreak();
+    }
+    if((Pte & Attributes) != Attributes)
+        MiFlushTlb(Pt, Address);
+    else
+        MmUnmapPageTable(Pt);
 }
 
 /*

Modified: trunk/reactos/ntoskrnl/mm/pageop.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/pageop.c?rev=5…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/pageop.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/pageop.c [iso-8859-1] Tue Nov 22 22:01:06 2011
@@ -145,6 +145,8 @@
    ULONG_PTR Hash;
    KIRQL oldIrql;
    PMM_PAGEOP PageOp;
+   
+   Address = (PVOID)PAGE_ROUND_DOWN(Address);
 
    /*
     * Calcuate the hash value for pageop structure