Re: [edk2-devel] [PATCH v4 0/8] Add Variable Flash Info HOB

["Michael Kubacki" <mikuback@linux.microsoft.com>]

Hello MdeModulePkg maintainers,

This patch series has received reviews for all patches. I have placed a 
rebased branch with up-to-date review tags in the following PR:

https://github.com/tianocore/edk2/pull/2812

Please let me know if anything further is needed for you to merge the PR.

Thanks,
Michael

On 4/12/2022 12:29 PM, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
> 
> The UEFI variable drivers such as VariableRuntimeDxe, VariableSmm,
> VariableStandaloneMm, etc. (and their dependent protocol/library
> stack), typically acquire UEFI variable store flash information
> with PCDs declared in MdeModulePkg.
> 
> For example:
> [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> 
> These PCDs work as-is in the StandaloneMm driver if they are not
> dynamic such as Dynamic or DynamicEx because PCD services are not
> readily available in the Standalone MM environment. Platforms that
> use Standalone MM today, must define these PCDs as FixedAtBuild in
> their platform build. However, the PCDs do allow platforms to treat
> the PCDs as Dynamic/DynamicEx and being able to support that is
> currently a gap for Standalone MM.
> 
> This patch series introduces a HOB that can be produced by the
> platform to provide the same information. The HOB list is
> available to Standalone MM.
> 
> The PCD declarations are left as-is in MdeModulePkg for backward
> compatibility. This means unless a platform wants to use the HOB,
> their code will continue to work with no change (they do not need
> to produce the HOB). Only if the HOB is found, is its value used
> instead of the PCDs.
> 
> Due to the large number of consumers of this information, access
> to the base address and size values is abstracted in a new library
> class (as requested in the v1 series) called VariableFlashInfoLib.
> 
> The API of VariableFlashInfoLib does not bind the underlying data
> structure to the information returned to library users to allow
> flexibility in the library implementation in the future.
> 
> V4 changes:
> 1. Add a UINT32 "Reserved" field to VARIABLE_FLASH_INFO.
> 2. Add a descriptive comment to VariableFlashInfo.h to explain
>     HOB usage.
> 
> V3 changes:
> 1. To better clarify usage, renamed the members
>     "NvStorageBaseAddress" and "NvStorageLength" in
>     "VARIABLE_FLASH_INFO" to "NvVariableBaseAddress" and
>     "NvVariableLength".
> 2. Added description comments to the fields in "VARIABLE_FLASH_INFO".
> 
> V2 changes:
> 1. Abstracted flash info data access with VariableFlashInfoLib.
> 2. Updated package builds in the repo that build the variable and
>     FTW drivers to include VariableFlashInfoLib.
> 3. Removed a redundant variable assignment in VariableSmm.c.
> 4. Updated comments in FtwMisc.c and FaultTolerantWritePei.c to
>     indicate driver assumption is UINTN (not UINT32)
> 5. Added a version field to the VARIABLE_FLASH_INFO structure.
> 
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Benjamin You <benjamin.you@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Nickle Wang <nickle.wang@hpe.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Michael Kubacki (8):
>    MdeModulePkg: Add Variable Flash Info HOB
>    MdeModulePkg/VariableFlashInfoLib: Add initial library
>    MdeModulePkg/Variable: Consume Variable Flash Info
>    MdeModulePkg/FaultTolerantWrite: Consume Variable Flash Info
>    ArmVirtPkg/ArmVirt.dsc.inc: Add VariableFlashInfoLib
>    EmulatorPkg: Add VariableFlashInfoLib
>    OvmfPkg: Add VariableFlashInfoLib
>    UefiPayloadPkg: Add VariableFlashInfoLib
> 
>   MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c        | 178 ++++++++++++++++++++
>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c                          |  41 +++--
>   MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c               |   7 +-
>   MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.c            |  28 +--
>   MdeModulePkg/Universal/Variable/Pei/Variable.c                                  |  14 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c                        |  16 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c                |  14 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c                        |  17 +-
>   ArmVirtPkg/ArmVirt.dsc.inc                                                      |   1 +
>   EmulatorPkg/EmulatorPkg.dsc                                                     |   1 +
>   MdeModulePkg/Include/Guid/VariableFlashInfo.h                                   | 111 ++++++++++++
>   MdeModulePkg/Include/Library/VariableFlashInfoLib.h                             |  68 ++++++++
>   MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf      |  48 ++++++
>   MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni      |  12 ++
>   MdeModulePkg/MdeModulePkg.dec                                                   |   8 +
>   MdeModulePkg/MdeModulePkg.dsc                                                   |   2 +
>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h               |   7 +-
>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf          |  10 +-
>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf          |  10 +-
>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf |  10 +-
>   MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf          |  10 +-
>   MdeModulePkg/Universal/Variable/Pei/Variable.h                                  |   2 +
>   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                             |   5 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                           |   7 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf               |   5 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                      |   5 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf             |   5 +-
>   OvmfPkg/AmdSev/AmdSevX64.dsc                                                    |   1 +
>   OvmfPkg/Bhyve/BhyveX64.dsc                                                      |   1 +
>   OvmfPkg/CloudHv/CloudHvX64.dsc                                                  |   1 +
>   OvmfPkg/IntelTdx/IntelTdxX64.dsc                                                |   1 +
>   OvmfPkg/Microvm/MicrovmX64.dsc                                                  |   1 +
>   OvmfPkg/OvmfPkgIa32.dsc                                                         |   1 +
>   OvmfPkg/OvmfPkgIa32X64.dsc                                                      |   1 +
>   OvmfPkg/OvmfPkgX64.dsc                                                          |   1 +
>   OvmfPkg/OvmfXen.dsc                                                             |   1 +
>   UefiPayloadPkg/UefiPayloadPkg.dsc                                               |   1 +
>   37 files changed, 558 insertions(+), 94 deletions(-)
>   create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.c
>   create mode 100644 MdeModulePkg/Include/Guid/VariableFlashInfo.h
>   create mode 100644 MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>   create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
>   create mode 100644 MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.uni
>