From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: =?UTF-8?B?UmU6IFtlZGsyLWRldmVsXSDlm57lpI06IFtlZGsyLWRldmVsXSBbUEFUQ0hdIE1kZU1vZHVsZVBrZyBWYXJpYWJsZVBlaTogQWRkIFZhcmlhYmxlIHN0YXRlIGNoZWNrIHdoZW4gZmluZCB2YXJpYWJsZSBpbiBJbmRleFRhYmxlLg==?= To: gaoliming ,devel@edk2.groups.io From: "Jiading Zhang" X-Originating-Location: Beijing, CN (218.247.145.3) X-Originating-Platform: Windows Chrome 106 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Sun, 09 Oct 2022 17:35:02 -0700 References: <01fa01d8d476$c9f2ed80$5dd8c880$@byosoft.com.cn> In-Reply-To: <01fa01d8d476$c9f2ed80$5dd8c880$@byosoft.com.cn> Message-ID: <28563.1665362102769346496@groups.io> Content-Type: multipart/alternative; boundary="uHFacSiHiQXprDGi3Iav" --uHFacSiHiQXprDGi3Iav Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi liming: Yes, NV Variable Data is not changed in PEI phase in normal case. This issu= e was found when we did a special coding, and when found variable in the In= dexTable, it found the variable before the last changed if=C2=A0 didn't add= the following condition: " if ((VariableHeader->State =3D=3D VAR_ADDED) || (VariableHeader->State = =3D=3D (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))" Maybe our specail coding had some defect=EF=BC=8Cand caused this issue. On Fri, Sep 30, 2022 at 10:46 AM, gaoliming wrote: >=20 >=20 >=20 > Jiading: >=20 >=20 >=20 > Hob Variable Store Info IndexTable is NULL. So, this logic doesn=E2=80=99= t work > for HOB variable store. NV Variable Store Info has IndexTable. When its > IndexTable is initialized, its IndexTable will only record the variable > with VAR_ADDED attribute. Because NV Variable Data is not changed in PEI > phase, this check is not required by NV variable. >=20 >=20 >=20 >=20 >=20 >=20 >=20 > Thanks >=20 >=20 >=20 > Liming >=20 >=20 >=20 > *=E5=8F=91=E4=BB=B6=E4=BA=BA :* devel@edk2.groups.io *=E4=BB=A3=E8=A1=A8* Jiading Zhang > *=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4 :* 2022 =E5=B9=B4 9 =E6=9C=88 28 = =E6=97=A5 11:05 > *=E6=94=B6=E4=BB=B6=E4=BA=BA :* devel@edk2.groups.io > *=E4=B8=BB=E9=A2=98 :* [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add= Variable state > check when find variable in IndexTable. >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 > When read a variable in PEI, it will find it first in the HOB, then find > in variable store. When find in variable store, it will check the variabl= e > state, but find in HOB, it doesn't check the state, so if the variable wa= s > changed, it will find the obsolete variable in the HOB. >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 > Signed-off-by: jdzhang < jdzhang@kunluntech.com.cn > >=20 >=20 >=20 >=20 > --- >=20 >=20 >=20 >=20 > MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 +++++++----- >=20 >=20 >=20 >=20 > 1 file changed, 7 insertions(+), 5 deletions(-) >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 > diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c > b/MdeModulePkg/Universal/Variable/Pei/Variable.c >=20 >=20 >=20 >=20 > index 26a4c73b45..dffbd8cdb1 100644 >=20 >=20 >=20 >=20 > --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c >=20 >=20 >=20 >=20 > +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c >=20 >=20 >=20 >=20 > @@ -866,11 +866,13 @@ FindVariableEx ( >=20 >=20 >=20 >=20 > Offset=C2=A0 +=3D IndexTable->Index[Index]; >=20 >=20 >=20 >=20 > MaxIndex =3D (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset); >=20 >=20 >=20 >=20 > GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader); >=20 >=20 >=20 >=20 > -=C2=A0 =C2=A0 =C2=A0 if (CompareWithValidVariable (StoreInfo, MaxIndex, = VariableHeader, > VariableName, VendorGuid, PtrTrack) =3D=3D EFI_SUCCESS) { >=20 >=20 >=20 >=20 > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (VariableHeader->State =3D=3D (VAR_IN_DEL= ETED_TRANSITION & > VAR_ADDED)) { >=20 >=20 >=20 >=20 > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 InDeletedVariable =3D PtrTrack->CurrP= tr; >=20 >=20 >=20 >=20 > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 } else { >=20 >=20 >=20 >=20 > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return EFI_SUCCESS; >=20 >=20 >=20 >=20 > +=C2=A0 =C2=A0 =C2=A0 if ((VariableHeader->State =3D=3D VAR_ADDED) || (Va= riableHeader->State > =3D=3D (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) { >=20 >=20 >=20 >=20 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (CompareWithValidVariable (StoreInfo, Max= Index, > VariableHeader, VariableName, VendorGuid, PtrTrack) =3D=3D EFI_SUCCESS) { >=20 >=20 >=20 >=20 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (VariableHeader->State =3D=3D (VAR= _IN_DELETED_TRANSITION & > VAR_ADDED)) { >=20 >=20 >=20 >=20 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 InDeletedVariable =3D PtrTrack= ->CurrPtr; >=20 >=20 >=20 >=20 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } else { >=20 >=20 >=20 >=20 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return EFI_SUCCESS; >=20 >=20 >=20 >=20 > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } >=20 >=20 >=20 >=20 > } >=20 >=20 >=20 >=20 > } >=20 >=20 >=20 >=20 > } >=20 >=20 >=20 >=20 > -- >=20 >=20 >=20 >=20 > 2.20.1.windows.1 >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 > --uHFacSiHiQXprDGi3Iav Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi liming:
=C2=A0 =C2=A0Yes, NV Variable Data is not changed in PEI pha= se in normal case. This issue was found when we did a special coding, and w= hen found variable in the IndexTable, it found the variable before the last= changed if=C2=A0 didn't add the following condition:
=C2=A0 "<= span style=3D"font-family: =E5=AE=8B=E4=BD=93; font-size: 16px;">if ((Varia= bleHeader->State =3D=3D VAR_ADDED) || (VariableHeader->State =3D=3D (= VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"

Maybe our= specail coding had some defect=EF=BC=8Cand caused this issue.

On = Fri, Sep 30, 2022 at 10:46 AM, gaoliming wrote:

Jiading:

=C2=A0Hob Var= iable Store Info IndexTable is NULL. So, this logic doesn=E2=80=99t work fo= r HOB variable store. NV Variable Store Info has IndexTable. When its Index= Table is initialized, its IndexTable will only record the variable with VAR= _ADDED attribute. Because NV Variable Data is not changed in PEI phase, thi= s check is not required by NV variable.

=C2=A0<= /p>

Thanks<= /p>

Liming<= /p>

=E5=8F=91=E4=BB=B6=E4= =BA=BA: devel@edk2.grou= ps.io <devel@edk2.groups.io> =E4=BB=A3=E8=A1=A8 Jiading Zhang
=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4<= span lang=3D"EN-US">: 2022=E5=B9=B49=E6=9C=8828=E6=97=A5 11:05
=E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io=E4=B8=BB=E9=A2=98: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Vari= able state check when find variable in IndexTable.

= =C2=A0

W= hen read a variable in PEI, it will find it first in the HOB, then find in = variable store. When find in variable store, it will check the variable sta= te, but find in HOB, it doesn't check the state, so if the variable was= changed, it will find the obsolete variable in the HOB.

= =C2=A0

S= igned-off-by: jdzhang <jdzh= ang@kunluntech.com.cn>

-= --

= =C2=A0MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 +++++++-----

= =C2=A01 file changed, 7 insertions(+), 5 deletions(-)

= =C2=A0

d= iff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/U= niversal/Variable/Pei/Variable.c

i= ndex 26a4c73b45..dffbd8cdb1 100644

-= -- a/MdeModulePkg/Universal/Variable/Pei/Variable.c

+= ++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c

@= @ -866,11 +866,13 @@ FindVariableEx (

= =C2=A0 =C2=A0 =C2=A0 =C2=A0Offset=C2=A0 +=3D IndexTable->Index[Index];

= =C2=A0 =C2=A0 =C2=A0 =C2=A0MaxIndex =3D (VARIABLE_HEADER *)((UINT8 *)IndexT= able->StartPtr + Offset);

= =C2=A0 =C2=A0 =C2=A0 =C2=A0GetVariableHeader (StoreInfo, MaxIndex, &Var= iableHeader);

-= =C2=A0 =C2=A0 =C2=A0 if (CompareWithValidVariable (StoreInfo, MaxIndex, Var= iableHeader, VariableName, VendorGuid, PtrTrack) =3D=3D EFI_SUCCESS) {

-= =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (VariableHeader->State =3D=3D (VAR_IN_DEL= ETED_TRANSITION & VAR_ADDED)) {

-= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 InDeletedVariable =3D PtrTrack->CurrP= tr;

-= =C2=A0 =C2=A0 =C2=A0 =C2=A0 } else {

-= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return EFI_SUCCESS;

+= =C2=A0 =C2=A0 =C2=A0 if ((VariableHeader->State =3D=3D VAR_ADDED) || (Va= riableHeader->State =3D=3D (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))= {

+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (CompareWithValidVariable (StoreInfo, MaxInd= ex, VariableHeader, VariableName, VendorGuid, PtrTrack) =3D=3D EFI_SUCCESS)= {

+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (VariableHeader->State =3D=3D (VAR= _IN_DELETED_TRANSITION & VAR_ADDED)) {

+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 InDeletedVariable =3D PtrTrack-&g= t;CurrPtr;

+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } else {

+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return EFI_SUCCESS;

+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }

= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}

= =C2=A0 =C2=A0 =C2=A0 =C2=A0}

= =C2=A0 =C2=A0 =C2=A0}

-= -=C2=A0

2= .20.1.windows.1

= =C2=A0

=C2=A0

--uHFacSiHiQXprDGi3Iav--