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

["gaoliming" <gaoliming@byosoft.com.cn>]

Michael:
  Thanks for your update. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> for this patch set.

Thanks
Liming
> -----邮件原件-----
> 发件人: Michael Kubacki <mikuback@linux.microsoft.com>
> 发送时间: 2022年4月13日 0:32
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; 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>; '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>
> 主题: Re: 回复: [PATCH v3 0/8] Add Variable Flash Info HOB
> 
> 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
> >
> >