Re: 回复: [PATCH v3 0/8] Add Variable Flash Info HOB

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

Hi Liming,

To help move reviews along, I went ahead and sent a v4 patch series with 
the following information:

1. More details about when the HOB is required.
2. More details about when the HOB should be installed in the boot flow.
3. A 4 byte reserved field after the "Version" field.

The details are in MdeModulePkg/Include/Guid/VariableFlashInfo.h

V4: https://edk2.groups.io/g/devel/message/88787
CI PR: https://github.com/tianocore/edk2/pull/2768

Regards,
Michael

On 4/11/2022 9:25 PM, gaoliming wrote:
> Michael:
>    Can you give the suggestion about how to install this variable flash info
> hob when it is required? Because VariablePei is early PEIM, how to make sure
> the variable flash info hob is installed before GetVariable is called?
> 
>    On VARIABLE_FLASH_INFO structure definition, the first field is UINT32,
> other fields are all UINT64. How about reserve 4 bytes after the first
> field? If so, all fields are at the nature alignment address.
> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
>> 发送时间: 2022年4月12日 1:42
>> 收件人: devel@edk2.groups.io
>> 抄送: Abner Chang <abner.chang@hpe.com>; Andrew Fish
>> <afish@apple.com>; Anthony Perard <anthony.perard@citrix.com>; Ard
>> Biesheuvel <ardb+tianocore@kernel.org>; Benjamin You
>> <benjamin.you@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Erdem
>> Aktas <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>;
>> Guo Dong <guo.dong@intel.com>; Hao A Wu <hao.a.wu@intel.com>; James
>> Bottomley <jejb@linux.ibm.com>; Jian J Wang <jian.j.wang@intel.com>;
>> Jiewen Yao <jiewen.yao@intel.com>; Jordan Justen
>> <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>; Leif Lindholm
>> <quic_llindhol@quicinc.com>; Liming Gao <gaoliming@byosoft.com.cn>;
>> Maurice Ma <maurice.ma@intel.com>; Min Xu <min.m.xu@intel.com>;
>> Nickle Wang <nickle.wang@hpe.com>; Peter Grehan <grehan@freebsd.org>;
>> Ray Ni <ray.ni@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Sami
>> Mujawar <sami.mujawar@arm.com>; Sean Rhodes <sean@starlabs.systems>;
>> Sebastien Boeuf <sebastien.boeuf@intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>
>> 主题: [PATCH v3 0/8] Add Variable Flash Info HOB
>>
>> 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.
>>
>> 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
>> |  78 +++++++++
>>   MdeModulePkg/Include/Library/VariableFlashInfoLib.h
>> |  68 ++++++++
>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.i
>> nf      |  48 ++++++
>>
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>> ni      |  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/FaultTolerantWriteStandal
>> oneMm.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, 525 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.i
>> nf
>>   create mode 100644
>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.u
>> ni
>>
>> --
>> 2.28.0.windows.1
> 
>