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