[edk2-devel] [edk2-platforms:PATCH V1] MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression.
Chiu, Chasel
chasel.chiu at intel.com
Mon May 8 19:36:55 UTC 2023
Hi Isaac,
Just my thoughts, I would vote for platform/bootloader to decide compressing the variable data before saving to NVRAM or not.
It should be optional and when platform having big SPI flash they might not want to do this.
Thanks,
Chasel
> -----Original Message-----
> From: Oram, Isaac W <isaac.w.oram at intel.com>
> Sent: Monday, May 8, 2023 12:15 PM
> To: Gudla, Raghava <raghava.gudla at intel.com>; devel at edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Kubacki, Michael
> <michael.kubacki at microsoft.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty at intel.com>
> Subject: RE: [edk2-platforms:PATCH V1] MinPlatformPkg/SaveMemoryConfig:
> Support NVS Data compression.
>
> The proposed implementation is fine and I will reviewed-by and push if that is
> the desired direction.
>
> My question is if we generally like the design of doing compression in common
> MinPlatform code, decompression in board specific FSP wrapper code. The
> alternative design is to do compression and decompression inside the FSP. This
> seems like a slightly simpler separation of responsibilities.
> The board code is responsible for save/restore, the FSP code is responsible for
> data blob creation and use. Data integrity, authentication, compression, etc all
> can be done based on more detailed knowledge of the silicon implementation
> requirements.
>
> I can see another argument though that doing it inside FSP effectively forces
> both bootloader and FSP to carry compression/decompression. The
> compression/decompression aren't likely to need to be silicon specific. And
> bootloader may have more requirements to balance than just the silicon
> requirements.
>
> Can I get some votes on preferred answer for compressing/decompressing FSP
> non-volatile data in bootloader or FSP?
>
> Thanks,
> Isaac
>
> -----Original Message-----
> From: Gudla, Raghava <raghava.gudla at intel.com>
> Sent: Friday, May 5, 2023 5:03 PM
> To: devel at edk2.groups.io
> Cc: Gudla, Raghava <raghava.gudla at intel.com>; Chiu, Chasel
> <chasel.chiu at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Oram, Isaac W <isaac.w.oram at intel.com>
> Subject: [edk2-platforms:PATCH V1] MinPlatformPkg/SaveMemoryConfig:
> Support NVS Data compression.
>
> Around 50KB "FspNonVolatileStorageHob" data can be compressed to
> approximately 3 KB which can save NVRAM space and enhance life of the SPI
> part by decreasing the number of reclaim cycles needed.
>
> This patch added support to compress "FspNonVolatileStorageHob" data before
> saving to NVRAM.
>
> A PcdEnableCompressFspNvsHob is introduced to enable/disable this feature per
> platform usage.
>
> Cc: Chasel Chiu <chasel.chiu at intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> Cc: Isaac Oram <isaac.w.oram at intel.com>
>
> Signed-off-by: Raghava Gudla <raghava.gudla at intel.com>
> ---
> .../SaveMemoryConfig/SaveMemoryConfig.c | 34 +++++++++++++++++++
> .../SaveMemoryConfig/SaveMemoryConfig.inf | 6 +++-
> .../Include/Dsc/CoreCommonLib.dsc | 1 +
> .../Intel/MinPlatformPkg/MinPlatformPkg.dec | 5 +++
> 4 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.c
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.c
> index 0215e8eed..8aa935b54 100644
> ---
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.c
> +++
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo
> +++ ryConfig.c
> @@ -21,6 +21,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/LargeVariableWriteLib.h> #include
> <Library/VariableWriteLib.h> #include
> <Guid/FspNonVolatileStorageHob2.h>+#include <Library/PcdLib.h>+#include
> <Library/CompressLib.h> /** This is the standard EFI driver point that detects
> whether there is a@@ -45,6 +47,9 @@ SaveMemoryConfigEntryPoint (
> UINTN DataSize; UINTN BufferSize; BOOLEAN
> DataIsIdentical;+ VOID *CompressedData;+ UINT64
> CompressedSize;+ UINTN CompressedAllocationPages; DataSize = 0;
> BufferSize = 0;@@ -73,6 +78,35 @@ SaveMemoryConfigEntryPoint (
> } } + if (PcdGetBool (PcdEnableCompressFspNvsHob) == 1) {+
> CompressedData = NULL;+ CompressedSize = 0;+
> CompressedAllocationPages = 0;++ DEBUG ((DEBUG_INFO, "compressing mem
> config nvs variable\n"));+ 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",
> __func__));+ ASSERT_EFI_ERROR (EFI_OUT_OF_RESOURCES);+ return
> EFI_OUT_OF_RESOURCES;+ }++ CompressedSize = EFI_PAGES_TO_SIZE
> (CompressedAllocationPages);+ Status = Compress (HobData, DataSize,
> CompressedData, &CompressedSize);+ if (EFI_ERROR (Status)) {+ DEBUG
> ((DEBUG_ERROR, "[%a] - failed to compress data. Status = %r\n", __func__,
> Status));+ ASSERT_EFI_ERROR (Status);+ FreePool(CompressedData);+
> return Status;+ } else {+ 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));diff --git
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.inf
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.inf
> index 61e85a658..77920d031 100644
> ---
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> yConfig.inf
> +++
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo
> +++ ryConfig.inf
> @@ -26,6 +26,7 @@
> LargeVariableReadLib LargeVariableWriteLib BaseLib+ CompressLib
> [Packages] MdePkg/MdePkg.dec@@ -45,6 +46,9 @@
> gFspNonVolatileStorageHob2Guid ## CONSUMES
> gFspNvsBufferVariableGuid ## PRODUCES +[Pcd]+
> gMinPlatformPkgTokenSpaceGuid.PcdEnableCompressFspNvsHob+ [Depex]
> gEfiVariableArchProtocolGuid AND- gEfiVariableWriteArchProtocolGuid
> \ No newline at end of file
> + gEfiVariableWriteArchProtocolGuiddiff --git
> + a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> + b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> index 5ce21cf31..dfe7d836d 100644
> --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> @@ -147,6 +147,7 @@
>
> BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLi
> b.inf
> LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableLib/BaseLarg
> eVariableReadLib.inf
> LargeVariableWriteLib|MinPlatformPkg/Library/BaseLargeVariableLib/BaseLarg
> eVariableWriteLib.inf+
> CompressLib|MinPlatformPkg/Library/CompressLib/CompressLib.inf # #
> CryptLibdiff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index 784abb828..e21d55fb3 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -348,6 +348,11 @@
>
> gMinPlatformPkgTokenSpaceGuid.PcdFadtFlags|0x000086A5|UINT32|0x90000
> 027
> gMinPlatformPkgTokenSpaceGuid.PcdFadtMajorVersion|0x06|UINT8|0x900000
> 30
> gMinPlatformPkgTokenSpaceGuid.PcdFadtMinorVersion|0x03|UINT8|0x900000
> 31+## Controls whether the Memory Config UEFI Variable is saved as
> compressed data.+# Data compression can significantly reduce variable storage
> usage for FSP NVS buffer data.+# Platforms that choose to compress the data
> will need to decompress the variable data upon+# extraction.+
> gMinPlatformPkgTokenSpaceGuid.PcdEnableCompressFspNvsHob|FALSE|BOOL
> EAN|0x90000032 [PcdsFixedAtBuild] --
> 2.19.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104286): https://edk2.groups.io/g/devel/message/104286
Mute This Topic: https://groups.io/mt/98719541/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list