[ros-diffs] [ion] 73718: [BOOTMGR]: Finish alloc/free test. It shows that it seems? to be working ok? :-) [BOOTLIB]: Wrap-up implementation of MmPapFreePhysicalPages. We no longer leak RAM.

Author: ion
Date: Sun Feb  5 22:02:24 2017
New Revision: 73718

URL: http://svn.reactos.org/svn/reactos?rev=73718&view=rev
Log:
[BOOTMGR]: Finish alloc/free test. It shows that it seems? to be working ok? :-)
[BOOTLIB]: Wrap-up implementation of MmPapFreePhysicalPages. We no longer leak RAM.

Modified:
    trunk/reactos/boot/environ/app/bootmgr/bootmgr.c
    trunk/reactos/boot/environ/lib/mm/pagealloc.c

Modified: trunk/reactos/boot/environ/app/bootmgr/bootmgr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/app/bootmgr/b…
==============================================================================
--- trunk/reactos/boot/environ/app/bootmgr/bootmgr.c	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/app/bootmgr/bootmgr.c	[iso-8859-1] Sun Feb  5 22:02:24
2017
@@ -2900,22 +2900,60 @@
     EfiPrintf(L"Performing memory allocator tests...\r\n");
     {
         NTSTATUS Status;
-        PHYSICAL_ADDRESS PhysicalAddress;
+        PHYSICAL_ADDRESS PhysicalAddress, PhysicalAddress2;
         PBL_MEMORY_DESCRIPTOR Found;
 
         /* Allocate 1 physical page */
         PhysicalAddress.QuadPart = 0;
         Status = BlMmAllocatePhysicalPages(&PhysicalAddress, BlLoaderData, 1, 0, 1);
-        EfiPrintf(L"Allocation status: %lx at address: %llx\r\n", Status,
PhysicalAddress.QuadPart);
-        EfiStall(10000);
-
-        Found = MmMdFindDescriptor(BL_MM_INCLUDE_UNMAPPED_ALLOCATED, 0,
PhysicalAddress.QuadPart >> PAGE_SHIFT);
-        EfiPrintf(L"Found descriptor: %p %llx\r\n", Found,
Found->BasePage);
-
+        if (Status != STATUS_SUCCESS)
+        {
+            EfiPrintf(L"FAIL: Allocation status: %lx at address: %llx\r\n",
Status, PhysicalAddress.QuadPart);
+            EfiStall(100000000);
+        }
+
+        /* Write some data */
+        *(PULONG)PhysicalAddress.QuadPart = 0x55555151;
+
+        /* Free it */
         Status = BlMmFreePhysicalPages(PhysicalAddress);
-        EfiPrintf(L"Memory free status: %lx\r\n", Status);
-    }
-
+        if (Status != STATUS_SUCCESS)
+        {
+            EfiPrintf(L"FAIL: Memory free status: %lx\r\n", Status);
+            EfiStall(100000000);
+        }
+
+        /* Allocate a page again */
+        PhysicalAddress2.QuadPart = 0;
+        Status = BlMmAllocatePhysicalPages(&PhysicalAddress2, BlLoaderData, 1, 0,
1);
+        if (Status != STATUS_SUCCESS)
+        {
+            EfiPrintf(L"FAIL: Allocation status: %lx at address: %llx\r\n",
Status, PhysicalAddress2.QuadPart);
+            EfiStall(100000000);
+        }
+
+        /* It should've given us the same page, since we freed it */
+        if (PhysicalAddress.QuadPart != PhysicalAddress2.QuadPart)
+        {
+            EfiPrintf(L"FAIL: Non-matching addresses: %llx %llx\r\n",
PhysicalAddress.QuadPart, PhysicalAddress2.QuadPart);
+            EfiStall(100000000);
+        }
+
+        /* The data should still be there, since zero-ing is not on for bootmgr */
+        if (*(PULONG)PhysicalAddress2.QuadPart != 0x55555151)
+        {
+            EfiPrintf(L"FAIL: Non-matching data: %lx %lx\r\n", 0x55555151,
*(PULONG)PhysicalAddress2.QuadPart);
+            EfiStall(100000000);
+        }
+
+        /* And free the second page again */
+        Status = BlMmFreePhysicalPages(PhysicalAddress);
+        if (Status != STATUS_SUCCESS)
+        {
+            EfiPrintf(L"FAIL: Memory free status: %lx\r\n", Status);
+            EfiStall(100000000);
+        }
+    }
 
     /* Write out the first XML tag */
     BlXmiWrite(L"<bootmgr/>");

Modified: trunk/reactos/boot/environ/lib/mm/pagealloc.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/lib/mm/pageal…
==============================================================================
--- trunk/reactos/boot/environ/lib/mm/pagealloc.c	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/lib/mm/pagealloc.c	[iso-8859-1] Sun Feb  5 22:02:24 2017
@@ -740,9 +740,43 @@
         Flags |= BL_MM_ADD_DESCRIPTOR_NEVER_COALESCE_FLAG;
     }
 
-    /* TBD */
-    EfiPrintf(L"Leaking memory: %p!\r\n", Address.QuadPart);
-    return STATUS_SUCCESS;
+    /* Check if the entire allocation is being free*/
+    if (PageCount == Descriptor->PageCount)
+    {
+        /* Remove the descriptor from the allocated list */
+        MmMdRemoveDescriptorFromList(&MmMdlUnmappedAllocated, Descriptor);
+
+        /* Mark the entire descriptor as free */
+        Descriptor->Type = BlConventionalMemory;
+    }
+    else
+    {
+        /* Init a descriptor for what we're actually freeing */
+        Descriptor = MmMdInitByteGranularDescriptor(Descriptor->Flags,
+                                                    BlConventionalMemory,
+                                                    Page,
+                                                    0,
+                                                    PageCount);
+        if (!Descriptor)
+        {
+            return STATUS_NO_MEMORY;
+        }
+
+        /* Remove the region from the existing descriptor */
+        Status = MmMdRemoveRegionFromMdlEx(&MmMdlUnmappedAllocated,
+                                           BL_MM_REMOVE_PHYSICAL_REGION_FLAG,
+                                           Page,
+                                           PageCount,
+                                           NULL);
+        if (!NT_SUCCESS(Status))
+        {
+            return Status;
+        }
+    }
+
+    /* Add the new descriptor into in the list (or the old, repurposed one) */
+    Descriptor->Flags &= ~BlMemoryCoalesced;
+    return MmMdAddDescriptorToList(&MmMdlUnmappedUnallocated, Descriptor, Flags);
 }
 
 NTSTATUS