From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web08.1583.1642561011857083454 for ; Tue, 18 Jan 2022 18:56:53 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Wed, 19 Jan 2022 10:56:46 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Chen, Lin Z'" , , "'Huang, Long1'" Cc: "'Bi, Dandan'" , "'Feng, Bob C'" , "'Zhang, Di'" , "'Li, Zhuangzhi'" References: <035601d80b90$cab5fc30$6021f490$@byosoft.com.cn> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIXSBNZGVNb2R1bGVQa2cvSGlpRGF0YWJhc2VEeGU6IEFkZCBTdXBwb3J0IGZvciBhdXRoZW50aWNhdGVkIHZhcmlhYmxl?= Date: Wed, 19 Jan 2022 10:56:48 +0800 Message-ID: <011701d80ce0$33d786c0$9b869440$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQK87V0gv5OJ8iWMUhYlz9G9fd5x7QLb/yTgAjpxELCqdxhh0A== Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Lin: > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Chen, Lin Z > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: = 2022=E5=B9=B41=E6=9C=8818=E6=97=A5 8:42 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Gao, Liming ; = devel@edk2.groups.io; > Huang, Long1 > =E6=8A=84=E9=80=81: Bi, Dandan ; Feng, Bob C > ; Zhang, Di ; Li, Zhuangzhi > > =E4=B8=BB=E9=A2=98: RE: [edk2-devel] [PATCH] = MdeModulePkg/HiiDatabaseDxe: Add > Support for authenticated variable >=20 > Hi Liming, >=20 > Pls see my comments below. >=20 > 1. > PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is same = to > PcdGetExPtr. This change is not required. >=20 > [Lin] It'll get alignment with PEI phase reference If using = PcdGetExPtr version. > Edk2/MdeModulePkg/Universal/PCD/Pei/Pcd.c#166 > DataBuffer =3D (UINT8 *)PeiPcdGetPtrEx = (&gEfiMdeModulePkgTokenSpaceGuid, > PcdToken (PcdNvStoreDefaultValueBuffer)); >=20 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.=20 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. >=20 > [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. >=20 Sorry, I miss the previous change in BaseTools. Normal variable storage = is enough for Tools and Code.=20 The default setting format can be always normal variable storage. The = consumer code logic can be simple.=20 I don't know what purpose to keep the alignment with FCE tool behavior. = Is there other tool or code to use=20 the generated PcdNvStoreDefaultValueBuffer? Thanks Liming > Thanks, > Lin >=20 > -----Original Message----- > From: gaoliming > Sent: Monday, January 17, 2022 6:56 PM > To: devel@edk2.groups.io; Huang, Long1 > Cc: Chen, Lin Z ; Bi, Dandan = ; > Feng, Bob C > Subject: =E5=9B=9E=E5=A4=8D: [edk2-devel] [PATCH] = MdeModulePkg/HiiDatabaseDxe: Add > Support for authenticated variable >=20 > Long: > I add my comments below. >=20 > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Long1 > Huang > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: = 2022=E5=B9=B41=E6=9C=8811=E6=97=A5 1:03 > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > > =E6=8A=84=E9=80=81: Huang Long ; Liming Gao > > ; Chen Lin Z ; = Dandan > > Bi > > =E4=B8=BB=E9=A2=98: [edk2-devel] [PATCH] = MdeModulePkg/HiiDatabaseDxe: Add Support > for > > authenticated variable > > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3796 > > > > 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 > > Cc: Chen Lin Z > > Cc: Dandan Bi > > > > Signed-off-by: Huang Long > > --- > > .../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 =3D (AUTHENTICATED_VARIABLE_HEADER > *)((UINT8 > > *)VariableStorage + VariableStorage->Size); > > > > + AuthVariableHeader =3D (AUTHENTICATED_VARIABLE_HEADER > > *)(VariableStorage + 1); > > > > + AuthVariableHeader =3D (AUTHENTICATED_VARIABLE_HEADER > > *)HEADER_ALIGN (AuthVariableHeader); > > > > + while (AuthVariableHeader < AuthVariableEnd) { > > > > + if (CompareGuid (&AuthVariableHeader->VendorGuid, VarGuid) && > > > > + (AuthVariableHeader->Attributes =3D=3D VarAttribute) && > > > > + (StrCmp (VarName, (CHAR16 *)(AuthVariableHeader + 1)) = =3D=3D 0)) > > > > + { > > > > + return AuthVariableHeader; > > > > + } > > > > + > > > > + AuthVariableHeader =3D (AUTHENTICATED_VARIABLE_HEADER > > *)((UINT8 *)AuthVariableHeader + sizeof > > (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize + > > AuthVariableHeader->DataSize); > > > > + AuthVariableHeader =3D (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 =3D=3D 0xFFFFFFFFFFFFFFFF) { > > > > gSkuId =3D LibPcdGetSku (); > > > > @@ -666,7 +707,7 @@ FindQuestionDefaultSetting ( > > } > > > > > > > > if (Link =3D=3D &gVarStorageList) { > > > > - DataBuffer =3D (UINT8 *)PcdGetPtr > > (PcdNvStoreDefaultValueBuffer); > > > > + DataBuffer =3D (UINT8 *)PcdGetExPtr > > (&gEfiMdeModulePkgTokenSpaceGuid, PcdNvStoreDefaultValueBuffer); > > > PcdNvStoreDefaultValueBuffer type is DynamicEx. Its PcdGetPtr is same = to > PcdGetExPtr. This change is not required. >=20 > > gNvDefaultStoreSize =3D > ((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 =3D FALSE; > > > > + if (CompareGuid (&VariableStorage->Signature, > > &gEfiAuthenticatedVariableGuid)) { > > > > + AuthFormat =3D TRUE; > > > > + } > > > > + > > >=20 >=20 > > // > > > > // Find the question default value from the variable storage > > > > // > > > > - VariableHeader =3D FindVariableData (VariableStorage, > > &EfiVarStore->Guid, > > EfiVarStore->Attributes, (CHAR16 *)EfiVarStore->Name); > > > > - if (VariableHeader =3D=3D NULL) { > > > > - return EFI_NOT_FOUND; > > > > + if(AuthFormat) { > > > > + AuthVariableHeader =3D FindAuthVariableData (VariableStorage, > > &EfiVarStore->Guid, EfiVarStore->Attributes, (CHAR16 > > *)EfiVarStore->Name); > > > > + if (AuthVariableHeader =3D=3D NULL) { > > > > + return EFI_NOT_FOUND; > > > > + } > > > > + } else { > > > > + VariableHeader =3D FindVariableData (VariableStorage, > > &EfiVarStore->Guid, EfiVarStore->Attributes, (CHAR16 > > *)EfiVarStore->Name); > > > > + if (VariableHeader =3D=3D 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. >=20 > Thanks > Liming > > > > > > StartBit =3D 0; > > > > @@ -770,20 +826,39 @@ FindQuestionDefaultSetting ( > > Width =3D 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 !=3D NULL) { > > > > - if (BitFieldQuestion) { > > > > - CopyMem (&BufferValue, (UINT8 *)VariableHeader + sizeof > > (VARIABLE_HEADER) + VariableHeader->NameSize + ByteOffset, Width); > > > > - BitFieldVal =3D 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 !=3D NULL) { > > > > + if (BitFieldQuestion) { > > > > + CopyMem (&BufferValue, (UINT8 *)AuthVariableHeader + sizeof > > (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize + > > ByteOffset, Width); > > > > + BitFieldVal =3D 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 !=3D NULL) { > > > > + if (BitFieldQuestion) { > > > > + CopyMem (&BufferValue, (UINT8 *)VariableHeader + sizeof > > (VARIABLE_HEADER) + VariableHeader->NameSize + ByteOffset, Width); > > > > + BitFieldVal =3D 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 > > > > > > > > -=3D-=3D-=3D-=3D-=3D-=3D > > 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] > > -=3D-=3D-=3D-=3D-=3D-=3D > > >=20 >=20