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

gaoliming gaoliming at byosoft.com.cn
Mon Jan 17 10:55:50 UTC 2022


Long:
  I add my comments below. 

> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Long1 Huang
> 发送时间: 2022年1月11日 1:03
> 收件人: devel at edk2.groups.io
> 抄送: Huang Long <long1.huang at intel.com>; Liming Gao
> <gaoliming at byosoft.com.cn>; Chen Lin Z <lin.z.chen at intel.com>; Dandan Bi
> <dandan.bi at intel.com>
> 主题: [edk2-devel] [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.
> 
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> Cc: Chen Lin Z <lin.z.chen at intel.com>
> Cc: Dandan Bi <dandan.bi at intel.com>
> 
> Signed-off-by: Huang Long <long1.huang at intel.com>
> ---
>  .../Universal/HiiDatabaseDxe/Database.c       | 147 +++++++++++++-----
>  .../HiiDatabaseDxe/HiiDatabaseDxe.inf         |   3 +
>  2 files changed, 114 insertions(+), 36 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> index 0b09c24d52..c055fa0f29 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,27 @@ 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;
> 
> +  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 +707,7 @@ FindQuestionDefaultSetting (
>    }
> 
> 
> 
>    if (Link == &gVarStorageList) {
> 
> -    DataBuffer          = (UINT8 *)PcdGetPtr
> (PcdNvStoreDefaultValueBuffer);
> 
> +    DataBuffer          = (UINT8 *)PcdGetExPtr
> (&gEfiMdeModulePkgTokenSpaceGuid, PcdNvStoreDefaultValueBuffer);
> 
PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is same to
PcdGetExPtr. This change is not required. 

>      gNvDefaultStoreSize = ((PCD_NV_STORE_DEFAULT_BUFFER_HEADER
> *)DataBuffer)->Length;
> 
>      //
> 
>      // The first section data includes NV storage default setting.
> 
> @@ -750,12 +791,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;
> 
> +    }
> 
>    }
> 
VariableStorage data buffer is from PcdNvStoreDefaultValueBuffer.
PcdNvStoreDefaultValueBuffer is auto generated by BaseTools. 
By design, its data format is always normal variable storage format. 
So, its value can't be auth variable format. 

Thanks
Liming
> 
> 
>    StartBit   = 0;
> 
> @@ -770,20 +826,39 @@ 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;
> 
> +    }
> 
> 
> 
> -  //
> 
> -  // Copy the question value
> 
> -  //
> 
> -  if (ValueBuffer != NULL) {
> 
> -    if (BitFieldQuestion) {
> 
> -      CopyMem (&BufferValue, (UINT8 *)VariableHeader + sizeof
> (VARIABLE_HEADER) + VariableHeader->NameSize + ByteOffset, Width);
> 
> -      BitFieldVal = BitFieldRead32 (BufferValue, StartBit, EndBit);
> 
> -      CopyMem (ValueBuffer, &BitFieldVal, Width);
> 
> -    } else {
> 
> -      CopyMem (ValueBuffer, (UINT8 *)VariableHeader + sizeof
> (VARIABLE_HEADER) + VariableHeader->NameSize +
> IfrQuestionHdr->VarStoreInfo.VarOffset, Width);
> 
> +    //
> 
> +    // Copy the question value
> 
> +    //
> 
> +    if (ValueBuffer != NULL) {
> 
> +      if (BitFieldQuestion) {
> 
> +        CopyMem (&BufferValue, (UINT8 *)AuthVariableHeader + sizeof
> (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize +
> ByteOffset, Width);
> 
> +        BitFieldVal = BitFieldRead32 (BufferValue, StartBit, EndBit);
> 
> +        CopyMem (ValueBuffer, &BitFieldVal, Width);
> 
> +      } else {
> 
> +        CopyMem (ValueBuffer, (UINT8 *)AuthVariableHeader + sizeof
> (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize +
> IfrQuestionHdr->VarStoreInfo.VarOffset, Width);
> 
> +      }
> 
> +    }
> 
> +  } else {
> 
> +    if (VariableHeader->DataSize < ByteOffset + Width) {
> 
> +      return EFI_INVALID_PARAMETER;
> 
> +    }
> 
> +
> 
> +    //
> 
> +    // Copy the question value
> 
> +    //
> 
> +    if (ValueBuffer != NULL) {
> 
> +      if (BitFieldQuestion) {
> 
> +        CopyMem (&BufferValue, (UINT8 *)VariableHeader + sizeof
> (VARIABLE_HEADER) + VariableHeader->NameSize + ByteOffset, Width);
> 
> +        BitFieldVal = BitFieldRead32 (BufferValue, StartBit, EndBit);
> 
> +        CopyMem (ValueBuffer, &BitFieldVal, Width);
> 
> +      } else {
> 
> +        CopyMem (ValueBuffer, (UINT8 *)VariableHeader + sizeof
> (VARIABLE_HEADER) + VariableHeader->NameSize +
> IfrQuestionHdr->VarStoreInfo.VarOffset, Width);
> 
> +      }
> 
>      }
> 
>    }
> 
> 
> 
> 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
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#85447): https://edk2.groups.io/g/devel/message/85447
> Mute This Topic: https://groups.io/mt/88319448/4905953
> Group Owner: devel+owner at edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming at byosoft.com.cn]
> -=-=-=-=-=-=
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85752): https://edk2.groups.io/g/devel/message/85752
Mute This Topic: https://groups.io/mt/88481550/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