Re: [edk2-devel] [PATCH 5/5] MdeModulePkg/PiSmmIpl: Use SecurityLockAuditLib for debug

["Laszlo Ersek" <lersek@redhat.com>]

On 07/22/19 06:02, Gao, Zhichao wrote:
> From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2006
> 
> Use SecurityLockAuditLib in PiSmmIpl to output debug message
> while lock the SMRAM.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c   | 2 ++
>  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> index 1cf8c93227..604eb1b98e 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
> @@ -33,6 +33,7 @@
>  #include <Library/UefiRuntimeLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/ReportStatusCodeLib.h>
> +#include <Library/SecurityLockAuditLib.h>
>  
>  #include "PiSmmCorePrivateData.h"
>  
> @@ -780,6 +781,7 @@ SmmIplReadyToLockEventNotify (
>    // Lock the SMRAM (Note: Locking SMRAM may not be supported on all platforms)
>    //
>    mSmmAccess->Lock (mSmmAccess);
> +  SECURITY_LOCK_REPORT_EVENT ("Lock SMRAM", HARDWARE_LOCK);
>  
>    //
>    // Close protocol and event notification events that do not apply after the
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
> index b6b1bbcdac..2240ab3c5f 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
> @@ -46,6 +46,7 @@
>    DxeServicesLib
>    PcdLib
>    ReportStatusCodeLib
> +  SecurityLockAuditLib
>  
>  [Protocols]
>    gEfiSmmBase2ProtocolGuid                      ## PRODUCES
> 

Here a new lib class dependency is being introduced to "PiSmmIpl.inf".
In addition, the new lib class itself is introduced as a new feature, to
"MdeModulePkg.dec", in patch #1.

This means that every platform DSC that currently consumes
"PiSmmIpl.inf" will fail to build, after this patch set is merged.

That is fine for platform DSCs that live outside of the edk2 repository.
It is not fine for platform DSCs that live inside edk2.

Whenever implementing such patches, please always grep the entire edk2
repo for matches, and implement the necessary updates (you can of course
ask for details before submitting v1).

In the present case, we have

$ git grep -F PiSmmIpl.inf -- '*dsc*'
MdeModulePkg/MdeModulePkg.dsc:  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
OvmfPkg/OvmfPkgIa32.dsc:  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
OvmfPkg/OvmfPkgIa32X64.dsc:  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
OvmfPkg/OvmfPkgX64.dsc:  MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf

MdeModulePkg.dsc is updated in patch #4.

(1) Thus, please *prepend* a patch to patch#5, for OvmfPkg:

In the [LibraryClasses] section of all three DSC files, please resolve
the SecurityLockAuditLib class to the SecurityLockAuditDebugLib instance.

Doing things in this order will keep the tree bisectable -- at no stage
of the patch series will OVMF fail to build.

Thanks
Laszlo