Hi Liming, On Thu, Nov 29, 2018 at 9:27 PM Gao, Liming <[email protected]> wrote: > > My comment is below. > > 1. Please don't update MemoryFence() implementation. It will impact all > consumer code. AsmLfence() is X86 specific API. You can implement the > internal function in the arch specific source file to call AsmLfence() for > X86 and call MemoryFence() for ARM. This internal function will be called in > the common logic. > 2. On StandaloneMmServicesTableLib.h, I suggest to add it into MdePkg, and > add StandaloneMmRuntimeDxe library into MdePkg. This library sets gMmst is > NULL, and always return FALSE in InMm(). > 3. On PcdStandaloneMmEnable, I also suggest to add it into MdePkg. It can be > used to control the driver logic in the different packages. > > With 2 & 3, other edk2 packages don't need to depend on > StandaloneMmPkg/StandaloneMmPkg.dec
Thank you for your comments. All your comments have been addressed and the next version of this patchset will include appropriate changes based on your comments. Regards, Jagadeesh. > > > -----Original Message----- > > From: Jagadeesh Ujja [mailto:[email protected]] > > Sent: Wednesday, November 28, 2018 5:35 PM > > To: [email protected]; Gao, Liming <[email protected]>; Zhang, > > Chao B <[email protected]>; [email protected]; > > [email protected] > > Subject: [RFC PATCH v3 00/11] Extend secure variable service to be usable > > from Standalone MM > > > > Changes since v2: > > - Added 'Contributed-under' tag, removed Change-ID tag and > > maintained a single signed-off-by for the all the patches. > > > > Changes since 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 RFC 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. > > > > This patch series is being posted as an RFC to get feedback on the approach > > taken > > in these patches. > > > > Jagadeesh Ujja (11): > > MdeModulePkg/Variable: replace all uses of AsmLfence with MemoryFence > > StandaloneMmPkg: Pull in additonal libraries from staging branch > > MdeModulePkg/Library: Add StandaloneMmRuntimeDxe library > > ArmPlatformPkg/NorFlashDxe: allow reusability as a MM driver > > 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 > > SecurityPkg/AuthVariableLib: allow MM_STANDALONE drivers to use this > > library > > MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this > > library > > CryptoPkg/BaseCryptLib: allow MM_STANDALONE drivers to use this > > library > > CryptoPkg/BaseCryptLib: Hack to get time in MM Standalone mode > > > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > > | 3 + > > ArmPlatformPkg/Drivers/NorFlashDxe/{NorFlashDxe.inf => > > NorFlashStandaloneMm.inf} > > | 28 +- > > CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > > | 8 +- > > CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > > | 5 + > > MdeModulePkg/Library/{VarCheckLib/VarCheckLib.inf => > > StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.inf} > > | 22 +- > > MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > > | 5 +- > > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > > | 2 + > > MdeModulePkg/Universal/FaultTolerantWriteDxe/{FaultTolerantWriteDxe.inf => > > FaultTolerantWriteStandaloneMm.inf} | > > 53 +- > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > > | 2 + > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > > | 4 + > > MdeModulePkg/Universal/Variable/RuntimeDxe/{VariableRuntimeDxe.inf => > > VariableStandaloneMm.inf} > > | 107 ++- > > SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf > > | 5 +- > > StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf > > | 2 +- > > StandaloneMmPkg/Library/{StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf > > => > > StandaloneMmHobLib/StandaloneMmHobLib.inf} | 11 +- > > > > StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf > > | 45 ++ > > > > StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf > > | 36 + > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > > | 5 +- > > MdeModulePkg/Include/Library/StandaloneMmRuntimeDxe.h > > | 39 + > > StandaloneMmPkg/Include/Library/StandaloneMmServicesTableLib.h > > | 47 ++ > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c > > | 2 +- > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > > | 211 ++++- > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > > | 88 ++- > > CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c > > | 27 +- > > MdeModulePkg/Library/StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.c > > | 36 + > > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c > > | 207 +++-- > > MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c > > | 27 +- > > MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c > > | 2 +- > > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > > | 37 +- > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > > | 201 ++++- > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > > | 31 +- > > MdePkg/Library/BaseLib/X86MemoryFence.c > > | 2 +- > > > > StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneMmCoreHobLibInternal.c > > | 64 ++ > > StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c > > | 655 ++++++++++++++++ > > > > StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c > > | 824 ++++++++++++++++++++ > > > > StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c > > | 64 ++ > > 35 files changed, 2564 insertions(+), 343 deletions(-) > > copy ArmPlatformPkg/Drivers/NorFlashDxe/{NorFlashDxe.inf => > > NorFlashStandaloneMm.inf} (71%) > > copy MdeModulePkg/Library/{VarCheckLib/VarCheckLib.inf => > > StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.inf} (51%) > > copy > > MdeModulePkg/Universal/FaultTolerantWriteDxe/{FaultTolerantWriteDxe.inf => > > FaultTolerantWriteStandaloneMm.inf} (54%) > > copy MdeModulePkg/Universal/Variable/RuntimeDxe/{VariableRuntimeDxe.inf => > > VariableStandaloneMm.inf} (54%) > > copy > > StandaloneMmPkg/Library/{StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf > > => > > StandaloneMmHobLib/StandaloneMmHobLib.inf} (79%) > > create mode 100644 > > StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf > > create mode 100644 > > StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf > > create mode 100644 MdeModulePkg/Include/Library/StandaloneMmRuntimeDxe.h > > create mode 100644 > > StandaloneMmPkg/Include/Library/StandaloneMmServicesTableLib.h > > create mode 100644 > > MdeModulePkg/Library/StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.c > > create mode 100644 > > StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneMmCoreHobLibInternal.c > > create mode 100644 > > StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c > > create mode 100644 > > StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c > > create mode 100644 > > StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c > > > > -- > > 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
