Re: [edk2-platforms:PATCH v2] MinPlatformPkg: Fix SetLargeVariable fail issue

["Chiu, Chasel" <chasel.chiu@intel.com>]

Patch merged:
https://github.com/tianocore/edk2-platforms/commit/b71f2bda9e4fc183068eef5d1d90a631181a2506

Thanks,
Chasel


> -----Original Message-----
> From: Shindo, Miki <miki.shindo@intel.com>
> Sent: Tuesday, May 23, 2023 5:44 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> Dong, Eric <eric.dong@intel.com>; Zhang, Xiaoqiang
> <xiaoqiang.zhang@intel.com>
> Subject: [edk2-platforms:PATCH v2] MinPlatformPkg: Fix SetLargeVariable fail
> issue
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4454
> 
> On Server platform, when the large variable "FspNvsBuffer" is already in the
> UEFI variable store and the remaining variable storage space is less than the
> large variable size, and also not in OS runtime, then we need to add the size of
> the current data that will end up being replaced by the new data to the
> remaining space before we decide that there is not enough space to store the
> large variable.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Co-authored-by: Xiaoqiang Zhang <xiaoqiang.zhang@intel.com>
> Signed-off-by: Miki Shindo <miki.shindo@intel.com>
> ---
> 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWri
> teLib.c                   | 10 +++++++++-
> 
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunti
> meVariableWriteLib.c        | 15 +++++++++++++++
> 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWrit
> eCommon.c                   | 16 ++++++++++++++++
> 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVa
> riableWriteLibConstructor.c  | 30 ++++++++++++++++++++++++++++++
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVa
> riableWriteLibConstructor.c | 30 ++++++++++++++++++++++++++++++
>  Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
> | 12 ++++++++++++
> 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariabl
> eWriteLib.inf             |  1 +
> 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVa
> riableWriteLib.inf           |  3 ++-
> 
> Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVa
> riableWriteLib.inf          |  3 ++-
>  9 files changed, 117 insertions(+), 3 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariable
> WriteLib.c
> b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariable
> WriteLib.c
> index de23ae6160..4bf9a6994f 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariable
> WriteLib.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVa
> +++ riableWriteLib.c
> @@ -22,7 +22,7 @@
>  #include <Library/PrintLib.h> #include <Library/VariableReadLib.h> #include
> <Library/VariableWriteLib.h>-+#include <Library/LargeVariableReadLib.h>
> #include "LargeVariableCommon.h"  /**@@ -270,6 +270,7 @@
> SetLargeVariable (
>    UINT8         *OffsetPtr;   UINTN         BytesRemaining;   UINTN
> SizeToSave;+  UINTN         BufferSize = 0;    //   // Check input parameters.@@ -
> 365,6 +366,13 @@ SetLargeVariable (
>      // Non-Volatile storage to store the data.     //     RemainingVariableStorage =
> GetRemainingVariableStorageSpace ();+    //+    // Check if current variable
> already existed in NV storage variable space+    //+    Status = GetLargeVariable
> (VariableName, VendorGuid, &BufferSize, NULL);+    if ((Status ==
> EFI_BUFFER_TOO_SMALL) && (BufferSize != 0) && !VarLibAtOsRuntime ()) {+
> RemainingVariableStorage = RemainingVariableStorage + BufferSize;+    }     if
> (DataSize > RemainingVariableStorage) {       DEBUG ((DEBUG_ERROR,
> "SetLargeVariable: Not enough NV storage space to store the data\n"));
> Status = EFI_OUT_OF_RESOURCES;diff --git
> a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.c
> b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.c
> index 28730f858b..9ca4734f24 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/D
> +++ xeRuntimeVariableWriteLib.c
> @@ -195,6 +195,21 @@ VarLibVariableRequestToLock (
>    return Status; } +/**+  Indicator of whether it is runtime or not.++  @retval
> TRUE        It is Runtime.+  @retval FALSE       It is not
> Runtime.+**/+BOOLEAN+EFIAPI+VarLibAtOsRuntime (+  VOID+  )+{+  return
> (mVariableWriteLibVariablePolicy == NULL) ? TRUE : FALSE;+}+ /**   Close
> events when driver unloaded. diff --git
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableW
> riteCommon.c
> b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableW
> riteCommon.c
> index 50ebb544b8..cd7118d1fb 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableW
> riteCommon.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVaria
> +++ bleWriteCommon.c
> @@ -18,6 +18,7 @@
>  #include <Protocol/SmmVariable.h>  EFI_SMM_VARIABLE_PROTOCOL
> *mVariableWriteLibSmmVariable = NULL;+BOOLEAN                    mEfiAtRuntime =
> FALSE;  /**   Sets the value of a variable.@@ -169,3 +170,18 @@
> VarLibVariableRequestToLock (
>    //   return EFI_UNSUPPORTED; }++/**+  Indicator of whether it is runtime or
> not.++  @retval TRUE        It is Runtime.+  @retval FALSE       It is not
> Runtime.+**/+BOOLEAN+EFIAPI+VarLibAtOsRuntime (+  VOID+  )+{+  return
> mEfiAtRuntime;+}diff --git
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMm
> VariableWriteLibConstructor.c
> b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMm
> VariableWriteLibConstructor.c
> index d39418abd2..8c2b7d18f5 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMm
> VariableWriteLibConstructor.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/Standalo
> +++ neMmVariableWriteLibConstructor.c
> @@ -20,6 +20,28 @@
>  #include <Library/MmServicesTableLib.h>  extern
> EFI_SMM_VARIABLE_PROTOCOL  *mVariableWriteLibSmmVariable;+extern
> BOOLEAN                    mEfiAtRuntime;++/**+  Callback for ExitBootService, which
> is registered at the constructor.+  This callback sets a global variable
> mEfiAtRuntime to indicate whether+  it is after ExitBootService.++  @param[in]
> Protocol        Protocol unique ID.+  @param[in] Interface       Interface instance.+
> @param[in] Handle          The handle on which the interface is
> installed.+**/+EFI_STATUS+EFIAPI+VarLibExitBootServicesCallback (+  IN
> CONST EFI_GUID   *Protocol,+  IN      VOID             *Interface,+  IN
> EFI_HANDLE        Handle+  )+{+  mEfiAtRuntime = TRUE;+  return EFI_SUCCESS;+}
> /**   The constructor function acquires the EFI SMM Variable Services@@ -
> 41,11 +63,19 @@ StandaloneMmVariableWriteLibConstructor (
>    ) {   EFI_STATUS    Status;+  VOID          *Registration = NULL;    //   // Locate
> SmmVariableProtocol.   //   Status = gMmst->MmLocateProtocol
> (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)
> &mVariableWriteLibSmmVariable);   ASSERT_EFI_ERROR (Status);++  //+  //
> Register VarLibExitBootServicesCallback for
> gEdkiiSmmExitBootServicesProtocolGuid.+  //+  Status =
> SmmRegisterProtocolNotify (&gEdkiiSmmExitBootServicesProtocolGuid,
> VarLibExitBootServicesCallback, &Registration);+  ASSERT_EFI_ERROR (Status);+
> return Status; }diff --git
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMm
> VariableWriteLibConstructor.c
> b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMm
> VariableWriteLibConstructor.c
> index d142527e17..abc1e25cde 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMm
> VariableWriteLibConstructor.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/Traditio
> +++ nalMmVariableWriteLibConstructor.c
> @@ -20,6 +20,28 @@
>  #include <Library/SmmServicesTableLib.h>  extern
> EFI_SMM_VARIABLE_PROTOCOL  *mVariableWriteLibSmmVariable;+extern
> BOOLEAN                    mEfiAtRuntime;++/**+  Callback for ExitBootService, which
> is registered at the constructor.+  This callback sets a global variable
> mEfiAtRuntime to indicate whether+  it is after ExitBootService.++  @param[in]
> Protocol        Protocol unique ID.+  @param[in] Interface       Interface instance.+
> @param[in] Handle          The handle on which the interface is
> installed.+**/+EFI_STATUS+EFIAPI+VarLibExitBootServicesCallback (+  IN
> CONST EFI_GUID   *Protocol,+  IN      VOID             *Interface,+  IN
> EFI_HANDLE        Handle+  )+{+  mEfiAtRuntime = TRUE;+  return EFI_SUCCESS;+}
> /**   The constructor function acquires the EFI SMM Variable Services@@ -
> 41,11 +63,19 @@ TraditionalMmVariableWriteLibConstructor (
>    ) {   EFI_STATUS    Status;+  VOID          *Registration = NULL;    //   // Locate
> SmmVariableProtocol.   //   Status = gSmst->SmmLocateProtocol
> (&gEfiSmmVariableProtocolGuid, NULL, (VOID **)
> &mVariableWriteLibSmmVariable);   ASSERT_EFI_ERROR (Status);++  //+  //
> Register VarLibExitBootServicesCallback for
> gEdkiiSmmExitBootServicesProtocolGuid.+  //+  Status =
> SmmRegisterProtocolNotify (&gEdkiiSmmExitBootServicesProtocolGuid,
> VarLibExitBootServicesCallback, &Registration);+  ASSERT_EFI_ERROR (Status);+
> return Status; }diff --git
> a/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
> b/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
> index fab87f2e48..bc0b52d782 100644
> --- a/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
> +++ b/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
> @@ -135,4 +135,16 @@ VarLibVariableRequestToLock (
>    IN  EFI_GUID                     *VendorGuid   ); +/**+  Indicator of whether it is
> runtime or not.++  @retval TRUE        It is Runtime.+  @retval FALSE       It is not
> Runtime.+**/+BOOLEAN+EFIAPI+VarLibAtOsRuntime (+  VOID+  );+ #endif  //
> _VARIABLE_WRITE_LIB_H_diff --git
> a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVaria
> bleWriteLib.inf
> b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVaria
> bleWriteLib.inf
> index 2493a94596..cbc2a5d93a 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVaria
> bleWriteLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLar
> +++ geVariableWriteLib.inf
> @@ -49,3 +49,4 @@
>    PrintLib   VariableReadLib   VariableWriteLib+  LargeVariableReadLibdiff --git
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMm
> VariableWriteLib.inf
> b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMm
> VariableWriteLib.inf
> index 0d1c63a297..868be49630 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMm
> VariableWriteLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/Standalo
> +++ neMmVariableWriteLib.inf
> @@ -39,7 +39,8 @@
>    MmServicesTableLib  [Protocols]-  gEfiSmmVariableProtocolGuid   ##
> CONSUMES+  gEfiSmmVariableProtocolGuid             ## CONSUMES+
> gEdkiiSmmExitBootServicesProtocolGuid   ## CONSUMES  [Depex]
> gEfiSmmVariableProtocolGuiddiff --git
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMm
> VariableWriteLib.inf
> b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMm
> VariableWriteLib.inf
> index 5d833b7e0f..4aaab069ab 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMm
> VariableWriteLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/Traditio
> +++ nalMmVariableWriteLib.inf
> @@ -38,7 +38,8 @@
>    SmmServicesTableLib  [Protocols]-  gEfiSmmVariableProtocolGuid   ##
> CONSUMES+  gEfiSmmVariableProtocolGuid             ## CONSUMES+
> gEdkiiSmmExitBootServicesProtocolGuid   ## CONSUMES  [Depex]
> gEfiSmmVariableProtocolGuid--
> 2.39.1.windows.1