public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
@ 2022-01-10 17:02 Long1 Huang
  2022-01-17 10:55 ` 回复: [edk2-devel] " gaoliming
  0 siblings, 1 reply; 8+ messages in thread
From: Long1 Huang @ 2022-01-10 17:02 UTC (permalink / raw)
  To: devel; +Cc: Huang Long, Liming Gao, Chen Lin Z, Dandan Bi

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);
     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;
+    }
   }
 
   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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
  2022-01-17  1:13   ` Dandan Bi
@ 2022-01-17  3:11     ` gaoliming
  0 siblings, 0 replies; 8+ messages in thread
From: gaoliming @ 2022-01-17  3:11 UTC (permalink / raw)
  To: devel, dandan.bi, 'Huang, Long1'
  Cc: 'Feng, Bob C', 'Chen, Lin Z',
	'Li, Zhuangzhi'

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
> 
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 8+ messages in thread

* 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
  2022-01-10 17:02 [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable Long1 Huang
@ 2022-01-17 10:55 ` gaoliming
  2022-01-18  0:42   ` Chen Lin Z
  0 siblings, 1 reply; 8+ messages in thread
From: gaoliming @ 2022-01-17 10:55 UTC (permalink / raw)
  To: devel, long1.huang
  Cc: 'Chen Lin Z', 'Dandan Bi', 'Feng, Bob C'

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]
> -=-=-=-=-=-=
> 




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
  2022-01-17 10:55 ` 回复: [edk2-devel] " gaoliming
@ 2022-01-18  0:42   ` Chen Lin Z
  2022-01-19  2:56     ` 回复: " gaoliming
  0 siblings, 1 reply; 8+ messages in thread
From: Chen Lin Z @ 2022-01-18  0:42 UTC (permalink / raw)
  To: Gao, Liming, devel@edk2.groups.io, Huang, Long1
  Cc: Bi, Dandan, Feng, Bob C, Zhang, Di, Li, Zhuangzhi

Hi Liming, 
  
   Pls see my comments below.

1. 
PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is same to PcdGetExPtr. This change is not required.

[Lin] It'll get alignment with PEI phase reference If using PcdGetExPtr version.
Edk2/MdeModulePkg/Universal/PCD/Pei/Pcd.c#166
DataBuffer = (UINT8 *)PeiPcdGetPtrEx (&gEfiMdeModulePkgTokenSpaceGuid, PcdToken (PcdNvStoreDefaultValueBuffer));

2. 
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.

[Lin] BaseTools can generate authenticated variable storage format now (see https://edk2.groups.io/g/devel/message/83329), 
 since previous FCE tool generates authenticated format, we want to keep variable storage format no changes after switching to StructurePcd.

Thanks,
Lin

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Monday, January 17, 2022 6:56 PM
To: devel@edk2.groups.io; Huang, Long1 <long1.huang@intel.com>
Cc: Chen, Lin Z <lin.z.chen@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable

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]
> -=-=-=-=-=-=
> 




^ permalink raw reply	[flat|nested] 8+ messages in thread

* 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
  2022-01-18  0:42   ` Chen Lin Z
@ 2022-01-19  2:56     ` gaoliming
  2022-01-20  6:05       ` Zhang, Di
  0 siblings, 1 reply; 8+ messages in thread
From: gaoliming @ 2022-01-19  2:56 UTC (permalink / raw)
  To: 'Chen, Lin Z', devel, 'Huang, Long1'
  Cc: 'Bi, Dandan', 'Feng, Bob C', 'Zhang, Di',
	'Li, Zhuangzhi'

Lin:

> -----邮件原件-----
> 发件人: Chen, Lin Z <lin.z.chen@intel.com>
> 发送时间: 2022年1月18日 8:42
> 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> Huang, Long1 <long1.huang@intel.com>
> 抄送: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>; Zhang, Di <di.zhang@intel.com>; Li, Zhuangzhi
> <zhuangzhi.li@intel.com>
> 主题: RE: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> Support for authenticated variable
> 
> Hi Liming,
> 
>    Pls see my comments below.
> 
> 1.
> PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is same to
> PcdGetExPtr. This change is not required.
> 
> [Lin] It'll get alignment with PEI phase reference If using PcdGetExPtr version.
> Edk2/MdeModulePkg/Universal/PCD/Pei/Pcd.c#166
> DataBuffer = (UINT8 *)PeiPcdGetPtrEx (&gEfiMdeModulePkgTokenSpaceGuid,
> PcdToken (PcdNvStoreDefaultValueBuffer));
> 
PcdPei is different. It is Pcd driver. It can't consume the real PcdLib. So, it doesn't use PcdLib PcdGetPtr() API.
It uses the internal function PeiPcdGetPtrEx() to get PCD value and size. 

HiiDataBase is normal DXE driver. It can depend on the real PcdLib. So, it uses PcdLib PcdGetPtr() API.
Seemly, current code brings confuse. If so, I am OK to make this change. And, please also update PcdGetSize (PcdNvStoreDefaultValueBuffer) with LibPcdGetExSize()

> 2.
> 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.
> 
> [Lin] BaseTools can generate authenticated variable storage format now (see
> https://edk2.groups.io/g/devel/message/83329),
>  since previous FCE tool generates authenticated format, we want to keep
> variable storage format no changes after switching to StructurePcd.
> 
Sorry, I miss the previous change in BaseTools. Normal variable storage is enough for Tools and Code. 
The default setting format can be always normal variable storage. The consumer code logic can be simple. 
I don't know what purpose to keep the alignment with FCE tool behavior. Is there other tool or code to use 
the generated PcdNvStoreDefaultValueBuffer?

Thanks
Liming
> Thanks,
> Lin
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Monday, January 17, 2022 6:56 PM
> To: devel@edk2.groups.io; Huang, Long1 <long1.huang@intel.com>
> Cc: Chen, Lin Z <lin.z.chen@intel.com>; Bi, Dandan <dandan.bi@intel.com>;
> Feng, Bob C <bob.c.feng@intel.com>
> Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> Support for authenticated variable
> 
> 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]
> > -=-=-=-=-=-=
> >
> 
> 




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
  2022-01-19  2:56     ` 回复: " gaoliming
@ 2022-01-20  6:05       ` Zhang, Di
  2022-01-21  5:15         ` 回复: " gaoliming
  0 siblings, 1 reply; 8+ messages in thread
From: Zhang, Di @ 2022-01-20  6:05 UTC (permalink / raw)
  To: Gao, Liming, Chen, Lin Z, devel@edk2.groups.io, Huang, Long1
  Cc: Bi, Dandan, Feng, Bob C, Li, Zhuangzhi


Liming:

> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Wednesday, January 19, 2022 10:57 AM
> To: Chen, Lin Z <lin.z.chen@intel.com>; devel@edk2.groups.io; Huang, Long1
> <long1.huang@intel.com>
> Cc: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C <bob.c.feng@intel.com>;
> Zhang, Di <di.zhang@intel.com>; Li, Zhuangzhi <zhuangzhi.li@intel.com>
> Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> Support for authenticated variable
> 
> Lin:
> 
> > -----邮件原件-----
> > 发件人: Chen, Lin Z <lin.z.chen@intel.com>
> > 发送时间: 2022年1月18日 8:42
> > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> > Huang, Long1 <long1.huang@intel.com>
> > 抄送: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> > <bob.c.feng@intel.com>; Zhang, Di <di.zhang@intel.com>; Li, Zhuangzhi
> > <zhuangzhi.li@intel.com>
> > 主题: RE: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> > Support for authenticated variable
> >
> > Hi Liming,
> >
> >    Pls see my comments below.
> >
> > 1.
> > PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is same to
> > PcdGetExPtr. This change is not required.
> >
> > [Lin] It'll get alignment with PEI phase reference If using PcdGetExPtr
> version.
> > Edk2/MdeModulePkg/Universal/PCD/Pei/Pcd.c#166
> > DataBuffer = (UINT8 *)PeiPcdGetPtrEx (&gEfiMdeModulePkgTokenSpaceGuid,
> > PcdToken (PcdNvStoreDefaultValueBuffer));
> >
> PcdPei is different. It is Pcd driver. It can't consume the real PcdLib.
> So, it doesn't use PcdLib PcdGetPtr() API.
> It uses the internal function PeiPcdGetPtrEx() to get PCD value and size.
> 
> HiiDataBase is normal DXE driver. It can depend on the real PcdLib. So, it
> uses PcdLib PcdGetPtr() API.
> Seemly, current code brings confuse. If so, I am OK to make this change.
> And, please also update PcdGetSize (PcdNvStoreDefaultValueBuffer) with
> LibPcdGetExSize()
> 
> > 2.
> > 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.
> >
> > [Lin] BaseTools can generate authenticated variable storage format now
> (see
> > https://edk2.groups.io/g/devel/message/83329),
> >  since previous FCE tool generates authenticated format, we want to keep
> > variable storage format no changes after switching to StructurePcd.
> >
> Sorry, I miss the previous change in BaseTools. Normal variable storage is
> enough for Tools and Code.
> The default setting format can be always normal variable storage. The
> consumer code logic can be simple.
> I don't know what purpose to keep the alignment with FCE tool behavior. Is
> there other tool or code to use
> the generated PcdNvStoreDefaultValueBuffer?
> 

We have the usage that writing the entire VPD (which has default setup value)
to variable region upon first boot. And the variable region needs to be in
authenticated format when secure boot is enabled, so we also need the VPD in
authenticated format. Hope it clarifies your question.

> Thanks
> Liming
> > Thanks,
> > Lin
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Monday, January 17, 2022 6:56 PM
> > To: devel@edk2.groups.io; Huang, Long1 <long1.huang@intel.com>
> > Cc: Chen, Lin Z <lin.z.chen@intel.com>; Bi, Dandan <dandan.bi@intel.com>;
> > Feng, Bob C <bob.c.feng@intel.com>
> > Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> > Support for authenticated variable
> >
> > 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]
> > > -=-=-=-=-=-=
> > >
> >
> >
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
  2022-01-20  6:05       ` Zhang, Di
@ 2022-01-21  5:15         ` gaoliming
  2022-01-24  2:13           ` Zhang, Di
  0 siblings, 1 reply; 8+ messages in thread
From: gaoliming @ 2022-01-21  5:15 UTC (permalink / raw)
  To: 'Zhang, Di', 'Chen, Lin Z', devel,
	'Huang, Long1', 'Kinney, Michael D'
  Cc: 'Bi, Dandan', 'Feng, Bob C',
	'Li, Zhuangzhi'

Di:

> -----邮件原件-----
> 发件人: Zhang, Di <di.zhang@intel.com>
> 发送时间: 2022年1月20日 14:05
> 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; Chen, Lin Z
> <lin.z.chen@intel.com>; devel@edk2.groups.io; Huang, Long1
> <long1.huang@intel.com>
> 抄送: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>; Li, Zhuangzhi <zhuangzhi.li@intel.com>
> 主题: RE: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> Support for authenticated variable
> 
> 
> Liming:
> 
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Wednesday, January 19, 2022 10:57 AM
> > To: Chen, Lin Z <lin.z.chen@intel.com>; devel@edk2.groups.io; Huang,
> Long1
> > <long1.huang@intel.com>
> > Cc: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>;
> > Zhang, Di <di.zhang@intel.com>; Li, Zhuangzhi <zhuangzhi.li@intel.com>
> > Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> > Support for authenticated variable
> >
> > Lin:
> >
> > > -----邮件原件-----
> > > 发件人: Chen, Lin Z <lin.z.chen@intel.com>
> > > 发送时间: 2022年1月18日 8:42
> > > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>;
> devel@edk2.groups.io;
> > > Huang, Long1 <long1.huang@intel.com>
> > > 抄送: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> > > <bob.c.feng@intel.com>; Zhang, Di <di.zhang@intel.com>; Li, Zhuangzhi
> > > <zhuangzhi.li@intel.com>
> > > 主题: RE: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> > > Support for authenticated variable
> > >
> > > Hi Liming,
> > >
> > >    Pls see my comments below.
> > >
> > > 1.
> > > PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is same
> to
> > > PcdGetExPtr. This change is not required.
> > >
> > > [Lin] It'll get alignment with PEI phase reference If using PcdGetExPtr
> > version.
> > > Edk2/MdeModulePkg/Universal/PCD/Pei/Pcd.c#166
> > > DataBuffer = (UINT8 *)PeiPcdGetPtrEx
> (&gEfiMdeModulePkgTokenSpaceGuid,
> > > PcdToken (PcdNvStoreDefaultValueBuffer));
> > >
> > PcdPei is different. It is Pcd driver. It can't consume the real PcdLib.
> > So, it doesn't use PcdLib PcdGetPtr() API.
> > It uses the internal function PeiPcdGetPtrEx() to get PCD value and size.
> >
> > HiiDataBase is normal DXE driver. It can depend on the real PcdLib. So, it
> > uses PcdLib PcdGetPtr() API.
> > Seemly, current code brings confuse. If so, I am OK to make this change.
> > And, please also update PcdGetSize (PcdNvStoreDefaultValueBuffer) with
> > LibPcdGetExSize()
> >
> > > 2.
> > > 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.
> > >
> > > [Lin] BaseTools can generate authenticated variable storage format now
> > (see
> > > https://edk2.groups.io/g/devel/message/83329),
> > >  since previous FCE tool generates authenticated format, we want to
> keep
> > > variable storage format no changes after switching to StructurePcd.
> > >
> > Sorry, I miss the previous change in BaseTools. Normal variable storage is
> > enough for Tools and Code.
> > The default setting format can be always normal variable storage. The
> > consumer code logic can be simple.
> > I don't know what purpose to keep the alignment with FCE tool behavior. Is
> > there other tool or code to use
> > the generated PcdNvStoreDefaultValueBuffer?
> >
> 
> We have the usage that writing the entire VPD (which has default setup value)
> to variable region upon first boot. And the variable region needs to be in
> authenticated format when secure boot is enabled, so we also need the VPD
> in
> authenticated format. Hope it clarifies your question.
> 
Current usage is to build VariableHob based on PcdNvStoreDefaultValueBuffer data in PEI, 
and write VariableHob into NV space in DXE. Variable driver is responsible to convert 
normal variable format to the auth variable format.

Have you another usage model to write entire VPD data to variable region? How is 
VPD data placed into variable region? Is VPD data wrapped as the variable format? 
If so, VPD data is also variable data. I think Variable driver should check NV region 
to decide the variable format instead of VPD data.

Can you add the background about new usage model into https://bugzilla.tianocore.org/show_bug.cgi?id=3796?

Thanks
Liming
> > Thanks
> > Liming
> > > Thanks,
> > > Lin
> > >
> > > -----Original Message-----
> > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > Sent: Monday, January 17, 2022 6:56 PM
> > > To: devel@edk2.groups.io; Huang, Long1 <long1.huang@intel.com>
> > > Cc: Chen, Lin Z <lin.z.chen@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>;
> > > Feng, Bob C <bob.c.feng@intel.com>
> > > Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe:
> Add
> > > Support for authenticated variable
> > >
> > > 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]
> > > > -=-=-=-=-=-=
> > > >
> > >
> > >
> >
> >




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable
  2022-01-21  5:15         ` 回复: " gaoliming
@ 2022-01-24  2:13           ` Zhang, Di
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Di @ 2022-01-24  2:13 UTC (permalink / raw)
  To: Gao, Liming, Chen, Lin Z, devel@edk2.groups.io, Huang, Long1,
	Kinney, Michael D
  Cc: Bi, Dandan, Feng, Bob C, Li, Zhuangzhi


Liming:

> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Friday, January 21, 2022 1:16 PM
> To: Zhang, Di <di.zhang@intel.com>; Chen, Lin Z <lin.z.chen@intel.com>;
> devel@edk2.groups.io; Huang, Long1 <long1.huang@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C <bob.c.feng@intel.com>;
> Li, Zhuangzhi <zhuangzhi.li@intel.com>
> Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> Support for authenticated variable
> 
> Di:
> 
> > -----邮件原件-----
> > 发件人: Zhang, Di <di.zhang@intel.com>
> > 发送时间: 2022年1月20日 14:05
> > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; Chen, Lin Z
> > <lin.z.chen@intel.com>; devel@edk2.groups.io; Huang, Long1
> > <long1.huang@intel.com>
> > 抄送: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> > <bob.c.feng@intel.com>; Li, Zhuangzhi <zhuangzhi.li@intel.com>
> > 主题: RE: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> > Support for authenticated variable
> >
> >
> > Liming:
> >
> > > -----Original Message-----
> > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > Sent: Wednesday, January 19, 2022 10:57 AM
> > > To: Chen, Lin Z <lin.z.chen@intel.com>; devel@edk2.groups.io; Huang,
> > Long1
> > > <long1.huang@intel.com>
> > > Cc: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> > <bob.c.feng@intel.com>;
> > > Zhang, Di <di.zhang@intel.com>; Li, Zhuangzhi <zhuangzhi.li@intel.com>
> > > Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> > > Support for authenticated variable
> > >
> > > Lin:
> > >
> > > > -----邮件原件-----
> > > > 发件人: Chen, Lin Z <lin.z.chen@intel.com>
> > > > 发送时间: 2022年1月18日 8:42
> > > > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>;
> > devel@edk2.groups.io;
> > > > Huang, Long1 <long1.huang@intel.com>
> > > > 抄送: Bi, Dandan <dandan.bi@intel.com>; Feng, Bob C
> > > > <bob.c.feng@intel.com>; Zhang, Di <di.zhang@intel.com>; Li,
> Zhuangzhi
> > > > <zhuangzhi.li@intel.com>
> > > > 主题: RE: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe: Add
> > > > Support for authenticated variable
> > > >
> > > > Hi Liming,
> > > >
> > > >    Pls see my comments below.
> > > >
> > > > 1.
> > > > PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is
> same
> > to
> > > > PcdGetExPtr. This change is not required.
> > > >
> > > > [Lin] It'll get alignment with PEI phase reference If using
> PcdGetExPtr
> > > version.
> > > > Edk2/MdeModulePkg/Universal/PCD/Pei/Pcd.c#166
> > > > DataBuffer = (UINT8 *)PeiPcdGetPtrEx
> > (&gEfiMdeModulePkgTokenSpaceGuid,
> > > > PcdToken (PcdNvStoreDefaultValueBuffer));
> > > >
> > > PcdPei is different. It is Pcd driver. It can't consume the real
> PcdLib.
> > > So, it doesn't use PcdLib PcdGetPtr() API.
> > > It uses the internal function PeiPcdGetPtrEx() to get PCD value and
> size.
> > >
> > > HiiDataBase is normal DXE driver. It can depend on the real PcdLib. So,
> it
> > > uses PcdLib PcdGetPtr() API.
> > > Seemly, current code brings confuse. If so, I am OK to make this
> change.
> > > And, please also update PcdGetSize (PcdNvStoreDefaultValueBuffer) with
> > > LibPcdGetExSize()
> > >
> > > > 2.
> > > > 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.
> > > >
> > > > [Lin] BaseTools can generate authenticated variable storage format
> now
> > > (see
> > > > https://edk2.groups.io/g/devel/message/83329),
> > > >  since previous FCE tool generates authenticated format, we want to
> > keep
> > > > variable storage format no changes after switching to StructurePcd.
> > > >
> > > Sorry, I miss the previous change in BaseTools. Normal variable
> storage is
> > > enough for Tools and Code.
> > > The default setting format can be always normal variable storage. The
> > > consumer code logic can be simple.
> > > I don't know what purpose to keep the alignment with FCE tool behavior.
> Is
> > > there other tool or code to use
> > > the generated PcdNvStoreDefaultValueBuffer?
> > >
> >
> > We have the usage that writing the entire VPD (which has default setup
> value)
> > to variable region upon first boot. And the variable region needs to be
> in
> > authenticated format when secure boot is enabled, so we also need the
> VPD
> > in
> > authenticated format. Hope it clarifies your question.
> >
> Current usage is to build VariableHob based on
> PcdNvStoreDefaultValueBuffer data in PEI,
> and write VariableHob into NV space in DXE. Variable driver is responsible
> to convert
> normal variable format to the auth variable format.
> 
> Have you another usage model to write entire VPD data to variable region?
> How is
> VPD data placed into variable region? Is VPD data wrapped as the variable
> format?
> If so, VPD data is also variable data. I think Variable driver should
> check NV region
> to decide the variable format instead of VPD data.
> 
> Can you add the background about new usage model into
> https://bugzilla.tianocore.org/show_bug.cgi?id=3796?
> 

Thanks for pointing it out, we didn't notice DXE Variable driver has done
this converting job already since FCE was using auth format.
We have verified the normal VPD + auth variable store and it works.
We will drop this patch (also reverting the basetools change) since normal
VPD should already meet our current usage model. 

> Thanks
> Liming
> > > Thanks
> > > Liming
> > > > Thanks,
> > > > Lin
> > > >
> > > > -----Original Message-----
> > > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > > Sent: Monday, January 17, 2022 6:56 PM
> > > > To: devel@edk2.groups.io; Huang, Long1 <long1.huang@intel.com>
> > > > Cc: Chen, Lin Z <lin.z.chen@intel.com>; Bi, Dandan
> > <dandan.bi@intel.com>;
> > > > Feng, Bob C <bob.c.feng@intel.com>
> > > > Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/HiiDatabaseDxe:
> > Add
> > > > Support for authenticated variable
> > > >
> > > > 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]
> > > > > -=-=-=-=-=-=
> > > > >
> > > >
> > > >
> > >
> > >
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-01-24  2:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-10 17:02 [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable Long1 Huang
2022-01-17 10:55 ` 回复: [edk2-devel] " gaoliming
2022-01-18  0:42   ` 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox