[edk2-devel] [PATCH v2 5/6] SecurityPkg: Tcg2Smm: Added support for Standalone Mm
Yao, Jiewen
jiewen.yao at intel.com
Mon Mar 1 08:28:43 UTC 2021
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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Kun Qin
Sent: Thursday, February 25, 2021 10:26 AM
To: devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>
Cc: Wang, Jian J <jian.j.wang at intel.com>; Zhang, Qi1 <qi1.zhang at intel.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>
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<mailto:kun.q at outlook.com>
Sent: Tuesday, February 23, 2021 17:40
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>; jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>
Cc: Wang, Jian J<mailto:jian.j.wang at intel.com>; Zhang, Qi1<mailto:qi1.zhang at intel.com>; Kumar, Rahul1<mailto:rahul1.kumar at intel.com>
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 at 326598e (github.com)<https://github.com/tianocore/edk2/commit/326598e9b7591dc4117c453b270811f645d099b7#diff-a0bf18927da79063fc4535344728f85944571ac4dccb77448fe00d79a385e494>
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 {
<LibraryClasses>
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<mailto:jiewen.yao at intel.com>
Sent: Tuesday, February 23, 2021 17:26
To: Kun Qin<mailto:kun.q at outlook.com>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Wang, Jian J<mailto:jian.j.wang at intel.com>; Zhang, Qi1<mailto:qi1.zhang at intel.com>; Kumar, Rahul1<mailto:rahul1.kumar at intel.com>
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 <kun.q at outlook.com<mailto:kun.q at outlook.com>>
> Sent: Wednesday, February 10, 2021 9:25 AM
> To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
> Cc: Yao, Jiewen <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>; Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>;
> Zhang, Qi1 <qi1.zhang at intel.com<mailto:qi1.zhang at intel.com>>; Kumar, Rahul1 <rahul1.kumar at intel.com<mailto:rahul1.kumar at intel.com>>
> 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 <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>
> Cc: Jian J Wang <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>
> Cc: Qi Zhang <qi1.zhang at intel.com<mailto:qi1.zhang at intel.com>>
> Cc: Rahul Kumar <rahul1.kumar at intel.com<mailto:rahul1.kumar at intel.com>>
>
> Signed-off-by: Kun Qin <kun.q at outlook.com<mailto:kun.q at outlook.com>>
> ---
>
> 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 <PiDxe.h>
> +
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +/**
> + 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.<BR>
> +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "Tcg2Smm.h"
> +#include <Library/StandaloneMmMemLib.h>
> +
> +/**
> + 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.<BR><BR>
> + # NOTE: This Pcd must be FixedAtBuild if Standalone MM is used
> # TPM_DEVICE_INTERFACE_NONE means disable.<BR>
> # TPM_DEVICE_INTERFACE_TPM12 means TPM 1.2 DTPM.<BR>
> # TPM_DEVICE_INTERFACE_DTPM2 means TPM 2.0 DTPM.<BR>
> 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.<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
> +# 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72267): https://edk2.groups.io/g/devel/message/72267
Mute This Topic: https://groups.io/mt/80522089/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210301/20614a34/attachment.htm>
More information about the edk2-devel-archive
mailing list