[edk2-devel] [PATCH v1 3/9] MdeModulePkg: Fix Bugs in MAT Logic

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

Fix the bugs in ImagePropertiesRecordLib (which is a copy of the
current UEFI 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 fd127ee167e1..6e15a76cccae 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -1017,7 +1017,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 46f4f401a4d5..a787e376b551 100644
--- a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
+++ b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
@@ -519,7 +519,7 @@ GetMaxSplitRecordCount (
       break;
     }
 
-    SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 1);
+    SplitRecordCount += (2 * ImageRecord->CodeSegmentCount + 3);
     PhysicalStart     = ImageRecord->ImageBase + ImageRecord->ImageSize;
   } while ((ImageRecord != NULL) && (PhysicalStart < PhysicalEnd));
 
@@ -565,7 +565,6 @@ SplitRecord (
   UINT64                   PhysicalEnd;
   UINTN                    NewRecordCount;
   UINTN                    TotalNewRecordCount;
-  BOOLEAN                  IsLastRecordData;
 
   if (MaxSplitRecordCount == 0) {
     CopyMem (NewRecord, OldRecord, DescriptorSize);
@@ -586,35 +585,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;
@@ -622,6 +602,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
     //
@@ -707,11 +705,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.
   //
@@ -719,7 +718,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);
     //
@@ -733,16 +733,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--;
   }
 
@@ -751,16 +749,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.2



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