Re: [edk2-platforms: PATCH v3 0/9] MinPlatformPkg: Support FSP 2.3 FSP_NON_VOLATILE_STORAGE_HOB2.


Oram, Isaac W
 

The "VariableReadLib.*BaseVariableReadLibNull.inf" belongs in edk2-platforms\Platform\Intel\MinPlatformPkg\Include\Dsc\CorePeiLib.dsc. The purpose of the common include is so we can make the same change in one place, not every board port DSC file. Board ports should be exceptions to the basic single default answer.
Note it might be nice to split SEC content into separate files. But for now, CorePeiLib.dsc is where this default library class instances for SEC belongs.

Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiFspMiscUpdUpdateLib.c, lines 47 and 48.
There is a strange whitespace use here. Better to vertically align initialization and add a newline to separate init and use.

Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c and PeiLib.h seem to have some trailing whitespace issues. With existing code, not added code, but still, nice to clean up while you are here.
I also found the function description a bit confusing. I would suggest:
/**
This function returns a "large variable". A large variable is stored across multiple
UEFI Variables. This function retrieves the multiple UEFI Variables using
ReadOnlyVariable2 PPI GetVariable().
The function uses AllocatePages () to allocate the buffer.
The buffer is not expected to be freed, as PEI does not support a FreePages ()
functionality.

If Name is NULL, then ASSERT ().
If Guid is NULL, then ASSERT ().
If Value is NULL, then ASSERT ().

@param[in] Name The Unicode UEFI Variable name used to store the large variable.
@param[in] Guid The GUID used to store the large variable
@param[out] Value The buffer point saved the variable info.
@param[out] Size The buffer size of the variable.

@return EFI_OUT_OF_RESOURCES Allocate buffer failed.
@return EFI_SUCCESS Found the specified variable.
@return Others Errors Return errors from call to gRT->GetVariable.
**/

I am not sure I like the PeiLib design, but as that is existing, I think that this is a fair extension.

Regards,
Isaac

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com>
Sent: Thursday, October 7, 2021 11:43 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Luo, Heng <heng.luo@intel.com>; Jeremy Soller <jeremy@system76.com>; Benjamin Doron <benjamin.doron00@gmail.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Kethi Reddy, Deepika <deepika.kethi.reddy@intel.com>; Esakkithevar, Kathappan <kathappan.esakkithevar@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [edk2-platforms: PATCH v3 0/9] MinPlatformPkg: Support FSP 2.3 FSP_NON_VOLATILE_STORAGE_HOB2.

V3:
Fix another GCC build failure.

V2:
Fix GCC build failures.

V1:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3678

Implementation should search FSP_NON_VOLATILE_STORAGE_HOB2 firstly and only search FSP_NON_VOLATILE_STORAGE_HOB when former one is not found.

Also added PeiGetLargeVariable () to support the scenarios where the variable data size is bigger than a single variable size limit.

Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Heng Luo <heng.luo@intel.com>
Cc: Jeremy Soller <jeremy@system76.com>
Cc: Benjamin Doron <benjamin.doron00@gmail.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@intel.com>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>

Chasel Chiu (9):
MinPlatformPkg: Support FSP 2.3 FSP_NON_VOLATILE_STORAGE_HOB2.
CometlakeOpenBoardPkg: Use same variable name for FspNvsHob.
KabylakeOpenBoardPkg/AspireVn7Dash572G:Use same variable name for
FspNvsHob
KabylakeOpenBoardPkg/GalagoPro3: Use same variable name for FspNvsHob.
KabylakeOpenBoardPkg/KabylakeRvp3: Use same variable name for
FspNvsHob.
TigerlakeOpenBoardPkg: Use same variable name for FspNvsHob.
WhiskeylakeOpenBoardPkg: Use same variable name for FspNvsHob.
WhitleyOpenBoardPkg: Support FSP 2.3 FSP_NON_VOLATILE_STORAGE_HOB2.
WhitleySiliconPkg: Use same variable name for FspNvsHob.

Platform/Intel/CometlakeOpenBoardPkg/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiFspMiscUpdUpdateLib.c | 63 ++++++++++++++-------------------------------------------------
Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiFspMiscUpdUpdateLib.c | 24 ++++++++++--------------
Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.c | 23 +++++++++--------------
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiFspMiscUpdUpdateLib.c | 24 ++++++++++--------------
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiFspMiscUpdUpdateLib.c | 25 ++++++++++---------------
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.c | 23 +++++++++--------------
Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c | 39 ++++++++++++++++++++++++++++-----------
Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c | 4 ++--
Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.c | 21 ++++++++++++++++++---
Platform/Intel/WhiskeylakeOpenBoardPkg/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiFspMiscUpdUpdateLib.c | 63 ++++++++++++---------------------------------------------------
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiFspMiscUpdUpdateLib.c | 63 ++++++++++++---------------------------------------------------
Platform/Intel/WhitleyOpenBoardPkg/Platform/Dxe/S3NvramSave/S3NvramSave.c | 29 +++++++++++++++++++++++------
Silicon/Intel/WhitleySiliconPkg/Library/FspWrapperPlatformLib/FspWrapperPlatformLib.c | 35 +++++++++--------------------------
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc | 3 +++
Platform/Intel/CometlakeOpenBoardPkg/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf | 5 ++---
Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf | 7 ++++---
Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf | 2 +-
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf | 5 ++---
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc | 3 +++
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf | 5 ++---
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf | 2 +-
Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf | 5 ++++-
Platform/Intel/MinPlatformPkg/Include/Library/PeiLib.h | 33 ++++++++++++++++++++++++++++++++-
Platform/Intel/MinPlatformPkg/Library/PeiLib/PeiLib.inf | 4 +++-
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec | 1 +
Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf | 1 +
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.dsc | 3 +++
Platform/Intel/WhiskeylakeOpenBoardPkg/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf | 5 ++---
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf | 4 ++--
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Include/Fdf/FlashMapInclude.fdf | 18 +++++++++---------
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc | 3 +++
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc | 3 +++
Platform/Intel/WhitleyOpenBoardPkg/Platform/Dxe/S3NvramSave/S3NvramSave.inf | 4 +++-
Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc | 1 +
Silicon/Intel/WhitleySiliconPkg/Library/FspWrapperPlatformLib/FspWrapperPlatformLib.inf | 3 ++-
36 files changed, 321 insertions(+), 304 deletions(-)

--
2.28.0.windows.1

Join devel@edk2.groups.io to automatically receive all group messages.