[edk2-devel] [PATCH V4 02/10] MdeModulePkg/Variable: Parameterize GetNextVariableInternal () stores
Wu, Hao A
hao.a.wu at intel.com
Wed Oct 16 07:53:16 UTC 2019
> -----Original Message-----
> From: Kubacki, Michael A
> Sent: Tuesday, October 15, 2019 7:30 AM
> To: devel at edk2.groups.io
> Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming; Kinney,
> Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen
> Subject: [PATCH V4 02/10] MdeModulePkg/Variable: Parameterize
> GetNextVariableInternal () stores
>
> The majority of logic related to GetNextVariableName () is currently
> implemented in VariableServiceGetNextVariableInternal (). The list
> of variable stores to search for the given variable name and variable
> GUID is defined in the function body. This change adds a new parameter
> so that the caller must pass in the list of variable stores to be used
> in the variable search.
Since all my previous comments have been addressed, the patch looks good to me:
Reviewed-by: Hao A Wu <hao.a.wu at intel.com>
Best Regards,
Hao Wu
>
> Cc: Dandan Bi <dandan.bi at intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Liming Gao <liming.gao at intel.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Jian J Wang <jian.j.wang at intel.com>
> Cc: Hao A Wu <hao.a.wu at intel.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki at intel.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h | 13 ++-
> -
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 12 ++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c | 6 ++
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c | 82
> ++++++++++++--------
> 4 files changed, 73 insertions(+), 40 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h
> index b0d7f76bd8..6555316f52 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h
> @@ -248,18 +248,20 @@ FindVariableEx (
> );
>
> /**
> - This code Finds the Next available variable.
> + This code finds the next available variable.
>
> Caution: This function may receive untrusted input.
> This function may be invoked in SMM mode. This function will do basic
> validation, before parse the data.
>
> - @param[in] VariableName Pointer to variable name.
> - @param[in] VendorGuid Variable Vendor Guid.
> - @param[out] VariablePtr Pointer to variable header address.
> + @param[in] VariableName Pointer to variable name.
> + @param[in] VendorGuid Variable Vendor Guid.
> + @param[in] VariableStoreList A list of variable stores that should be used
> to get the next variable.
> + The maximum number of entries is the max value of
> VARIABLE_STORE_TYPE.
> + @param[out] VariablePtr Pointer to variable header address.
>
> @retval EFI_SUCCESS The function completed successfully.
> @retval EFI_NOT_FOUND The next variable was not found.
> - @retval EFI_INVALID_PARAMETER If VariableName is not an empty string,
> while VendorGuid is NULL.
> + @retval EFI_INVALID_PARAMETER If VariableName is nt an empty string,
> while VendorGuid is NULL.
> @retval EFI_INVALID_PARAMETER The input values of VariableName and
> VendorGuid are not a name and
> GUID of an existing variable.
>
> @@ -269,6 +271,7 @@ EFIAPI
> VariableServiceGetNextVariableInternal (
> IN CHAR16 *VariableName,
> IN EFI_GUID *VendorGuid,
> + IN VARIABLE_STORE_HEADER **VariableStoreList,
> OUT VARIABLE_HEADER **VariablePtr
> );
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 76536308e6..70af86db24 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2358,6 +2358,7 @@ VariableServiceGetNextVariableName (
> UINTN MaxLen;
> UINTN VarNameSize;
> VARIABLE_HEADER *VariablePtr;
> + VARIABLE_STORE_HEADER
> *VariableStoreHeader[VariableStoreTypeMax];
>
> if (VariableNameSize == NULL || VariableName == NULL || VendorGuid ==
> NULL) {
> return EFI_INVALID_PARAMETER;
> @@ -2377,7 +2378,16 @@ VariableServiceGetNextVariableName (
>
> AcquireLockOnlyAtBootTime(&mVariableModuleGlobal-
> >VariableGlobal.VariableServicesLock);
>
> - Status = VariableServiceGetNextVariableInternal (VariableName,
> VendorGuid, &VariablePtr);
> + //
> + // 0: Volatile, 1: HOB, 2: Non-Volatile.
> + // The index and attributes mapping must be kept in this order as
> FindVariable
> + // makes use of this mapping to implement search algorithm.
> + //
> + VariableStoreHeader[VariableStoreTypeVolatile] =
> (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal-
> >VariableGlobal.VolatileVariableBase;
> + VariableStoreHeader[VariableStoreTypeHob] =
> (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal-
> >VariableGlobal.HobVariableBase;
> + VariableStoreHeader[VariableStoreTypeNv] = mNvVariableCache;
> +
> + Status = VariableServiceGetNextVariableInternal (VariableName,
> VendorGuid, VariableStoreHeader, &VariablePtr);
> if (!EFI_ERROR (Status)) {
> VarNameSize = NameSizeOfVariable (VariablePtr);
> ASSERT (VarNameSize != 0);
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c
> index dc78f68fa9..c787ddba5b 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c
> @@ -98,10 +98,16 @@ VariableExLibFindNextVariable (
> EFI_STATUS Status;
> VARIABLE_HEADER *VariablePtr;
> AUTHENTICATED_VARIABLE_HEADER *AuthVariablePtr;
> + VARIABLE_STORE_HEADER
> *VariableStoreHeader[VariableStoreTypeMax];
> +
> + VariableStoreHeader[VariableStoreTypeVolatile] =
> (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal-
> >VariableGlobal.VolatileVariableBase;
> + VariableStoreHeader[VariableStoreTypeHob] =
> (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal-
> >VariableGlobal.HobVariableBase;
> + VariableStoreHeader[VariableStoreTypeNv] = mNvVariableCache;
>
> Status = VariableServiceGetNextVariableInternal (
> VariableName,
> VendorGuid,
> + VariableStoreHeader,
> &VariablePtr
> );
> if (EFI_ERROR (Status)) {
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c
> index 5698a1a5e4..d6bb916e68 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c
> @@ -476,14 +476,16 @@ FindVariableEx (
> }
>
> /**
> - This code Finds the Next available variable.
> + This code finds the next available variable.
>
> Caution: This function may receive untrusted input.
> This function may be invoked in SMM mode. This function will do basic
> validation, before parse the data.
>
> - @param[in] VariableName Pointer to variable name.
> - @param[in] VendorGuid Variable Vendor Guid.
> - @param[out] VariablePtr Pointer to variable header address.
> + @param[in] VariableName Pointer to variable name.
> + @param[in] VendorGuid Variable Vendor Guid.
> + @param[in] VariableStoreList A list of variable stores that should be used
> to get the next variable.
> + The maximum number of entries is the max value of
> VARIABLE_STORE_TYPE.
> + @param[out] VariablePtr Pointer to variable header address.
>
> @retval EFI_SUCCESS The function completed successfully.
> @retval EFI_NOT_FOUND The next variable was not found.
> @@ -497,26 +499,47 @@ EFIAPI
> VariableServiceGetNextVariableInternal (
> IN CHAR16 *VariableName,
> IN EFI_GUID *VendorGuid,
> + IN VARIABLE_STORE_HEADER **VariableStoreList,
> OUT VARIABLE_HEADER **VariablePtr
> )
> {
> - VARIABLE_STORE_TYPE Type;
> + EFI_STATUS Status;
> + VARIABLE_STORE_TYPE StoreType;
> VARIABLE_POINTER_TRACK Variable;
> VARIABLE_POINTER_TRACK VariableInHob;
> VARIABLE_POINTER_TRACK VariablePtrTrack;
> - EFI_STATUS Status;
> - VARIABLE_STORE_HEADER *VariableStoreHeader[VariableStoreTypeMax];
>
> - Status = FindVariable (VariableName, VendorGuid, &Variable,
> &mVariableModuleGlobal->VariableGlobal, FALSE);
> + Status = EFI_NOT_FOUND;
> +
> + if (VariableStoreList == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + // Check if the variable exists in the given variable store list
> + for (StoreType = (VARIABLE_STORE_TYPE) 0; StoreType <
> VariableStoreTypeMax; StoreType++) {
> + if (VariableStoreList[StoreType] == NULL) {
> + continue;
> + }
> +
> + Variable.StartPtr = GetStartPointer (VariableStoreList[StoreType]);
> + Variable.EndPtr = GetEndPointer (VariableStoreList[StoreType]);
> + Variable.Volatile = (BOOLEAN) (StoreType == VariableStoreTypeVolatile);
> +
> + Status = FindVariableEx (VariableName, VendorGuid, FALSE, &Variable);
> + if (!EFI_ERROR (Status)) {
> + break;
> + }
> + }
> +
> if (Variable.CurrPtr == NULL || EFI_ERROR (Status)) {
> //
> - // For VariableName is an empty string, FindVariable() will try to find and
> return
> - // the first qualified variable, and if FindVariable() returns error
> (EFI_NOT_FOUND)
> + // For VariableName is an empty string, FindVariableEx() will try to find
> and return
> + // the first qualified variable, and if FindVariableEx() returns error
> (EFI_NOT_FOUND)
> // as no any variable is found, still go to return the error
> (EFI_NOT_FOUND).
> //
> if (VariableName[0] != 0) {
> //
> - // For VariableName is not an empty string, and FindVariable() returns
> error as
> + // For VariableName is not an empty string, and FindVariableEx() returns
> error as
> // VariableName and VendorGuid are not a name and GUID of an existing
> variable,
> // there is no way to get next variable, follow spec to return
> EFI_INVALID_PARAMETER.
> //
> @@ -527,39 +550,30 @@ VariableServiceGetNextVariableInternal (
>
> if (VariableName[0] != 0) {
> //
> - // If variable name is not NULL, get next variable.
> + // If variable name is not empty, get next variable.
> //
> Variable.CurrPtr = GetNextVariablePtr (Variable.CurrPtr);
> }
>
> - //
> - // 0: Volatile, 1: HOB, 2: Non-Volatile.
> - // The index and attributes mapping must be kept in this order as
> FindVariable
> - // makes use of this mapping to implement search algorithm.
> - //
> - VariableStoreHeader[VariableStoreTypeVolatile] =
> (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal-
> >VariableGlobal.VolatileVariableBase;
> - VariableStoreHeader[VariableStoreTypeHob] =
> (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal-
> >VariableGlobal.HobVariableBase;
> - VariableStoreHeader[VariableStoreTypeNv] = mNvVariableCache;
> -
> while (TRUE) {
> //
> - // Switch from Volatile to HOB, to Non-Volatile.
> + // Switch to the next variable store if needed
> //
> while (!IsValidVariableHeader (Variable.CurrPtr, Variable.EndPtr)) {
> //
> // Find current storage index
> //
> - for (Type = (VARIABLE_STORE_TYPE) 0; Type < VariableStoreTypeMax;
> Type++) {
> - if ((VariableStoreHeader[Type] != NULL) && (Variable.StartPtr ==
> GetStartPointer (VariableStoreHeader[Type]))) {
> + for (StoreType = (VARIABLE_STORE_TYPE) 0; StoreType <
> VariableStoreTypeMax; StoreType++) {
> + if ((VariableStoreList[StoreType] != NULL) && (Variable.StartPtr ==
> GetStartPointer (VariableStoreList[StoreType]))) {
> break;
> }
> }
> - ASSERT (Type < VariableStoreTypeMax);
> + ASSERT (StoreType < VariableStoreTypeMax);
> //
> // Switch to next storage
> //
> - for (Type++; Type < VariableStoreTypeMax; Type++) {
> - if (VariableStoreHeader[Type] != NULL) {
> + for (StoreType++; StoreType < VariableStoreTypeMax; StoreType++) {
> + if (VariableStoreList[StoreType] != NULL) {
> break;
> }
> }
> @@ -568,12 +582,12 @@ VariableServiceGetNextVariableInternal (
> // 1. current storage is the last one, or
> // 2. no further storage
> //
> - if (Type == VariableStoreTypeMax) {
> + if (StoreType == VariableStoreTypeMax) {
> Status = EFI_NOT_FOUND;
> goto Done;
> }
> - Variable.StartPtr = GetStartPointer (VariableStoreHeader[Type]);
> - Variable.EndPtr = GetEndPointer (VariableStoreHeader[Type]);
> + Variable.StartPtr = GetStartPointer (VariableStoreList[StoreType]);
> + Variable.EndPtr = GetEndPointer (VariableStoreList[StoreType]);
> Variable.CurrPtr = Variable.StartPtr;
> }
>
> @@ -605,11 +619,11 @@ VariableServiceGetNextVariableInternal (
> //
> // Don't return NV variable when HOB overrides it
> //
> - if ((VariableStoreHeader[VariableStoreTypeHob] != NULL) &&
> (VariableStoreHeader[VariableStoreTypeNv] != NULL) &&
> - (Variable.StartPtr == GetStartPointer
> (VariableStoreHeader[VariableStoreTypeNv]))
> + if ((VariableStoreList[VariableStoreTypeHob] != NULL) &&
> (VariableStoreList[VariableStoreTypeNv] != NULL) &&
> + (Variable.StartPtr == GetStartPointer
> (VariableStoreList[VariableStoreTypeNv]))
> ) {
> - VariableInHob.StartPtr = GetStartPointer
> (VariableStoreHeader[VariableStoreTypeHob]);
> - VariableInHob.EndPtr = GetEndPointer
> (VariableStoreHeader[VariableStoreTypeHob]);
> + VariableInHob.StartPtr = GetStartPointer
> (VariableStoreList[VariableStoreTypeHob]);
> + VariableInHob.EndPtr = GetEndPointer
> (VariableStoreList[VariableStoreTypeHob]);
> Status = FindVariableEx (
> GetVariableNamePtr (Variable.CurrPtr),
> GetVendorGuidPtr (Variable.CurrPtr),
> --
> 2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49064): https://edk2.groups.io/g/devel/message/49064
Mute This Topic: https://groups.io/mt/34540088/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list