From nobody Fri May 17 04:49:27 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+86549+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+86549+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1644476350; cv=none; d=zohomail.com; s=zohoarc; b=bO/Vei+1TgQ+1ZA9eMCuOkVCIk0RmPjE4D3C1lHGzV+finKq1T7Ie37e0FfU+XtvyeKyBpItLz3v1EFdXtxHBtygVPG9q2BYWwT8bIDjQo2KeN65czROWqXyTct+uZIFT0M20T67hJJLy4bHdCmIZgEAthrnwJU9QP0i5JpoW2A= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1644476350; h=Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=WJjbnTHs+e8JUfmMrCPuNje+hYOyKq7QSQMb7Z+++4U=; b=KWGIsRqjMKlx70nY88TwULijloldFaa7F6+YGBGDGYu0R3ZV6BhkyFd2R+ZASVOmotbvuCeoMsZNWhlqyVWkRmm9iVb96tqs4l+MDNQ05j6t3jiD4feP7ahFwxOe0bTcOkIWkhoIAAIRy8ZKdB7APshnWyw4XxscVYLMgw59wfs= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+86549+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1644476350019580.2590544419189; Wed, 9 Feb 2022 22:59:10 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id BH4LYY1788612xoHjAG7ZEdZ; Wed, 09 Feb 2022 22:59:09 -0800 X-Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web09.7038.1644476348630495726 for ; Wed, 09 Feb 2022 22:59:09 -0800 X-IronPort-AV: E=McAfee;i="6200,9189,10253"; a="248258492" X-IronPort-AV: E=Sophos;i="5.88,358,1635231600"; d="scan'208";a="248258492" X-Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Feb 2022 22:59:08 -0800 X-IronPort-AV: E=Sophos;i="5.88,358,1635231600"; d="scan'208";a="541487183" X-Received: from cchiu4-mobl.gar.corp.intel.com ([10.252.189.32]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Feb 2022 22:59:06 -0800 From: "Chiu, Chasel" To: devel@edk2.groups.io Cc: Chasel Chiu , Nate DeSimone , Liming Gao , Eric Dong Subject: [edk2-devel] [edk2-platforms: PATCH v2] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked. Date: Thu, 10 Feb 2022 14:59:01 +0800 Message-Id: <20220210065901.714-1-chasel.chiu@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,chasel.chiu@intel.com X-Gm-Message-State: mKGtePpRalR2PnLK59ntMsqix1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1644476349; bh=0ufdfCeYdr/Nk6kYDCRVS4+f3IjfOEI/9jCi5dTzAN0=; h=Cc:Date:From:Reply-To:Subject:To; b=gi1/j5Vzg+LqiKhAmJ9Pj2HpM1V32PABeoTLOyB7VnnwOSky+u8ZDXlWmU7m3VKPzeu RwUPXjZo1G32T7mVqwLgDyQfkM2AKVANdhMZUQrXMzPylBh9btAaEWF92ZX22aWFst2Vp Ka+JoIc/98ogCZXs4qgCmhmKH056OMaydok= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1644476351998100004 Content-Type: text/plain; charset="utf-8" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3829 Fixed the bug that existing variable will not be locked when it is identical with hob data by creating LockLargeVariable function, also switched to VariablePolicyProtocol for locking variables. This patch also modified SaveMemoryConfig driver to be unloaded after execution because it does not produce any service protocol. To achieve this goal the DxeRuntimeVariableWriteLib should close registered ExitBootService events in its DESTRUCTOR. Cc: Nate DeSimone Cc: Liming Gao Cc: Eric Dong Signed-off-by: Chasel Chiu --- V2 : Created LockLargeVariable function to support locking multiple variabl= e case. Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig= .c | 12 +++++++++--- Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWr= iteLib.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++-- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntim= eVariableWriteLib.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++--= -------------- Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h = | 23 ++++++++++++++++++++++- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntim= eVariableWriteLib.inf | 8 +++++--- 5 files changed, 177 insertions(+), 25 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/Save= MemoryConfig.c b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/= SaveMemoryConfig.c index 820585f676..6e521bdce6 100644 --- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC= onfig.c +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC= onfig.c @@ -2,7 +2,7 @@ This is the driver that locates the MemoryConfigurationData HOB, if it exists, and saves the data to nvRAM. =20 -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
+Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent =20 **/ @@ -18,6 +18,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include #include #include +#include #include =20 /** @@ -86,6 +87,11 @@ SaveMemoryConfigEntryPoint ( Status =3D GetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVa= riableGuid, &BufferSize, VariableData); if (!EFI_ERROR (Status) && (BufferSize =3D=3D DataSize) && (0 = =3D=3D CompareMem (HobData, VariableData, DataSize))) { DataIsIdentical =3D TRUE; + // + // No need to update Variable, only lock it. + // + Status =3D LockLargeVariable (L"FspNvsBuffer", &gFspNvsBuff= erVariableGuid); + ASSERT_EFI_ERROR (Status); } FreePool (VariableData); } @@ -106,7 +112,7 @@ SaveMemoryConfigEntryPoint ( } =20 // - // This driver cannot be unloaded because DxeRuntimeVariableWriteLib con= structor will register ExitBootServices callback. + // This driver does not produce any protocol services, so always unload = it. // - return EFI_SUCCESS; + return EFI_REQUEST_UNLOAD_IMAGE; } diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/Lar= geVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVaria= bleLib/LargeVariableWriteLib.c index e4b97ef1df..5bee2d6751 100644 --- a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVaria= bleWriteLib.c +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVaria= bleWriteLib.c @@ -10,7 +10,7 @@ integer number will be added to the end of the variable name. This number will be incremented for each variable as needed to store the entire data= set. =20 - Copyright (c) 2021, Intel Corporation. All rights reserved.
+ Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent =20 **/ @@ -412,7 +412,7 @@ SetLargeVariable ( // all data is saved. // if (LockVariable) { - for (Index =3D 0; Index < VariablesSaved; Index++) { + for (Index =3D 0; Index <=3D VariablesSaved; Index++) { ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE); UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", = VariableName, Index); =20 @@ -448,3 +448,96 @@ Done: DEBUG ((DEBUG_ERROR, "SetLargeVariable: Status =3D %r\n", Status)); return Status; } + +/** + Locks the existing large variable. + + @param[in] VariableName A Null-terminated string that is the name= of the vendor's variable. + Each VariableName is unique for each Vend= orGuid. VariableName must + contain 1 or more characters. If Variable= Name is an empty string, + then EFI_INVALID_PARAMETER is returned. + @param[in] VendorGuid A unique identifier for the vendor. + @retval EFI_SUCCESS The firmware has successfully locked the = variable. + @retval EFI_INVALID_PARAMETER An invalid combination of variable name a= nd GUID was supplied + @retval EFI_UNSUPPORTED The service for locking variable is not r= eady. + @retval EFI_NOT_FOUND The targeting variable for locking is not= present. + +**/ +EFI_STATUS +EFIAPI +LockLargeVariable ( + IN CHAR16 *VariableName, + IN EFI_GUID *VendorGuid + ) +{ + CHAR16 TempVariableName[MAX_VARIABLE_NAME_SIZE]; + UINT64 VariableSize; + EFI_STATUS Status; + UINTN Index; + + // + // Check input parameters. + // + if (VariableName =3D=3D NULL || VariableName[0] =3D=3D 0 || VendorGuid = =3D=3D NULL) { + return EFI_INVALID_PARAMETER; + } + + if (!VarLibIsVariableRequestToLockSupported ()) { + return EFI_UNSUPPORTED; + } + + VariableSize =3D 0; + Index =3D 0; + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE); + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", Variab= leName, Index); + Status =3D VarLibGetVariable (TempVariableName, VendorGuid, NULL, &Varia= bleSize, NULL); + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { + // + // Lock multiple variables. + // + + // + // Lock first variable and continue to rest of the variables. + // + DEBUG ((DEBUG_INFO, "Locking %s, Guid =3D %g\n", TempVariableName, Ven= dorGuid)); + Status =3D VarLibVariableRequestToLock (TempVariableName, VendorGuid); + for (Index =3D 1; Index < MAX_VARIABLE_SPLIT; Index++) { + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE); + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", Va= riableName, Index); + + VariableSize =3D 0; + Status =3D VarLibGetVariable (TempVariableName, VendorGuid, NULL, &V= ariableSize, NULL); + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { + DEBUG ((DEBUG_INFO, "Locking %s, Guid =3D %g\n", TempVariableName,= VendorGuid)); + Status =3D VarLibVariableRequestToLock (TempVariableName, VendorGu= id); + ASSERT_EFI_ERROR (Status); + } else if (Status =3D=3D EFI_NOT_FOUND) { + // + // No more variables need to lock. + // + return EFI_SUCCESS; + } + } // End of for loop + } else if (Status =3D=3D EFI_NOT_FOUND) { + // + // Check if it is single variable scenario. + // + VariableSize =3D 0; + Status =3D VarLibGetVariable (VariableName, VendorGuid, NULL, &Variabl= eSize, NULL); + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { + // + // Lock single variable. + // + DEBUG ((DEBUG_INFO, "Locking %s, Guid =3D %g\n", VariableName, Vendo= rGuid)); + Status =3D VarLibVariableRequestToLock (VariableName, VendorGuid); + ASSERT_EFI_ERROR (Status); + return EFI_SUCCESS; + } + } + + // + // Here probably means variable not present. + // + return Status; + +} diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteL= ib/DxeRuntimeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/Dxe= RuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c index 9ed59f8827..e7d0c5ec34 100644 --- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeR= untimeVariableWriteLib.c +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeR= untimeVariableWriteLib.c @@ -10,7 +10,7 @@ Using this library allows code to be written in a generic manner that ca= n be used in DXE or SMM without modification. =20 - Copyright (c) 2021, Intel Corporation. All rights reserved.
+ Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent =20 **/ @@ -18,14 +18,16 @@ #include =20 #include -#include +#include =20 #include #include #include #include =20 -STATIC EDKII_VARIABLE_LOCK_PROTOCOL *mVariableWriteLibVariableLock =3D NU= LL; +STATIC EDKII_VARIABLE_POLICY_PROTOCOL *mVariableWriteLibVariablePolicy = =3D NULL; +EFI_EVENT mExitBootServiceEvent; +EFI_EVENT mLegacyBootEvent; =20 /** Sets the value of a variable. @@ -144,7 +146,7 @@ VarLibIsVariableRequestToLockSupported ( VOID ) { - if (mVariableWriteLibVariableLock !=3D NULL) { + if (mVariableWriteLibVariablePolicy !=3D NULL) { return TRUE; } else { return FALSE; @@ -178,16 +180,46 @@ VarLibVariableRequestToLock ( { EFI_STATUS Status =3D EFI_UNSUPPORTED; =20 - if (mVariableWriteLibVariableLock !=3D NULL) { - Status =3D mVariableWriteLibVariableLock->RequestToLock ( - mVariableWriteLibVariableLoc= k, - VariableName, - VendorGuid - ); + if (mVariableWriteLibVariablePolicy !=3D NULL) { + Status =3D RegisterBasicVariablePolicy ( + mVariableWriteLibVariablePolicy, + (CONST EFI_GUID*) VendorGuid, + (CONST CHAR16 *) VariableName, + VARIABLE_POLICY_NO_MIN_SIZE, + VARIABLE_POLICY_NO_MAX_SIZE, + VARIABLE_POLICY_NO_MUST_ATTR, + VARIABLE_POLICY_NO_CANT_ATTR, + VARIABLE_POLICY_TYPE_LOCK_NOW + ); + ASSERT_EFI_ERROR (Status); } return Status; } =20 +/** + Close events when driver unloaded. + + @param[in] ImageHandle A handle for the image that is initializing this= driver + @param[in] SystemTable A pointer to the EFI system table + + @retval EFI_SUCCESS The initialization finished successfully. +**/ +EFI_STATUS +EFIAPI +DxeRuntimeVariableWriteLibDestructor ( + IN EFI_HANDLE ImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + if (mExitBootServiceEvent !=3D 0) { + gBS->CloseEvent (mExitBootServiceEvent); + } + if (mLegacyBootEvent !=3D 0) { + gBS->CloseEvent (mLegacyBootEvent); + } + return EFI_SUCCESS; +} + /** Exit Boot Services Event notification handler. =20 @@ -202,7 +234,7 @@ DxeRuntimeVariableWriteLibOnExitBootServices ( IN VOID *Context ) { - mVariableWriteLibVariableLock =3D NULL; + mVariableWriteLibVariablePolicy =3D NULL; } =20 /** @@ -227,13 +259,11 @@ DxeRuntimeVariableWriteLibConstructor ( ) { EFI_STATUS Status; - EFI_EVENT ExitBootServiceEvent; - EFI_EVENT LegacyBootEvent; =20 // // Locate VariableLockProtocol. // - Status =3D gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (= VOID **)&mVariableWriteLibVariableLock); + Status =3D gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL,= (VOID **)&mVariableWriteLibVariablePolicy); ASSERT_EFI_ERROR (Status); =20 // @@ -245,7 +275,7 @@ DxeRuntimeVariableWriteLibConstructor ( DxeRuntimeVariableWriteLibOnExitBootServices, NULL, &gEfiEventExitBootServicesGuid, - &ExitBootServiceEvent + &mExitBootServiceEvent ); ASSERT_EFI_ERROR (Status); =20 @@ -257,7 +287,7 @@ DxeRuntimeVariableWriteLibConstructor ( TPL_NOTIFY, DxeRuntimeVariableWriteLibOnExitBootServices, NULL, - &LegacyBootEvent + &mLegacyBootEvent ); ASSERT_EFI_ERROR (Status); =20 diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWri= teLib.h b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteL= ib.h index c847d7f152..83b5e78506 100644 --- a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h +++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h @@ -16,7 +16,7 @@ is possible, adjusting the value of PcdMaxVariableSize may provide a sim= pler solution to this problem. =20 - Copyright (c) 2021, Intel Corporation. All rights reserved.
+ Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent =20 **/ @@ -66,4 +66,25 @@ SetLargeVariable ( IN VOID *Data ); =20 +/** + Locks the existing large variable. + + @param[in] VariableName A Null-terminated string that is the name= of the vendor's variable. + Each VariableName is unique for each Vend= orGuid. VariableName must + contain 1 or more characters. If Variable= Name is an empty string, + then EFI_INVALID_PARAMETER is returned. + @param[in] VendorGuid A unique identifier for the vendor. + @retval EFI_SUCCESS The firmware has successfully locked the = variable. + @retval EFI_INVALID_PARAMETER An invalid combination of variable name a= nd GUID was supplied + @retval EFI_UNSUPPORTED The service for locking variable is not r= eady. + @retval EFI_NOT_FOUND The targeting variable for locking is not= present. + +**/ +EFI_STATUS +EFIAPI +LockLargeVariable ( + IN CHAR16 *VariableName, + IN EFI_GUID *VendorGuid + ); + #endif // _LARGE_VARIABLE_WRITE_LIB_H_ diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteL= ib/DxeRuntimeVariableWriteLib.inf b/Platform/Intel/MinPlatformPkg/Library/D= xeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf index 704a8ac7cc..f83090c847 100644 --- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeR= untimeVariableWriteLib.inf +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeR= untimeVariableWriteLib.inf @@ -10,7 +10,7 @@ # Using this library allows code to be written in a generic manner that ca= n be # used in DXE or SMM without modification. # -# Copyright (c) 2021, Intel Corporation. All rights reserved.
+# Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
# # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -24,6 +24,7 @@ MODULE_TYPE =3D DXE_RUNTIME_DRIVER LIBRARY_CLASS =3D VariableWriteLib|DXE_CORE DXE_DRIVER = DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER CONSTRUCTOR =3D DxeRuntimeVariableWriteLibConstructor + DESTRUCTOR =3D DxeRuntimeVariableWriteLibDestructor =20 [Packages] MdePkg/MdePkg.dec @@ -37,13 +38,14 @@ UefiLib UefiBootServicesTableLib UefiRuntimeServicesTableLib + VariablePolicyHelperLib =20 [Guids] gEfiEventExitBootServicesGuid ## CONSUMES ## Event =20 [Protocols] gEfiVariableWriteArchProtocolGuid ## CONSUMES - gEdkiiVariableLockProtocolGuid ## CONSUMES + gEdkiiVariablePolicyProtocolGuid ## CONSUMES =20 [Depex] - gEfiVariableWriteArchProtocolGuid AND gEdkiiVariableLockProtocolGuid + gEfiVariableWriteArchProtocolGuid AND gEdkiiVariablePolicyProtocolGuid --=20 2.28.0.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86549): https://edk2.groups.io/g/devel/message/86549 Mute This Topic: https://groups.io/mt/89041158/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-