From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from walk.intel-email.com (walk.intel-email.com [101.227.64.242]) by mx.groups.io with SMTP id smtpd.web10.2677.1665712652674160632 for ; Thu, 13 Oct 2022 18:57:33 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@byosoft.com.cn header.s=cloud-union header.b=rTOxhhAc; spf=pass (domain: byosoft.com.cn, ip: 101.227.64.242, mailfrom: gaoliming@byosoft.com.cn) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id E3543CD1F7CA for ; Fri, 14 Oct 2022 09:57:27 +0800 (CST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=byosoft.com.cn; s=cloud-union; t=1665712648; bh=xPJXe+DFj885HPIufabAioLDhyWE+aMUguABB7JiZz0=; h=From:To:References:In-Reply-To:Subject:Date; b=rTOxhhAcUeMW8F8+zHWA65rqv1Zj86rv8wIjWGAVx5mWgUgPTAF8SM2HRUicw/zgs IWggNW3qNSKnIJb6RcLj6h9GzxX7OV6/gYcLgTYOamC6aYpS4zf/5JQ6GeDo8M7Miw Yh2zoOsKJmVsxLFqHlJupNSK3P9tJuUATeWVqmOo= Received: from localhost (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id DE54BCD1F7A3 for ; Fri, 14 Oct 2022 09:57:27 +0800 (CST) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id 900F5CD1F770 for ; Fri, 14 Oct 2022 09:57:27 +0800 (CST) Authentication-Results: walk.intel-email.com; none Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by walk.intel-email.com (Postfix) with SMTP id 57B99CD1F7CA for ; Fri, 14 Oct 2022 09:57:24 +0800 (CST) Received: from DESKTOPS6D0PVI ([101.86.146.168]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Fri, 14 Oct 2022 09:57:21 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 101.86.146.168 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Jiading Zhang'" , References: <01df01d8dc49$1a7b2130$4f716390$@byosoft.com.cn> <28563.1665390985077437700@groups.io> In-Reply-To: <28563.1665390985077437700@groups.io> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0g5Zue5aSNOiBbZWRrMi1kZXZlbF0g5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIXSBNZGVNb2R1bGVQa2cgVmFyaWFibGVQZWk6IEFkZCBWYXJpYWJsZSBzdGF0ZSBjaGVjayB3aGVuIGZpbmQgdmFyaWFibGUgaW4gSW5kZXhUYWJsZS4=?= Date: Fri, 14 Oct 2022 09:57:23 +0800 Message-ID: <03d301d8df70$4d789930$e869cb90$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHz7EQdTw8ELNXB3a6vENYUgVlYsgN4UiSUrbtDR0A= Sender: "gaoliming" Content-Type: multipart/alternative; boundary="----=_NextPart_000_03D4_01D8DFB3.5B9D86E0" Content-Language: zh-cn ------=_NextPart_000_03D4_01D8DFB3.5B9D86E0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Jiading: HOB is created in volatile memory (cache or physical memory). And, HOB = is re-created for every boot (normal boot, S3 boot). When the first call = GetVariable, gEfiVariableIndexTableGuid guid hob should not exist. If = this guid hob exits, it should be created by other module. Please check. = =20 Thanks Liming =E5=8F=91=E4=BB=B6=E4=BA=BA: Jiading Zhang =20 =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: = 2022=E5=B9=B410=E6=9C=8810=E6=97=A5 16:36 =E6=94=B6=E4=BB=B6=E4=BA=BA: gaoliming ; = devel@edk2.groups.io =E4=B8=BB=E9=A2=98: Re: [edk2-devel] =E5=9B=9E=E5=A4=8D: [edk2-devel] = =E5=9B=9E=E5=A4=8D: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add = Variable state check when find variable in IndexTable. =20 Hi liming: I checked the code, and found the root cause of the issue. As the following code in edk2, in my code, after the first time = to read the variable in PEI phase, it cached the variable store info = IndexTable into a Hob in a special non-volatile memory, but after the = code running in dxe phase, the variable was changed. And after a warm = reboot, maybe call s3 sleep is more exactly, when read the variable, it = first get the IndexTable from the Hob but not build the IndexTable = again, so it read the deleted variable if hasn't the condition check. = This is really a special case. GuidHob =3D GetFirstGuidHob (&gEfiVariableIndexTableGuid); if (GuidHob !=3D NULL) { StoreInfo->IndexTable =3D GET_GUID_HOB_DATA (GuidHob); } else { // // If it's the first time to access variable region in flash, = create a guid hob to record // VAR_ADDED type variable info. // Note that as the resource of PEI phase is limited, only = store the limited number of // VAR_ADDED type variables to reduce access time. // StoreInfo->IndexTable =3D (VARIABLE_INDEX_TABLE *) = BuildGuidHob (&gEfiVariableIndexTableGuid, sizeof = (VARIABLE_INDEX_TABLE)); StoreInfo->IndexTable->Length =3D 0; StoreInfo->IndexTable->StartPtr =3D GetStartPointer = (VariableStoreHeader); StoreInfo->IndexTable->EndPtr =3D GetEndPointer = (VariableStoreHeader); StoreInfo->IndexTable->GoneThrough =3D 0; } =20 On Mon, Oct 10, 2022 at 09:39 AM, gaoliming wrote: Jiading:=20 Please check why NV variable data is required to be changed in PEI = phase. This will be helpful for this issue. =20 Thanks Liming =E5=8F=91=E4=BB=B6=E4=BA=BA: Jiading Zhang >=20 =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: = 2022=E5=B9=B410=E6=9C=8810=E6=97=A5 8:35 =E6=94=B6=E4=BB=B6=E4=BA=BA: gaoliming >; devel@edk2.groups.io = =20 =E4=B8=BB=E9=A2=98: Re: [edk2-devel] =E5=9B=9E=E5=A4=8D: [edk2-devel] = [PATCH] MdeModulePkg VariablePei: Add Variable state check when find = variable in IndexTable. =20 Hi liming: Yes, NV Variable Data is not changed in PEI phase in normal case. = This issue was found when we did a special coding, and when found = variable in the IndexTable, it found the variable before the last = changed if 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: Jiading: Hob Variable Store Info IndexTable is NULL. So, this logic = doesn=E2=80=99t 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 Thanks Liming =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=B49=E6=9C=8828=E6=97=A5 = 11:05 =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =20 =E4=B8=BB=E9=A2=98: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add = Variable state check when find variable in IndexTable. =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 = variable state, 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. =20 Signed-off-by: jdzhang > --- MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) =20 diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c = b/MdeModulePkg/Universal/Variable/Pei/Variable.c index 26a4c73b45..dffbd8cdb1 100644 --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c @@ -866,11 +866,13 @@ FindVariableEx ( Offset +=3D IndexTable->Index[Index]; MaxIndex =3D (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + = Offset); GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader); - if (CompareWithValidVariable (StoreInfo, MaxIndex, = VariableHeader, VariableName, VendorGuid, PtrTrack) =3D=3D EFI_SUCCESS) = { - if (VariableHeader->State =3D=3D (VAR_IN_DELETED_TRANSITION & = VAR_ADDED)) { - InDeletedVariable =3D PtrTrack->CurrPtr; - } else { - return EFI_SUCCESS; + if ((VariableHeader->State =3D=3D VAR_ADDED) || = (VariableHeader->State =3D=3D (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) = { + if (CompareWithValidVariable (StoreInfo, MaxIndex, = VariableHeader, VariableName, VendorGuid, PtrTrack) =3D=3D EFI_SUCCESS) = { + if (VariableHeader->State =3D=3D (VAR_IN_DELETED_TRANSITION & = VAR_ADDED)) { + InDeletedVariable =3D PtrTrack->CurrPtr; + } else { + return EFI_SUCCESS; + } } } } --=20 2.20.1.windows.1 =20 =20 ------=_NextPart_000_03D4_01D8DFB3.5B9D86E0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

Jiading:

=C2=A0HOB is = created in volatile memory (cache or physical memory). And, HOB is = re-created for every boot (normal boot, S3 boot). When the first call = GetVariable, gEfiVariableIndexTableGuid guid hob should not exist. If = this guid hob exits, it should be created by other module. Please check. =

 

Thanks

Liming

=E5=8F=91=E4=BB= =B6=E4=BA=BA: Jiading Zhang = <jdzhang@kunluntech.com.cn>
=E5=8F=91=E9=80= =81=E6=97=B6=E9=97=B4: = 2022=E5=B9=B410=E6=9C=8810=E6=97=A5 = 16:36
=E6=94=B6=E4=BB=B6=E4=BA=BA: gaoliming = <gaoliming@byosoft.com.cn>; = devel@edk2.groups.io
=E4=B8=BB=E9=A2=98: Re: [edk2-devel] = =E5=9B=9E=E5=A4=8D: [edk2-devel] = =E5=9B=9E=E5=A4=8D: [edk2-devel] [PATCH] = MdeModulePkg VariablePei: Add Variable state check when find variable in = IndexTable.

 

Hi = liming:
    I checked the code, and found the root cause of = the issue.
    As the following code in edk2,  =  in my code,  after the  first time to read the = variable in PEI phase,  it cached the variable store info  = IndexTable into  a Hob in a special non-volatile memory,  but = after the code running in dxe phase, the variable was changed.  And = after a warm reboot, maybe call s3 sleep is more exactly, when read the = variable, it  first get the IndexTable from the Hob but not build = the IndexTable again, so it read the deleted variable if hasn't the = condition check.  This is really a special = case.

      GuidHob =3D GetFirstGuidHob = (&gEfiVariableIndexTableGuid);

        if = (GuidHob !=3D NULL) {

          = StoreInfo->IndexTable =3D GET_GUID_HOB_DATA = (GuidHob);

        } else = {

          = //

          // If it's the first = time to access variable region in flash, create a guid hob to = record

          // VAR_ADDED type = variable info.

          = // Note that as the resource of PEI phase is limited, only store the = limited number of

          = // VAR_ADDED type variables to reduce access = time.

          = //

          StoreInfo->IndexTable = =3D (VARIABLE_INDEX_TABLE *) BuildGuidHob = (&gEfiVariableIndexTableGuid, sizeof = (VARIABLE_INDEX_TABLE));

          = StoreInfo->IndexTable->Length      =3D = 0;

          = StoreInfo->IndexTable->StartPtr    =3D GetStartPointer = (VariableStoreHeader);

          = StoreInfo->IndexTable->EndPtr      =3D = GetEndPointer  =  (VariableStoreHeader);

          = StoreInfo->IndexTable->GoneThrough =3D = 0;

      =   }

     

On Mon, Oct 10, 2022 at 09:39 = AM, gaoliming wrote:

Jiading: =

  Ple= ase check why NV variable data is required to be changed in PEI phase. = This will be helpful for this issue.

 

Thanks

Liming

=E5=8F=91=E4=BB= =B6=E4=BA=BA: = Jiading Zhang <jdzhang@kunluntech.com.cn&g= t;
=E5=8F=91=E9=80= =81=E6=97=B6=E9=97=B4: = 2022=E5=B9=B410=E6=9C=8810=E6=97=A5 = 8:35
=E6=94=B6=E4=BB=B6=E4=BA=BA: gaoliming = <gaoliming@byosoft.com.cn>= ; devel@edk2.groups.io
<= strong>=E4=B8=BB=E9=A2=98: Re: = [edk2-devel] =E5=9B=9E=E5=A4=8D: [edk2-devel] = [PATCH] MdeModulePkg VariablePei: Add Variable state check when find = variable in IndexTable.

 

Hi liming:
   Yes, = NV Variable Data is not changed in PEI phase in normal case. This issue = was found when we did a special coding, and when found variable in the = IndexTable, it found the variable before the last changed if  = 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:

Jiading:=

 Hob = Variable Store Info IndexTable is NULL. So, this logic doesn=E2=80=99t 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. =

 

Thanks

Liming

=E5=8F=91=E4=BB= =B6=E4=BA=BA: = devel@edk2.groups.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: = 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
<= strong>=E4=B8=BB=E9=A2=98: [edk2-devel] = [PATCH] MdeModulePkg VariablePei: Add Variable state check when find = variable in IndexTable.

 

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 variable = state, 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.

 

Signed-off-by: jdzhang <jdzhang@kunluntech.com.cn&g= t;

---

 MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 = +++++++-----

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

 

diff = --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c = b/MdeModulePkg/Universal/Variable/Pei/Variable.c

index 26a4c73b45..dffbd8cdb1 = 100644

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

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

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

      =  Offset  +=3D = IndexTable->Index[Index];

  =      MaxIndex =3D (VARIABLE_HEADER *)((UINT8 = *)IndexTable->StartPtr + Offset);

  =      GetVariableHeader (StoreInfo, MaxIndex, = &VariableHeader);

-      if (CompareWithValidVariable = (StoreInfo, MaxIndex, VariableHeader, VariableName, VendorGuid, = PtrTrack) =3D=3D EFI_SUCCESS) {

-        if (VariableHeader->State = =3D=3D (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) = {

-        =   InDeletedVariable =3D = PtrTrack->CurrPtr;

-        } else = {

-        =   return EFI_SUCCESS;

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

+        if = (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader, = VariableName, VendorGuid, PtrTrack) =3D=3D EFI_SUCCESS) = {

+        =   if (VariableHeader->State =3D=3D (VAR_IN_DELETED_TRANSITION = & VAR_ADDED)) {

+            = InDeletedVariable =3D = PtrTrack->CurrPtr;

+          } else = {

+        =     return EFI_SUCCESS;

+          = }

        =  }

      =  }

    =  }

-- 

2.20.1.windows.1

 

 

<= /div>
------=_NextPart_000_03D4_01D8DFB3.5B9D86E0--