From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web10.4560.1630480663427203026 for ; Wed, 01 Sep 2021 00:17:44 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=mM32Uh3T; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 28C8724002C for ; Wed, 1 Sep 2021 09:17:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1630480660; bh=4gX90DcekG2FQfiz5yVr7w7hjVe9S5JoX5ksu0YRCOc=; h=Subject:To:Cc:From:Date:From; b=mM32Uh3Ttz2OMzdrR7z7+2q2cWIgsOqWYpqGrUX1LnN/6uqmvnqTt/n9YdVSazy2U A6K5lrJ7fZf2rpz0r5qZBI/WUhUEL8GwgIQSOGiz7LcKOiihdJYVesZzf8lc1G4dUK C3sOeBnRogzFYyPcPk5x74f1kl2WIGMJ/nXfzxQb/JlMTXFU54Mrlf2B+uyst2urQJ +lR1V89z745XTumCaA6W+FzpqLdHMj3GOAb5yw2q0NEqIg/ADY04/9ctMmQBLxjqXI 8VMFjT7s1bLsjstfOqBc5QZqAOPP8qoSONxmf8klLmPQOG3RTUPKhgONFdLlJNVSPP JyGTMnJlZ39cg== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4GzwPG4R7pz6tmc; Wed, 1 Sep 2021 09:17:38 +0200 (CEST) Subject: Re: [PATCH 1/1] MdeModulePkg: Move PiSmmCoreMemoryAllocationLib into PiSmmCore To: "Ni, Ray" , "devel@edk2.groups.io" Cc: "Wang, Jian J" , "Wu, Hao A" , "Dong, Eric" , Vitaly Cheptsov References: From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: Date: Wed, 1 Sep 2021 07:17:38 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Hey Ray, Thanks for your response! 1) It would disrupt platform builds that use this INF. 2) We'd need a new library to satisfy MemoryAllocationLib dependencies.=20 If using the generic SMM one, libraries linked against the core would=20 start using the indirect table calls over the direct calls for=20 practically no reason, at least I see none at the moment. More or less I saw no reason to do it, as this is a change that needs no=20 platform maintainer attention, but if you have suggestion on how to=20 improve the patch, I'd be open to it of course. Best regards, Marvin On 01/09/2021 06:21, Ni, Ray wrote: > Marvin, > Your patch moves the memory allocation lib implementation to PiSmmCore. > Can you remove the PiSmmCoreMemoryAllocationLib.inf completely? (or wha= t forbids you remove this lib instance?) > > Thanks, > Ray > > -----Original Message----- > From: Marvin H=C3=A4user > Sent: Sunday, August 22, 2021 3:56 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A ; Dong, Eric ; Ni, Ray ; Vitaly C= heptsov > Subject: [PATCH 1/1] MdeModulePkg: Move PiSmmCoreMemoryAllocationLib in= to PiSmmCore > > PiSmmCoreMemoryAllocationLib duplicates private definitions of > PiSmmCore, namely the SMM_CORE_PRIVATE_DATA structure. Move this code > into PiSmmCore, so that the struct definition can be consumed > directly instead. > > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Eric Dong > Cc: Ray Ni > Cc: Vitaly Cheptsov > Signed-off-by: Marvin H=C3=A4user > --- > MdeModulePkg/{Library/PiSmmCoreMemoryAllocationLib/MemoryAllocationLi= b.c =3D> Core/PiSmmCore/MemoryAllocation.c} | 3 +- > MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf = | 1 + > MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemoryAllo= cationLib.inf | 5 +- > MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemoryAllo= cationProfileLib.inf | 6 +- > MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemoryAllo= cationServices.h | 185 -------------------- > 5 files changed, 10 insertions(+), 190 deletions(-) > > diff --git a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAl= locationLib.c b/MdeModulePkg/Core/PiSmmCore/MemoryAllocation.c > similarity index 96% > rename from MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAll= ocationLib.c > rename to MdeModulePkg/Core/PiSmmCore/MemoryAllocation.c > index fd20a779cdcc..fb99174c9d8d 100644 > --- a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocatio= nLib.c > +++ b/MdeModulePkg/Core/PiSmmCore/MemoryAllocation.c > @@ -22,7 +22,8 @@ > #include > > #include > > #include > > -#include "PiSmmCoreMemoryAllocationServices.h" > > +#include "PiSmmCore.h" > > +#include "PiSmmCorePrivateData.h" > > =20 > > #include > > =20 > > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/C= ore/PiSmmCore/PiSmmCore.inf > index c8bfae3860fc..85628f927134 100644 > --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf > +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf > @@ -37,6 +37,7 @@ [Sources] > SmiHandlerProfile.c > > HeapGuard.c > > HeapGuard.h > > + MemoryAllocation.c > > =20 > > [Packages] > > MdePkg/MdePkg.dec > > diff --git a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCor= eMemoryAllocationLib.inf b/MdeModulePkg/Library/PiSmmCoreMemoryAllocation= Lib/PiSmmCoreMemoryAllocationLib.inf > index 5c51c48b0b1e..8812c9604103 100644 > --- a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemory= AllocationLib.inf > +++ b/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemory= AllocationLib.inf > @@ -19,6 +19,9 @@ [Defines] > VERSION_STRING =3D 1.0 > > PI_SPECIFICATION_VERSION =3D 0x0001000A > > LIBRARY_CLASS =3D MemoryAllocationLib|SMM_CORE > > + # > > + # This function is defined in PiSmmCore. > > + # > > CONSTRUCTOR =3D PiSmmCoreMemoryAllocationLibCons= tructor > > =20 > > # > > @@ -28,8 +31,6 @@ [Defines] > # > > =20 > > [Sources] > > - MemoryAllocationLib.c > > - PiSmmCoreMemoryAllocationServices.h > > PiSmmCoreMemoryProfileLibNull.c > > =20 > > [Packages] > > diff --git a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCor= eMemoryAllocationProfileLib.inf b/MdeModulePkg/Library/PiSmmCoreMemoryAll= ocationLib/PiSmmCoreMemoryAllocationProfileLib.inf > index 89658c0f6ccb..c3b8a4fdce7b 100644 > --- a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemory= AllocationProfileLib.inf > +++ b/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemory= AllocationProfileLib.inf > @@ -19,6 +19,9 @@ [Defines] > VERSION_STRING =3D 1.0 > > PI_SPECIFICATION_VERSION =3D 0x0001000A > > LIBRARY_CLASS =3D MemoryAllocationLib|SMM_CORE > > + # > > + # This function is defined in PiSmmCore. > > + # > > CONSTRUCTOR =3D PiSmmCoreMemoryAllocationLibCons= tructor > > LIBRARY_CLASS =3D MemoryProfileLib|SMM_CORE > > CONSTRUCTOR =3D PiSmmCoreMemoryProfileLibConstru= ctor > > @@ -30,8 +33,6 @@ [Defines] > # > > =20 > > [Sources] > > - MemoryAllocationLib.c > > - PiSmmCoreMemoryAllocationServices.h > > PiSmmCoreMemoryProfileLib.c > > PiSmmCoreMemoryProfileServices.h > > =20 > > @@ -43,6 +44,7 @@ [LibraryClasses] > DebugLib > > BaseMemoryLib > > UefiBootServicesTableLib > > + MemoryAllocationLib > > =20 > > [Guids] > > gEdkiiMemoryProfileGuid ## SOMETIMES_CONSUMES ## GUID # Locate = protocol > > diff --git a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCor= eMemoryAllocationServices.h b/MdeModulePkg/Library/PiSmmCoreMemoryAllocat= ionLib/PiSmmCoreMemoryAllocationServices.h > deleted file mode 100644 > index 789fcf2c01ea..000000000000 > --- a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/PiSmmCoreMemory= AllocationServices.h > +++ /dev/null > @@ -1,185 +0,0 @@ > -/** @file > > - Contains function prototypes for Memory Services in the SMM Core. > > - > > - This header file borrows the PiSmmCore Memory Allocation services as= the primitive > > - for memory allocation. > > - > > - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved. > > - SPDX-License-Identifier: BSD-2-Clause-Patent > > - > > -**/ > > - > > -#ifndef _PI_SMM_CORE_MEMORY_ALLOCATION_SERVICES_H_ > > -#define _PI_SMM_CORE_MEMORY_ALLOCATION_SERVICES_H_ > > - > > -// > > -// It should be aligned with the definition in PiSmmCore. > > -// > > -typedef struct { > > - UINTN Signature; > > - > > - /// > > - /// The ImageHandle passed into the entry point of the SMM IPL. Thi= s ImageHandle > > - /// is used by the SMM Core to fill in the ParentImageHandle field o= f the Loaded > > - /// Image Protocol for each SMM Driver that is dispatched by the SMM= Core. > > - /// > > - EFI_HANDLE SmmIplImageHandle; > > - > > - /// > > - /// The number of SMRAM ranges passed from the SMM IPL to the SMM Co= re. The SMM > > - /// Core uses these ranges of SMRAM to initialize the SMM Core memor= y manager. > > - /// > > - UINTN SmramRangeCount; > > - > > - /// > > - /// A table of SMRAM ranges passed from the SMM IPL to the SMM Core.= The SMM > > - /// Core uses these ranges of SMRAM to initialize the SMM Core memor= y manager. > > - /// > > - EFI_SMRAM_DESCRIPTOR *SmramRanges; > > - > > - /// > > - /// The SMM Foundation Entry Point. The SMM Core fills in this fiel= d when the > > - /// SMM Core is initialized. The SMM IPL is responsbile for registe= ring this entry > > - /// point with the SMM Configuration Protocol. The SMM Configuratio= n Protocol may > > - /// not be available at the time the SMM IPL and SMM Core are starte= d, so the SMM IPL > > - /// sets up a protocol notification on the SMM Configuration Protoco= l and registers > > - /// the SMM Foundation Entry Point as soon as the SMM Configuration = Protocol is > > - /// available. > > - /// > > - EFI_SMM_ENTRY_POINT SmmEntryPoint; > > - > > - /// > > - /// Boolean flag set to TRUE while an SMI is being processed by the = SMM Core. > > - /// > > - BOOLEAN SmmEntryPointRegistered; > > - > > - /// > > - /// Boolean flag set to TRUE while an SMI is being processed by the = SMM Core. > > - /// > > - BOOLEAN InSmm; > > - > > - /// > > - /// This field is set by the SMM Core then the SMM Core is initializ= ed. This field is > > - /// used by the SMM Base 2 Protocol and SMM Communication Protocol i= mplementations in > > - /// the SMM IPL. > > - /// > > - EFI_SMM_SYSTEM_TABLE2 *Smst; > > - > > - /// > > - /// This field is used by the SMM Communicatioon Protocol to pass a = buffer into > > - /// a software SMI handler and for the software SMI handler to pass = a buffer back to > > - /// the caller of the SMM Communication Protocol. > > - /// > > - VOID *CommunicationBuffer; > > - > > - /// > > - /// This field is used by the SMM Communicatioon Protocol to pass th= e size of a buffer, > > - /// in bytes, into a software SMI handler and for the software SMI h= andler to pass the > > - /// size, in bytes, of a buffer back to the caller of the SMM Commun= ication Protocol. > > - /// > > - UINTN BufferSize; > > - > > - /// > > - /// This field is used by the SMM Communication Protocol to pass the= return status from > > - /// a software SMI handler back to the caller of the SMM Communicati= on Protocol. > > - /// > > - EFI_STATUS ReturnStatus; > > - > > - EFI_PHYSICAL_ADDRESS PiSmmCoreImageBase; > > - UINT64 PiSmmCoreImageSize; > > - EFI_PHYSICAL_ADDRESS PiSmmCoreEntryPoint; > > -} SMM_CORE_PRIVATE_DATA; > > - > > -/** > > - Called to initialize the memory service. > > - > > - @param SmramRangeCount Number of SMRAM Regions > > - @param SmramRanges Pointer to SMRAM Descriptors > > - > > -**/ > > -VOID > > -SmmInitializeMemoryServices ( > > - IN UINTN SmramRangeCount, > > - IN EFI_SMRAM_DESCRIPTOR *SmramRanges > > - ); > > - > > -/** > > - Allocates pages from the memory map. > > - > > - @param Type The type of allocation to perform > > - @param MemoryType The type of memory to turn the alloca= ted pages > > - into > > - @param NumberOfPages The number of pages to allocate > > - @param Memory A pointer to receive the base allocat= ed memory > > - address > > - > > - @retval EFI_INVALID_PARAMETER Parameters violate checking rules def= ined in spec. > > - @retval EFI_NOT_FOUND Could not allocate pages match the re= quirement. > > - @retval EFI_OUT_OF_RESOURCES No enough pages to allocate. > > - @retval EFI_SUCCESS Pages successfully allocated. > > - > > -**/ > > -EFI_STATUS > > -EFIAPI > > -SmmAllocatePages ( > > - IN EFI_ALLOCATE_TYPE Type, > > - IN EFI_MEMORY_TYPE MemoryType, > > - IN UINTN NumberOfPages, > > - OUT EFI_PHYSICAL_ADDRESS *Memory > > - ); > > - > > -/** > > - Frees previous allocated pages. > > - > > - @param Memory Base address of memory being freed > > - @param NumberOfPages The number of pages to free > > - > > - @retval EFI_NOT_FOUND Could not find the entry that covers = the range > > - @retval EFI_INVALID_PARAMETER Address not aligned > > - @return EFI_SUCCESS Pages successfully freed. > > - > > -**/ > > -EFI_STATUS > > -EFIAPI > > -SmmFreePages ( > > - IN EFI_PHYSICAL_ADDRESS Memory, > > - IN UINTN NumberOfPages > > - ); > > - > > -/** > > - Allocate pool of a particular type. > > - > > - @param PoolType Type of pool to allocate > > - @param Size The amount of pool to allocate > > - @param Buffer The address to return a pointer to th= e allocated > > - pool > > - > > - @retval EFI_INVALID_PARAMETER PoolType not valid > > - @retval EFI_OUT_OF_RESOURCES Size exceeds max pool size or allocat= ion failed. > > - @retval EFI_SUCCESS Pool successfully allocated. > > - > > -**/ > > -EFI_STATUS > > -EFIAPI > > -SmmAllocatePool ( > > - IN EFI_MEMORY_TYPE PoolType, > > - IN UINTN Size, > > - OUT VOID **Buffer > > - ); > > - > > -/** > > - Frees pool. > > - > > - @param Buffer The allocated pool entry to free > > - > > - @retval EFI_INVALID_PARAMETER Buffer is not a valid value. > > - @retval EFI_SUCCESS Pool successfully freed. > > - > > -**/ > > -EFI_STATUS > > -EFIAPI > > -SmmFreePool ( > > - IN VOID *Buffer > > - ); > > - > > -#endif >