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 <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 <nathaniel.l.desim...@intel.com>; Oram, Isaac W
> <isaac.w.o...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; Dong,
> Eric <eric.d...@intel.com>; Gudla, Raghava <raghava.gu...@intel.com>
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add
> FspNvsBuffer compression option
>
>
> Reviewed-by: Chasel Chiu <chasel.c...@intel.com>
>
> Thanks,
> Chasel
>
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <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 <chasel.c...@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desim...@intel.com>; Oram, Isaac W
> > <isaac.w.o...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>;
> > Dong, Eric <eric.d...@intel.com>; Gudla, Raghava
> > <raghava.gu...@intel.com>
> > Subject: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg:
> > Add FspNvsBuffer compression option
> >
> > From: Michael Kubacki <michael.kuba...@microsoft.com>
> >
> > 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 <chasel.c...@intel.com>
> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
> > Cc: Isaac Oram <isaac.w.o...@intel.com>
> > Cc: Liming Gao <gaolim...@byosoft.com.cn>
> > Cc: Eric Dong <eric.d...@intel.com>
> > Cc: Raghava Gudla <raghava.gu...@intel.com>
> > Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com>
> > ---
> >
> > 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.<BR>
> > +Copyright (c) Microsoft Corporation.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -10,6 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include <Base.h> #include <Uefi.h> #include <Library/BaseLib.h>
> > +#include <Library/CompressLib.h>
> > #include <Library/UefiBootServicesTableLib.h>
> > #include <Library/UefiRuntimeServicesTableLib.h>
> > #include <Library/HobLib.h>
> > @@ -19,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include <Library/BaseMemoryLib.h> #include
> > <Library/LargeVariableReadLib.h> #include
> > <Library/LargeVariableWriteLib.h>
> > +#include <Library/PcdLib.h>
> > #include <Library/VariableWriteLib.h> #include
> > <Guid/FspNonVolatileStorageHob2.h>
> >
> > @@ -38,20 +41,26 @@ SaveMemoryConfigEntryPoint (
> > IN EFI_SYSTEM_TABLE *SystemTable
> > )
> > {
> > - EFI_STATUS Status;
> > - 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;
> > + BOOLEAN DataIsIdentical;
> > + 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,
> > &CompressedSize);
> > + 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/SaveMemor
> > yConfig.inf
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > yConfig.inf
> > index 61e85a658693..0f12deb131ca 100644
> > ---
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > yConfig.inf
> > +++
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo
> > +++ ryConfig.inf
> > @@ -26,6 +26,7 @@ [LibraryClasses]
> > LargeVariableReadLib
> > LargeVariableWriteLib
> > BaseLib
> > + CompressLib
> >
> > [Packages]
> > MdePkg/MdePkg.dec
> > @@ -45,6 +46,9 @@ [Guids]
> > gFspNonVolatileStorageHob2Guid ## CONSUMES
> > gFspNvsBufferVariableGuid ## PRODUCES
> >
> > +[Pcd]
> > + gMinPlatformPkgTokenSpaceGuid.PcdEnableCompressedFspNvsBuffer
> > +
> > [Depex]
> > gEfiVariableArchProtocolGuid AND
> > gEfiVariableWriteArchProtocolGuid
> > \ No newline at end of file
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> > b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> > index 5ce21cf31e17..dfe7d836d32a 100644
> > --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> > +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> > @@ -147,6 +147,7 @@ [LibraryClasses.common]
> >
> >
> BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLi
> > b.inf
> >
> > LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableLib/BaseL
> > LargeVariableReadLib|arg
> > eVariableReadLib.inf
> >
> > LargeVariableWriteLib|MinPlatformPkg/Library/BaseLargeVariableLib/Base
> > LargeVariableWriteLib|Larg
> > eVariableWriteLib.inf
> > + CompressLib|MinPlatformPkg/Library/CompressLib/CompressLib.inf
> >
> > #
> > # CryptLib
> > diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > index 784abb828e76..45c75f8ec2c9 100644
> > --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > @@ -311,6 +311,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
> > PcdsDynamic, PcdsDynamicEx]
> >
> >
> >
> gMinPlatformPkgTokenSpaceGuid.PcdPlatformMemoryCheckLevel|0|UINT32|0
> > x30000009
> >
> > + ## Controls whether the FSP NVS buffer 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.PcdEnableCompressedFspNvsBuffer|FALSE|B
> > O
> > + OLEAN|0x30000010
> > +
> > ## This PCD is to control which device is the potential trusted
> > console input device.<BR><BR>
> > # For example:<BR>
> > # USB Short Form: UsbHID(0xFFFF,0xFFFF,0x1,0x1)<BR>
> > --
> > 2.40.1.windows.1
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#104478):
> > https://edk2.groups.io/g/devel/message/104478
> > Mute This Topic: https://groups.io/mt/98799191/1777047
> > Group Owner: devel+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [chasel.c...@intel.com] -=-=-=-=-=-=
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104490): https://edk2.groups.io/g/devel/message/104490
Mute This Topic: https://groups.io/mt/98799191/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
