For HeapGuard.c part,
Reviewed-by: Jian J Wang
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>
Regards,
Jian
From: Ni, Ruiyu
Sent: Thursday, August 09, 2018 11:12 AM
To: Zhang, Shenglei <shenglei.zh...@intel.com>; edk2-devel@lists.01.org
Cc: Dong, Eric <eric.d...@intel.com>; Zeng, Star <star.z...@intel.com>; Wang,
Jian J <jian.j.w...@intel.com>
Subject: RE: [edk2] [PATCH 17/26] MdeModulePkg PiSmmCore: Remove redundant
functions
+ Wang Jian.
Thanks/Ray
> -----Original Message-----
> From: edk2-devel
> <edk2-devel-boun...@lists.01.org<mailto:edk2-devel-boun...@lists.01.org>> On
> Behalf Of shenglei
> Sent: Wednesday, August 8, 2018 4:47 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Dong, Eric <eric.d...@intel.com<mailto:eric.d...@intel.com>>; Zeng, Star
> <star.z...@intel.com<mailto:star.z...@intel.com>>
> Subject: [edk2] [PATCH 17/26] MdeModulePkg PiSmmCore: Remove
> redundant functions
>
> The functions that are never called have been removed.
> They are IsImageInsideSmram,FindImageRecord,SmmRemoveImageRecord,
> SmmMemoryAttributesTableConsistencyCheck,DumpSmmMemoryMapEntr
> y,
> SmmMemoryMapConsistencyCheckRange,SmmMemoryMapConsistencyCh
> eck,
> DumpSmmMemoryMap,ClearGuardMapBit,SetGuardMapBit,AdjustMemory
> A,
> AdjustMemoryS,IsHeadGuard and IsTailGuard.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1062
>
> Cc: Star Zeng <star.z...@intel.com<mailto:star.z...@intel.com>>
> Cc: Eric Dong <eric.d...@intel.com<mailto:eric.d...@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shenglei
> <shenglei.zh...@intel.com<mailto:shenglei.zh...@intel.com>>
> ---
> MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 166 ------------------
> .../Core/PiSmmCore/MemoryAttributesTable.c | 131 --------------
> MdeModulePkg/Core/PiSmmCore/Page.c | 121 -------------
> 3 files changed, 418 deletions(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index d9e54b96cb..f7ae9ae286 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -455,51 +455,6 @@ GetGuardMapBit (
> return 0;
> }
>
> -/**
> - Set the bit in bitmap table for the given address.
> -
> - @param[in] Address The address to set for.
> -
> - @return VOID.
> -**/
> -VOID
> -EFIAPI
> -SetGuardMapBit (
> - IN EFI_PHYSICAL_ADDRESS Address
> - )
> -{
> - UINT64 *GuardMap;
> - UINT64 BitMask;
> -
> - FindGuardedMemoryMap (Address, TRUE, &GuardMap);
> - if (GuardMap != NULL) {
> - BitMask = LShiftU64 (1, GUARDED_HEAP_MAP_ENTRY_BIT_INDEX
> (Address));
> - *GuardMap |= BitMask;
> - }
> -}
> -
> -/**
> - Clear the bit in bitmap table for the given address.
> -
> - @param[in] Address The address to clear for.
> -
> - @return VOID.
> -**/
> -VOID
> -EFIAPI
> -ClearGuardMapBit (
> - IN EFI_PHYSICAL_ADDRESS Address
> - )
> -{
> - UINT64 *GuardMap;
> - UINT64 BitMask;
> -
> - FindGuardedMemoryMap (Address, TRUE, &GuardMap);
> - if (GuardMap != NULL) {
> - BitMask = LShiftU64 (1, GUARDED_HEAP_MAP_ENTRY_BIT_INDEX
> (Address));
> - *GuardMap &= ~BitMask;
> - }
> -}
>
> /**
> Check to see if the page at the given address is a Guard page or not.
> @@ -526,39 +481,7 @@ IsGuardPage (
> return ((BitMap == BIT0) || (BitMap == BIT2) || (BitMap == (BIT2 |
> BIT0))); }
>
> -/**
> - Check to see if the page at the given address is a head Guard page or not.
>
> - @param[in] Address The address to check for.
> -
> - @return TRUE The page at Address is a head Guard page.
> - @return FALSE The page at Address is not a head Guard page.
> -**/
> -BOOLEAN
> -EFIAPI
> -IsHeadGuard (
> - IN EFI_PHYSICAL_ADDRESS Address
> - )
> -{
> - return (GetGuardedMemoryBits (Address, 2) == BIT1); -}
> -
> -/**
> - Check to see if the page at the given address is a tail Guard page or not.
> -
> - @param[in] Address The address to check for.
> -
> - @return TRUE The page at Address is a tail Guard page.
> - @return FALSE The page at Address is not a tail Guard page.
> -**/
> -BOOLEAN
> -EFIAPI
> -IsTailGuard (
> - IN EFI_PHYSICAL_ADDRESS Address
> - )
> -{
> - return (GetGuardedMemoryBits (Address - EFI_PAGE_SIZE, 2) == BIT0); -}
>
> /**
> Check to see if the page at the given address is guarded or not.
> @@ -864,66 +787,7 @@ UnsetGuardForMemory (
> ClearGuardedMemoryBits(Memory, NumberOfPages); }
>
> -/**
> - Adjust address of free memory according to existing and/or required
> Guard.
> -
> - This function will check if there're existing Guard pages of adjacent
> - memory blocks, and try to use it as the Guard page of the memory to be
> - allocated.
>
> - @param[in] Start Start address of free memory block.
> - @param[in] Size Size of free memory block.
> - @param[in] SizeRequested Size of memory to allocate.
> -
> - @return The end address of memory block found.
> - @return 0 if no enough space for the required size of memory and its
> Guard.
> -**/
> -UINT64
> -AdjustMemoryS (
> - IN UINT64 Start,
> - IN UINT64 Size,
> - IN UINT64 SizeRequested
> - )
> -{
> - UINT64 Target;
> -
> - //
> - // UEFI spec requires that allocated pool must be 8-byte aligned. If it's
> - // indicated to put the pool near the Tail Guard, we need extra bytes to
> - // make sure alignment of the returned pool address.
> - //
> - if ((PcdGet8 (PcdHeapGuardPropertyMask) & BIT7) == 0) {
> - SizeRequested = ALIGN_VALUE(SizeRequested, 8);
> - }
> -
> - Target = Start + Size - SizeRequested;
> - ASSERT (Target >= Start);
> - if (Target == 0) {
> - return 0;
> - }
> -
> - if (!IsGuardPage (Start + Size)) {
> - // No Guard at tail to share. One more page is needed.
> - Target -= EFI_PAGES_TO_SIZE (1);
> - }
> -
> - // Out of range?
> - if (Target < Start) {
> - return 0;
> - }
> -
> - // At the edge?
> - if (Target == Start) {
> - if (!IsGuardPage (Target - EFI_PAGES_TO_SIZE (1))) {
> - // No enough space for a new head Guard if no Guard at head to share.
> - return 0;
> - }
> - }
> -
> - // OK, we have enough pages for memory and its Guards. Return the End
> of the
> - // free space.
> - return Target + SizeRequested - 1;
> -}
>
> /**
> Adjust the start address and number of pages to free according to Guard.
> @@ -1049,36 +913,6 @@ AdjustMemoryF (
> *NumberOfPages = PagesToFree;
> }
>
> -/**
> - Adjust the base and number of pages to really allocate according to Guard.
> -
> - @param[in,out] Memory Base address of free memory.
> - @param[in,out] NumberOfPages Size of memory to allocate.
> -
> - @return VOID.
> -**/
> -VOID
> -AdjustMemoryA (
> - IN OUT EFI_PHYSICAL_ADDRESS *Memory,
> - IN OUT UINTN *NumberOfPages
> - )
> -{
> - //
> - // FindFreePages() has already taken the Guard into account. It's safe to
> - // adjust the start address and/or number of pages here, to make sure that
> - // the Guards are also "allocated".
> - //
> - if (!IsGuardPage (*Memory + EFI_PAGES_TO_SIZE (*NumberOfPages))) {
> - // No tail Guard, add one.
> - *NumberOfPages += 1;
> - }
> -
> - if (!IsGuardPage (*Memory - EFI_PAGE_SIZE)) {
> - // No head Guard, add one.
> - *Memory -= EFI_PAGE_SIZE;
> - *NumberOfPages += 1;
> - }
> -}
>
> /**
> Adjust the pool head position to make sure the Guard page is adjavent to
> diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
> b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
> index 36ccf65fa3..1682d0f9e4 100644
> --- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
> +++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
> @@ -114,34 +114,6 @@ EfiSizeToPages (
> return RShiftU64 (Size, EFI_PAGE_SHIFT) + ((((UINTN)Size) &
> EFI_PAGE_MASK) ? 1 : 0); }
>
> -/**
> - Check the consistency of Smm memory attributes table.
> -
> - @param[in] MemoryAttributesTable PI SMM memory attributes table -
> **/ -VOID -SmmMemoryAttributesTableConsistencyCheck (
> - IN EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE
> *MemoryAttributesTable
> - )
> -{
> - EFI_MEMORY_DESCRIPTOR *MemoryMap;
> - UINTN MemoryMapEntryCount;
> - UINTN DescriptorSize;
> - UINTN Index;
> - UINT64 Address;
> -
> - Address = 0;
> - MemoryMapEntryCount = MemoryAttributesTable->NumberOfEntries;
> - DescriptorSize = MemoryAttributesTable->DescriptorSize;
> - MemoryMap = (EFI_MEMORY_DESCRIPTOR *)(MemoryAttributesTable +
> 1);
> - for (Index = 0; Index < MemoryMapEntryCount; Index++) {
> - if (Address != 0) {
> - ASSERT (Address == MemoryMap->PhysicalStart);
> - }
> - Address = MemoryMap->PhysicalStart + EfiPagesToSize(MemoryMap-
> >NumberOfPages);
> - MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap,
> DescriptorSize);
> - }
> -}
>
> /**
> Sort memory map entries based upon PhysicalStart, from low to high.
> @@ -1224,85 +1196,6 @@ Finish:
> return ;
> }
>
> -/**
> - Find image record according to image base and size.
> -
> - @param[in] ImageBase Base of PE image
> - @param[in] ImageSize Size of PE image
> -
> - @return image record
> -**/
> -STATIC
> -IMAGE_PROPERTIES_RECORD *
> -FindImageRecord (
> - IN EFI_PHYSICAL_ADDRESS ImageBase,
> - IN UINT64 ImageSize
> - )
> -{
> - IMAGE_PROPERTIES_RECORD *ImageRecord;
> - LIST_ENTRY *ImageRecordLink;
> - LIST_ENTRY *ImageRecordList;
> -
> - ImageRecordList = &mImagePropertiesPrivateData.ImageRecordList;
> -
> - for (ImageRecordLink = ImageRecordList->ForwardLink;
> - ImageRecordLink != ImageRecordList;
> - ImageRecordLink = ImageRecordLink->ForwardLink) {
> - ImageRecord = CR (
> - ImageRecordLink,
> - IMAGE_PROPERTIES_RECORD,
> - Link,
> - IMAGE_PROPERTIES_RECORD_SIGNATURE
> - );
> -
> - if ((ImageBase == ImageRecord->ImageBase) &&
> - (ImageSize == ImageRecord->ImageSize)) {
> - return ImageRecord;
> - }
> - }
> -
> - return NULL;
> -}
> -
> -/**
> - Remove Image record.
> -
> - @param[in] DriverEntry Driver information
> -**/
> -VOID
> -SmmRemoveImageRecord (
> - IN EFI_SMM_DRIVER_ENTRY *DriverEntry
> - )
> -{
> - IMAGE_PROPERTIES_RECORD *ImageRecord;
> - LIST_ENTRY *CodeSegmentListHead;
> - IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> -
> - DEBUG ((DEBUG_VERBOSE, "SMM RemoveImageRecord - 0x%x\n",
> DriverEntry));
> - DEBUG ((DEBUG_VERBOSE, "SMM RemoveImageRecord - 0x%016lx -
> 0x%016lx\n", DriverEntry->ImageBuffer, DriverEntry->NumberOfPage));
> -
> - ImageRecord = FindImageRecord (DriverEntry->ImageBuffer,
> EfiPagesToSize(DriverEntry->NumberOfPage));
> - if (ImageRecord == NULL) {
> - DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! ImageRecord not
> found !!!!!!!!\n"));
> - return ;
> - }
> -
> - CodeSegmentListHead = &ImageRecord->CodeSegmentList;
> - while (!IsListEmpty (CodeSegmentListHead)) {
> - ImageRecordCodeSection = CR (
> - CodeSegmentListHead->ForwardLink,
> - IMAGE_PROPERTIES_RECORD_CODE_SECTION,
> - Link,
> - IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
> - );
> - RemoveEntryList (&ImageRecordCodeSection->Link);
> - FreePool (ImageRecordCodeSection);
> - }
> -
> - RemoveEntryList (&ImageRecord->Link);
> - FreePool (ImageRecord);
> - mImagePropertiesPrivateData.ImageRecordCount--;
> -}
>
> /**
> Publish MemoryAttributesTable to SMM configuration table.
> @@ -1386,30 +1279,6 @@ PublishMemoryAttributesTable (
> ASSERT_EFI_ERROR (Status);
> }
>
> -/**
> - This function returns if image is inside SMRAM.
> -
> - @param[in] LoadedImage LoadedImage protocol instance for an image.
> -
> - @retval TRUE the image is inside SMRAM.
> - @retval FALSE the image is outside SMRAM.
> -**/
> -BOOLEAN
> -IsImageInsideSmram (
> - IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage
> - )
> -{
> - UINTN Index;
> -
> - for (Index = 0; Index < mFullSmramRangeCount; Index++) {
> - if ((mFullSmramRanges[Index].PhysicalStart <= (UINTN)LoadedImage-
> >ImageBase)&&
> - (mFullSmramRanges[Index].PhysicalStart +
> mFullSmramRanges[Index].PhysicalSize >= (UINTN)LoadedImage-
> >ImageBase + LoadedImage->ImageSize)) {
> - return TRUE;
> - }
> - }
> -
> - return FALSE;
> -}
>
> /**
> This function installs all SMM image record information.
> diff --git a/MdeModulePkg/Core/PiSmmCore/Page.c
> b/MdeModulePkg/Core/PiSmmCore/Page.c
> index 3699af7424..cd7d7ece0c 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Page.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Page.c
> @@ -451,128 +451,7 @@ GetSmmMemoryMapEntryCount (
> return Count;
> }
>
> -/**
> - Dump Smm memory map entry.
> -**/
> -VOID
> -DumpSmmMemoryMapEntry (
> - VOID
> - )
> -{
> - LIST_ENTRY *Link;
> - MEMORY_MAP *Entry;
> - EFI_PHYSICAL_ADDRESS Last;
> -
> - Last = 0;
> - DEBUG ((DEBUG_INFO, "DumpSmmMemoryMapEntry:\n"));
> - Link = gMemoryMap.ForwardLink;
> - while (Link != &gMemoryMap) {
> - Entry = CR (Link, MEMORY_MAP, Link, MEMORY_MAP_SIGNATURE);
> - Link = Link->ForwardLink;
> -
> - if ((Last != 0) && (Last != (UINT64)-1)) {
> - if (Last + 1 != Entry->Start) {
> - Last = (UINT64)-1;
> - } else {
> - Last = Entry->End;
> - }
> - } else if (Last == 0) {
> - Last = Entry->End;
> - }
> -
> - DEBUG ((DEBUG_INFO, "Entry (Link - 0x%x)\n", &Entry->Link));
> - DEBUG ((DEBUG_INFO, " Signature - 0x%x\n", Entry->Signature));
> - DEBUG ((DEBUG_INFO, " Link.ForwardLink - 0x%x\n", Entry-
> >Link.ForwardLink));
> - DEBUG ((DEBUG_INFO, " Link.BackLink - 0x%x\n", Entry-
> >Link.BackLink));
> - DEBUG ((DEBUG_INFO, " Type - 0x%x\n", Entry->Type));
> - DEBUG ((DEBUG_INFO, " Start - 0x%016lx\n", Entry->Start));
> - DEBUG ((DEBUG_INFO, " End - 0x%016lx\n", Entry->End));
> - }
> -
> - ASSERT (Last != (UINT64)-1);
> -}
> -
> -/**
> - Dump Smm memory map.
> -**/
> -VOID
> -DumpSmmMemoryMap (
> - VOID
> - )
> -{
> - LIST_ENTRY *Node;
> - FREE_PAGE_LIST *Pages;
> -
> - DEBUG ((DEBUG_INFO, "DumpSmmMemoryMap\n"));
> -
> - Pages = NULL;
> - Node = mSmmMemoryMap.ForwardLink;
> - while (Node != &mSmmMemoryMap) {
> - Pages = BASE_CR (Node, FREE_PAGE_LIST, Link);
> - DEBUG ((DEBUG_INFO, "Pages - 0x%x\n", Pages));
> - DEBUG ((DEBUG_INFO, "Pages->NumberOfPages - 0x%x\n", Pages-
> >NumberOfPages));
> - Node = Node->ForwardLink;
> - }
> -}
>
> -/**
> - Check if a Smm base~length is in Smm memory map.
> -
> - @param[in] Base The base address of Smm memory to be checked.
> - @param[in] Length THe length of Smm memory to be checked.
> -
> - @retval TRUE Smm base~length is in smm memory map.
> - @retval FALSE Smm base~length is in smm memory map.
> -**/
> -BOOLEAN
> -SmmMemoryMapConsistencyCheckRange (
> - IN EFI_PHYSICAL_ADDRESS Base,
> - IN UINTN Length
> - )
> -{
> - LIST_ENTRY *Link;
> - MEMORY_MAP *Entry;
> - BOOLEAN Result;
> -
> - Result = FALSE;
> - Link = gMemoryMap.ForwardLink;
> - while (Link != &gMemoryMap) {
> - Entry = CR (Link, MEMORY_MAP, Link, MEMORY_MAP_SIGNATURE);
> - Link = Link->ForwardLink;
> -
> - if (Entry->Type != EfiConventionalMemory) {
> - continue;
> - }
> - if (Entry->Start == Base && Entry->End == Base + Length - 1) {
> - Result = TRUE;
> - break;
> - }
> - }
> -
> - return Result;
> -}
> -
> -/**
> - Check the consistency of Smm memory map.
> -**/
> -VOID
> -SmmMemoryMapConsistencyCheck (
> - VOID
> - )
> -{
> - LIST_ENTRY *Node;
> - FREE_PAGE_LIST *Pages;
> - BOOLEAN Result;
> -
> - Pages = NULL;
> - Node = mSmmMemoryMap.ForwardLink;
> - while (Node != &mSmmMemoryMap) {
> - Pages = BASE_CR (Node, FREE_PAGE_LIST, Link);
> - Result = SmmMemoryMapConsistencyCheckRange
> ((EFI_PHYSICAL_ADDRESS)(UINTN)Pages,
> (UINTN)EFI_PAGES_TO_SIZE(Pages->NumberOfPages));
> - ASSERT (Result);
> - Node = Node->ForwardLink;
> - }
> -}
>
> /**
> Internal Function. Allocate n pages from given free page node.
> --
> 2.18.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
