From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.151; helo=mga17.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 86785210E38AB for ; Wed, 8 Aug 2018 20:44:33 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Aug 2018 20:44:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,213,1531810800"; d="scan'208,217";a="71364482" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by FMSMGA003.fm.intel.com with ESMTP; 08 Aug 2018 20:44:32 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 8 Aug 2018 20:44:31 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.240]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.150]) with mapi id 14.03.0319.002; Thu, 9 Aug 2018 11:44:29 +0800 From: "Wang, Jian J" To: "Ni, Ruiyu" , "Zhang, Shenglei" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , "Zeng, Star" Thread-Topic: [edk2] [PATCH 17/26] MdeModulePkg PiSmmCore: Remove redundant functions Thread-Index: AQHULvSj4IByioogxE+EfNie+zQuIaS2OXgAgACO/TA= Date: Thu, 9 Aug 2018 03:44:29 +0000 Message-ID: References: <20180808084712.34696-1-shenglei.zhang@intel.com> <20180808084712.34696-18-shenglei.zhang@intel.com> <734D49CCEBEEF84792F5B80ED585239D5BDC95BF@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BDC95BF@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGRkNTI3YjctNjg3ZS00ODQ2LWExYmItOTdlNmI2NGNjMGJmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiXC9SYlNlYzNQNkdQWlwveXV6S1ZCMTFzUndvZUhYK2lDRmM4RlNSZmpSdkd3c0RnNkZkN2ZTVk5hU0NMVWgwd3BCIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.27 Subject: Re: [PATCH 17/26] MdeModulePkg PiSmmCore: Remove redundant functions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Aug 2018 03:44:33 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable For HeapGuard.c part, Reviewed-by: Jian J Wang > Regards, Jian From: Ni, Ruiyu Sent: Thursday, August 09, 2018 11:12 AM To: Zhang, Shenglei ; edk2-devel@lists.01.org Cc: Dong, Eric ; Zeng, Star ; Wan= g, Jian J Subject: RE: [edk2] [PATCH 17/26] MdeModulePkg PiSmmCore: Remove redundant = functions + Wang Jian. Thanks/Ray > -----Original Message----- > From: edk2-devel > On Behalf Of shenglei > Sent: Wednesday, August 8, 2018 4:47 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric >; Zeng, S= tar > > 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=3D1062 > > Cc: Star Zeng > > Cc: Eric Dong > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: shenglei > > --- > 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 !=3D NULL) { > - BitMask =3D LShiftU64 (1, GUARDED_HEAP_MAP_ENTRY_BIT_INDEX > (Address)); > - *GuardMap |=3D 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 !=3D NULL) { > - BitMask =3D LShiftU64 (1, GUARDED_HEAP_MAP_ENTRY_BIT_INDEX > (Address)); > - *GuardMap &=3D ~BitMask; > - } > -} > > /** > Check to see if the page at the given address is a Guard page or not. > @@ -526,39 +481,7 @@ IsGuardPage ( > return ((BitMap =3D=3D BIT0) || (BitMap =3D=3D BIT2) || (BitMap =3D=3D= (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) =3D=3D 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) =3D=3D 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 i= t'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) =3D=3D 0) { > - SizeRequested =3D ALIGN_VALUE(SizeRequested, 8); > - } > - > - Target =3D Start + Size - SizeRequested; > - ASSERT (Target >=3D Start); > - if (Target =3D=3D 0) { > - return 0; > - } > - > - if (!IsGuardPage (Start + Size)) { > - // No Guard at tail to share. One more page is needed. > - Target -=3D EFI_PAGES_TO_SIZE (1); > - } > - > - // Out of range? > - if (Target < Start) { > - return 0; > - } > - > - // At the edge? > - if (Target =3D=3D Start) { > - if (!IsGuardPage (Target - EFI_PAGES_TO_SIZE (1))) { > - // No enough space for a new head Guard if no Guard at head to sha= re. > - 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 Guar= d. > @@ -1049,36 +913,6 @@ AdjustMemoryF ( > *NumberOfPages =3D PagesToFree; > } > > -/** > - Adjust the base and number of pages to really allocate according to Gu= ard. > - > - @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 +=3D 1; > - } > - > - if (!IsGuardPage (*Memory - EFI_PAGE_SIZE)) { > - // No head Guard, add one. > - *Memory -=3D EFI_PAGE_SIZE; > - *NumberOfPages +=3D 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 =3D 0; > - MemoryMapEntryCount =3D MemoryAttributesTable->NumberOfEntries; > - DescriptorSize =3D MemoryAttributesTable->DescriptorSize; > - MemoryMap =3D (EFI_MEMORY_DESCRIPTOR *)(MemoryAttributesTable + > 1); > - for (Index =3D 0; Index < MemoryMapEntryCount; Index++) { > - if (Address !=3D 0) { > - ASSERT (Address =3D=3D MemoryMap->PhysicalStart); > - } > - Address =3D MemoryMap->PhysicalStart + EfiPagesToSize(MemoryMap- > >NumberOfPages); > - MemoryMap =3D 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 =3D &mImagePropertiesPrivateData.ImageRecordList; > - > - for (ImageRecordLink =3D ImageRecordList->ForwardLink; > - ImageRecordLink !=3D ImageRecordList; > - ImageRecordLink =3D ImageRecordLink->ForwardLink) { > - ImageRecord =3D CR ( > - ImageRecordLink, > - IMAGE_PROPERTIES_RECORD, > - Link, > - IMAGE_PROPERTIES_RECORD_SIGNATURE > - ); > - > - if ((ImageBase =3D=3D ImageRecord->ImageBase) && > - (ImageSize =3D=3D 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 =3D FindImageRecord (DriverEntry->ImageBuffer, > EfiPagesToSize(DriverEntry->NumberOfPage)); > - if (ImageRecord =3D=3D NULL) { > - DEBUG ((DEBUG_ERROR, "SMM !!!!!!!! ImageRecord not > found !!!!!!!!\n")); > - return ; > - } > - > - CodeSegmentListHead =3D &ImageRecord->CodeSegmentList; > - while (!IsListEmpty (CodeSegmentListHead)) { > - ImageRecordCodeSection =3D CR ( > - CodeSegmentListHead->ForwardLink, > - IMAGE_PROPERTIES_RECORD_CODE_SECTION, > - Link, > - IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGN= ATURE > - ); > - 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 =3D 0; Index < mFullSmramRangeCount; Index++) { > - if ((mFullSmramRanges[Index].PhysicalStart <=3D (UINTN)LoadedImage- > >ImageBase)&& > - (mFullSmramRanges[Index].PhysicalStart + > mFullSmramRanges[Index].PhysicalSize >=3D (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 =3D 0; > - DEBUG ((DEBUG_INFO, "DumpSmmMemoryMapEntry:\n")); > - Link =3D gMemoryMap.ForwardLink; > - while (Link !=3D &gMemoryMap) { > - Entry =3D CR (Link, MEMORY_MAP, Link, MEMORY_MAP_SIGNATURE); > - Link =3D Link->ForwardLink; > - > - if ((Last !=3D 0) && (Last !=3D (UINT64)-1)) { > - if (Last + 1 !=3D Entry->Start) { > - Last =3D (UINT64)-1; > - } else { > - Last =3D Entry->End; > - } > - } else if (Last =3D=3D 0) { > - Last =3D 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 !=3D (UINT64)-1); > -} > - > -/** > - Dump Smm memory map. > -**/ > -VOID > -DumpSmmMemoryMap ( > - VOID > - ) > -{ > - LIST_ENTRY *Node; > - FREE_PAGE_LIST *Pages; > - > - DEBUG ((DEBUG_INFO, "DumpSmmMemoryMap\n")); > - > - Pages =3D NULL; > - Node =3D mSmmMemoryMap.ForwardLink; > - while (Node !=3D &mSmmMemoryMap) { > - Pages =3D 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 =3D 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 =3D FALSE; > - Link =3D gMemoryMap.ForwardLink; > - while (Link !=3D &gMemoryMap) { > - Entry =3D CR (Link, MEMORY_MAP, Link, MEMORY_MAP_SIGNATURE); > - Link =3D Link->ForwardLink; > - > - if (Entry->Type !=3D EfiConventionalMemory) { > - continue; > - } > - if (Entry->Start =3D=3D Base && Entry->End =3D=3D Base + Length - 1)= { > - Result =3D TRUE; > - break; > - } > - } > - > - return Result; > -} > - > -/** > - Check the consistency of Smm memory map. > -**/ > -VOID > -SmmMemoryMapConsistencyCheck ( > - VOID > - ) > -{ > - LIST_ENTRY *Node; > - FREE_PAGE_LIST *Pages; > - BOOLEAN Result; > - > - Pages =3D NULL; > - Node =3D mSmmMemoryMap.ForwardLink; > - while (Node !=3D &mSmmMemoryMap) { > - Pages =3D BASE_CR (Node, FREE_PAGE_LIST, Link); > - Result =3D SmmMemoryMapConsistencyCheckRange > ((EFI_PHYSICAL_ADDRESS)(UINTN)Pages, > (UINTN)EFI_PAGES_TO_SIZE(Pages->NumberOfPages)); > - ASSERT (Result); > - Node =3D 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 > https://lists.01.org/mailman/listinfo/edk2-devel