Hi Liming, On Tue, Dec 18, 2018 at 10:07 AM Gao, Liming <[email protected]> wrote: > > Jagadeesh: > StandaloneMmServicesTableLib library class header file is added into > MdePkg. Its library implementation is in MdePkg and StandaloneMmPkg. The one > in MdePkg produces the dummy implementation, and the one in StandaloneMmPkg > produces the real implementation. I don't see the reason to separate this > library class. >
In this patchset series, the Variable service/Fault tolerant/Nor Flash driver are refactored to be usable as MM_STANDALONE driver. These drivers uses the following libraries from “StandaloneMmPkg”. - MmServicesTableLib|StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf - MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf Variable MM_STANDALONE driver is located at - MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c - MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf FaultTolerant MM_STANDALONE is driver located at - MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c - MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf These drivers look for “gMmst” which is defined in “MmServicesTableLib”. Ideally, “StandaloneMmPkg/Include/Library/StandaloneMmServicesTableLib.h” should have defined “gMmst” as an “extern EFI_MM_SYSTEM_TABLE *gMmst;”. In which case, we would have to add “StandaloneMmPkg/StandaloneMmPkg.dec” in other drivers listed below. - MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf - MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf This will make “edk2 packages” to be depended on "StandaloneMmPkg/StandaloneMmPkg.dec". To avoid this, “StandaloneMmPkg/Include/Library/StandaloneMmServicesTableLib.h” is moved to “MdePkg/Include/Library/StandaloneMmServicesTableLib.h”. But, the implementation of “MmServicesTableLib” comes from “StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf”. Thanks Jagadeesh > Thanks > Liming > >-----Original Message----- > >From: Jagadeesh Ujja [mailto:[email protected]] > >Sent: Monday, December 17, 2018 7:47 PM > >To: Gao, Liming <[email protected]> > >Cc: [email protected]; Zhang, Chao B <[email protected]>; > >[email protected]; [email protected] > >Subject: Re: [edk2] [PATCH 00/13] Extend secure variable service to be usable > >from Standalone MM > > > >Hi Liming, > > > >On Mon, Dec 17, 2018 at 7:15 AM Gao, Liming <[email protected]> wrote: > >> > >> One question here. Why separate StandaloneMmServicesTableLib to two > >library classes? Current MdePkg\Include\Library\SmmServicesTableLib.h is > >one library class. > >MdePkg\Library\SmmServicesTableLib\SmmServicesTableLib.inf is its > >implementation. StandaloneMmServicesTableLib should be same to it. > >> StandaloneMmServicesTableLib is the library class. > >MdePkg\Library\StandaloneMmRuntimeDxe is its library instance. > >> > >Thanks for your review. > > > >The implementation of the "StandaloneMmServicesTableLib" library class > >is at "StandaloneMmPkg/Library/StandaloneMmServicesTableLib/". As this > >patchset reuses some of the DXE_DRIVER drivers as MM_STANDALONE > >drivers, the "StandaloneMmServicesTableLib" library class definition > >was placed within MdePkg. The reason for splitting the library class > >definition (in MdePkg) and its implementation (in StandaloneMmPkg) was > >due to your comment that "edk2 packages" should not have any reference > >to StandaloneMmPkg.dec. > > > >The "StandaloneMmRuntimeDxe" library now just has an implementation of > >InMm(). And so, this can be kept as a separate library with no > >dependency on StandaloneMmPkg. So this was the reason to split > >"StandaloneMmRuntimeDxe" and "StandaloneMmServicesTableLib" into two > >separate libraries. > > > >thanks > >Jagadeesh > >> Thanks > >> Liming > >> >-----Original Message----- > >> >From: Jagadeesh Ujja [mailto:[email protected]] > >> >Sent: Friday, December 14, 2018 8:13 PM > >> >To: [email protected]; Gao, Liming <[email protected]>; Zhang, > >> >Chao B <[email protected]>; [email protected]; > >> >[email protected] > >> >Subject: [PATCH 00/13] Extend secure variable service to be usable from > >> >Standalone MM > >> > > >> >Changes since RFC v4: > >> >- Addressed all the comments from Liming Gao > >> > - Added an additional PCD 'PcdStandaloneMmCodeEnabled' to indicate > >> > presence of StandaloneMM support. > >> > - MdePkg.dec file updated to include StandaloneMmServiceTableLib and > >> > StandaloneMmRuntimeDxe library. > >> > - Platform specific changes will be posted in a seperate patchset. > >> > - AsmLfence wrapper function is supported for AArch64 platforms. > >> > - All the patches in this series can be pulled from > >> > https://github.com/jagadeeshujja/edk2 (branch: > >> >topics/aarch64_secure_vars) > >> > > >> >Changes since RFC v3: > >> >- Addressed all the comments from Liming Gao > >> > - Added a AArch64 implementation of AsmLfence which is a wrapper for > >> > MemoryFence. The changes in variable service driver in v3 of this > >> > patchset that used MemoryFence instead of AsmLfence have been > >> >removed. > >> > - Added StandaloneMmServicesTableLib.h and > >StandaloneMmRuntimeDxe > >> > library into MdePkg. > >> > - Renamed PcdStandaloneMmEnable as > >PcdStandaloneMmVariableEnabled > >> >and > >> > added to in to MdePkg. > >> > - Now with above changes, edk2 packages don't need to depend on > >> > StandaloneMmPkg/StandaloneMmPkg.dec > >> >- Addressed comments from Ting Ye > >> > - Removed the hacks in the v3 version. > >> > - Will relook into the “TimerWrapp.c” file and add a appropriate > >> > implementation of this for MM Standalone mode code. > >> > > >> >Changes since RFC v2: > >> >- Added 'Contributed-under' tag, removed Change-ID tag and > >> > maintained a single signed-off-by for the all the patches. > >> > > >> >Changes since RFC v1: > >> >- Addressed all the comments from Liming Gao > >> > - Removed the use of #ifdef/#else/#endif and used a Pcd instead to > >> > select between MM and non-MM paths. > >> > - Removed all dependencies on edk2-platforms. > >> > - Dropped the use of mMmst and used gSmst instead. > >> > - Added a dummy implementation UefiRuntimeServiceTableLib for > >> > MM_STANDALONE usage > >> >- Replaced all uses of AsmLfence with MemoryFence from variable > >> > service code. > >> >- Add a new StandaloneMmRuntimeDxe library to for use by non-MM > >code. > >> > > >> >This patch series extends the existing secure variable service support for > >> >use with Standalone MM. This is applicable to paltforms that use > >Standalone > >> >Management Mode to protect access to non-volatile memory (NOR flash > >in > >> >case > >> >of these patches) used to store the secure EFI variables. > >> > > >> >The first patch pulls in additional libraries from the staging branch of > >> >StandaloneMmPkg into the edk2's StandaloneMmPkg. The existing secure > >> >variable > >> >service implementation supports only the traditional MM mode and so the > >> >rest > >> >of the patches extends the existing secure variable service support to be > >> >useable with Standalone MM mode as well. > >> > > >> >Jagadeesh Ujja (13): > >> > StandaloneMmPkg: Pull in additonal libraries from staging branch > >> > MdePkg: Add a PCD that indicates presence of Standalone MM mode > >> > MdeModulePkg: Add a PCD to indicate Standalone MM supports secure > >> > variable > >> > MdePkg/Include: add StandaloneMmServicesTableLib header file > >> > MdePkg/Library/BaseLib/AArch64: Add AsmLfence function > >> > MdePkg/Library: Add StandaloneMmRuntimeDxe library > >> > MdeModulePkg/FaultTolerantWriteDxe: allow reusability as a MM driver > >> > MdeModulePkg/Variable/RuntimeDxe: adapt for usability with MM > >> > Standalone > >> > MdeModulePkg/Variable/RuntimeDxe: adapt as a MM Standalone driver > >> > MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this > >> > library > >> > ArmPlatformPkg/NorFlashDxe: allow reusability as a MM driver > >> > SecurityPkg/AuthVariableLib: allow MM_STANDALONE drivers to use this > >> > library > >> > CryptoPkg/BaseCryptLib: allow MM_STANDALONE drivers to use this > >> > library > >> > > >> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c > >> >| 2 +- > >> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > >| > >> >210 ++++- > >> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > >| > >> >5 +- > >> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > >| > >> >2 + > >> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > >> >| 96 +-- > >> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf > >> >| 76 ++ > >> > CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > >> > | > >7 > >> >+- > >> > CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > >| 4 > >> >+ > >> > CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c > >| > >> >15 +- > >> > MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > >| > >> >5 +- > >> > MdeModulePkg/MdeModulePkg.dec > >> > | 5 + > >> > > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > >> >| 1 + > >> > > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c > >> >| 203 +++-- > >> > > >> >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStan > >dal > >> >oneMm.inf | 101 +++ > >> > MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c > >> >| 27 +- > >> > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >> >| 37 +- > >> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > >> >| 1 + > >> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > >> >| 201 ++++- > >> > > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > >> >| 31 +- > >> > > >> >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > >e.i > >> >nf | 3 + > >> > > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > >> >| 132 ++++ > >> > MdePkg/Include/Library/BaseLib.h > >> > | 33 +- > >> > MdePkg/Include/Library/StandaloneMmRuntimeDxe.h > >> >| 39 + > >> > MdePkg/Include/Library/StandaloneMmServicesTableLib.h > >> >| 25 + > >> > MdePkg/Library/BaseLib/AArch64/AsmLfence.S > >> > | > >42 > >> >+ > >> > MdePkg/Library/BaseLib/AArch64/AsmLfence.asm > >| > >> >41 + > >> > MdePkg/Library/BaseLib/BaseLib.inf > >> > | 2 + > >> > > >MdePkg/Library/StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.c > >> >| 36 + > >> > > >MdePkg/Library/StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.inf > >> >| 36 + > >> > MdePkg/MdePkg.dec > >> > | 12 + > >> > SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf > >| > >> >5 +- > >> > > >> >StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCor > >eH > >> >obLib.inf | 2 +- > >> > > >> >StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneM > >mC > >> >oreHobLibInternal.c | 64 ++ > >> > > >StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c > >> >| 655 ++++++++++++++++ > >> > > >StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf > >> >| 48 ++ > >> > > >> >StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo > >ne > >> >MmMemoryAllocationLib.c | 824 ++++++++++++++++++++ > >> > > >> >StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo > >ne > >> >MmMemoryAllocationLib.inf | 45 ++ > >> > > >> >StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneM > >mS > >> >ervicesTableLib.c | 64 ++ > >> > > >> >StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneM > >mS > >> >ervicesTableLib.inf | 36 + > >> > 39 files changed, 2929 insertions(+), 244 deletions(-) > >> > create mode 100644 > >> >ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf > >> > create mode 100644 > >> >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStan > >dal > >> >oneMm.inf > >> > create mode 100644 > >> >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i > >nf > >> > create mode 100644 > >MdePkg/Include/Library/StandaloneMmRuntimeDxe.h > >> > create mode 100644 > >> >MdePkg/Include/Library/StandaloneMmServicesTableLib.h > >> > create mode 100644 MdePkg/Library/BaseLib/AArch64/AsmLfence.S > >> > create mode 100644 MdePkg/Library/BaseLib/AArch64/AsmLfence.asm > >> > create mode 100644 > >> >MdePkg/Library/StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe. > >c > >> > create mode 100644 > >> >MdePkg/Library/StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.i > >nf > >> > create mode 100644 > >> >StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneM > >mC > >> >oreHobLibInternal.c > >> > create mode 100644 > >> >StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c > >> > create mode 100644 > >> >StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.i > >nf > >> > create mode 100644 > >> >StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo > >ne > >> >MmMemoryAllocationLib.c > >> > create mode 100644 > >> >StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalo > >ne > >> >MmMemoryAllocationLib.inf > >> > create mode 100644 > >> >StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneM > >mS > >> >ervicesTableLib.c > >> > create mode 100644 > >> >StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneM > >mS > >> >ervicesTableLib.inf > >> > > >> >-- > >> >2.7.4 > >> > >> _______________________________________________ > >> edk2-devel mailing list > >> [email protected] > >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
