On Wed, 20 Dec 2023 at 10:11, Ray Ni <ray...@intel.com> wrote:
>
> Load-module-at-fixed-address feature does not work in standalone MM core.
>
> The patch removes the 2 dead functions and related global variables
> that are related to the feature.
>
> Signed-off-by: Ray Ni <ray...@intel.com>
> Cc: Sami Mujawar <sami.muja...@arm.com>
Reviewed-by: Ard Biesheuvel <a...@kernel.org>
One nit below
> ---
> StandaloneMmPkg/Core/Dispatcher.c | 185 +-----------------------
> StandaloneMmPkg/Core/StandaloneMmCore.h | 3 +-
> 2 files changed, 2 insertions(+), 186 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index 7b4a3c4c55..6d43e53851 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -29,7 +29,7 @@
> Depex - Dependency Expresion.
>
> Copyright (c) 2014, Hewlett-Packard Development Company, L.P.
> - Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
I know this is standard procedure, but you are extending Intel's
copyright claim on this file by 9 years by /removing/ 180 lines?
> Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -97,189 +97,6 @@ BOOLEAN gDispatcherRunning = FALSE;
> //
> BOOLEAN gRequestDispatch = FALSE;
>
> -//
> -// The global variable is defined for Loading modules at fixed address
> feature to track the MM code
> -// memory range usage. It is a bit mapped array in which every bit indicates
> the correspoding
> -// memory page available or not.
> -//
> -GLOBAL_REMOVE_IF_UNREFERENCED UINT64 *mMmCodeMemoryRangeUsageBitMap =
> NULL;
> -
> -/**
> - To check memory usage bit map array to figure out if the memory range in
> which the image will be loaded
> - is available or not. If memory range is avaliable, the function will mark
> the corresponding bits to 1
> - which indicates the memory range is used. The function is only invoked
> when load modules at fixed address
> - feature is enabled.
> -
> - @param ImageBase The base addres the image will be loaded
> at.
> - @param ImageSize The size of the image
> -
> - @retval EFI_SUCCESS The memory range the image will be loaded
> in is available
> - @retval EFI_NOT_FOUND The memory range the image will be loaded
> in is not available
> -**/
> -EFI_STATUS
> -CheckAndMarkFixLoadingMemoryUsageBitMap (
> - IN EFI_PHYSICAL_ADDRESS ImageBase,
> - IN UINTN ImageSize
> - )
> -{
> - UINT32 MmCodePageNumber;
> - UINT64 MmCodeSize;
> - EFI_PHYSICAL_ADDRESS MmCodeBase;
> - UINTN BaseOffsetPageNumber;
> - UINTN TopOffsetPageNumber;
> - UINTN Index;
> -
> - //
> - // Build tool will calculate the smm code size and then patch the
> PcdLoadFixAddressMmCodePageNumber
> - //
> - MmCodePageNumber = 0;
> - MmCodeSize = EFI_PAGES_TO_SIZE (MmCodePageNumber);
> - MmCodeBase = gLoadModuleAtFixAddressMmramBase;
> -
> - //
> - // If the memory usage bit map is not initialized, do it. Every bit in
> the array
> - // indicate the status of the corresponding memory page, available or not
> - //
> - if (mMmCodeMemoryRangeUsageBitMap == NULL) {
> - mMmCodeMemoryRangeUsageBitMap = AllocateZeroPool (((MmCodePageNumber /
> 64) + 1) * sizeof (UINT64));
> - }
> -
> - //
> - // If the Dxe code memory range is not allocated or the bit map array
> allocation failed, return EFI_NOT_FOUND
> - //
> - if (mMmCodeMemoryRangeUsageBitMap == NULL) {
> - return EFI_NOT_FOUND;
> - }
> -
> - //
> - // see if the memory range for loading the image is in the MM code range.
> - //
> - if ((MmCodeBase + MmCodeSize < ImageBase + ImageSize) || (MmCodeBase >
> ImageBase)) {
> - return EFI_NOT_FOUND;
> - }
> -
> - //
> - // Test if the memory is available or not.
> - //
> - BaseOffsetPageNumber = (UINTN)EFI_SIZE_TO_PAGES ((UINT32)(ImageBase -
> MmCodeBase));
> - TopOffsetPageNumber = (UINTN)EFI_SIZE_TO_PAGES ((UINT32)(ImageBase +
> ImageSize - MmCodeBase));
> - for (Index = BaseOffsetPageNumber; Index < TopOffsetPageNumber; Index++) {
> - if ((mMmCodeMemoryRangeUsageBitMap[Index / 64] & LShiftU64 (1, (Index %
> 64))) != 0) {
> - //
> - // This page is already used.
> - //
> - return EFI_NOT_FOUND;
> - }
> - }
> -
> - //
> - // Being here means the memory range is available. So mark the bits for
> the memory range
> - //
> - for (Index = BaseOffsetPageNumber; Index < TopOffsetPageNumber; Index++) {
> - mMmCodeMemoryRangeUsageBitMap[Index / 64] |= LShiftU64 (1, (Index % 64));
> - }
> -
> - return EFI_SUCCESS;
> -}
> -
> -/**
> - Get the fixed loading address from image header assigned by build tool.
> This function only be called
> - when Loading module at Fixed address feature enabled.
> -
> - @param ImageContext Pointer to the image context structure
> that describes the PE/COFF
> - image that needs to be examined by this
> function.
> - @retval EFI_SUCCESS An fixed loading address is assigned to
> this image by build tools .
> - @retval EFI_NOT_FOUND The image has no assigned fixed loadding
> address.
> -
> -**/
> -EFI_STATUS
> -GetPeCoffImageFixLoadingAssignedAddress (
> - IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext
> - )
> -{
> - UINTN SectionHeaderOffset;
> - EFI_STATUS Status;
> - EFI_IMAGE_SECTION_HEADER SectionHeader;
> - EFI_IMAGE_OPTIONAL_HEADER_UNION *ImgHdr;
> - EFI_PHYSICAL_ADDRESS FixLoadingAddress;
> - UINT16 Index;
> - UINTN Size;
> - UINT16 NumberOfSections;
> - UINT64 ValueInSectionHeader;
> -
> - FixLoadingAddress = 0;
> - Status = EFI_NOT_FOUND;
> -
> - //
> - // Get PeHeader pointer
> - //
> - ImgHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((CHAR8
> *)ImageContext->Handle + ImageContext->PeCoffHeaderOffset);
> - SectionHeaderOffset = ImageContext->PeCoffHeaderOffset + sizeof (UINT32) +
> sizeof (EFI_IMAGE_FILE_HEADER) +
> - ImgHdr->Pe32.FileHeader.SizeOfOptionalHeader;
> - NumberOfSections = ImgHdr->Pe32.FileHeader.NumberOfSections;
> -
> - //
> - // Get base address from the first section header that doesn't point to
> code section.
> - //
> - for (Index = 0; Index < NumberOfSections; Index++) {
> - //
> - // Read section header from file
> - //
> - Size = sizeof (EFI_IMAGE_SECTION_HEADER);
> - Status = ImageContext->ImageRead (
> - ImageContext->Handle,
> - SectionHeaderOffset,
> - &Size,
> - &SectionHeader
> - );
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> -
> - Status = EFI_NOT_FOUND;
> -
> - if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_CNT_CODE) == 0) {
> - //
> - // Build tool will save the address in PointerToRelocations &
> PointerToLineNumbers fields
> - // in the first section header that doesn't point to code section in
> image header. So there
> - // is an assumption that when the feature is enabled, if a module with
> a loading address
> - // assigned by tools, the PointerToRelocations & PointerToLineNumbers
> fields should not be
> - // Zero, or else, these 2 fields should be set to Zero
> - //
> - ValueInSectionHeader = ReadUnaligned64 ((UINT64
> *)&SectionHeader.PointerToRelocations);
> - if (ValueInSectionHeader != 0) {
> - //
> - // Found first section header that doesn't point to code section in
> which build tool saves the
> - // offset to SMRAM base as image base in PointerToRelocations &
> PointerToLineNumbers fields
> - //
> - FixLoadingAddress =
> (EFI_PHYSICAL_ADDRESS)(gLoadModuleAtFixAddressMmramBase +
> (INT64)ValueInSectionHeader);
> - //
> - // Check if the memory range is available.
> - //
> - Status = CheckAndMarkFixLoadingMemoryUsageBitMap (FixLoadingAddress,
> (UINTN)(ImageContext->ImageSize + ImageContext->SectionAlignment));
> - if (!EFI_ERROR (Status)) {
> - //
> - // The assigned address is valid. Return the specified loading
> address
> - //
> - ImageContext->ImageAddress = FixLoadingAddress;
> - }
> - }
> -
> - break;
> - }
> -
> - SectionHeaderOffset += sizeof (EFI_IMAGE_SECTION_HEADER);
> - }
> -
> - DEBUG ((
> - DEBUG_INFO|DEBUG_LOAD,
> - "LOADING MODULE FIXED INFO: Loading module at fixed address %x, Status =
> %r\n",
> - FixLoadingAddress,
> - Status
> - ));
> - return Status;
> -}
> -
> /**
> Loads an EFI image into SMRAM.
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index da23b8dc3c..3d71bc84f8 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -2,7 +2,7 @@
> The internal header file includes the common header files, defines
> internal structure and functions used by MmCore module.
>
> - Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -177,7 +177,6 @@ typedef struct {
> extern MM_CORE_PRIVATE_DATA *gMmCorePrivate;
> extern EFI_MM_SYSTEM_TABLE gMmCoreMmst;
> extern LIST_ENTRY gHandleList;
> -extern EFI_PHYSICAL_ADDRESS gLoadModuleAtFixAddressMmramBase;
>
> /**
> Called to initialize the memory service.
> --
> 2.39.1.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112763): https://edk2.groups.io/g/devel/message/112763
Mute This Topic: https://groups.io/mt/103278283/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
