From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web11.2690.1571212401354132985 for ; Wed, 16 Oct 2019 00:53:21 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Oct 2019 00:53:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,303,1566889200"; d="scan'208";a="347347808" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga004.jf.intel.com with ESMTP; 16 Oct 2019 00:53:20 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 16 Oct 2019 00:53:20 -0700 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 16 Oct 2019 00:53:19 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.166]) by SHSMSX154.ccr.corp.intel.com ([10.239.6.54]) with mapi id 14.03.0439.000; Wed, 16 Oct 2019 15:53:17 +0800 From: "Wu, Hao A" To: "Kubacki, Michael A" , "devel@edk2.groups.io" CC: "Bi, Dandan" , Ard Biesheuvel , "Dong, Eric" , Laszlo Ersek , "Gao, Liming" , "Kinney, Michael D" , "Ni, Ray" , "Wang, Jian J" , "Yao, Jiewen" Subject: Re: [PATCH V4 02/10] MdeModulePkg/Variable: Parameterize GetNextVariableInternal () stores Thread-Topic: [PATCH V4 02/10] MdeModulePkg/Variable: Parameterize GetNextVariableInternal () stores Thread-Index: AQHVgudin79Cmyt77k+0uFVaKDkBeadc56Pg Date: Wed, 16 Oct 2019 07:53:16 +0000 Message-ID: References: <20191014233001.33024-1-michael.a.kubacki@intel.com> <20191014233001.33024-3-michael.a.kubacki@intel.com> In-Reply-To: <20191014233001.33024-3-michael.a.kubacki@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Kubacki, Michael A > Sent: Tuesday, October 15, 2019 7:30 AM > To: devel@edk2.groups.io > Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming; Ki= nney, > Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen > Subject: [PATCH V4 02/10] MdeModulePkg/Variable: Parameterize > GetNextVariableInternal () stores >=20 > 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 Best Regards, Hao Wu >=20 > Cc: Dandan Bi > Cc: Ard Biesheuvel > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Liming Gao > Cc: Michael D Kinney > Cc: Ray Ni > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Jiewen Yao > Signed-off-by: Michael Kubacki > --- > 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(-) >=20 > 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 ( > ); >=20 > /** > - This code Finds the Next available variable. > + This code finds the next available variable. >=20 > 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. >=20 > - @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. >=20 > @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. >=20 > @@ -269,6 +271,7 @@ EFIAPI > VariableServiceGetNextVariableInternal ( > IN CHAR16 *VariableName, > IN EFI_GUID *VendorGuid, > + IN VARIABLE_STORE_HEADER **VariableStoreList, > OUT VARIABLE_HEADER **VariablePtr > ); >=20 > 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]; >=20 > if (VariableNameSize =3D=3D NULL || VariableName =3D=3D NULL || Vendor= Guid =3D=3D > NULL) { > return EFI_INVALID_PARAMETER; > @@ -2377,7 +2378,16 @@ VariableServiceGetNextVariableName ( >=20 > AcquireLockOnlyAtBootTime(&mVariableModuleGlobal- > >VariableGlobal.VariableServicesLock); >=20 > - Status =3D 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] =3D > (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal- > >VariableGlobal.VolatileVariableBase; > + VariableStoreHeader[VariableStoreTypeHob] =3D > (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal- > >VariableGlobal.HobVariableBase; > + VariableStoreHeader[VariableStoreTypeNv] =3D mNvVariableCache; > + > + Status =3D VariableServiceGetNextVariableInternal (VariableName, > VendorGuid, VariableStoreHeader, &VariablePtr); > if (!EFI_ERROR (Status)) { > VarNameSize =3D NameSizeOfVariable (VariablePtr); > ASSERT (VarNameSize !=3D 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] =3D > (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal- > >VariableGlobal.VolatileVariableBase; > + VariableStoreHeader[VariableStoreTypeHob] =3D > (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal- > >VariableGlobal.HobVariableBase; > + VariableStoreHeader[VariableStoreTypeNv] =3D mNvVariableCache; >=20 > Status =3D 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 ( > } >=20 > /** > - This code Finds the Next available variable. > + This code finds the next available variable. >=20 > 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. >=20 > - @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. >=20 > @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]; >=20 > - Status =3D FindVariable (VariableName, VendorGuid, &Variable, > &mVariableModuleGlobal->VariableGlobal, FALSE); > + Status =3D EFI_NOT_FOUND; > + > + if (VariableStoreList =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + // Check if the variable exists in the given variable store list > + for (StoreType =3D (VARIABLE_STORE_TYPE) 0; StoreType < > VariableStoreTypeMax; StoreType++) { > + if (VariableStoreList[StoreType] =3D=3D NULL) { > + continue; > + } > + > + Variable.StartPtr =3D GetStartPointer (VariableStoreList[StoreType])= ; > + Variable.EndPtr =3D GetEndPointer (VariableStoreList[StoreType])= ; > + Variable.Volatile =3D (BOOLEAN) (StoreType =3D=3D VariableStoreTypeV= olatile); > + > + Status =3D FindVariableEx (VariableName, VendorGuid, FALSE, &Variabl= e); > + if (!EFI_ERROR (Status)) { > + break; > + } > + } > + > if (Variable.CurrPtr =3D=3D NULL || EFI_ERROR (Status)) { > // > - // For VariableName is an empty string, FindVariable() will try to f= ind 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 err= or > (EFI_NOT_FOUND) > // as no any variable is found, still go to return the error > (EFI_NOT_FOUND). > // > if (VariableName[0] !=3D 0) { > // > - // For VariableName is not an empty string, and FindVariable() ret= urns > error as > + // For VariableName is not an empty string, and FindVariableEx() r= eturns > error as > // VariableName and VendorGuid are not a name and GUID of an exist= ing > variable, > // there is no way to get next variable, follow spec to return > EFI_INVALID_PARAMETER. > // > @@ -527,39 +550,30 @@ VariableServiceGetNextVariableInternal ( >=20 > if (VariableName[0] !=3D 0) { > // > - // If variable name is not NULL, get next variable. > + // If variable name is not empty, get next variable. > // > Variable.CurrPtr =3D GetNextVariablePtr (Variable.CurrPtr); > } >=20 > - // > - // 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] =3D > (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal- > >VariableGlobal.VolatileVariableBase; > - VariableStoreHeader[VariableStoreTypeHob] =3D > (VARIABLE_STORE_HEADER *) (UINTN) mVariableModuleGlobal- > >VariableGlobal.HobVariableBase; > - VariableStoreHeader[VariableStoreTypeNv] =3D 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 =3D (VARIABLE_STORE_TYPE) 0; Type < VariableStoreTypeMax= ; > Type++) { > - if ((VariableStoreHeader[Type] !=3D NULL) && (Variable.StartPtr = =3D=3D > GetStartPointer (VariableStoreHeader[Type]))) { > + for (StoreType =3D (VARIABLE_STORE_TYPE) 0; StoreType < > VariableStoreTypeMax; StoreType++) { > + if ((VariableStoreList[StoreType] !=3D NULL) && (Variable.StartP= tr =3D=3D > GetStartPointer (VariableStoreList[StoreType]))) { > break; > } > } > - ASSERT (Type < VariableStoreTypeMax); > + ASSERT (StoreType < VariableStoreTypeMax); > // > // Switch to next storage > // > - for (Type++; Type < VariableStoreTypeMax; Type++) { > - if (VariableStoreHeader[Type] !=3D NULL) { > + for (StoreType++; StoreType < VariableStoreTypeMax; StoreType++) { > + if (VariableStoreList[StoreType] !=3D NULL) { > break; > } > } > @@ -568,12 +582,12 @@ VariableServiceGetNextVariableInternal ( > // 1. current storage is the last one, or > // 2. no further storage > // > - if (Type =3D=3D VariableStoreTypeMax) { > + if (StoreType =3D=3D VariableStoreTypeMax) { > Status =3D EFI_NOT_FOUND; > goto Done; > } > - Variable.StartPtr =3D GetStartPointer (VariableStoreHeader[Type]); > - Variable.EndPtr =3D GetEndPointer (VariableStoreHeader[Type]); > + Variable.StartPtr =3D GetStartPointer (VariableStoreList[StoreType= ]); > + Variable.EndPtr =3D GetEndPointer (VariableStoreList[StoreType= ]); > Variable.CurrPtr =3D Variable.StartPtr; > } >=20 > @@ -605,11 +619,11 @@ VariableServiceGetNextVariableInternal ( > // > // Don't return NV variable when HOB overrides it > // > - if ((VariableStoreHeader[VariableStoreTypeHob] !=3D NULL) && > (VariableStoreHeader[VariableStoreTypeNv] !=3D NULL) && > - (Variable.StartPtr =3D=3D GetStartPointer > (VariableStoreHeader[VariableStoreTypeNv])) > + if ((VariableStoreList[VariableStoreTypeHob] !=3D NULL) && > (VariableStoreList[VariableStoreTypeNv] !=3D NULL) && > + (Variable.StartPtr =3D=3D GetStartPointer > (VariableStoreList[VariableStoreTypeNv])) > ) { > - VariableInHob.StartPtr =3D GetStartPointer > (VariableStoreHeader[VariableStoreTypeHob]); > - VariableInHob.EndPtr =3D GetEndPointer > (VariableStoreHeader[VariableStoreTypeHob]); > + VariableInHob.StartPtr =3D GetStartPointer > (VariableStoreList[VariableStoreTypeHob]); > + VariableInHob.EndPtr =3D GetEndPointer > (VariableStoreList[VariableStoreTypeHob]); > Status =3D FindVariableEx ( > GetVariableNamePtr (Variable.CurrPtr), > GetVendorGuidPtr (Variable.CurrPtr), > -- > 2.16.2.windows.1