That sounds very reasonable to me. I have done something similar with basic PPI/Protocol services to abstract phase specific differences that a lot of code simply doesn't need to care about. I even like the idea of a general "phase agnostic services library" that handles these kinds of things that would suffice for a lot of code that doesn't need driver model complexities or some other phase specific complexities. That would likely be more an RFC discussion though. I think case by case is also fine and more aligned with the current library design.
Regards, Isaac From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io Sent: Wednesday, September 8, 2021 3:34 PM To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.o...@intel.com> Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Liming Gao <gaolim...@byosoft.com.cn>; Dong, Eric <eric.d...@intel.com> Subject: Re: [EXTERNAL] [edk2-devel][edk2-platforms][PATCH V1 1/1] MinPlatformPkg/Variable*Lib: Build VariableRead and VariableWrite libs Question: I've recently brought up with some that the GetVariable##() functions in UefiLib are probably incorrect as they are written (because they assume things like gRT and gBS availability). Would it perhaps make sense to move this interface up into the MdePkg scope and leverage it to start removing some of those assumptions? I already have a TODO to start splitting UefiLib along phase lines (PEI, DXE, MM, etc), but that work could largely be obviated with this abstraction. - Bret From: Oram, Isaac W via groups.io<mailto:isaac.w.oram=intel....@groups.io> Sent: Wednesday, September 8, 2021 3:29 PM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Chasel Chiu<mailto:chasel.c...@intel.com>; Desimone, Nathaniel L<mailto:nathaniel.l.desim...@intel.com>; Liming Gao<mailto:gaolim...@byosoft.com.cn>; Dong, Eric<mailto:eric.d...@intel.com> Subject: [EXTERNAL] [edk2-devel][edk2-platforms][PATCH V1 1/1] MinPlatformPkg/Variable*Lib: Build VariableRead and VariableWrite libs Add the VariableReadLib and VariableWriteLib instances to Components to ensure build when building MinPlatformPkg.dsc. Add a NULL library instance that provides the non-functional library instance for VariableReadLib designed for all phase use. Cc: Chasel Chiu <chasel.c...@intel.com<mailto:chasel.c...@intel.com>> Cc: Nate DeSimone <nathaniel.l.desim...@intel.com<mailto:nathaniel.l.desim...@intel.com>> Cc: Liming Gao <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>> Cc: Eric Dong <eric.d...@intel.com<mailto:eric.d...@intel.com>> Signed-off-by: Isaac Oram <isaac.w.o...@intel.com<mailto:isaac.w.o...@intel.com>> --- Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.c | 75 ++++++++++++++++++++ Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.inf | 37 ++++++++++ Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 6 +- 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.c b/Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.c new file mode 100644 index 0000000000..f276b7b6b4 --- /dev/null +++ b/Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.c @@ -0,0 +1,75 @@ +/** @file + NULL implementation of Variable Read Lib + + This library provides phase agnostic access to the UEFI Variable Services. + This is done by implementing a wrapper on top of the phase specific mechanism + for reading from UEFI variables. For example, the PEI implementation of this + library uses EFI_PEI_READ_ONLY_VARIABLE2_PPI. The DXE implementation accesses + the UEFI Runtime Services Table, and the SMM implementation uses + EFI_SMM_VARIABLE_PROTOCOL. + + Using this library allows code to be written in a generic manner that can be + used in PEI, DXE, or SMM without modification. + + @copyright + Copyright 2021 Intel Corporation. <BR> + + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#include <Uefi/UefiBaseType.h> + +/** + Returns the value of a variable. + + @param[in] VariableName A Null-terminated string that is the name of the vendor's + variable. + @param[in] VendorGuid A unique identifier for the vendor. + @param[out] Attributes If not NULL, a pointer to the memory location to return the + attributes bitmask for the variable. + @param[in, out] DataSize On input, the size in bytes of the return Data buffer. + On output the size of data returned in Data. + @param[out] Data The buffer to return the contents of the variable. May be NULL + with a zero DataSize in order to determine the size buffer needed. + + @retval EFI_UNSUPPORTED This function is not implemented by this instance of the LibraryClass + +**/ +EFI_STATUS +EFIAPI +VarLibGetVariable ( + IN CHAR16 *VariableName, + IN EFI_GUID *VendorGuid, + OUT UINT32 *Attributes, OPTIONAL + IN OUT UINTN *DataSize, + OUT VOID *Data OPTIONAL + ) +{ + return EFI_UNSUPPORTED; +} + +/** + Enumerates the current variable names. + + @param[in, out] VariableNameSize The size of the VariableName buffer. The size must be large + enough to fit input string supplied in VariableName buffer. + @param[in, out] VariableName On input, supplies the last VariableName that was returned + by GetNextVariableName(). On output, returns the Nullterminated + string of the current variable. + @param[in, out] VendorGuid On input, supplies the last VendorGuid that was returned by + GetNextVariableName(). On output, returns the + VendorGuid of the current variable. + + @retval EFI_UNSUPPORTED This function is not implemented by this instance of the LibraryClass + +**/ +EFI_STATUS +EFIAPI +VarLibGetNextVariableName ( + IN OUT UINTN *VariableNameSize, + IN OUT CHAR16 *VariableName, + IN OUT EFI_GUID *VendorGuid + ) +{ + return EFI_UNSUPPORTED; +} diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.inf b/Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.inf new file mode 100644 index 0000000000..3a397998a9 --- /dev/null +++ b/Platform/Intel/MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.inf @@ -0,0 +1,37 @@ +## @file +# Component description file for NULL implementation of Variable Read Lib +# +# This library provides phase agnostic access to the UEFI Variable Services. +# This is done by implementing a wrapper on top of the phase specific mechanism +# for reading from UEFI variables. For example, the PEI implementation of this +# library uses EFI_PEI_READ_ONLY_VARIABLE2_PPI. The DXE implementation accesses +# the UEFI Runtime Services Table, and the SMM implementation uses +# EFI_SMM_VARIABLE_PROTOCOL. +# +# Using this library allows code to be written in a generic manner that can be +# used in PEI, DXE, or SMM without modification. +# +# @copyright +# Copyright 2021 Intel Corporation. <BR> +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +## + +## +## NOTICE: This library is also available in MinPlatformPkg. This copy was added +## for the convience of those that are using an older MinPlatformPkg. +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = BaseVariableReadLibNull + FILE_GUID = 5C9E2489-329F-4D2A-90F1-F5CB2A88A3E6 + VERSION_STRING = 1.0 + MODULE_TYPE = BASE + LIBRARY_CLASS = VariableReadLib + +[Packages] + MdePkg/MdePkg.dec + +[Sources] + BaseVariableReadLibNull.c diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc index 07b776cecd..a09f8db3ab 100644 --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc @@ -74,7 +74,7 @@ FspWrapperApiTestLib|IntelFsp2WrapperPkg/Library/PeiFspWrapperApiTestLib/PeiFspWrapperApiTestLib.inf FspWrapperHobProcessLib|MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf PlatformSecLib|MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf - + VariableReadLib|MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.inf FspWrapperPlatformLib|MinPlatformPkg/FspWrapper/Library/PeiFspWrapperPlatformLib/PeiFspWrapperPlatformLib.inf BoardInitLib|MinPlatformPkg/PlatformInit/Library/BoardInitLibNull/BoardInitLibNull.inf @@ -214,5 +214,9 @@ MinPlatformPkg/Tcg/Tcg2PlatformPei/Tcg2PlatformPei.inf MinPlatformPkg/Tcg/Tcg2PlatformDxe/Tcg2PlatformDxe.inf + MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariableReadLibNull.inf + MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLib.inf + MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf + [BuildOptions] *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES -- 2.27.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80383): https://edk2.groups.io/g/devel/message/80383 Mute This Topic: https://groups.io/mt/85472211/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
