回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable

["gaoliming" <gaoliming@byosoft.com.cn>]

Dandan:
  I will review this patch today. Please wait one day for me. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Dandan Bi
> 发送时间: 2022年1月17日 9:13
> 收件人: Huang, Long1 <long1.huang@intel.com>; devel@edk2.groups.io
> 抄送: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Chen, Lin Z <lin.z.chen@intel.com>; Li,
> Zhuangzhi <zhuangzhi.li@intel.com>
> 主题: Re: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> Support for authenticated variable
> 
>  I will push this patch today if no other comment.
> 
> 
> Thanks,
> Dandan
> 
> > -----Original Message-----
> > From: Bi, Dandan
> > Sent: Thursday, January 13, 2022 1:35 PM
> > To: Huang, Long1 <long1.huang@intel.com>; devel@edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Chen, Lin Z <lin.z.chen@intel.com>; Li,
> > Zhuangzhi <zhuangzhi.li@intel.com>
> > Subject: RE: [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for
> > authenticated variable
> >
> > The change is ok to me. Reviewed-by: Dandan Bi <dandan.bi@intel.com>
> >
> > Hi Liming,
> >
> > Could you also help review it?
> >
> >
> > Thanks,
> > Dandan
> > > -----Original Message-----
> > > From: Huang, Long1 <long1.huang@intel.com>
> > > Sent: Thursday, January 13, 2022 1:06 AM
> > > To: devel@edk2.groups.io
> > > Cc: Huang, Long1 <long1.huang@intel.com>; Feng, Bob C
> > > <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Bi,
> > > Dandan <dandan.bi@intel.com>; Chen, Lin Z <lin.z.chen@intel.com>; Li,
> > > Zhuangzhi <zhuangzhi.li@intel.com>
> > > Subject: [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for
> > > authenticated variable
> > >
> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3796
> > >
> > > Database.c:
> > > 	1. Replace PcdGetExPtr with PcdGetExPtr.
> > > 	2. Add FindAuthVariableData function to parse authenticated
> > variable
> > > type for getting a correct default value in
> > > PcdNvStoreDefaultValueBuffer.
> > >
> > > Signed-off-by: Huang Long <long1.huang@intel.com>
> > >
> > > Cc: Bob Feng <bob.c.feng@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Lin Z Chen <lin.z.chen@intel.com>
> > > Cc: Zhuangzhi Li <zhuangzhi.li@intel.com>
> > > ---
> > >  .../Universal/HiiDatabaseDxe/Database.c       | 130
> ++++++++++++++----
> > >  .../HiiDatabaseDxe/HiiDatabaseDxe.inf         |   3 +
> > >  2 files changed, 105 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> > > b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> > > index 0b09c24d52..c7a92d6aed 100644
> > > --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> > > +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> > > @@ -603,6 +603,45 @@ FindVariableData (
> > >    return NULL;
> > >
> > >  }
> > >
> > >
> > >
> > > +/**
> > >
> > > +  Find the matched authenticated variable from the input variable
> storage.
> > >
> > > +
> > >
> > > +  @param[in] VariableStorage Point to the variable storage header.
> > >
> > > +  @param[in] VarGuid         A unique identifier for the variable.
> > >
> > > +  @param[in] VarAttribute    The attributes bitmask for the variable.
> > >
> > > +  @param[in] VarName         A Null-terminated ascii string that is
> the name
> > of
> > > the variable.
> > >
> > > +
> > >
> > > +  @return Pointer to the matched variable header or NULL if not
found.
> > >
> > > +**/
> > >
> > > +AUTHENTICATED_VARIABLE_HEADER *
> > >
> > > +FindAuthVariableData (
> > >
> > > +  IN  VARIABLE_STORE_HEADER  *VariableStorage,
> > >
> > > +  IN  EFI_GUID               *VarGuid,
> > >
> > > +  IN  UINT32                 VarAttribute,
> > >
> > > +  IN  CHAR16                 *VarName
> > >
> > > +  )
> > >
> > > +{
> > >
> > > +  AUTHENTICATED_VARIABLE_HEADER  *AuthVariableHeader;
> > >
> > > +  AUTHENTICATED_VARIABLE_HEADER  *AuthVariableEnd;
> > >
> > > +
> > >
> > > +  AuthVariableEnd    = (AUTHENTICATED_VARIABLE_HEADER
> *)((UINT8
> > > *)VariableStorage + VariableStorage->Size);
> > >
> > > +  AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER
> > > *)(VariableStorage + 1);
> > >
> > > +  AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER
> > > *)HEADER_ALIGN (AuthVariableHeader);
> > >
> > > +  while (AuthVariableHeader < AuthVariableEnd) {
> > >
> > > +    if (CompareGuid (&AuthVariableHeader->VendorGuid, VarGuid) &&
> > >
> > > +        (AuthVariableHeader->Attributes == VarAttribute) &&
> > >
> > > +        (StrCmp (VarName, (CHAR16 *)(AuthVariableHeader + 1)) ==
> 0))
> > >
> > > +    {
> > >
> > > +      return AuthVariableHeader;
> > >
> > > +    }
> > >
> > > +
> > >
> > > +    AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER
> *)((UINT8
> > > *)AuthVariableHeader + sizeof (AUTHENTICATED_VARIABLE_HEADER) +
> > > AuthVariableHeader->NameSize + AuthVariableHeader->DataSize);
> > >
> > > +    AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER
> > > *)HEADER_ALIGN (AuthVariableHeader);
> > >
> > > +  }
> > >
> > > +
> > >
> > > +  return NULL;
> > >
> > > +}
> > >
> > > +
> > >
> > >  /**
> > >
> > >    Find question default value from PcdNvStoreDefaultValueBuffer
> > >
> > >
> > >
> > > @@ -626,25 +665,29 @@ FindQuestionDefaultSetting (
> > >    IN  BOOLEAN                  BitFieldQuestion
> > >
> > >    )
> > >
> > >  {
> > >
> > > -  VARIABLE_HEADER          *VariableHeader;
> > >
> > > -  VARIABLE_STORE_HEADER    *VariableStorage;
> > >
> > > -  LIST_ENTRY               *Link;
> > >
> > > -  VARSTORAGE_DEFAULT_DATA  *Entry;
> > >
> > > -  VARIABLE_STORE_HEADER    *NvStoreBuffer;
> > >
> > > -  UINT8                    *DataBuffer;
> > >
> > > -  UINT8                    *BufferEnd;
> > >
> > > -  BOOLEAN                  IsFound;
> > >
> > > -  UINTN                    Index;
> > >
> > > -  UINT32                   BufferValue;
> > >
> > > -  UINT32                   BitFieldVal;
> > >
> > > -  UINTN                    BitOffset;
> > >
> > > -  UINTN                    ByteOffset;
> > >
> > > -  UINTN                    BitWidth;
> > >
> > > -  UINTN                    StartBit;
> > >
> > > -  UINTN                    EndBit;
> > >
> > > -  PCD_DEFAULT_DATA         *DataHeader;
> > >
> > > -  PCD_DEFAULT_INFO         *DefaultInfo;
> > >
> > > -  PCD_DATA_DELTA           *DeltaData;
> > >
> > > +  VARIABLE_HEADER                   *VariableHeader;
> > >
> > > +  AUTHENTICATED_VARIABLE_HEADER     *AuthVariableHeader;
> > >
> > > +  VARIABLE_STORE_HEADER             *VariableStorage;
> > >
> > > +  LIST_ENTRY                        *Link;
> > >
> > > +  VARSTORAGE_DEFAULT_DATA           *Entry;
> > >
> > > +  VARIABLE_STORE_HEADER             *NvStoreBuffer;
> > >
> > > +  VOID                              *ValueSource;
> > >
> > > +  VOID                              *BitValueSource;
> > >
> > > +  UINT8                             *DataBuffer;
> > >
> > > +  UINT8                             *BufferEnd;
> > >
> > > +  BOOLEAN                           AuthFormat;
> > >
> > > +  BOOLEAN                           IsFound;
> > >
> > > +  UINTN                             Index;
> > >
> > > +  UINT32                            BufferValue;
> > >
> > > +  UINT32                            BitFieldVal;
> > >
> > > +  UINTN                             BitOffset;
> > >
> > > +  UINTN                             ByteOffset;
> > >
> > > +  UINTN                             BitWidth;
> > >
> > > +  UINTN                             StartBit;
> > >
> > > +  UINTN                             EndBit;
> > >
> > > +  PCD_DEFAULT_DATA                  *DataHeader;
> > >
> > > +  PCD_DEFAULT_INFO                  *DefaultInfo;
> > >
> > > +  PCD_DATA_DELTA                    *DeltaData;
> > >
> > >
> > >
> > >    if (gSkuId == 0xFFFFFFFFFFFFFFFF) {
> > >
> > >      gSkuId = LibPcdGetSku ();
> > >
> > > @@ -666,7 +709,7 @@ FindQuestionDefaultSetting (
> > >    }
> > >
> > >
> > >
> > >    if (Link == &gVarStorageList) {
> > >
> > > -    DataBuffer          = (UINT8 *)PcdGetPtr
> (PcdNvStoreDefaultValueBuffer);
> > >
> > > +    DataBuffer          = (UINT8 *)PcdGetExPtr
> > > (&gEfiMdeModulePkgTokenSpaceGuid, PcdNvStoreDefaultValueBuffer);
> > >
> > >      gNvDefaultStoreSize =
> ((PCD_NV_STORE_DEFAULT_BUFFER_HEADER
> > > *)DataBuffer)->Length;
> > >
> > >      //
> > >
> > >      // The first section data includes NV storage default setting.
> > >
> > > @@ -750,12 +793,27 @@ FindQuestionDefaultSetting (
> > >      return EFI_NOT_FOUND;
> > >
> > >    }
> > >
> > >
> > >
> > > +  //
> > >
> > > +  // Judge if the variable type is authenticated, default is false
> > >
> > > +  //
> > >
> > > +  AuthFormat = FALSE;
> > >
> > > +  if (CompareGuid (&VariableStorage->Signature,
> > > &gEfiAuthenticatedVariableGuid)) {
> > >
> > > +    AuthFormat = TRUE;
> > >
> > > +  }
> > >
> > > +
> > >
> > >    //
> > >
> > >    // Find the question default value from the variable storage
> > >
> > >    //
> > >
> > > -  VariableHeader = FindVariableData (VariableStorage,
> > > &EfiVarStore->Guid,
> > > EfiVarStore->Attributes, (CHAR16 *)EfiVarStore->Name);
> > >
> > > -  if (VariableHeader == NULL) {
> > >
> > > -    return EFI_NOT_FOUND;
> > >
> > > +  if(AuthFormat) {
> > >
> > > +    AuthVariableHeader = FindAuthVariableData (VariableStorage,
> > > &EfiVarStore->Guid, EfiVarStore->Attributes, (CHAR16 *)EfiVarStore-
> > > >Name);
> > >
> > > +    if (AuthVariableHeader == NULL) {
> > >
> > > +      return EFI_NOT_FOUND;
> > >
> > > +    }
> > >
> > > +  } else {
> > >
> > > +    VariableHeader = FindVariableData (VariableStorage,
> > > + &EfiVarStore->Guid,
> > > EfiVarStore->Attributes, (CHAR16 *)EfiVarStore->Name);
> > >
> > > +    if (VariableHeader == NULL) {
> > >
> > > +      return EFI_NOT_FOUND;
> > >
> > > +    }
> > >
> > >    }
> > >
> > >
> > >
> > >    StartBit   = 0;
> > >
> > > @@ -770,8 +828,24 @@ FindQuestionDefaultSetting (
> > >      Width      = EndBit / 8 + 1;
> > >
> > >    }
> > >
> > >
> > >
> > > -  if (VariableHeader->DataSize < ByteOffset + Width) {
> > >
> > > -    return EFI_INVALID_PARAMETER;
> > >
> > > +  if (AuthFormat) {
> > >
> > > +    if (AuthVariableHeader->DataSize < ByteOffset + Width) {
> > >
> > > +      return EFI_INVALID_PARAMETER;
> > >
> > > +    }
> > >
> > > +
> > >
> > > +    ValueSource = (UINT8 *)AuthVariableHeader + sizeof
> > > (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize
> > +
> > > IfrQuestionHdr->VarStoreInfo.VarOffset;
> > >
> > > +    if (BitFieldQuestion) {
> > >
> > > +      BitValueSource = (UINT8 *)AuthVariableHeader + sizeof
> > > (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize
> > +
> > > ByteOffset;
> > >
> > > +    }
> > >
> > > +  } else {
> > >
> > > +    if (VariableHeader->DataSize < ByteOffset + Width) {
> > >
> > > +      return EFI_INVALID_PARAMETER;
> > >
> > > +    }
> > >
> > > +
> > >
> > > +    ValueSource = (UINT8 *)VariableHeader + sizeof
> (VARIABLE_HEADER)
> > > + +
> > > VariableHeader->NameSize + IfrQuestionHdr->VarStoreInfo.VarOffset;
> > >
> > > +    if (BitFieldQuestion) {
> > >
> > > +      BitValueSource = (UINT8 *)VariableHeader + sizeof
> > > + (VARIABLE_HEADER)
> > > + VariableHeader->NameSize + ByteOffset;
> > >
> > > +    }
> > >
> > >    }
> > >
> > >
> > >
> > >    //
> > >
> > > @@ -779,11 +853,11 @@ FindQuestionDefaultSetting (
> > >    //
> > >
> > >    if (ValueBuffer != NULL) {
> > >
> > >      if (BitFieldQuestion) {
> > >
> > > -      CopyMem (&BufferValue, (UINT8 *)VariableHeader + sizeof
> > > (VARIABLE_HEADER) + VariableHeader->NameSize + ByteOffset, Width);
> > >
> > > +      CopyMem (&BufferValue, BitValueSource, Width);
> > >
> > >        BitFieldVal = BitFieldRead32 (BufferValue, StartBit, EndBit);
> > >
> > >        CopyMem (ValueBuffer, &BitFieldVal, Width);
> > >
> > >      } else {
> > >
> > > -      CopyMem (ValueBuffer, (UINT8 *)VariableHeader + sizeof
> > > (VARIABLE_HEADER) + VariableHeader->NameSize + IfrQuestionHdr-
> > > >VarStoreInfo.VarOffset, Width);
> > >
> > > +      CopyMem (ValueBuffer, ValueSource, Width);
> > >
> > >      }
> > >
> > >    }
> > >
> > >
> > >
> > > @@ -832,7 +906,7 @@ UpdateDefaultSettingInFormPackage (
> > >    // If no default setting, do nothing
> > >
> > >    //
> > >
> > >    if (gNvDefaultStoreSize == 0) {
> > >
> > > -    gNvDefaultStoreSize = PcdGetSize (PcdNvStoreDefaultValueBuffer);
> > >
> > > +    gNvDefaultStoreSize = PcdGetExSize
> > > (&gEfiMdeModulePkgTokenSpaceGuid, PcdNvStoreDefaultValueBuffer);
> > >
> > >    }
> > >
> > >
> > >
> > >    if (gNvDefaultStoreSize < sizeof
> > > (PCD_NV_STORE_DEFAULT_BUFFER_HEADER)) {
> > >
> > > diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> > > b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> > > index 0116fb6ecb..dac4d614a8 100644
> > > --- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> > > +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> > > @@ -86,6 +86,9 @@
> > >    gEfiHiiImageDecoderNameJpegGuid
> > > |gEfiMdeModulePkgTokenSpaceGuid.PcdSupportHiiImageProtocol  ##
> > > SOMETIMES_CONSUMES ## GUID
> > >
> > >    gEfiHiiImageDecoderNamePngGuid
> > > |gEfiMdeModulePkgTokenSpaceGuid.PcdSupportHiiImageProtocol  ##
> > > SOMETIMES_CONSUMES ## GUID
> > >
> > >    gEdkiiIfrBitVarstoreGuid
> ##
> > > SOMETIMES_CONSUMES ## GUID
> > >
> > > +  gEfiAuthenticatedVariableGuid
> > >
> > > +  gEfiVariableGuid
> > >
> > > +  gEfiMdeModulePkgTokenSpaceGuid
> > >
> > >
> > >
> > >  [Depex]
> > >
> > >    TRUE
> > >
> > > --
> > > 2.25.1
> 
> 
> 
> 
>