[edk2,edk2-platforms,v1,04/16] Hisilicon/D06: Fix access variable fail issue

Message ID 20190201133436.10500-5-ming.huang@linaro.org
State Superseded
Headers show
Series
  • Fix issues and improve D0x
Related show

Commit Message

Ming Huang Feb. 1, 2019, 1:34 p.m.
From: Jason Zhang <zhangjinsong2@huawei.com>


BmcWdtEnable is a field of OemConfigData structure, need have
runtime service attribution if use it during exit boot service

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>

---
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr | 2 +-
 Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.9.5

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Feb. 12, 2019, 3:17 p.m. | #1
On Fri, Feb 01, 2019 at 09:34:24PM +0800, Ming Huang wrote:
> From: Jason Zhang <zhangjinsong2@huawei.com>

> 

> BmcWdtEnable is a field of OemConfigData structure, need have

> runtime service attribution if use it during exit boot service


This sounds like a very shady thing to do.
Which module is seeing issues, and what issues are it seeing, during
ExitBootServices?

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ming Huang <ming.huang@linaro.org>

> ---

>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr | 2 +-

>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c      | 2 +-

>  2 files changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr

> index 470e9ace3dcf..08236704fbfe 100644

> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr

> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr

> @@ -23,7 +23,7 @@ formset

>    help      = STRING_TOKEN(STR_OEM_CONFIG),

>    classguid = gEfiIfrFrontPageGuid,  // for MdeModule Bds.

>    efivarstore OEM_CONFIG_DATA,

> -    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,

> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS,

>      name  = OemConfig,

>      guid  = gOemConfigGuid;

>  

> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c

> index 012d45bc0214..6668103af027 100644

> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c

> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c

> @@ -316,7 +316,7 @@ OemConfigUiLibConstructor (

>        Status = gRT->SetVariable (

>                        OEM_CONFIG_NAME,

>                        &gOemConfigGuid,

> -                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,

> +                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,

>                        sizeof (OEM_CONFIG_DATA),

>                        &Configuration

>                        );

> -- 

> 2.9.5

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ming Huang Feb. 13, 2019, 2:21 a.m. | #2
On 2/12/2019 11:17 PM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:24PM +0800, Ming Huang wrote:

>> From: Jason Zhang <zhangjinsong2@huawei.com>

>>

>> BmcWdtEnable is a field of OemConfigData structure, need have

>> runtime service attribution if use it during exit boot service

> 

> This sounds like a very shady thing to do.

> Which module is seeing issues, and what issues are it seeing, during

> ExitBootServices?


Yes,WatchDog module read the OemConfigData.BmcWdtEnable during ExitBootServices
and will get fail log before boot kernel:

Get Variable failed. Status Not Found
[    0.000000] Booting Linux on physical CPU 0x0000010000 [0x480fd010]

Thanks.

> 

> /

>     Leif

> 

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Ming Huang <ming.huang@linaro.org>

>> ---

>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr | 2 +-

>>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c      | 2 +-

>>  2 files changed, 2 insertions(+), 2 deletions(-)

>>

>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr

>> index 470e9ace3dcf..08236704fbfe 100644

>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr

>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr

>> @@ -23,7 +23,7 @@ formset

>>    help      = STRING_TOKEN(STR_OEM_CONFIG),

>>    classguid = gEfiIfrFrontPageGuid,  // for MdeModule Bds.

>>    efivarstore OEM_CONFIG_DATA,

>> -    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,

>> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS,

>>      name  = OemConfig,

>>      guid  = gOemConfigGuid;

>>  

>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c

>> index 012d45bc0214..6668103af027 100644

>> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c

>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c

>> @@ -316,7 +316,7 @@ OemConfigUiLibConstructor (

>>        Status = gRT->SetVariable (

>>                        OEM_CONFIG_NAME,

>>                        &gOemConfigGuid,

>> -                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,

>> +                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,

>>                        sizeof (OEM_CONFIG_DATA),

>>                        &Configuration

>>                        );

>> -- 

>> 2.9.5

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
index 470e9ace3dcf..08236704fbfe 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.vfr
@@ -23,7 +23,7 @@  formset
   help      = STRING_TOKEN(STR_OEM_CONFIG),
   classguid = gEfiIfrFrontPageGuid,  // for MdeModule Bds.
   efivarstore OEM_CONFIG_DATA,
-    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS,
     name  = OemConfig,
     guid  = gOemConfigGuid;
 
diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
index 012d45bc0214..6668103af027 100644
--- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
+++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
@@ -316,7 +316,7 @@  OemConfigUiLibConstructor (
       Status = gRT->SetVariable (
                       OEM_CONFIG_NAME,
                       &gOemConfigGuid,
-                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+                      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
                       sizeof (OEM_CONFIG_DATA),
                       &Configuration
                       );