From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io, chasel.chiu@intel.com
Cc: "Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>,
"Oram, Isaac W" <isaac.w.oram@intel.com>,
"Gao, Liming" <gaoliming@byosoft.com.cn>,
"Dong, Eric" <eric.dong@intel.com>,
"Gudla, Raghava" <raghava.gudla@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option
Date: Wed, 10 May 2023 11:03:32 -0400 [thread overview]
Message-ID: <22565d46-1eb1-7262-59a1-8f0a6d830694@linux.microsoft.com> (raw)
In-Reply-To: <BN9PR11MB548305EC9FD2585BDB02CCC1E6779@BN9PR11MB5483.namprd11.prod.outlook.com>
Sent in v4 patch:
https://edk2.groups.io/g/devel/message/104568
Can you please add your R-b when merging?
On 5/10/2023 2:03 AM, Chiu, Chasel wrote:
>
> 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; mikuback@linux.microsoft.com
>> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Oram, Isaac W
>> <isaac.w.oram@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Dong,
>> Eric <eric.dong@intel.com>; Gudla, Raghava <raghava.gudla@intel.com>
>> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add
>> FspNvsBuffer compression option
>>
>>
>> Reviewed-by: Chasel Chiu <chasel.chiu@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.chiu@intel.com>; Desimone, Nathaniel L
>>> <nathaniel.l.desimone@intel.com>; Oram, Isaac W
>>> <isaac.w.oram@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
>>> Dong, Eric <eric.dong@intel.com>; Gudla, Raghava
>>> <raghava.gudla@intel.com>
>>> Subject: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg:
>>> Add FspNvsBuffer compression option
>>>
>>> From: Michael Kubacki <michael.kubacki@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.chiu@intel.com>
>>> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
>>> Cc: Isaac Oram <isaac.w.oram@intel.com>
>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> Cc: Raghava Gudla <raghava.gudla@intel.com>
>>> Signed-off-by: Michael Kubacki <michael.kubacki@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+owner@edk2.groups.io
>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
>>> [chasel.chiu@intel.com] -=-=-=-=-=-=
>>>
>>
>>
>>
>>
>>
>
>
>
>
>
prev parent reply other threads:[~2023-05-10 15:03 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-10 3:20 [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option Michael Kubacki
2023-05-10 3:27 ` [edk2-devel] " Chiu, Chasel
[not found] ` <175DAA7A0E18189B.14362@groups.io>
2023-05-10 6:03 ` Chiu, Chasel
2023-05-10 15:03 ` Michael Kubacki [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=22565d46-1eb1-7262-59a1-8f0a6d830694@linux.microsoft.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox