Re: [PATCH V4 0/8] Enable secure-boot when lauch OVMF with -bios parameter

["Min Xu" <min.m.xu@intel.com>]

Hi, Gerd
Do you have any comments to this patch-set?

Thanks Min
> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Friday, July 1, 2022 7:29 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Chang, Abner <abner.chang@hpe.com>; Schaefer, Daniel
> <daniel.schaefer@hpe.com>; Aktas, Erdem <erdemaktas@google.com>;
> James Bottomley <jejb@linux.ibm.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V4 0/8] Enable secure-boot when lauch OVMF with -bios
> parameter
> 
> Secure-Boot related variables include the PK/KEK/DB/DBX and they are
> stored in NvVarStore (OVMF_VARS.fd). When lauching with -pflash,
> QEMU/OVMF will use emulated flash, and fully support UEFI variables.
> But when launching with -bios parameter, UEFI variables will be partially
> emulated, and non-volatile variables may lose their contents after a reboot.
> See OvmfPkg/README.
> 
> Tdx guest is an example that -pflash is not supported. So this patch-set is
> designed to initialize the NvVarStore with the content of in OVMF_VARS.fd.
> 
> patch 1:
>  Add a new function (AllocateRuntimePages) in PrePiMemoryAllocationLib.
>  This function will be used in PeilessStartupLib which will run  in SEC phase.
> 
> patch 2:
>  Delete the TdxValidateCfv in PeilessStartupLib. Because it is going to  be
> renamed to PlatformValidateNvVarStore and be moved to PlatformInitLib.
> 
> patch 3 - 7:
>  Then we add functions for EmuVariableNvStore in PlatformInitLib. This  lib
> will then be called in OvmfPkg/PlatformPei and PeilessStartupLib.
>  We also shortcut ConnectNvVarsToFileSystem in secure-boot.
> 
> patch 8:
>  At last a build-flag (SECURE_BOOT_FEATURE_ENABLED) is introduced in  the
> dsc in OvmfPkg. Because the copy over of OVMR_VARS.fd to
> EmuVariableNvStore is only required when secure-boot is enabled.
> 
> Code: https://github.com/mxu9/edk2/tree/secure-boot.v4
> 
> v4 chagnes:
>  - "EmbeddedPkg: Add AllocateRuntimePages in PrePiMemoryAllocationLib"
> is
>    missed in v3. It is added in this version.
>  - No other changes.
> 
> v3 changes:
>  - Renamed TdxValidateCfv to PlatformValidateNvVarStore and implemented
>    in PlatformInitlLib/Platform.c.
>  - Shortcut ConnectNvVarsToFileSystem in secure-boot.
>  - Other minor changes, such as adding log in
>    PlatformInitEmuVariableNvStore.
> 
> v2 changes:
>  - The v1 title is "Enable Secure-Boot in Tdx guest". Because the
>    patch-setwe was first designed to fix the gap when secure-boot feature
>    was enabled in Tdx guest. After discussing with the community (see
>    the disuccsions under https://edk2.groups.io/g/devel/message/90589)
>    this patch-set can fix the secure-boot issue when OVMF is lauched
>    with -bios parameter. So the title is updated.
>  - Add a new function (AllocateRuntimePages) in PrePiMemoryAllocationLib.
>  - Add build-flag SECURE_BOOT_FEATURE_ENABLED to control the copy over
>    of OVMF_VARS.fd to EmuVariableNvStore.
> 
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com> [jejb]
> Cc: Jiewen Yao <jiewen.yao@intel.com> [jyao1]
> Cc: Tom Lendacky <thomas.lendacky@amd.com> [tlendacky]
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min M Xu (8):
>   EmbeddedPkg: Add AllocateRuntimePages in PrePiMemoryAllocationLib
>   OvmfPkg/PeilessStartupLib: Delete TdxValidateCfv
>   OvmfPkg/PlatformInitLib: Add functions for EmuVariableNvStore
>   OvmfPkg/PlatformPei: Update ReserveEmuVariableNvStore
>   OvmfPkg: Reserve and init EmuVariableNvStore in Pei-less Startup
>   OvmfPkg/NvVarsFileLib: Shortcut ConnectNvVarsToFileSystem in
>     secure-boot
>   OvmfPkg/TdxDxe: Set PcdEmuVariableNvStoreReserved
>   OvmfPkg: Add build-flag SECURE_BOOT_FEATURE_ENABLED
> 
>  EmbeddedPkg/Include/Library/PrePiLib.h        |  19 ++
>  .../MemoryAllocationLib.c                     |  64 +++--
>  OvmfPkg/CloudHv/CloudHvX64.dsc                |   9 +
>  OvmfPkg/Include/Library/PlatformInitLib.h     |  51 ++++
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |   9 +
>  OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.c |   7 +
>  OvmfPkg/Library/PeilessStartupLib/IntelTdx.c  | 153 -----------
>  .../PeilessStartupLib/PeilessStartup.c        |  15 +-
>  .../PeilessStartupInternal.h                  |  17 --
>  OvmfPkg/Library/PlatformInitLib/Platform.c    | 238 ++++++++++++++++++
>  .../PlatformInitLib/PlatformInitLib.inf       |   3 +
>  OvmfPkg/OvmfPkgIa32.dsc                       |   9 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |   9 +
>  OvmfPkg/OvmfPkgX64.dsc                        |   9 +
>  OvmfPkg/PlatformPei/Platform.c                |  25 +-
>  OvmfPkg/TdxDxe/TdxDxe.c                       |   2 +
>  OvmfPkg/TdxDxe/TdxDxe.inf                     |   1 +
>  17 files changed, 428 insertions(+), 212 deletions(-)
> 
> --
> 2.29.2.windows.2