[jgardou] 54392: [NTOSKRNL/MM] - Some more cleanup. - Work with page rounded down addresses in SectionView page faults. - You should not create a virtual mapping on a non page-aligned address range...

Author: jgardou
Date: Tue Nov 15 23:50:04 2011
New Revision: 54392

URL: http://svn.reactos.org/svn/reactos?rev=54392&view=rev
Log:
[NTOSKRNL/MM]
- Some more cleanup.
- Work with page rounded down addresses in SectionView page faults.
- You should not create a virtual mapping on a non page-aligned address range : ASSERT
about that.
- Flush Tlb each time we change it
- Fix logic I broke in MmCreateVirtualMapping (thanks to Thomas Faber)

Modified:
    trunk/reactos/ntoskrnl/mm/i386/page.c
    trunk/reactos/ntoskrnl/mm/section.c

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 15 23:50:04 2011
@@ -284,37 +284,34 @@
     PageDir = (PULONG)MiAddressToPde(Address);
     if (0 == InterlockedCompareExchangePte(PageDir, 0, 0))
     {
-        if (Address >= MmSystemRangeStart)
-        {
-            if (0 ==
InterlockedCompareExchangePte(&MmGlobalKernelPageDirectory[PdeOffset], 0, 0))
-            {
-                if (Create == FALSE)
-                {
-                    return NULL;
-                }
-                MI_SET_USAGE(MI_USAGE_LEGACY_PAGE_DIRECTORY);
-                if (Process) MI_SET_PROCESS2(Process->ImageFileName);
-                if (!Process) MI_SET_PROCESS2("Kernel Legacy");
-                Status = MmRequestPageMemoryConsumer(MC_SYSTEM, FALSE, &Pfn);
-                if (!NT_SUCCESS(Status) || Pfn == 0)
-                {
-                    KeBugCheck(MEMORY_MANAGEMENT);
-                }
-                Entry = PFN_TO_PTE(Pfn) | PA_PRESENT | PA_READWRITE;
-                if(0 !=
InterlockedCompareExchangePte(&MmGlobalKernelPageDirectory[PdeOffset], Entry, 0))
-                {
-                    MmReleasePageMemoryConsumer(MC_SYSTEM, Pfn);
-                }
-                InterlockedExchangePte(PageDir, MmGlobalKernelPageDirectory[PdeOffset]);
-                RtlZeroMemory(MiPteToAddress(PageDir), PAGE_SIZE);
-                return (PULONG)MiAddressToPte(Address);
+        if (0 ==
InterlockedCompareExchangePte(&MmGlobalKernelPageDirectory[PdeOffset], 0, 0))
+        {
+            if (Create == FALSE)
+            {
+                return NULL;
+            }
+            MI_SET_USAGE(MI_USAGE_LEGACY_PAGE_DIRECTORY);
+            if (Process) MI_SET_PROCESS2(Process->ImageFileName);
+            if (!Process) MI_SET_PROCESS2("Kernel Legacy");
+            Status = MmRequestPageMemoryConsumer(MC_SYSTEM, FALSE, &Pfn);
+            if (!NT_SUCCESS(Status) || Pfn == 0)
+            {
+                KeBugCheck(MEMORY_MANAGEMENT);
+            }
+            Entry = PFN_TO_PTE(Pfn) | PA_PRESENT | PA_READWRITE;
+            if(0 !=
InterlockedCompareExchangePte(&MmGlobalKernelPageDirectory[PdeOffset], Entry, 0))
+            {
+                MmReleasePageMemoryConsumer(MC_SYSTEM, Pfn);
             }
             InterlockedExchangePte(PageDir, MmGlobalKernelPageDirectory[PdeOffset]);
-        }
-        else
-        {
-            
-        }
+            /* 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);
 }
@@ -700,15 +697,8 @@
     }
     Pte = *Pt;
     InterlockedExchangePte(Pt, SwapEntry << 1);
-    if (Pte != 0)
-    {
-        MiFlushTlb(Pt, Address);
-    }
-    else
-    {
-        MmUnmapPageTable(Pt);
-    }
-    
+    MiFlushTlb(Pt, Address);
+        
     return(STATUS_SUCCESS);
 }
 
@@ -729,6 +719,8 @@
     ULONG Pte;
     DPRINT("MmCreateVirtualMappingUnsafe(%x, %x, %x, %x (%x), %d)\n",
            Process, Address, flProtect, Pages, *Pages, PageCount);
+           
+    ASSERT(((ULONG_PTR)Address % PAGE_SIZE) == 0);
     
     if (Process == NULL)
     {
@@ -772,6 +764,8 @@
     }
     
     Addr = Address;
+    /* MmGetPageTableForProcess should be called on the first run, so
+     * let this trigger it */
     oldPdeOffset = ADDR_TO_PDE_OFFSET(Addr) + 1;
     for (i = 0; i < PageCount; i++, Addr = (PVOID)((ULONG_PTR)Addr + PAGE_SIZE))
     {
@@ -785,6 +779,7 @@
         PdeOffset = ADDR_TO_PDE_OFFSET(Addr);
         if (oldPdeOffset != PdeOffset)
         {
+            if(Pt) MmUnmapPageTable(Pt);
             Pt = MmGetPageTableForProcess(Process, Addr, TRUE);
             if (Pt == NULL)
             {
@@ -805,8 +800,18 @@
             KeBugCheck(MEMORY_MANAGEMENT);
         }
         InterlockedExchangePte(Pt, PFN_TO_PTE(Pages[i]) | Attributes);
-        MiFlushTlb(Pt, Addr);
-    }
+        /* 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))
+        {
+            KeInvalidateTlbEntry(Addr);
+        }
+    }
+    
+    ASSERT(Addr > Address);
+    MmUnmapPageTable(Pt);
     
     return(STATUS_SUCCESS);
 }

Modified: trunk/reactos/ntoskrnl/mm/section.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] Tue Nov 15 23:50:04 2011
@@ -1314,7 +1314,6 @@
    ULONG Offset;
    PFN_NUMBER Page;
    NTSTATUS Status;
-   PVOID PAddress;
    PROS_SECTION_OBJECT Section;
    PMM_SECTION_SEGMENT Segment;
    ULONG_PTR Entry;
@@ -1343,8 +1342,8 @@
       return(STATUS_UNSUCCESSFUL);
    }
 
-   PAddress = MM_ROUND_DOWN(Address, PAGE_SIZE);
-   Offset = (ULONG)((ULONG_PTR)PAddress - (ULONG_PTR)MemoryArea->StartingAddress
+   Address = MM_ROUND_DOWN(Address, PAGE_SIZE);
+   Offset = (ULONG)((ULONG_PTR)Address - (ULONG_PTR)MemoryArea->StartingAddress
             + MemoryArea->Data.SectionData.ViewOffset);
 
    Segment = MemoryArea->Data.SectionData.Segment;
@@ -1432,7 +1431,7 @@
       if (!MmIsPagePresent(Process, Address))
       {
          Entry = MmGetPageEntrySectionSegment(Segment, Offset);
-         HasSwapEntry = MmIsPageSwapEntry(Process, (PVOID)PAddress);
+         HasSwapEntry = MmIsPageSwapEntry(Process, (PVOID)Address);
 
          if (PAGE_FROM_SSE(Entry) == 0 || HasSwapEntry)
          {
@@ -1461,7 +1460,7 @@
             DPRINT1("Unable to create virtual mapping\n");
             KeBugCheck(MEMORY_MANAGEMENT);
          }
-         MmInsertRmap(Page, Process, (PVOID)PAddress);
+         MmInsertRmap(Page, Process, Address);
       }
       MmUnlockSectionSegment(Segment);
       PageOp->Status = STATUS_SUCCESS;
@@ -1470,7 +1469,7 @@
       return(STATUS_SUCCESS);
    }
 
-   HasSwapEntry = MmIsPageSwapEntry(Process, (PVOID)PAddress);
+   HasSwapEntry = MmIsPageSwapEntry(Process, Address);
    if (HasSwapEntry)
    {
       /*
@@ -1488,7 +1487,7 @@
       }
 
       MmUnlockSectionSegment(Segment);
-      MmDeletePageFileMapping(Process, (PVOID)PAddress, &SwapEntry);
+      MmDeletePageFileMapping(Process, Address, &SwapEntry);
 
       MmUnlockAddressSpace(AddressSpace);
       MI_SET_USAGE(MI_USAGE_SECTION);
@@ -1527,8 +1526,7 @@
       /*
        * Add the page to the process's working set
        */
-      MmInsertRmap(Page, Process, (PVOID)PAddress);
-
+      MmInsertRmap(Page, Process, Address);
       /*
        * Finish the operation
        */
@@ -1600,7 +1598,7 @@
           KeBugCheck(MEMORY_MANAGEMENT);
          return(Status);
       }
-      MmInsertRmap(Page, Process, (PVOID)PAddress);
+      MmInsertRmap(Page, Process, Address);
 
       /*
        * Cleanup and release locks
@@ -1699,7 +1697,7 @@
          DPRINT1("Unable to create virtual mapping\n");
           KeBugCheck(MEMORY_MANAGEMENT);
       }
-      MmInsertRmap(Page, Process, (PVOID)PAddress);
+      MmInsertRmap(Page, Process, Address);
 
       PageOp->Status = STATUS_SUCCESS;
       MmspCompleteAndReleasePageOp(PageOp);
@@ -1772,7 +1770,7 @@
          DPRINT1("Unable to create virtual mapping\n");
           KeBugCheck(MEMORY_MANAGEMENT);
       }
-      MmInsertRmap(Page, Process, (PVOID)PAddress);
+      MmInsertRmap(Page, Process, Address);
       PageOp->Status = STATUS_SUCCESS;
       MmspCompleteAndReleasePageOp(PageOp);
       DPRINT("Address 0x%.8X\n", Address);
@@ -1800,7 +1798,7 @@
          DPRINT1("Unable to create virtual mapping\n");
           KeBugCheck(MEMORY_MANAGEMENT);
       }
-      MmInsertRmap(Page, Process, (PVOID)PAddress);
+      MmInsertRmap(Page, Process, Address);
       PageOp->Status = STATUS_SUCCESS;
       MmspCompleteAndReleasePageOp(PageOp);
       DPRINT("Address 0x%.8X\n", Address);
@@ -1819,7 +1817,6 @@
    PFN_NUMBER OldPage;
    PFN_NUMBER NewPage;
    NTSTATUS Status;
-   PVOID PAddress;
    ULONG Offset;
    PMM_PAGEOP PageOp;
    PMM_REGION Region;
@@ -1840,8 +1837,8 @@
    /*
     * Find the offset of the page
     */
-   PAddress = MM_ROUND_DOWN(Address, PAGE_SIZE);
-   Offset = (ULONG)((ULONG_PTR)PAddress - (ULONG_PTR)MemoryArea->StartingAddress
+   Address = MM_ROUND_DOWN(Address, PAGE_SIZE);
+   Offset = (ULONG)((ULONG_PTR)Address - (ULONG_PTR)MemoryArea->StartingAddress
             + MemoryArea->Data.SectionData.ViewOffset);
 
    Segment = MemoryArea->Data.SectionData.Segment;
@@ -1874,7 +1871,7 @@
        PFN_FROM_SSE(Entry) != OldPage)
    {
       /* This is a private page. We must only change the page protection. */
-      MmSetPageProtect(Process, PAddress, Region->Protect);
+      MmSetPageProtect(Process, Address, Region->Protect);
       return(STATUS_SUCCESS);
    }
 
@@ -1938,7 +1935,7 @@
    /*
     * Copy the old page
     */
-   MiCopyFromUserPage(NewPage, PAddress);
+   MiCopyFromUserPage(NewPage, Address);
 
    MmLockAddressSpace(AddressSpace);
    /*
@@ -1969,8 +1966,8 @@
    /*
     * Unshare the old page.
     */
-   MmDeleteRmap(OldPage, Process, PAddress);
-   MmInsertRmap(NewPage, Process, PAddress);
+   MmDeleteRmap(OldPage, Process, Address);
+   MmInsertRmap(NewPage, Process, Address);
    MmLockSectionSegment(Segment);
    MmUnsharePageEntrySectionSegment(Section, Segment, Offset, FALSE, FALSE);
    MmUnlockSectionSegment(Segment);