From: "Wang, Jian J" <jian.j.wang@intel.com>
To: Jagadeesh Ujja <jagadeesh.ujja@arm.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Gao, Liming" <liming.gao@intel.com>,
"Zhang, Chao B" <chao.b.zhang@intel.com>,
"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
"ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH 00/13] Extend secure variable service to be usable from Standalone MM
Date: Fri, 21 Dec 2018 02:57:51 +0000 [thread overview]
Message-ID: <D827630B58408649ACB04F44C510003624ED368A@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1544789607-11316-1-git-send-email-jagadeesh.ujja@arm.com>
Jagadeesh,
There're many places in this patch series where code similar to following is added.
It'd better to wrap them into module private functions or even a library, if necessary.
This can make the code cleaner (no if/else) and easier (central place) to maintain in
the future.
+ if (!PcdGetBool (PcdStandaloneMmVariableEnabled)) {
+ Status = gSmst->SmmLocateProtocol (
+ &gEfiSmmSwapAddressRangeProtocolGuid,
+ NULL,
+ SarProtocol
+ );
+ } else {
+ Status = gMmst->MmLocateProtocol (
+ &gEfiSmmSwapAddressRangeProtocolGuid,
+ NULL,
+ SarProtocol
+ );
+ }
Regards,
Jian
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jagadeesh Ujja
> Sent: Friday, December 14, 2018 8:13 PM
> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhang, Chao
> B <chao.b.zhang@intel.com>; leif.lindholm@linaro.org;
> ard.biesheuvel@linaro.org
> Subject: [edk2] [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/FaultTolerantWriteStandalon
> eMm.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/VariableSmmRuntimeDxe.inf
> | 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/StandaloneMmCoreHob
> Lib.inf | 2 +-
>
> StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneMmCore
> HobLibInternal.c | 64 ++
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> | 655 ++++++++++++++++
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> | 48 ++
>
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMm
> MemoryAllocationLib.c | 824 ++++++++++++++++++++
>
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMm
> MemoryAllocationLib.inf | 45 ++
>
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServi
> cesTableLib.c | 64 ++
>
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServi
> cesTableLib.inf | 36 +
> 39 files changed, 2929 insertions(+), 244 deletions(-)
> create mode 100644
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> create mode 100644
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon
> eMm.inf
> create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> 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.inf
> create mode 100644
> StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneMmCore
> HobLibInternal.c
> create mode 100644
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
> create mode 100644
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> create mode 100644
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMm
> MemoryAllocationLib.c
> create mode 100644
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMm
> MemoryAllocationLib.inf
> create mode 100644
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServi
> cesTableLib.c
> create mode 100644
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServi
> cesTableLib.inf
>
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2018-12-21 2:57 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-14 12:13 [PATCH 00/13] Extend secure variable service to be usable from Standalone MM Jagadeesh Ujja
2018-12-14 12:13 ` [PATCH 01/13] StandaloneMmPkg: Pull in additonal libraries from staging branch Jagadeesh Ujja
2018-12-21 8:58 ` Ard Biesheuvel
2018-12-14 12:13 ` [PATCH 02/13] MdePkg: Add a PCD that indicates presence of Standalone MM mode Jagadeesh Ujja
2018-12-21 9:13 ` Ard Biesheuvel
2018-12-14 12:13 ` [PATCH 03/13] MdeModulePkg: Add a PCD to indicate Standalone MM supports secure variable Jagadeesh Ujja
2018-12-21 9:13 ` Ard Biesheuvel
2018-12-14 12:13 ` [PATCH 04/13] MdePkg/Include: add StandaloneMmServicesTableLib header file Jagadeesh Ujja
2018-12-14 12:13 ` [PATCH 05/13] MdePkg/Library/BaseLib/AArch64: Add AsmLfence function Jagadeesh Ujja
2018-12-14 13:53 ` Ard Biesheuvel
2018-12-17 2:04 ` Gao, Liming
2018-12-17 3:29 ` Yao, Jiewen
2018-12-17 7:45 ` Ard Biesheuvel
2018-12-17 8:10 ` Ard Biesheuvel
2018-12-17 8:24 ` Yao, Jiewen
2018-12-17 8:30 ` Yao, Jiewen
2018-12-17 8:35 ` Ard Biesheuvel
2018-12-17 8:44 ` Yao, Jiewen
2018-12-17 9:27 ` Ard Biesheuvel
2018-12-18 2:08 ` Yao, Jiewen
2018-12-18 2:12 ` Gao, Liming
2018-12-18 2:19 ` Yao, Jiewen
2018-12-20 9:00 ` Jagadeesh Ujja
2018-12-20 9:10 ` Ard Biesheuvel
2018-12-14 12:13 ` [PATCH 06/13] MdePkg/Library: Add StandaloneMmRuntimeDxe library Jagadeesh Ujja
2018-12-14 12:13 ` [PATCH 07/13] MdeModulePkg/FaultTolerantWriteDxe: allow reusability as a MM driver Jagadeesh Ujja
2018-12-14 12:13 ` [PATCH 08/13] MdeModulePkg/Variable/RuntimeDxe: adapt for usability with MM Standalone Jagadeesh Ujja
2018-12-14 12:13 ` [PATCH 09/13] MdeModulePkg/Variable/RuntimeDxe: adapt as a MM Standalone driver Jagadeesh Ujja
2018-12-14 12:13 ` [PATCH 10/13] MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this library Jagadeesh Ujja
2019-01-02 13:05 ` Ard Biesheuvel
2019-01-02 13:23 ` Gao, Liming
2019-01-02 14:23 ` Ard Biesheuvel
2019-01-02 16:54 ` Ard Biesheuvel
2019-01-02 13:27 ` Jagadeesh Ujja
2018-12-14 12:13 ` [PATCH 11/13] ArmPlatformPkg/NorFlashDxe: allow reusability as a MM driver Jagadeesh Ujja
2018-12-21 11:07 ` Ard Biesheuvel
2018-12-14 12:13 ` [PATCH 12/13] SecurityPkg/AuthVariableLib: allow MM_STANDALONE drivers to use this library Jagadeesh Ujja
2019-01-02 13:05 ` Ard Biesheuvel
2018-12-14 12:13 ` [PATCH 13/13] CryptoPkg/BaseCryptLib: " Jagadeesh Ujja
2018-12-21 10:13 ` Ard Biesheuvel
2018-12-17 1:45 ` [PATCH 00/13] Extend secure variable service to be usable from Standalone MM Gao, Liming
2018-12-17 11:46 ` Jagadeesh Ujja
2018-12-18 4:37 ` Gao, Liming
2018-12-18 11:19 ` Jagadeesh Ujja
2018-12-20 14:23 ` Gao, Liming
2019-01-02 17:15 ` Ard Biesheuvel
2019-01-03 7:43 ` Jagadeesh Ujja
2019-01-03 9:52 ` Ard Biesheuvel
2019-01-03 10:35 ` Ard Biesheuvel
2018-12-21 2:57 ` Wang, Jian J [this message]
2019-01-02 13:19 ` Jagadeesh Ujja
2019-01-03 2:37 ` Wang, Jian J
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D827630B58408649ACB04F44C510003624ED368A@SHSMSX103.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox