From: "gaoliming" <gaoliming@byosoft.com.cn>
To: <devel@edk2.groups.io>, <long1.huang@intel.com>
Cc: "'Chen Lin Z'" <lin.z.chen@intel.com>,
"'Dandan Bi'" <dandan.bi@intel.com>,
"'Feng, Bob C'" <bob.c.feng@intel.com>
Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
Date: Mon, 17 Jan 2022 18:55:50 +0800 [thread overview]
Message-ID: <035601d80b90$cab5fc30$6021f490$@byosoft.com.cn> (raw)
In-Reply-To: <cb3649b335fc2204ed7746696f22332f8ee97473.1641834028.git.long1.huang@intel.com>
Long:
I add my comments below.
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Long1 Huang
> 发送时间: 2022年1月11日 1:03
> 收件人: devel@edk2.groups.io
> 抄送: Huang Long <long1.huang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Chen Lin Z <lin.z.chen@intel.com>; Dandan Bi
> <dandan.bi@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@byosoft.com.cn>
> Cc: Chen Lin Z <lin.z.chen@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
>
> Signed-off-by: Huang Long <long1.huang@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@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
>
next prev parent reply other threads:[~2022-01-17 10:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-10 17:02 [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable Long1 Huang
2022-01-17 10:55 ` gaoliming [this message]
2022-01-18 0:42 ` [edk2-devel] " Chen Lin Z
2022-01-19 2:56 ` 回复: " gaoliming
2022-01-20 6:05 ` Zhang, Di
2022-01-21 5:15 ` 回复: " gaoliming
2022-01-24 2:13 ` Zhang, Di
-- strict thread matches above, loose matches on Subject: below --
2022-01-12 17:06 Long1 Huang
2022-01-13 5:34 ` Dandan Bi
2022-01-17 1:13 ` Dandan Bi
2022-01-17 3:11 ` 回复: [edk2-devel] " gaoliming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='035601d80b90$cab5fc30$6021f490$@byosoft.com.cn' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox