[edk2-devel] [PATCH v4 09/14] MdeModulePkg: Fix Bugs in MAT Logic

["Taylor Beebe" <t@taylorbeebe.com>]

From: Taylor Beebe <taylor.d.beebe@gmail.com>

Fix the bugs in the MAT logic before switching the
UEFI and SMM MAT logic to use the new library.

Signed-off-by: Taylor Beebe <t@taylorbeebe.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c                       |  2 +-
 MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c | 98 ++++++++++----------
 2 files changed, 49 insertions(+), 51 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index 51630f504ea1..af6c26244cc0 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -517,7 +517,7 @@ CoreGetMemoryMapWithSeparatedImageSection (
 
   CoreAcquiremMemoryAttributesTableLock ();
 
-  AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 1) * mImagePropertiesPrivateData.ImageRecordCount;
+  AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 3) * mImagePropertiesPrivateData.ImageRecordCount;
 
   OldMemoryMapSize = *MemoryMapSize;
   Status           = CoreGetMemoryMap (MemoryMapSize, MemoryMap, MapKey, DescriptorSize, DescriptorVersion);
diff --git a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
index 9fb3b922038f..379eb0c6cccd 100644
--- a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
+++ b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
@@ -277,7 +277,7 @@ GetMaxSplitRecordCount (
       break;
     }
 
-    SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 1);
+    SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 3);
     PhysicalStart     = ImageRecord->ImageBase + ImageRecord->ImageSize;
   } while ((ImageRecord != NULL) && (PhysicalStart < PhysicalEnd));
 
@@ -323,7 +323,6 @@ SplitRecord (
   UINT64                   PhysicalEnd;
   UINTN                    NewRecordCount;
   UINTN                    TotalNewRecordCount;
-  BOOLEAN                  IsLastRecordData;
 
   if (MaxSplitRecordCount == 0) {
     CopyMem (NewRecord, OldRecord, DescriptorSize);
@@ -344,35 +343,16 @@ SplitRecord (
     NewImageRecord = GetImageRecordByAddress (PhysicalStart, PhysicalEnd - PhysicalStart, ImageRecordList);
     if (NewImageRecord == NULL) {
       //
-      // No more image covered by this range, stop
+      // No more images cover this range, check if we've reached the end of the old descriptor. If not,
+      // add the remaining range to the new descriptor list.
       //
-      if ((PhysicalEnd > PhysicalStart) && (ImageRecord != NULL)) {
-        //
-        // If this is still address in this record, need record.
-        //
-        NewRecord        = PREVIOUS_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
-        IsLastRecordData = FALSE;
-        if ((NewRecord->Attribute & EFI_MEMORY_XP) != 0) {
-          IsLastRecordData = TRUE;
-        }
-
-        if (IsLastRecordData) {
-          //
-          // Last record is DATA, just merge it.
-          //
-          NewRecord->NumberOfPages = EfiSizeToPages (PhysicalEnd - NewRecord->PhysicalStart);
-        } else {
-          //
-          // Last record is CODE, create a new DATA entry.
-          //
-          NewRecord                = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
-          NewRecord->Type          = TempRecord.Type;
-          NewRecord->PhysicalStart = TempRecord.PhysicalStart;
-          NewRecord->VirtualStart  = 0;
-          NewRecord->NumberOfPages = TempRecord.NumberOfPages;
-          NewRecord->Attribute     = TempRecord.Attribute | EFI_MEMORY_XP;
-          TotalNewRecordCount++;
-        }
+      if (PhysicalEnd > PhysicalStart) {
+        NewRecord->Type          = TempRecord.Type;
+        NewRecord->PhysicalStart = PhysicalStart;
+        NewRecord->VirtualStart  = 0;
+        NewRecord->NumberOfPages = EfiSizeToPages (PhysicalEnd - PhysicalStart);
+        NewRecord->Attribute     = TempRecord.Attribute;
+        TotalNewRecordCount++;
       }
 
       break;
@@ -380,6 +360,24 @@ SplitRecord (
 
     ImageRecord = NewImageRecord;
 
+    //
+    // Update PhysicalStart to exclude the portion before the image buffer
+    //
+    if (TempRecord.PhysicalStart < ImageRecord->ImageBase) {
+      NewRecord->Type          = TempRecord.Type;
+      NewRecord->PhysicalStart = TempRecord.PhysicalStart;
+      NewRecord->VirtualStart  = 0;
+      NewRecord->NumberOfPages = EfiSizeToPages (ImageRecord->ImageBase - TempRecord.PhysicalStart);
+      NewRecord->Attribute     = TempRecord.Attribute;
+      TotalNewRecordCount++;
+
+      PhysicalStart            = ImageRecord->ImageBase;
+      TempRecord.PhysicalStart = PhysicalStart;
+      TempRecord.NumberOfPages = EfiSizeToPages (PhysicalEnd - PhysicalStart);
+
+      NewRecord = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)NewRecord + DescriptorSize);
+    }
+
     //
     // Set new record
     //
@@ -465,11 +463,12 @@ SplitTable (
 {
   INTN   IndexOld;
   INTN   IndexNew;
+  INTN   IndexNewStarting;
   UINTN  MaxSplitRecordCount;
   UINTN  RealSplitRecordCount;
-  UINTN  TotalSplitRecordCount;
+  UINTN  TotalSkippedRecords;
 
-  TotalSplitRecordCount = 0;
+  TotalSkippedRecords = 0;
   //
   // Let old record point to end of valid MemoryMap buffer.
   //
@@ -477,7 +476,8 @@ SplitTable (
   //
   // Let new record point to end of full MemoryMap buffer.
   //
-  IndexNew = ((*MemoryMapSize) / DescriptorSize) - 1 + NumberOfAdditionalDescriptors;
+  IndexNew         = ((*MemoryMapSize) / DescriptorSize) - 1 + NumberOfAdditionalDescriptors;
+  IndexNewStarting = IndexNew;
   for ( ; IndexOld >= 0; IndexOld--) {
     MaxSplitRecordCount = GetMaxSplitRecordCount ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + IndexOld * DescriptorSize), ImageRecordList);
     //
@@ -491,16 +491,14 @@ SplitTable (
                              DescriptorSize,
                              ImageRecordList
                              );
-    //
-    // Adjust IndexNew according to real split.
-    //
-    CopyMem (
-      ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitRecordCount) * DescriptorSize),
-      ((UINT8 *)MemoryMap + IndexNew * DescriptorSize),
-      RealSplitRecordCount * DescriptorSize
-      );
-    IndexNew               = IndexNew + MaxSplitRecordCount - RealSplitRecordCount;
-    TotalSplitRecordCount += RealSplitRecordCount;
+
+    // If we didn't utilize all the extra allocated descriptor slots, set the physical address of the unused slots
+    // to MAX_ADDRESS so they are moved to the bottom of the list when sorting.
+    for ( ; RealSplitRecordCount < MaxSplitRecordCount; RealSplitRecordCount++) {
+      ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + ((IndexNew + RealSplitRecordCount + 1) * DescriptorSize)))->PhysicalStart = MAX_ADDRESS;
+      TotalSkippedRecords++;
+    }
+
     IndexNew--;
   }
 
@@ -509,16 +507,16 @@ SplitTable (
   //
   CopyMem (
     MemoryMap,
-    (UINT8 *)MemoryMap + (NumberOfAdditionalDescriptors - TotalSplitRecordCount) * DescriptorSize,
-    (*MemoryMapSize) + TotalSplitRecordCount * DescriptorSize
+    (UINT8 *)MemoryMap + ((IndexNew + 1) * DescriptorSize),
+    (IndexNewStarting - IndexNew) * DescriptorSize
     );
 
-  *MemoryMapSize = (*MemoryMapSize) + DescriptorSize * TotalSplitRecordCount;
+  //
+  // Sort from low to high to filter out the MAX_ADDRESS records.
+  //
+  SortMemoryMap (MemoryMap, (IndexNewStarting - IndexNew) * DescriptorSize, DescriptorSize);
 
-  //
-  // Sort from low to high (Just in case)
-  //
-  SortMemoryMap (MemoryMap, *MemoryMapSize, DescriptorSize);
+  *MemoryMapSize = (IndexNewStarting - IndexNew - TotalSkippedRecords) * DescriptorSize;
 
   return;
 }
-- 
2.41.0.windows.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107595): https://edk2.groups.io/g/devel/message/107595
Mute This Topic: https://groups.io/mt/100553440/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-