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

Chiu, Chasel chasel.chiu at intel.com
Wed May 24 01:07:34 UTC 2023



Reviewed-by: Chasel Chiu <chasel.chiu at intel.com<mailto:chasel.chiu at intel.com>>

Thanks,
Chasel


From: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>
Sent: Tuesday, May 23, 2023 5:59 PM
To: Shindo, Miki <miki.shindo at intel.com>; devel at edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>; Dong, Eric <eric.dong at intel.com>; Zhang, Xiaoqiang <xiaoqiang.zhang at intel.com>
Subject: Re: [edk2-platforms:PATCH v2] MinPlatformPkg: Fix SetLargeVariable fail issue

Thank you, Miki, and Xiaoqiang!

Reviewed-by: Nate DeSimone <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at intel.com>>

From: Shindo, Miki <miki.shindo at intel.com<mailto:miki.shindo at intel.com>>
Date: Tuesday, May 23, 2023 at 5:44 PM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io> <devel at edk2.groups.io<mailto:devel at edk2.groups.io>>
Cc: Chiu, Chasel <chasel.chiu at intel.com<mailto:chasel.chiu at intel.com>>, Desimone, Nathaniel L <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at intel.com>>, Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at byosoft.com.cn>>, Dong, Eric <eric.dong at intel.com<mailto:eric.dong at intel.com>>, Zhang, Xiaoqiang <xiaoqiang.zhang at intel.com<mailto:xiaoqiang.zhang at 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 at intel.com<mailto:chasel.chiu at intel.com>>
Cc: Nate DeSimone <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at intel.com>>
Cc: Liming Gao <gaoliming at byosoft.com.cn<mailto:gaoliming at byosoft.com.cn>>
Cc: Eric Dong <eric.dong at intel.com<mailto:eric.dong at intel.com>>
Co-authored-by: Xiaoqiang Zhang <xiaoqiang.zhang at intel.com<mailto:xiaoqiang.zhang at intel.com>>
Signed-off-by: Miki Shindo <miki.shindo at intel.com<mailto:miki.shindo at intel.com>>
---
 Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c                   | 10 +++++++++-
 Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c        | 15 +++++++++++++++
 Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWriteCommon.c                   | 16 ++++++++++++++++
 Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLibConstructor.c  | 30 ++++++++++++++++++++++++++++++
 Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLibConstructor.c | 30 ++++++++++++++++++++++++++++++
 Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h                                     | 12 ++++++++++++
 Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.inf             |  1 +
 Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf           |  3 ++-
 Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf          |  3 ++-
 9 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
index de23ae6160..4bf9a6994f 100644
--- a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
+++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.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/DxeRuntimeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
index 28730f858b..9ca4734f24 100644
--- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
+++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.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/SmmVariableWriteCommon.c b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWriteCommon.c
index 50ebb544b8..cd7118d1fb 100644
--- a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWriteCommon.c
+++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWriteCommon.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/StandaloneMmVariableWriteLibConstructor.c b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLibConstructor.c
index d39418abd2..8c2b7d18f5 100644
--- a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLibConstructor.c
+++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLibConstructor.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/TraditionalMmVariableWriteLibConstructor.c b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLibConstructor.c
index d142527e17..abc1e25cde 100644
--- a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLibConstructor.c
+++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLibConstructor.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/BaseLargeVariableWriteLib.inf b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.inf
index 2493a94596..cbc2a5d93a 100644
--- a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.inf
@@ -49,3 +49,4 @@
   PrintLib

   VariableReadLib

   VariableWriteLib

+  LargeVariableReadLib

diff --git a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf
index 0d1c63a297..868be49630 100644
--- a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf
@@ -39,7 +39,8 @@
   MmServicesTableLib



 [Protocols]

-  gEfiSmmVariableProtocolGuid   ## CONSUMES

+  gEfiSmmVariableProtocolGuid             ## CONSUMES

+  gEdkiiSmmExitBootServicesProtocolGuid   ## CONSUMES



 [Depex]

   gEfiSmmVariableProtocolGuid

diff --git a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
index 5d833b7e0f..4aaab069ab 100644
--- a/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
@@ -38,7 +38,8 @@
   SmmServicesTableLib



 [Protocols]

-  gEfiSmmVariableProtocolGuid   ## CONSUMES

+  gEfiSmmVariableProtocolGuid             ## CONSUMES

+  gEdkiiSmmExitBootServicesProtocolGuid   ## CONSUMES



 [Depex]

   gEfiSmmVariableProtocolGuid

--
2.39.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105211): https://edk2.groups.io/g/devel/message/105211
Mute This Topic: https://groups.io/mt/99099949/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20230524/e4959afd/attachment-0001.htm>


More information about the edk2-devel-archive mailing list