From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.19971.1683731014794345201 for ; Wed, 10 May 2023 08:03:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=o2yKrXvp; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 9B68020AECFC; Wed, 10 May 2023 08:03:33 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 9B68020AECFC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1683731014; bh=xY5yT+0tFuNFO5PzzpQXRrHHAhEsvNPJjypKgGh6Nuc=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=o2yKrXvp4wxCGykUiytu6f9OntJV+W9+rMzC9QRFjc2msI2p8slHuLqLEBu3obWNT b2omZQbgkX5YDa3XB7+RkfWO24FmWG6i+CWF0SmTGhUe7JhuYtLqWM6nBVXH61mRMv 516WGeH4Xk3R8eWisfvuOMUBocu/ggUwKmn+2k0g= Message-ID: <22565d46-1eb1-7262-59a1-8f0a6d830694@linux.microsoft.com> Date: Wed, 10 May 2023 11:03:32 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 1/1] MinPlatformPkg: Add FspNvsBuffer compression option To: devel@edk2.groups.io, chasel.chiu@intel.com Cc: "Desimone, Nathaniel L" , "Oram, Isaac W" , "Gao, Liming" , "Dong, Eric" , "Gudla, Raghava" References: <20230510032052.1225-1-mikuback@linux.microsoft.com> <175DAA7A0E18189B.14362@groups.io> From: "Michael Kubacki" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 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 ; 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_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.

>>> # For example:
>>> # USB Short Form: UsbHID(0xFFFF,0xFFFF,0x1,0x1)
>>> -- >>> 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] -=-=-=-=-=-= >>> >> >> >> >> >> > > > > >