[edk2-devel] [PATCH v5 06/16] MdeModulePkg: Update MemoryAttributesTable.c to Reduce Global Variable Use

["Taylor Beebe" <taylor.d.beebe@gmail.com>]

This patch updates MemoryAttributesTable.c to reduce reliance on global
variables and allow some logic to move to a library.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Dandan Bi <dandan.bi@intel.com>
Signed-off-by: Taylor Beebe <taylor.d.beebe@gmail.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 102 +++++++++++---------
 1 file changed, 54 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index fd127ee167e1..64b0aa1ff5e5 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -541,8 +541,9 @@ EnforceMemoryMapAttribute (
 /**
   Return the first image record, whose [ImageBase, ImageSize] covered by [Buffer, Length].
 
-  @param Buffer  Start Address
-  @param Length  Address length
+  @param Buffer           Start Address
+  @param Length           Address length
+  @param ImageRecordList  Image record list
 
   @return first image record covered by [buffer, length]
 **/
@@ -550,14 +551,12 @@ STATIC
 IMAGE_PROPERTIES_RECORD *
 GetImageRecordByAddress (
   IN EFI_PHYSICAL_ADDRESS  Buffer,
-  IN UINT64                Length
+  IN UINT64                Length,
+  IN LIST_ENTRY            *ImageRecordList
   )
 {
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
   LIST_ENTRY               *ImageRecordLink;
-  LIST_ENTRY               *ImageRecordList;
-
-  ImageRecordList = &mImagePropertiesPrivateData.ImageRecordList;
 
   for (ImageRecordLink = ImageRecordList->ForwardLink;
        ImageRecordLink != ImageRecordList;
@@ -692,7 +691,8 @@ SetNewRecord (
 STATIC
 UINTN
 GetMaxSplitRecordCount (
-  IN EFI_MEMORY_DESCRIPTOR  *OldRecord
+  IN EFI_MEMORY_DESCRIPTOR  *OldRecord,
+  IN LIST_ENTRY             *ImageRecordList
   )
 {
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
@@ -705,7 +705,7 @@ GetMaxSplitRecordCount (
   PhysicalEnd      = OldRecord->PhysicalStart + EfiPagesToSize (OldRecord->NumberOfPages);
 
   do {
-    ImageRecord = GetImageRecordByAddress (PhysicalStart, PhysicalEnd - PhysicalStart);
+    ImageRecord = GetImageRecordByAddress (PhysicalStart, PhysicalEnd - PhysicalStart, ImageRecordList);
     if (ImageRecord == NULL) {
       break;
     }
@@ -725,13 +725,16 @@ GetMaxSplitRecordCount (
   Split the memory map to new entries, according to one old entry,
   based upon PE code section and data section.
 
-  @param  OldRecord              A pointer to one old memory map entry.
-  @param  NewRecord              A pointer to several new memory map entries.
-                                 The caller gurantee the buffer size be 1 +
-                                 (SplitRecordCount * DescriptorSize) calculated
-                                 below.
-  @param  MaxSplitRecordCount    The max number of splitted entries
-  @param  DescriptorSize         Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
+  @param        OldRecord             A pointer to one old memory map entry.
+  @param        NewRecord             A pointer to several new memory map entries.
+                                      The caller gurantee the buffer size be 1 +
+                                      (SplitRecordCount * DescriptorSize) calculated
+                                      below.
+  @param        MaxSplitRecordCount   The max number of splitted entries
+  @param        DescriptorSize        Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
+  @param        ImageRecordList       A list of IMAGE_PROPERTIES_RECORD entries used when searching
+                                      for an image record contained by the memory range described in
+                                      the existing EFI memory map descriptor OldRecord
 
   @retval  0 no entry is splitted.
   @return  the real number of splitted record.
@@ -742,7 +745,8 @@ SplitRecord (
   IN EFI_MEMORY_DESCRIPTOR      *OldRecord,
   IN OUT EFI_MEMORY_DESCRIPTOR  *NewRecord,
   IN UINTN                      MaxSplitRecordCount,
-  IN UINTN                      DescriptorSize
+  IN UINTN                      DescriptorSize,
+  IN LIST_ENTRY                 *ImageRecordList
   )
 {
   EFI_MEMORY_DESCRIPTOR    TempRecord;
@@ -770,7 +774,7 @@ SplitRecord (
 
   ImageRecord = NULL;
   do {
-    NewImageRecord = GetImageRecordByAddress (PhysicalStart, PhysicalEnd - PhysicalStart);
+    NewImageRecord = GetImageRecordByAddress (PhysicalStart, PhysicalEnd - PhysicalStart, ImageRecordList);
     if (NewImageRecord == NULL) {
       //
       // No more image covered by this range, stop
@@ -867,23 +871,29 @@ SplitRecord (
    | Record Y      |
    +---------------+
 
-  @param  MemoryMapSize          A pointer to the size, in bytes, of the
-                                 MemoryMap buffer. On input, this is the size of
-                                 old MemoryMap before split. The actual buffer
-                                 size of MemoryMap is MemoryMapSize +
-                                 (AdditionalRecordCount * DescriptorSize) calculated
-                                 below. On output, it is the size of new MemoryMap
-                                 after split.
-  @param  MemoryMap              A pointer to the buffer in which firmware places
-                                 the current memory map.
-  @param  DescriptorSize         Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
+  @param  MemoryMapSize                   A pointer to the size, in bytes, of the
+                                          MemoryMap buffer. On input, this is the size of
+                                          old MemoryMap before split. The actual buffer
+                                          size of MemoryMap is MemoryMapSize +
+                                          (AdditionalRecordCount * DescriptorSize) calculated
+                                          below. On output, it is the size of new MemoryMap
+                                          after split.
+  @param  MemoryMap                       A pointer to the buffer in which firmware places
+                                          the current memory map.
+  @param  DescriptorSize                  Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
+  @param  ImageRecordList                 A list of IMAGE_PROPERTIES_RECORD entries used when searching
+                                          for an image record contained by the memory range described in
+                                          EFI memory map descriptors.
+  @param  NumberOfAdditionalDescriptors   The number of unused descriptors at the end of the input MemoryMap.
 **/
 STATIC
 VOID
 SplitTable (
   IN OUT UINTN                  *MemoryMapSize,
   IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
-  IN UINTN                      DescriptorSize
+  IN     UINTN                  DescriptorSize,
+  IN     LIST_ENTRY             *ImageRecordList,
+  IN     UINTN                  NumberOfAdditionalDescriptors
   )
 {
   INTN   IndexOld;
@@ -891,9 +901,6 @@ SplitTable (
   UINTN  MaxSplitRecordCount;
   UINTN  RealSplitRecordCount;
   UINTN  TotalSplitRecordCount;
-  UINTN  AdditionalRecordCount;
-
-  AdditionalRecordCount = (2 * mImagePropertiesPrivateData.CodeSegmentCountMax + 1) * mImagePropertiesPrivateData.ImageRecordCount;
 
   TotalSplitRecordCount = 0;
   //
@@ -903,9 +910,9 @@ SplitTable (
   //
   // Let new record point to end of full MemoryMap buffer.
   //
-  IndexNew = ((*MemoryMapSize) / DescriptorSize) - 1 + AdditionalRecordCount;
+  IndexNew = ((*MemoryMapSize) / DescriptorSize) - 1 + NumberOfAdditionalDescriptors;
   for ( ; IndexOld >= 0; IndexOld--) {
-    MaxSplitRecordCount = GetMaxSplitRecordCount ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + IndexOld * DescriptorSize));
+    MaxSplitRecordCount = GetMaxSplitRecordCount ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + IndexOld * DescriptorSize), ImageRecordList);
     //
     // Split this MemoryMap record
     //
@@ -914,7 +921,8 @@ SplitTable (
                              (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + IndexOld * DescriptorSize),
                              (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + IndexNew * DescriptorSize),
                              MaxSplitRecordCount,
-                             DescriptorSize
+                             DescriptorSize,
+                             ImageRecordList
                              );
     //
     // Adjust IndexNew according to real split.
@@ -934,7 +942,7 @@ SplitTable (
   //
   CopyMem (
     MemoryMap,
-    (UINT8 *)MemoryMap + (AdditionalRecordCount - TotalSplitRecordCount) * DescriptorSize,
+    (UINT8 *)MemoryMap + (NumberOfAdditionalDescriptors - TotalSplitRecordCount) * DescriptorSize,
     (*MemoryMapSize) + TotalSplitRecordCount * DescriptorSize
     );
 
@@ -1035,7 +1043,7 @@ CoreGetMemoryMapWithSeparatedImageSection (
       //
       // Split PE code/data
       //
-      SplitTable (MemoryMapSize, MemoryMap, *DescriptorSize);
+      SplitTable (MemoryMapSize, MemoryMap, *DescriptorSize, &mImagePropertiesPrivateData.ImageRecordList, AdditionalRecordCount);
     }
   }
 
@@ -1233,11 +1241,13 @@ SwapImageRecord (
 
 /**
   Sort image record based upon the ImageBase from low to high.
+
+  @param ImageRecordList    Image record list to be sorted
 **/
 STATIC
 VOID
 SortImageRecord (
-  VOID
+  IN LIST_ENTRY  *ImageRecordList
   )
 {
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
@@ -1245,9 +1255,6 @@ SortImageRecord (
   LIST_ENTRY               *ImageRecordLink;
   LIST_ENTRY               *NextImageRecordLink;
   LIST_ENTRY               *ImageRecordEndLink;
-  LIST_ENTRY               *ImageRecordList;
-
-  ImageRecordList = &mImagePropertiesPrivateData.ImageRecordList;
 
   ImageRecordLink     = ImageRecordList->ForwardLink;
   NextImageRecordLink = ImageRecordLink->ForwardLink;
@@ -1456,7 +1463,7 @@ InsertImageRecord (
     mImagePropertiesPrivateData.CodeSegmentCountMax = ImageRecord->CodeSegmentCount;
   }
 
-  SortImageRecord ();
+  SortImageRecord (&mImagePropertiesPrivateData.ImageRecordList);
 
 Finish:
   return;
@@ -1465,8 +1472,9 @@ Finish:
 /**
   Find image record according to image base and size.
 
-  @param  ImageBase    Base of PE image
-  @param  ImageSize    Size of PE image
+  @param  ImageBase           Base of PE image
+  @param  ImageSize           Size of PE image
+  @param  ImageRecordList     Image record list to be searched
 
   @return image record
 **/
@@ -1474,14 +1482,12 @@ STATIC
 IMAGE_PROPERTIES_RECORD *
 FindImageRecord (
   IN EFI_PHYSICAL_ADDRESS  ImageBase,
-  IN UINT64                ImageSize
+  IN UINT64                ImageSize,
+  IN LIST_ENTRY            *ImageRecordList
   )
 {
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
   LIST_ENTRY               *ImageRecordLink;
-  LIST_ENTRY               *ImageRecordList;
-
-  ImageRecordList = &mImagePropertiesPrivateData.ImageRecordList;
 
   for (ImageRecordLink = ImageRecordList->ForwardLink;
        ImageRecordLink != ImageRecordList;
@@ -1526,7 +1532,7 @@ RemoveImageRecord (
     return;
   }
 
-  ImageRecord = FindImageRecord ((EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize);
+  ImageRecord = FindImageRecord ((EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize, &mImagePropertiesPrivateData.ImageRecordList);
   if (ImageRecord == NULL) {
     DEBUG ((DEBUG_ERROR, "!!!!!!!! ImageRecord not found !!!!!!!!\n"));
     return;
-- 
2.42.0.windows.2



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