subject:"\[edk2\-devel\] \[edk2\-platforms\]\[PATCH v3 1\/1\] MinPlatformPkg\: Add FspNvsBuffer compression option"

Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option

2023-05-10 Thread Chiu, Chasel



Just found one minor issue but I think it can be fixed during merging.

We also need to add below libraryClass in MinPlatform.dsc:
  CompressLib|MinPlatformPkg/Library/CompressLib/CompressLib.inf


Thanks,
Chasel


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chiu, Chasel
> Sent: Tuesday, May 9, 2023 8:28 PM
> To: devel@edk2.groups.io; mikub...@linux.microsoft.com
> Cc: Desimone, Nathaniel L ; Oram, Isaac W
> ; Gao, Liming ; Dong,
> Eric ; Gudla, Raghava 
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add
> FspNvsBuffer compression option
> 
> 
> Reviewed-by: Chasel Chiu 
> 
> Thanks,
> Chasel
> 
> 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Michael
> > Kubacki
> > Sent: Tuesday, May 9, 2023 8:21 PM
> > To: devel@edk2.groups.io
> > Cc: Chiu, Chasel ; Desimone, Nathaniel L
> > ; Oram, Isaac W
> > ; Gao, Liming ;
> > Dong, Eric ; Gudla, Raghava
> > 
> > Subject: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg:
> > Add FspNvsBuffer compression option
> >
> > From: Michael Kubacki 
> >
> > Adds a PCD called "PcdEnableCompressedFspNvsBuffer" that allows the
> > "FspNvsBuffer" UEFI variable data to be saved as compressed data.
> >
> > Especially due to the nature of the data saved in this variable, it 
> > compresses
> well.
> > For example, it has been found to reduce ~63KB of data to ~13KB. Boot
> > time impact has been found to be negligible.
> >
> > The default value is FALSE to keep default behavior consistent.
> >
> > Decompression can be performed on the variable data using the standard
> > UefiDecompressLib.
> >
> > Cc: Chasel Chiu 
> > Cc: Nate DeSimone 
> > Cc: Isaac Oram 
> > Cc: Liming Gao 
> > Cc: Eric Dong 
> > Cc: Raghava Gudla 
> > Signed-off-by: Michael Kubacki 
> > ---
> >
> > Notes:
> > v3:
> >   - Rebase onto 7ac91ec277db
> >   - Add CompressLib instance to CoreCommonLib.dsc
> > v2: Rebase onto 9769bf28d1fc
> >
> >
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> > onfig.c   | 62 
> >
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> > onfig.inf |  4 ++
> >  Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> > |
> > 1 +
> >  Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec   
> > |  6 ++
> >  4 files changed, 60 insertions(+), 13 deletions(-)
> >
> > diff --git
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > yConfig.c
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > yConfig.c
> > index 0215e8eeddfb..95b8cef8b32b 100644
> > ---
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > yConfig.c
> > +++
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo
> > +++ ryConfig.c
> > @@ -3,6 +3,7 @@
> >exists, and saves the data to nvRAM.
> >
> >  Copyright (c) 2017 - 2022, Intel Corporation. All rights
> > reserved.
> > +Copyright (c) Microsoft Corporation.
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -10,6 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include   #include   #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -19,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include   #include
> >  #include
> > 
> > +#include 
> >  #include   #include
> > 
> >
> > @@ -38,20 +41,26 @@ SaveMemoryConfigEntryPoint (
> >IN EFI_SYSTEM_TABLE   *SystemTable
> >)
> >  {
> > -  EFI_STATUSStatus;
> > -  EFI_HOB_GUID_TYPE *GuidHob;
> > -  VOID  *HobData;
> > -  VOID  *VariableData;
> > -  UINTN DataSize;
> > -  UINTN BufferSize;
> > -  BOOLEAN   DataIsIdentical;
> > +  EFI_STATUS Status;
> > +  EFI_HOB_GUID_TYPE  *GuidHob;
> > +  VOID   *HobData;
> > +  VOID   *VariableData;
> > +  UINTN  DataSize;
> > +  UINTN  BufferSize;
> > +  BOOLEANDataIsIdentical;
> > +  VOID   *CompressedData;
> > +  UINT64 CompressedSize;
> > +  UINTN

Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option

2023-05-09 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 

Thanks,
Chasel



> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Michael
> Kubacki
> Sent: Tuesday, May 9, 2023 8:21 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Oram, Isaac W ;
> Gao, Liming ; Dong, Eric ;
> Gudla, Raghava 
> Subject: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add
> FspNvsBuffer compression option
> 
> From: Michael Kubacki 
> 
> Adds a PCD called "PcdEnableCompressedFspNvsBuffer" that allows the
> "FspNvsBuffer" UEFI variable data to be saved as compressed data.
> 
> Especially due to the nature of the data saved in this variable, it 
> compresses well.
> For example, it has been found to reduce ~63KB of data to ~13KB. Boot time
> impact has been found to be negligible.
> 
> The default value is FALSE to keep default behavior consistent.
> 
> Decompression can be performed on the variable data using the standard
> UefiDecompressLib.
> 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Isaac Oram 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Raghava Gudla 
> Signed-off-by: Michael Kubacki 
> ---
> 
> Notes:
> v3:
>   - Rebase onto 7ac91ec277db
>   - Add CompressLib instance to CoreCommonLib.dsc
> v2: Rebase onto 9769bf28d1fc
> 
> 
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> onfig.c   | 62 
> 
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> onfig.inf |  4 ++
>  Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc  
>   |
> 1 +
>  Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec 
>   |  6 ++
>  4 files changed, 60 insertions(+), 13 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.c
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.c
> index 0215e8eeddfb..95b8cef8b32b 100644
> ---
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.c
> +++
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo
> +++ ryConfig.c
> @@ -3,6 +3,7 @@
>exists, and saves the data to nvRAM.
> 
>  Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -10,6 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include
>   #include   #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -19,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include
>   #include 
> #include 
> +#include 
>  #include 
>  #include 
> 
> @@ -38,20 +41,26 @@ SaveMemoryConfigEntryPoint (
>IN EFI_SYSTEM_TABLE   *SystemTable
>)
>  {
> -  EFI_STATUSStatus;
> -  EFI_HOB_GUID_TYPE *GuidHob;
> -  VOID  *HobData;
> -  VOID  *VariableData;
> -  UINTN DataSize;
> -  UINTN BufferSize;
> -  BOOLEAN   DataIsIdentical;
> +  EFI_STATUS Status;
> +  EFI_HOB_GUID_TYPE  *GuidHob;
> +  VOID   *HobData;
> +  VOID   *VariableData;
> +  UINTN  DataSize;
> +  UINTN  BufferSize;
> +  BOOLEANDataIsIdentical;
> +  VOID   *CompressedData;
> +  UINT64 CompressedSize;
> +  UINTN  CompressedAllocationPages;
> 
> -  DataSize= 0;
> -  BufferSize  = 0;
> -  VariableData= NULL;
> -  GuidHob = NULL;
> -  HobData = NULL;
> -  DataIsIdentical = FALSE;
> +  DataSize  = 0;
> +  BufferSize= 0;
> +  VariableData  = NULL;
> +  GuidHob   = NULL;
> +  HobData   = NULL;
> +  DataIsIdentical   = FALSE;
> +  CompressedData= NULL;
> +  CompressedSize= 0;
> +  CompressedAllocationPages = 0;
> 
>//
>// Search for the Memory Configuration GUID HOB.  If it is not present, 
> then
> @@ -73,6 +82,29 @@ SaveMemoryConfigEntryPoint (
>  }
>}
> 
> +  if (PcdGetBool (PcdEnableCompressedFspNvsBuffer)) {
> +if (DataSize > 0) {
> +  CompressedAllocationPages = EFI_SIZE_TO_PAGES (DataSize);
> +  CompressedData= AllocatePages (CompressedAllocationPages);
> +  if (CompressedData == NULL) {
> +DEBUG ((DEBUG_ERROR, "[%a] - Failed to allocate compressed data
> buffer.\n", __FUNCTION__));
> +ASSERT_EFI_ERROR (EFI_OUT_OF_RESOURCES);
> +return EFI_OUT_OF_RESO

[edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option

2023-05-09 Thread Michael Kubacki

From: Michael Kubacki 

Adds a PCD called "PcdEnableCompressedFspNvsBuffer" that allows the
"FspNvsBuffer" UEFI variable data to be saved as compressed data.

Especially due to the nature of the data saved in this variable, it
compresses well. For example, it has been found to reduce ~63KB
of data to ~13KB. Boot time impact has been found to be negligible.

The default value is FALSE to keep default behavior consistent.

Decompression can be performed on the variable data using the
standard UefiDecompressLib.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Isaac Oram 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Raghava Gudla 
Signed-off-by: Michael Kubacki 
---

Notes:
v3:
  - Rebase onto 7ac91ec277db
  - Add CompressLib instance to CoreCommonLib.dsc
v2: Rebase onto 9769bf28d1fc

 Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c   
| 62 
 Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf 
|  4 ++
 Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
|  1 +
 Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec   
|  6 ++
 4 files changed, 60 insertions(+), 13 deletions(-)

diff --git 
a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c 
b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
index 0215e8eeddfb..95b8cef8b32b 100644
--- 
a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
+++ 
b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
@@ -3,6 +3,7 @@
   exists, and saves the data to nvRAM.
 
 Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -10,6 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -19,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -38,20 +41,26 @@ SaveMemoryConfigEntryPoint (
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 {
-  EFI_STATUSStatus;
-  EFI_HOB_GUID_TYPE *GuidHob;
-  VOID  *HobData;
-  VOID  *VariableData;
-  UINTN DataSize;
-  UINTN BufferSize;
-  BOOLEAN   DataIsIdentical;
+  EFI_STATUS Status;
+  EFI_HOB_GUID_TYPE  *GuidHob;
+  VOID   *HobData;
+  VOID   *VariableData;
+  UINTN  DataSize;
+  UINTN  BufferSize;
+  BOOLEANDataIsIdentical;
+  VOID   *CompressedData;
+  UINT64 CompressedSize;
+  UINTN  CompressedAllocationPages;
 
-  DataSize= 0;
-  BufferSize  = 0;
-  VariableData= NULL;
-  GuidHob = NULL;
-  HobData = NULL;
-  DataIsIdentical = FALSE;
+  DataSize  = 0;
+  BufferSize= 0;
+  VariableData  = NULL;
+  GuidHob   = NULL;
+  HobData   = NULL;
+  DataIsIdentical   = FALSE;
+  CompressedData= NULL;
+  CompressedSize= 0;
+  CompressedAllocationPages = 0;
 
   //
   // Search for the Memory Configuration GUID HOB.  If it is not present, then
@@ -73,6 +82,29 @@ SaveMemoryConfigEntryPoint (
 }
   }
 
+  if (PcdGetBool (PcdEnableCompressedFspNvsBuffer)) {
+if (DataSize > 0) {
+  CompressedAllocationPages = EFI_SIZE_TO_PAGES (DataSize);
+  CompressedData= AllocatePages (CompressedAllocationPages);
+  if (CompressedData == NULL) {
+DEBUG ((DEBUG_ERROR, "[%a] - Failed to allocate compressed data 
buffer.\n", __FUNCTION__));
+ASSERT_EFI_ERROR (EFI_OUT_OF_RESOURCES);
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  CompressedSize = EFI_PAGES_TO_SIZE (CompressedAllocationPages);
+  Status = Compress (HobData, DataSize, CompressedData, 
);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "[%a] - failed to compress data. Status = %r\n", 
__FUNCTION__, Status));
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+}
+
+HobData  = CompressedData;
+DataSize = (UINTN)CompressedSize;
+  }
+
   if (HobData != NULL) {
 DEBUG ((DEBUG_INFO, "FspNvsHob.NvsDataLength:%d\n", DataSize));
 DEBUG ((DEBUG_INFO, "FspNvsHob.NvsDataPtr   : 0x%x\n", HobData));
@@ -136,6 +168,10 @@ SaveMemoryConfigEntryPoint (
 DEBUG((DEBUG_ERROR, "Memory S3 Data HOB was not found\n"));
   }
 
+  if (CompressedData != NULL) {
+FreePages (CompressedData, CompressedAllocationPages);
+  }
+
   //
   // This driver does not produce any protocol services, so always unload it.
   //
diff --git 
a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
 
b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
index 61e85a658693..0f12deb131ca 100644
---

Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option

Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option

[edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option

3 matches

Site Navigation

Mail list logo

Footer information