public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
@ 2022-09-28  3:05 Jiading Zhang
  2022-09-30  2:46 ` 回复: [edk2-devel] " gaoliming
  2022-10-08  0:20 ` Jiading Zhang
  0 siblings, 2 replies; 10+ messages in thread
From: Jiading Zhang @ 2022-09-28  3:05 UTC (permalink / raw)
  To: devel


[-- Attachment #1.1: Type: text/plain, Size: 1780 bytes --]

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>
---
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  += IndexTable->Index[Index];
MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);
-      if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
-        if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) {
-          InDeletedVariable = PtrTrack->CurrPtr;
-        } else {
-          return EFI_SUCCESS;
+      if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
+        if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
+          if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) {
+            InDeletedVariable = PtrTrack->CurrPtr;
+          } else {
+            return EFI_SUCCESS;
+          }
}
}
}
--
2.20.1.windows.1

[-- Attachment #1.2: Type: text/html, Size: 2613 bytes --]

[-- Attachment #2: 0001-MdeModulePkg-VariablePei-Add-Variable-state-check-wh.patch --]
[-- Type: text/plain, Size: 2011 bytes --]

From d0a1e384ad04185cab915d2d577ac854169c3921 Mon Sep 17 00:00:00 2001
From: jdzhang <jdzhang@kunluntech.com.cn>
Date: Wed, 28 Sep 2022 10:57:47 +0800
Subject: [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>
---
 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  += IndexTable->Index[Index];
       MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
       GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);
-      if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
-        if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) {
-          InDeletedVariable = PtrTrack->CurrPtr;
-        } else {
-          return EFI_SUCCESS;
+      if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
+        if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
+          if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) {
+            InDeletedVariable = PtrTrack->CurrPtr;
+          } else {
+            return EFI_SUCCESS;
+          }
         }
       }
     }
-- 
2.20.1.windows.1


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

* 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-09-28  3:05 [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable Jiading Zhang
@ 2022-09-30  2:46 ` gaoliming
  2022-10-10  0:35   ` [edk2-devel] " Jiading Zhang
  2022-10-08  0:20 ` Jiading Zhang
  1 sibling, 1 reply; 10+ messages in thread
From: gaoliming @ 2022-09-30  2:46 UTC (permalink / raw)
  To: devel, jdzhang

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

Jiading:

 Hob Variable Store Info IndexTable is NULL. So, this logic doesn’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. 

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Jiading Zhang
发送时间: 2022年9月28日 11:05
收件人: devel@edk2.groups.io
主题: [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 <mailto:jdzhang@kunluntech.com.cn> >

---

 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  += IndexTable->Index[Index];

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

       GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);

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

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

-          InDeletedVariable = PtrTrack->CurrPtr;

-        } else {

-          return EFI_SUCCESS;

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

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

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

+            InDeletedVariable = PtrTrack->CurrPtr;

+          } else {

+            return EFI_SUCCESS;

+          }

         }

       }

     }

-- 

2.20.1.windows.1

 




[-- Attachment #2: Type: text/html, Size: 8499 bytes --]

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

* Re: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-09-28  3:05 [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable Jiading Zhang
  2022-09-30  2:46 ` 回复: [edk2-devel] " gaoliming
@ 2022-10-08  0:20 ` Jiading Zhang
  2022-10-08  1:29   ` 回复: " gaoliming
  1 sibling, 1 reply; 10+ messages in thread
From: Jiading Zhang @ 2022-10-08  0:20 UTC (permalink / raw)
  To: Jiading Zhang, devel

[-- Attachment #1: Type: text/plain, Size: 74 bytes --]

Hello,
Can anyone please help to review the patch?

Thanks very much!

[-- Attachment #2: Type: text/html, Size: 99 bytes --]

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

* 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-10-08  0:20 ` Jiading Zhang
@ 2022-10-08  1:29   ` gaoliming
  2022-10-08 10:05     ` [edk2-devel] " Jiading Zhang
  0 siblings, 1 reply; 10+ messages in thread
From: gaoliming @ 2022-10-08  1:29 UTC (permalink / raw)
  To: devel, jdzhang

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

Jiading:

 I have gave my comment on https://edk2.groups.io/g/devel/message/94555 for this patch. Have you got it?

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Jiading Zhang
发送时间: 2022年10月8日 8:21
收件人: Jiading Zhang <jdzhang@kunluntech.com.cn>; devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.

 

Hello, 
  Can anyone please help to review the patch?

Thanks very much! 




[-- Attachment #2: Type: text/html, Size: 4014 bytes --]

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

* Re: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-10-08  1:29   ` 回复: " gaoliming
@ 2022-10-08 10:05     ` Jiading Zhang
  0 siblings, 0 replies; 10+ messages in thread
From: Jiading Zhang @ 2022-10-08 10:05 UTC (permalink / raw)
  To: gaoliming, devel

[-- Attachment #1: Type: text/plain, Size: 81 bytes --]

Hi gaoliming:
I am sorry I missed it, I will take a look, thanks very much!
9

[-- Attachment #2: Type: text/html, Size: 208 bytes --]

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

* Re: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-09-30  2:46 ` 回复: [edk2-devel] " gaoliming
@ 2022-10-10  0:35   ` Jiading Zhang
  2022-10-10  1:39     ` 回复: " gaoliming
  0 siblings, 1 reply; 10+ messages in thread
From: Jiading Zhang @ 2022-10-10  0:35 UTC (permalink / raw)
  To: gaoliming, devel

[-- Attachment #1: Type: text/plain, Size: 3710 bytes --]

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 == VAR_ADDED) || (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"

Maybe our specail coding had some defect,and 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’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.
> 
> 
> 
> 
> 
> 
> 
> Thanks
> 
> 
> 
> Liming
> 
> 
> 
> *发件人 :* devel@edk2.groups.io <devel@edk2.groups.io> *代表* Jiading Zhang
> *发送时间 :* 2022 年 9 月 28 日 11:05
> *收件人 :* devel@edk2.groups.io
> *主题 :* [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 >
> 
> 
> 
> 
> ---
> 
> 
> 
> 
> 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  += IndexTable->Index[Index];
> 
> 
> 
> 
> MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
> 
> 
> 
> 
> GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);
> 
> 
> 
> 
> -      if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
> VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
> 
> 
> 
> 
> -        if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
> VAR_ADDED)) {
> 
> 
> 
> 
> -          InDeletedVariable = PtrTrack->CurrPtr;
> 
> 
> 
> 
> -        } else {
> 
> 
> 
> 
> -          return EFI_SUCCESS;
> 
> 
> 
> 
> +      if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State
> == (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
> 
> 
> 
> 
> +        if (CompareWithValidVariable (StoreInfo, MaxIndex,
> VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
> 
> 
> 
> 
> +          if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
> VAR_ADDED)) {
> 
> 
> 
> 
> +            InDeletedVariable = PtrTrack->CurrPtr;
> 
> 
> 
> 
> +          } else {
> 
> 
> 
> 
> +            return EFI_SUCCESS;
> 
> 
> 
> 
> +          }
> 
> 
> 
> 
> }
> 
> 
> 
> 
> }
> 
> 
> 
> 
> }
> 
> 
> 
> 
> --
> 
> 
> 
> 
> 2.20.1.windows.1
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>

[-- Attachment #2: Type: text/html, Size: 8633 bytes --]

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

* 回复: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-10-10  0:35   ` [edk2-devel] " Jiading Zhang
@ 2022-10-10  1:39     ` gaoliming
  2022-10-10  8:36       ` [edk2-devel] " Jiading Zhang
  0 siblings, 1 reply; 10+ messages in thread
From: gaoliming @ 2022-10-10  1:39 UTC (permalink / raw)
  To: 'Jiading Zhang', devel

[-- Attachment #1: Type: text/plain, Size: 3596 bytes --]

Jiading: 

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

 

Thanks

Liming

发件人: Jiading Zhang <jdzhang@kunluntech.com.cn> 
发送时间: 2022年10月10日 8:35
收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
主题: Re: [edk2-devel] 回复: [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 == VAR_ADDED) || (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"

Maybe our specail coding had some defect,and 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’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. 

 

Thanks

Liming

发件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>  <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > 代表 Jiading Zhang
发送时间: 2022年9月28日 11:05
收件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
主题: [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 <mailto:jdzhang@kunluntech.com.cn> >

---

 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  += IndexTable->Index[Index];

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

       GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);

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

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

-          InDeletedVariable = PtrTrack->CurrPtr;

-        } else {

-          return EFI_SUCCESS;

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

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

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

+            InDeletedVariable = PtrTrack->CurrPtr;

+          } else {

+            return EFI_SUCCESS;

+          }

         }

       }

     }

-- 

2.20.1.windows.1

 

 


[-- Attachment #2: Type: text/html, Size: 12717 bytes --]

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

* Re: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-10-10  1:39     ` 回复: " gaoliming
@ 2022-10-10  8:36       ` Jiading Zhang
  2022-10-14  1:57         ` 回复: " gaoliming
  0 siblings, 1 reply; 10+ messages in thread
From: Jiading Zhang @ 2022-10-10  8:36 UTC (permalink / raw)
  To: gaoliming, devel

[-- Attachment #1: Type: text/plain, Size: 6017 bytes --]

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 = GetFirstGuidHob (&gEfiVariableIndexTableGuid);
if (GuidHob != NULL) {
StoreInfo->IndexTable = 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 = (VARIABLE_INDEX_TABLE *) BuildGuidHob (&gEfiVariableIndexTableGuid, sizeof (VARIABLE_INDEX_TABLE));
StoreInfo->IndexTable->Length      = 0;
StoreInfo->IndexTable->StartPtr    = GetStartPointer (VariableStoreHeader);
StoreInfo->IndexTable->EndPtr      = GetEndPointer   (VariableStoreHeader);
StoreInfo->IndexTable->GoneThrough = 0;
}

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

> 
> 
> 
> Jiading:
> 
> 
> 
> Please check why NV variable data is required to be changed in PEI phase.
> This will be helpful for this issue.
> 
> 
> 
> 
> 
> 
> 
> Thanks
> 
> 
> 
> Liming
> 
> 
> 
> *发件人 :* Jiading Zhang <jdzhang@kunluntech.com.cn>
> *发送时间 :* 2022 年 10 月 10 日 8:35
> *收件人 :* gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> *主题 :* Re: [edk2-devel] 回复 : [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 == VAR_ADDED) || (VariableHeader->State ==
> (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"
> 
> Maybe our specail coding had some defect , and 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 ’ 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.
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> Thanks
>> 
>> 
>> 
>> Liming
>> 
>> 
>> 
>> *发件人 :* devel@edk2.groups.io < devel@edk2.groups.io > *代表* Jiading Zhang
>> *发送时间 :* 2022 年 9 月 28 日 11:05
>> *收件人 :* devel@edk2.groups.io
>> *主题 :* [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 >
>> 
>> 
>> 
>> 
>> ---
>> 
>> 
>> 
>> 
>> 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  += IndexTable->Index[Index];
>> 
>> 
>> 
>> 
>> MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
>> 
>> 
>> 
>> 
>> GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);
>> 
>> 
>> 
>> 
>> -      if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
>> VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>> 
>> 
>> 
>> 
>> -        if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
>> VAR_ADDED)) {
>> 
>> 
>> 
>> 
>> -          InDeletedVariable = PtrTrack->CurrPtr;
>> 
>> 
>> 
>> 
>> -        } else {
>> 
>> 
>> 
>> 
>> -          return EFI_SUCCESS;
>> 
>> 
>> 
>> 
>> +      if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State
>> == (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
>> 
>> 
>> 
>> 
>> +        if (CompareWithValidVariable (StoreInfo, MaxIndex,
>> VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>> 
>> 
>> 
>> 
>> +          if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
>> VAR_ADDED)) {
>> 
>> 
>> 
>> 
>> +            InDeletedVariable = PtrTrack->CurrPtr;
>> 
>> 
>> 
>> 
>> +          } else {
>> 
>> 
>> 
>> 
>> +            return EFI_SUCCESS;
>> 
>> 
>> 
>> 
>> +          }
>> 
>> 
>> 
>> 
>> }
>> 
>> 
>> 
>> 
>> }
>> 
>> 
>> 
>> 
>> }
>> 
>> 
>> 
>> 
>> --
>> 
>> 
>> 
>> 
>> 2.20.1.windows.1
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
> 
>

[-- Attachment #2: Type: text/html, Size: 13176 bytes --]

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

* 回复: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-10-10  8:36       ` [edk2-devel] " Jiading Zhang
@ 2022-10-14  1:57         ` gaoliming
  2022-10-14  2:29           ` [edk2-devel] " Jiading Zhang
  0 siblings, 1 reply; 10+ messages in thread
From: gaoliming @ 2022-10-14  1:57 UTC (permalink / raw)
  To: 'Jiading Zhang', devel

[-- Attachment #1: Type: text/plain, Size: 5999 bytes --]

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. 

 

Thanks

Liming

发件人: Jiading Zhang <jdzhang@kunluntech.com.cn> 
发送时间: 2022年10月10日 16:36
收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
主题: Re: [edk2-devel] 回复: [edk2-devel] 回复: [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 = GetFirstGuidHob (&gEfiVariableIndexTableGuid);

        if (GuidHob != NULL) {

          StoreInfo->IndexTable = 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 = (VARIABLE_INDEX_TABLE *) BuildGuidHob (&gEfiVariableIndexTableGuid, sizeof (VARIABLE_INDEX_TABLE));

          StoreInfo->IndexTable->Length      = 0;

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

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

          StoreInfo->IndexTable->GoneThrough = 0;

        }



     

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

Jiading: 

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

 

Thanks

Liming

发件人: Jiading Zhang <jdzhang@kunluntech.com.cn <mailto:jdzhang@kunluntech.com.cn> > 
发送时间: 2022年10月10日 8:35
收件人: gaoliming <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >; devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
主题: Re: [edk2-devel] 回复: [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 == VAR_ADDED) || (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"

Maybe our specail coding had some defect,and 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’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. 

 

Thanks

Liming

发件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>  <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > 代表 Jiading Zhang
发送时间: 2022年9月28日 11:05
收件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
主题: [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 <mailto:jdzhang@kunluntech.com.cn> >

---

 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  += IndexTable->Index[Index];

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

       GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);

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

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

-          InDeletedVariable = PtrTrack->CurrPtr;

-        } else {

-          return EFI_SUCCESS;

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

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

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

+            InDeletedVariable = PtrTrack->CurrPtr;

+          } else {

+            return EFI_SUCCESS;

+          }

         }

       }

     }

-- 

2.20.1.windows.1

 

 


[-- Attachment #2: Type: text/html, Size: 18886 bytes --]

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

* Re: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
  2022-10-14  1:57         ` 回复: " gaoliming
@ 2022-10-14  2:29           ` Jiading Zhang
  0 siblings, 0 replies; 10+ messages in thread
From: Jiading Zhang @ 2022-10-14  2:29 UTC (permalink / raw)
  To: gaoliming, devel

[-- Attachment #1: Type: text/plain, Size: 15339 bytes --]

Hi Liming:
My case is special, the HOB is created in a special memory, neither cache nor normal memory,  and it is non-volatile for a warm reboot, so the change works in my code. Maybe I should try to  create the hob in a volatile cache or memory, so I needn't change the open source code.

Thanks very much.
Jiading

On Fri, Oct 14, 2022 at 09:57 AM, gaoliming wrote:

> 
> 
> 
> 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.
> 
> 
> 
> 
> 
> 
> 
> Thanks
> 
> 
> 
> Liming
> 
> 
> 
> *发件人 :* Jiading Zhang <jdzhang@kunluntech.com.cn>
> *发送时间 :* 2022 年 10 月 10 日 16:36
> *收件人 :* gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> *主题 :* Re: [edk2-devel] 回复 : [edk2-devel] 回复 : [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 = GetFirstGuidHob (&gEfiVariableIndexTableGuid);
> 
> 
> 
> 
> if (GuidHob != NULL) {
> 
> 
> 
> 
> StoreInfo->IndexTable = 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 = (VARIABLE_INDEX_TABLE *) BuildGuidHob
> (&gEfiVariableIndexTableGuid, sizeof (VARIABLE_INDEX_TABLE));
> 
> 
> 
> 
> StoreInfo->IndexTable->Length      = 0;
> 
> 
> 
> 
> StoreInfo->IndexTable->StartPtr    = GetStartPointer
> (VariableStoreHeader);
> 
> 
> 
> 
> StoreInfo->IndexTable->EndPtr      = GetEndPointer  
> (VariableStoreHeader);
> 
> 
> 
> 
> StoreInfo->IndexTable->GoneThrough = 0;
> 
> 
> 
> 
> }
> 
> 
> 
> 
> 
> 
> 
> 
> On Mon, Oct 10, 2022 at 09:39 AM, gaoliming wrote:
> 
> 
>> 
>> 
>> Jiading:
>> 
>> 
>> 
>> Please check why NV variable data is required to be changed in PEI phase.
>> This will be helpful for this issue.
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> Thanks
>> 
>> 
>> 
>> Liming
>> 
>> 
>> 
>> *发件人 :* Jiading Zhang < jdzhang@kunluntech.com.cn >
>> *发送时间 :* 2022 年 10 月 10 日 8:35
>> *收件人 :* gaoliming < gaoliming@byosoft.com.cn >; devel@edk2.groups.io
>> *主题 :* Re: [edk2-devel] 回复 : [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 == VAR_ADDED) || (VariableHeader->State ==
>> (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"
>> 
>> Maybe our specail coding had some defect , and 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 ’ 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.
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> Thanks
>>> 
>>> 
>>> 
>>> Liming
>>> 
>>> 
>>> 
>>> *发件人 :* devel@edk2.groups.io < devel@edk2.groups.io > *代表* Jiading Zhang
>>> *发送时间 :* 2022 年 9 月 28 日 11:05
>>> *收件人 :* devel@edk2.groups.io
>>> *主题 :* [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 >
>>> 
>>> 
>>> 
>>> 
>>> ---
>>> 
>>> 
>>> 
>>> 
>>> 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  += IndexTable->Index[Index];
>>> 
>>> 
>>> 
>>> 
>>> MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
>>> 
>>> 
>>> 
>>> 
>>> GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);
>>> 
>>> 
>>> 
>>> 
>>> -      if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
>>> VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>>> 
>>> 
>>> 
>>> 
>>> -        if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
>>> VAR_ADDED)) {
>>> 
>>> 
>>> 
>>> 
>>> -          InDeletedVariable = PtrTrack->CurrPtr;
>>> 
>>> 
>>> 
>>> 
>>> -        } else {
>>> 
>>> 
>>> 
>>> 
>>> -          return EFI_SUCCESS;
>>> 
>>> 
>>> 
>>> 
>>> +      if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State
>>> == (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
>>> 
>>> 
>>> 
>>> 
>>> +        if (CompareWithValidVariable (StoreInfo, MaxIndex,
>>> VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>>> 
>>> 
>>> 
>>> 
>>> +          if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
>>> VAR_ADDED)) {
>>> 
>>> 
>>> 
>>> 
>>> +            InDeletedVariable = PtrTrack->CurrPtr;
>>> 
>>> 
>>> 
>>> 
>>> +          } else {
>>> 
>>> 
>>> 
>>> 
>>> +            return EFI_SUCCESS;
>>> 
>>> 
>>> 
>>> 
>>> +          }
>>> 
>>> 
>>> 
>>> 
>>> }
>>> 
>>> 
>>> 
>>> 
>>> }
>>> 
>>> 
>>> 
>>> 
>>> }
>>> 
>>> 
>>> 
>>> 
>>> --
>>> 
>>> 
>>> 
>>> 
>>> 2.20.1.windows.1
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>> 
>> 
> 
> 

On Fri, Oct 14, 2022 at 09:57 AM, gaoliming wrote:

> 
> 
> 
> 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.
> 
> 
> 
> 
> 
> 
> 
> Thanks
> 
> 
> 
> Liming
> 
> 
> 
> *发件人 :* Jiading Zhang <jdzhang@kunluntech.com.cn>
> *发送时间 :* 2022 年 10 月 10 日 16:36
> *收件人 :* gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> *主题 :* Re: [edk2-devel] 回复 : [edk2-devel] 回复 : [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 = GetFirstGuidHob (&gEfiVariableIndexTableGuid);
> 
> 
> 
> 
> if (GuidHob != NULL) {
> 
> 
> 
> 
> StoreInfo->IndexTable = 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 = (VARIABLE_INDEX_TABLE *) BuildGuidHob
> (&gEfiVariableIndexTableGuid, sizeof (VARIABLE_INDEX_TABLE));
> 
> 
> 
> 
> StoreInfo->IndexTable->Length      = 0;
> 
> 
> 
> 
> StoreInfo->IndexTable->StartPtr    = GetStartPointer
> (VariableStoreHeader);
> 
> 
> 
> 
> StoreInfo->IndexTable->EndPtr      = GetEndPointer  
> (VariableStoreHeader);
> 
> 
> 
> 
> StoreInfo->IndexTable->GoneThrough = 0;
> 
> 
> 
> 
> }
> 
> 
> 
> 
> 
> 
> 
> 
> On Mon, Oct 10, 2022 at 09:39 AM, gaoliming wrote:
> 
> 
>> 
>> 
>> Jiading:
>> 
>> 
>> 
>> Please check why NV variable data is required to be changed in PEI phase.
>> This will be helpful for this issue.
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> Thanks
>> 
>> 
>> 
>> Liming
>> 
>> 
>> 
>> *发件人 :* Jiading Zhang < jdzhang@kunluntech.com.cn >
>> *发送时间 :* 2022 年 10 月 10 日 8:35
>> *收件人 :* gaoliming < gaoliming@byosoft.com.cn >; devel@edk2.groups.io
>> *主题 :* Re: [edk2-devel] 回复 : [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 == VAR_ADDED) || (VariableHeader->State ==
>> (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"
>> 
>> Maybe our specail coding had some defect , and 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 ’ 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.
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> Thanks
>>> 
>>> 
>>> 
>>> Liming
>>> 
>>> 
>>> 
>>> *发件人 :* devel@edk2.groups.io < devel@edk2.groups.io > *代表* Jiading Zhang
>>> *发送时间 :* 2022 年 9 月 28 日 11:05
>>> *收件人 :* devel@edk2.groups.io
>>> *主题 :* [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 >
>>> 
>>> 
>>> 
>>> 
>>> ---
>>> 
>>> 
>>> 
>>> 
>>> 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  += IndexTable->Index[Index];
>>> 
>>> 
>>> 
>>> 
>>> MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
>>> 
>>> 
>>> 
>>> 
>>> GetVariableHeader (StoreInfo, MaxIndex, &VariableHeader);
>>> 
>>> 
>>> 
>>> 
>>> -      if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
>>> VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>>> 
>>> 
>>> 
>>> 
>>> -        if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
>>> VAR_ADDED)) {
>>> 
>>> 
>>> 
>>> 
>>> -          InDeletedVariable = PtrTrack->CurrPtr;
>>> 
>>> 
>>> 
>>> 
>>> -        } else {
>>> 
>>> 
>>> 
>>> 
>>> -          return EFI_SUCCESS;
>>> 
>>> 
>>> 
>>> 
>>> +      if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State
>>> == (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
>>> 
>>> 
>>> 
>>> 
>>> +        if (CompareWithValidVariable (StoreInfo, MaxIndex,
>>> VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>>> 
>>> 
>>> 
>>> 
>>> +          if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
>>> VAR_ADDED)) {
>>> 
>>> 
>>> 
>>> 
>>> +            InDeletedVariable = PtrTrack->CurrPtr;
>>> 
>>> 
>>> 
>>> 
>>> +          } else {
>>> 
>>> 
>>> 
>>> 
>>> +            return EFI_SUCCESS;
>>> 
>>> 
>>> 
>>> 
>>> +          }
>>> 
>>> 
>>> 
>>> 
>>> }
>>> 
>>> 
>>> 
>>> 
>>> }
>>> 
>>> 
>>> 
>>> 
>>> }
>>> 
>>> 
>>> 
>>> 
>>> --
>>> 
>>> 
>>> 
>>> 
>>> 2.20.1.windows.1
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>> 
>> 
> 
>

[-- Attachment #2: Type: text/html, Size: 35507 bytes --]

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

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

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-28  3:05 [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable Jiading Zhang
2022-09-30  2:46 ` 回复: [edk2-devel] " gaoliming
2022-10-10  0:35   ` [edk2-devel] " Jiading Zhang
2022-10-10  1:39     ` 回复: " gaoliming
2022-10-10  8:36       ` [edk2-devel] " Jiading Zhang
2022-10-14  1:57         ` 回复: " gaoliming
2022-10-14  2:29           ` [edk2-devel] " Jiading Zhang
2022-10-08  0:20 ` Jiading Zhang
2022-10-08  1:29   ` 回复: " gaoliming
2022-10-08 10:05     ` [edk2-devel] " Jiading Zhang

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