Inline comments below:
> -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Kubacki, Michael A > Sent: Saturday, September 28, 2019 9:47 AM > To: devel@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: [edk2-devel] [PATCH V2 4/9] MdeModulePkg/Variable: Add local > auth status in VariableParsing > > The file VariableParsing.c provides generic functionality related > to parsing variable related structures and information. In order to > calculate offsets for certain operations, the functions must know if > authenticated variables are enabled as this increases the size of > variable headers. > > This change removes linking against a global variable in an external file > in favor of a statically scoped variable in VariableParsing.c Because this > file is unaware of how the authenticated variable status is determined, the > variable is set through a function interface invoked during variable driver > initialization. > > Cc: Dandan Bi <dandan...@intel.com> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Liming Gao <liming....@intel.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Signed-off-by: Michael Kubacki <michael.a.kuba...@intel.com> > --- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h | 14 > +++++++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10 +++--- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c | 33 > ++++++++++++++++---- > 3 files changed, 45 insertions(+), 12 deletions(-) > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > index 6f2000f3ee..3eba590634 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > @@ -308,4 +308,18 @@ UpdateVariableInfo ( > IN OUT VARIABLE_INFO_ENTRY **VariableInfo > ); > > +/** > + Initializes context needed for variable parsing functions. > + > + @param[in] AuthFormat If true then indicates authenticated > variables are supported > + > + @retval EFI_SUCCESS Initialized successfully > + @retval Others An error occurred during > initialization > +**/ > +EFI_STATUS > +EFIAPI > +InitVariableParsing ( InitVariableParsing() seems an internal function, the 'EFIAPI' keyword can be dropped. Please help to update the function definition in .C file as well. > + IN BOOLEAN AuthFormat > + ); > + > #endif > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > index 1a57d7e1ba..53d797152c 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -3326,6 +3326,9 @@ InitNonVolatileVariableStore ( > mVariableModuleGlobal->MaxVariableSize = PcdGet32 > (PcdMaxVariableSize); > mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 > (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : > mVariableModuleGlobal->MaxVariableSize); > > + Status = InitVariableParsing (mVariableModuleGlobal- > >VariableGlobal.AuthFormat); > + ASSERT_EFI_ERROR (Status); > + After the above initialization, mVariableModuleGlobal->VariableGlobal.AuthFormat will be changed temporarily within ConvertNormalVarStorageToAuthVarStorage() if normal HOB variable store will be converted to the auth format: VOID * ConvertNormalVarStorageToAuthVarStorage ( VARIABLE_STORE_HEADER *NormalVarStorage ) { ... // // Set AuthFormat as FALSE for normal variable storage // mVariableModuleGlobal->VariableGlobal.AuthFormat = FALSE; ... // // Restore AuthFormat // mVariableModuleGlobal->VariableGlobal.AuthFormat = TRUE; return AuthVarStorage; } I think there will be issues in such converting, since I found that at least GetVariableHeaderSize() and NameSizeOfVariable() get called during the execution of ConvertNormalVarStorageToAuthVarStorage(). And they are checking 'mAuthFormat' rather than 'mVariableModuleGlobal->VariableGlobal.AuthFormat'. > // > // Parse non-volatile variable data and get last variable offset. > // > @@ -3756,18 +3759,13 @@ VariableCommonInitialize ( > > // > // mVariableModuleGlobal->VariableGlobal.AuthFormat > - // has been initialized in InitNonVolatileVariableStore(). > + // is initialized in InitNonVolatileVariableStore(). > // > if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > DEBUG ((EFI_D_INFO, "Variable driver will work with auth variable > format!\n")); > - // > - // Set AuthSupport to FALSE first, VariableWriteServiceInitialize() will > initialize it. > - // > - mVariableModuleGlobal->VariableGlobal.AuthSupport = FALSE; > VariableGuid = &gEfiAuthenticatedVariableGuid; > } else { > DEBUG ((EFI_D_INFO, "Variable driver will work without auth variable > support!\n")); > - mVariableModuleGlobal->VariableGlobal.AuthSupport = FALSE; Not sure why the above changes belong to this patch. Could you help to double confirm? Best Regards, Hao Wu > VariableGuid = &gEfiVariableGuid; > } > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > index 394195342d..0a47f6d10d 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > @@ -9,6 +9,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include "VariableParsing.h" > > +STATIC BOOLEAN mAuthFormat; > + > /** > > This code checks if variable header is valid or not. > @@ -88,7 +90,7 @@ GetVariableHeaderSize ( > { > UINTN Value; > > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > Value = sizeof (AUTHENTICATED_VARIABLE_HEADER); > } else { > Value = sizeof (VARIABLE_HEADER); > @@ -114,7 +116,7 @@ NameSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; > > AuthVariable = (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > if (AuthVariable->State == (UINT8) (-1) || > AuthVariable->DataSize == (UINT32) (-1) || > AuthVariable->NameSize == (UINT32) (-1) || > @@ -149,7 +151,7 @@ SetNameSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; > > AuthVariable = (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > AuthVariable->NameSize = (UINT32) NameSize; > } else { > Variable->NameSize = (UINT32) NameSize; > @@ -173,7 +175,7 @@ DataSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; > > AuthVariable = (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > if (AuthVariable->State == (UINT8) (-1) || > AuthVariable->DataSize == (UINT32) (-1) || > AuthVariable->NameSize == (UINT32) (-1) || > @@ -208,7 +210,7 @@ SetDataSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; > > AuthVariable = (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > AuthVariable->DataSize = (UINT32) DataSize; > } else { > Variable->DataSize = (UINT32) DataSize; > @@ -248,7 +250,7 @@ GetVendorGuidPtr ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; > > AuthVariable = (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > return &AuthVariable->VendorGuid; > } else { > return &Variable->VendorGuid; > @@ -746,3 +748,22 @@ UpdateVariableInfo ( > } > } > } > + > +/** > + Initializes context needed for variable parsing functions. > + > + @param[in] AuthFormat If true then indicates authenticated > variables are supported > + > + @retval EFI_SUCCESS Initialized successfully > + @retval Others An error occurred during > initialization > +**/ > +EFI_STATUS > +EFIAPI > +InitVariableParsing ( > + IN BOOLEAN AuthFormat > + ) > +{ > + mAuthFormat = AuthFormat; > + > + return EFI_SUCCESS; > +} > -- > 2.16.2.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48418): https://edk2.groups.io/g/devel/message/48418 Mute This Topic: https://groups.io/mt/34318587/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
