Personally, I prefer a separate DXE driver at this moment. The reason is that: It will be easy that we have a flexible feature ON/OFF in the binary level in the future. Linking different feature lib into one common driver will cause a big problem in such case. Sorry, I did not catch that in VariableStandaloneMM enabling time. I am not worried about too much on size, because the DXE FV will be compressed. The compression algo should be smart enough to catch the same code pattern. I prefer we use this patter for TcgSmm support in V4. And another patch for VariableSmm update. Thank you Yao Jiewen From: Kun Qin Sent: Monday, March 1, 2021 4:57 PM To: Yao, Jiewen ; devel@edk2.groups.io Cc: Wang, Jian J ; Zhang, Qi1 ; Kumar, Rahul1 ; Yao, Jiewen Subject: RE: [edk2-devel] [PATCH v2 5/6] SecurityPkg: Tcg2Smm: Added support for Standalone Mm Hi Jiewen, Sure, I can send the update with moving VariableMmDependency to MdeModulePkg for consistency. It will be my first time to move a module from one package to another, could you please let me know if I should make 2 patches for this purpose? (one patch for deleting the current instance, and a subsequent patch that adds the same instance to new location?) Or a single patch will be good enough? As per dependency library, do you mean you prefer to publish these dummy protocols from a DXE driver instead of anonymous library? Yes, that should work as well and it does seem cleaner and more flexible than linked library. The only drawback I could think of is the code size will be potentially larger than current solution due to library code linkages. Would you prefer me to make this change for both Tcg2Mm and VariableMm? I can send them in v4 patches. Thanks, Kun From: Yao, Jiewen Sent: Monday, March 1, 2021 00:28 To: devel@edk2.groups.io; kun.q@outlook.com Cc: Wang, Jian J; Zhang, Qi1; Kumar, Rahul1; Yao, Jiewen Subject: RE: [edk2-devel] [PATCH v2 5/6] SecurityPkg: Tcg2Smm: Added support for Standalone Mm Sorry for late response. I am thinking what is the best way to address such dependency issue. 1. If we take similar design, we need add XxxMmDependency in any StandaloneMm module with DXE communication capability, right? Now we have different rules: 1. The VariableMmDependency is in StandaloneMmPkg instead of MdeModulePkg 2. The Tcg2MmDependency is in SecurityPkg instead of StandaloneMmPkg. I think we have a consistence way to add the dependency module. I prefer to put it to the same package as the StandaloneMm module. Can we move VariableMmDependency to MdeModulePkg ? 1. Also, I don't think a Library is absolutely needed. It could be a DXE driver with gEfiMmCommunication2ProtocolGuid in dependency section, right? E.g. a Tcg2MmDependencyDxe in SecurityPkg/Tcg/Smm, and VariableMmDependencyDxe in MdeModulePkg/Universal/Variable Thank you Yao Jiewen From: devel@edk2.groups.io > On Behalf Of Kun Qin Sent: Thursday, February 25, 2021 10:26 AM To: devel@edk2.groups.io; Yao, Jiewen > Cc: Wang, Jian J >; Zhang, Qi1 >; Kumar, Rahul1 > Subject: Re: [edk2-devel] [PATCH v2 5/6] SecurityPkg: Tcg2Smm: Added support for Standalone Mm Hi Jiewen, Do you have any feedback on this patch based on my previous reply? By the way, the reason I did not add this dependency library in StandaloneMmPkg was because it will make standalone package to depend on SecurityPkg, which does not seem adequate. Please let me know how you think. Thanks in advance. Regards, Kun From: Kun Qin Sent: Tuesday, February 23, 2021 17:40 To: devel@edk2.groups.io; jiewen.yao@intel.com Cc: Wang, Jian J; Zhang, Qi1; Kumar, Rahul1 Subject: Re: [edk2-devel] [PATCH v2 5/6] SecurityPkg: Tcg2Smm: Added support for Standalone Mm Hi Jiewen, This is essentially following the example of VariableStandaloneMm model here: StandaloneMmPkg/Library: Install Variable Arch Protocol * tianocore/edk2@326598e (github.com) The intended usage for this library, in the context of Standalone MM, is to link this library to the MM IPL driver (or any other drivers that has a dependency on gEfiMmCommunication2ProtocolGuid), which will make sure MM communicate is ready to use (and all MM drivers dispatched) before DXE core dispatch Tcg2Acpi driver. I could add an example like below in the commit message if you think that will help on the intended usage: ``` MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.inf { NULL| SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.inf } ``` Or if you have any other ideas on making sure of the loading order, please let me know as well. Thanks, Kun From: Yao, Jiewen Sent: Tuesday, February 23, 2021 17:26 To: Kun Qin; devel@edk2.groups.io Cc: Wang, Jian J; Zhang, Qi1; Kumar, Rahul1 Subject: Re: [edk2-devel] [PATCH v2 5/6] SecurityPkg: Tcg2Smm: Added support for Standalone Mm I am not sure if Tcg2MmDependencyLib is the best solution. It seems NULL lib instance. But I am not sure how it is used. Can we have an example in SecurityPkg.dsc? > -----Original Message----- > From: Kun Qin > > Sent: Wednesday, February 10, 2021 9:25 AM > To: devel@edk2.groups.io > Cc: Yao, Jiewen >; Wang, Jian J >; > Zhang, Qi1 >; Kumar, Rahul1 > > Subject: [PATCH v2 5/6] SecurityPkg: Tcg2Smm: Added support for Standalone > Mm > > https://bugzilla.tianocore.org/show_bug.cgi?id=3169 > > This change added Standalone MM instance of Tcg2. The notify function for > Standalone MM instance is left empty. > > A designated dependency library was created for DXE drivers to link as an > anonymous library. > > Lastly, the support of CI build for Tcg2 Standalone MM module is added. > > Cc: Jiewen Yao > > Cc: Jian J Wang > > Cc: Qi Zhang > > Cc: Rahul Kumar > > > Signed-off-by: Kun Qin > > --- > > Notes: > v2: > - Newly added. > > SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.c | 48 > ++++++++++++ > SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c | 71 > ++++++++++++++++++ > SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.inf | 39 > ++++++++++ > SecurityPkg/SecurityPkg.ci.yaml | 1 + > SecurityPkg/SecurityPkg.dec | 1 + > SecurityPkg/SecurityPkg.dsc | 10 +++ > SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf | 77 > ++++++++++++++++++++ > 7 files changed, 247 insertions(+) > > diff --git > a/SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.c > b/SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.c > new file mode 100644 > index 000000000000..12b23813dce1 > --- /dev/null > +++ b/SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.c > @@ -0,0 +1,48 @@ > +/** @file > + Runtime DXE part corresponding to StandaloneMM Tcg2 module. > + > +This module installs gTcg2MmSwSmiRegisteredGuid to notify readiness of > +StandaloneMM Tcg2 module. > + > +Copyright (c) 2019 - 2021, Arm Ltd. All rights reserved. > +Copyright (c) Microsoft Corporation. > + > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +#include > +#include > + > +/** > + The constructor function installs gTcg2MmSwSmiRegisteredGuid to notify > + readiness of StandaloneMM Tcg2 module. > + > + @param ImageHandle The firmware allocated handle for the EFI image. > + @param SystemTable A pointer to the Management mode System Table. > + > + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > + > +**/ > +EFI_STATUS > +EFIAPI > +Tcg2MmDependencyLibConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + EFI_HANDLE Handle; > + > + Handle = NULL; > + Status = gBS->InstallProtocolInterface ( > + &Handle, > + &gTcg2MmSwSmiRegisteredGuid, > + EFI_NATIVE_INTERFACE, > + NULL > + ); > + ASSERT_EFI_ERROR (Status); > + return EFI_SUCCESS; > +} > diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c > b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c > new file mode 100644 > index 000000000000..9e0095efbc5e > --- /dev/null > +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c > @@ -0,0 +1,71 @@ > +/** @file > + TCG2 Standalone MM driver that updates TPM2 items in ACPI table and > registers > + SMI2 callback functions for Tcg2 physical presence, ClearMemory, and > + sample for dTPM StartMethod. > + > + Caution: This module requires additional review when modified. > + This driver will have external input - variable and ACPINvs data in SMM mode. > + This external input must be validated carefully to avoid security issue. > + > + PhysicalPresenceCallback() and MemoryClearCallback() will receive untrusted > input and do some check. > + > +Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation. > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include "Tcg2Smm.h" > +#include > + > +/** > + Notify the system that the SMM variable driver is ready. > +**/ > +VOID > +Tcg2NotifyMmReady ( > + VOID > + ) > +{ > + // Do nothing > +} > + > +/** > + This function is an abstraction layer for implementation specific Mm buffer > validation routine. > + > + @param Buffer The buffer start address to be checked. > + @param Length The buffer length to be checked. > + > + @retval TRUE This buffer is valid per processor architecture and not overlap > with SMRAM. > + @retval FALSE This buffer is not valid per processor architecture or overlap > with SMRAM. > +**/ > +BOOLEAN > +IsBufferOutsideMmValid ( > + IN EFI_PHYSICAL_ADDRESS Buffer, > + IN UINT64 Length > + ) > +{ > + return MmIsBufferOutsideMmValid (Buffer, Length); > +} > + > +/** > + The driver's entry point. > + > + It install callbacks for TPM physical presence and MemoryClear, and locate > + SMM variable to be used in the callback function. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI image. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The entry point is executed successfully. > + @retval Others Some error occurs when executing this entry point. > + > +**/ > +EFI_STATUS > +EFIAPI > +InitializeTcgStandaloneMm ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *SystemTable > + ) > +{ > + return InitializeTcgCommon (); > +} > diff --git > a/SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.inf > b/SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.inf > new file mode 100644 > index 000000000000..5533ce2b6e6e > --- /dev/null > +++ b/SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.inf > @@ -0,0 +1,39 @@ > +## @file > +# Runtime DXE part corresponding to StandaloneMM Tcg2 module. > +# > +# This module installs gTcg2MmSwSmiRegisteredGuid to notify readiness of > +# StandaloneMM Tcg2 module. > +# > +# Copyright (c) Microsoft Corporation. > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x0001001A > + BASE_NAME = Tcg2MmDependencyLib > + FILE_GUID = 94C210EA-3113-4563-ADEB-76FE759C2F46 > + MODULE_TYPE = DXE_DRIVER > + LIBRARY_CLASS = NULL > + CONSTRUCTOR = Tcg2MmDependencyLibConstructor > + > +# > +# The following information is for reference only and not required by the build > tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 > +# > +# > + > +[Sources] > + Tcg2MmDependencyLib.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + SecurityPkg/SecurityPkg.dec > + > +[Guids] > + gTcg2MmSwSmiRegisteredGuid ## PRODUCES ## GUID # Install > protocol > + > +[Depex] > + TRUE > diff --git a/SecurityPkg/SecurityPkg.ci.yaml b/SecurityPkg/SecurityPkg.ci.yaml > index 03be2e94ca97..d7b9e1f4e239 100644 > --- a/SecurityPkg/SecurityPkg.ci.yaml > +++ b/SecurityPkg/SecurityPkg.ci.yaml > @@ -31,6 +31,7 @@ > "MdePkg/MdePkg.dec", > "MdeModulePkg/MdeModulePkg.dec", > "SecurityPkg/SecurityPkg.dec", > + "StandaloneMmPkg/StandaloneMmPkg.dec", > "CryptoPkg/CryptoPkg.dec" > ], > # For host based unit tests > diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec > index 0970cae5c75e..dfbbb0365a2b 100644 > --- a/SecurityPkg/SecurityPkg.dec > +++ b/SecurityPkg/SecurityPkg.dec > @@ -383,6 +383,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, > PcdsDynamic, PcdsDynamicEx] > gEfiSecurityPkgTokenSpaceGuid.PcdTpmScrtmPolicy|1|UINT8|0x0001000E > > ## Guid name to identify TPM instance.

> + # NOTE: This Pcd must be FixedAtBuild if Standalone MM is used > # TPM_DEVICE_INTERFACE_NONE means disable.
> # TPM_DEVICE_INTERFACE_TPM12 means TPM 1.2 DTPM.
> # TPM_DEVICE_INTERFACE_DTPM2 means TPM 2.0 DTPM.
> diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc > index 928bff72baa3..37242da93f3d 100644 > --- a/SecurityPkg/SecurityPkg.dsc > +++ b/SecurityPkg/SecurityPkg.dsc > @@ -166,6 +166,14 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > > Tcg2PhysicalPresenceLib|SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/Sm > mTcg2PhysicalPresenceLib.inf > SmmIoLib|MdePkg/Library/SmmIoLib/SmmIoLib.inf > > +[LibraryClasses.common.MM_STANDALONE] > + > StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoin > t/StandaloneMmDriverEntryPoint.inf > + > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Standalo > neMmServicesTableLib.inf > + > Tcg2PhysicalPresenceLib|SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/Sta > ndaloneMmTcg2PhysicalPresenceLib.inf > + > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMe > mLib.inf > + > HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLi > b.inf > + > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAlloca > tionLib/StandaloneMmMemoryAllocationLib.inf > + > [PcdsDynamicDefault.common.DEFAULT] > gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0xb6, 0xe5, 0x01, 0x8b, > 0x19, 0x4f, 0xe8, 0x46, 0xab, 0x93, 0x1c, 0x53, 0x67, 0x1b, 0x90, 0xcc} > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy|1 > @@ -183,6 +191,7 @@ [PcdsDynamicHii.common.DEFAULT] > [Components] > SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf > > SecurityPkg/Library/DxeImageAuthenticationStatusLib/DxeImageAuthentication > StatusLib.inf > + SecurityPkg/Library/Tcg2MmDependencyLib/Tcg2MmDependencyLib.inf > > # > # TPM > @@ -317,6 +326,7 @@ [Components.IA32, Components.X64] > SecurityPkg/Tcg/MemoryOverwriteRequestControlLock/TcgMorLockSmm.inf > SecurityPkg/Tcg/TcgSmm/TcgSmm.inf > SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf > + SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf > SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.inf > > SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib > .inf > > SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/StandaloneMmTcg2PhysicalP > resenceLib.inf > diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf > b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf > new file mode 100644 > index 000000000000..746eda3e9fed > --- /dev/null > +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf > @@ -0,0 +1,77 @@ > +## @file > +# Provides ACPI methods for TPM 2.0 support > +# > +# Spec Compliance Info: > +# "TCG ACPI Specification Version 1.2 Revision 8" > +# "Physical Presence Interface Specification Version 1.30 Revision 00.52" > +# along with > +# "Errata Version 0.4 for TCG PC Client Platform Physical Presence Interface > Specification" > +# "Platform Reset Attack Mitigation Specification Version 1.00" > +# TPM2.0 ACPI device object > +# "TCG PC Client Platform Firmware Profile Specification for TPM Family 2.0 > Level 00 Revision 1.03 v51" > +# along with > +# "Errata for PC Client Specific Platform Firmware Profile Specification > Version 1.0 Revision 1.03" > +# > +# This driver implements TPM 2.0 definition block in ACPI table and > +# registers SMI callback functions for Tcg2 physical presence and > +# MemoryClear to handle the requests from ACPI method. > +# > +# Caution: This module requires additional review when modified. > +# This driver will have external input - variable and ACPINvs data in SMM mode. > +# This external input must be validated carefully to avoid security issue. > +# > +# Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
> +# Copyright (c) Microsoft Corporation.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = Tcg2StandaloneMm > + FILE_GUID = D40F321F-5349-4724-B667-131670587861 > + MODULE_TYPE = MM_STANDALONE > + PI_SPECIFICATION_VERSION = 0x00010032 > + VERSION_STRING = 1.0 > + ENTRY_POINT = InitializeTcgStandaloneMm > + > +[Sources] > + Tcg2Smm.h > + Tcg2Smm.c > + Tcg2StandaloneMm.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + SecurityPkg/SecurityPkg.dec > + StandaloneMmPkg/StandaloneMmPkg.dec > + > +[LibraryClasses] > + BaseLib > + BaseMemoryLib > + StandaloneMmDriverEntryPoint > + MmServicesTableLib > + DebugLib > + Tcg2PhysicalPresenceLib > + PcdLib > + MemLib > + > +[Guids] > + ## SOMETIMES_PRODUCES ## Variable:L"MemoryOverwriteRequestControl" > + ## SOMETIMES_CONSUMES ## Variable:L"MemoryOverwriteRequestControl" > + gEfiMemoryOverwriteControlDataGuid > + > + gEfiTpmDeviceInstanceTpm20DtpmGuid ## PRODUCES ## > GUID # TPM device identifier > + gTpmNvsMmGuid ## CONSUMES > + > +[Protocols] > + gEfiSmmSwDispatch2ProtocolGuid ## CONSUMES > + gEfiSmmVariableProtocolGuid ## CONSUMES > + gEfiMmReadyToLockProtocolGuid ## CONSUMES > + > +[Pcd] > + gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ## CONSUMES > + > +[Depex] > + gEfiSmmSwDispatch2ProtocolGuid AND > + gEfiSmmVariableProtocolGuid > -- > 2.30.0.windows.1