[edk2] [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT

Dandan Bi posted 1 patch 6 years, 9 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
[edk2] [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT
Posted by Dandan Bi 6 years, 9 months ago
Currently DxeCorePerformanceLib will get SMM performance data based
on SMM communication handler. If SMM communication handler returns error,
the library will ASSERT. In fact, if SMM perf data is not found.
DXE perf data can still be dumped. So using status check instead of
ASSERT is better.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 9b3224e..71d624f 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -281,13 +281,12 @@ AllocateBootPerformanceTable (
         // Get the size of boot records.
         //
         SmmCommData->Function       = SMM_FPDT_FUNCTION_GET_BOOT_RECORD_SIZE;
         SmmCommData->BootRecordData = NULL;
         Status = Communication->Communicate (Communication, SmmBootRecordCommBuffer, &CommSize);
-        ASSERT_EFI_ERROR (Status);
 
-        if (!EFI_ERROR (SmmCommData->ReturnStatus) && SmmCommData->BootRecordSize != 0) {
+        if (!EFI_ERROR (Status) && !EFI_ERROR (SmmCommData->ReturnStatus) && SmmCommData->BootRecordSize != 0) {
           //
           // Get all boot records
           //
           SmmCommData->Function       = SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET;
           SmmBootRecordDataSize       = SmmCommData->BootRecordSize;
@@ -315,11 +314,11 @@ AllocateBootPerformanceTable (
   //
   // Prepare memory for Boot Performance table.
   // Boot Performance table includes BasicBoot record, and one or more appended Boot Records.
   //
   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
-  if (SmmCommData != NULL) {
+  if (SmmCommData != NULL && SmmBootRecordData != NULL) {
     BootPerformanceDataSize += SmmBootRecordDataSize;
   }
 
   //
   // Try to allocate the same runtime buffer as last time boot.
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT
Posted by Gao, Liming 6 years, 9 months ago
Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dandan Bi
> Sent: Thursday, March 1, 2018 3:14 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT
> 
> Currently DxeCorePerformanceLib will get SMM performance data based
> on SMM communication handler. If SMM communication handler returns error,
> the library will ASSERT. In fact, if SMM perf data is not found.
> DXE perf data can still be dumped. So using status check instead of
> ASSERT is better.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> index 9b3224e..71d624f 100644
> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> @@ -281,13 +281,12 @@ AllocateBootPerformanceTable (
>          // Get the size of boot records.
>          //
>          SmmCommData->Function       = SMM_FPDT_FUNCTION_GET_BOOT_RECORD_SIZE;
>          SmmCommData->BootRecordData = NULL;
>          Status = Communication->Communicate (Communication, SmmBootRecordCommBuffer, &CommSize);
> -        ASSERT_EFI_ERROR (Status);
> 
> -        if (!EFI_ERROR (SmmCommData->ReturnStatus) && SmmCommData->BootRecordSize != 0) {
> +        if (!EFI_ERROR (Status) && !EFI_ERROR (SmmCommData->ReturnStatus) && SmmCommData->BootRecordSize != 0) {
>            //
>            // Get all boot records
>            //
>            SmmCommData->Function       = SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET;
>            SmmBootRecordDataSize       = SmmCommData->BootRecordSize;
> @@ -315,11 +314,11 @@ AllocateBootPerformanceTable (
>    //
>    // Prepare memory for Boot Performance table.
>    // Boot Performance table includes BasicBoot record, and one or more appended Boot Records.
>    //
>    BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32
> (PcdExtFpdtBootRecordPadSize);
> -  if (SmmCommData != NULL) {
> +  if (SmmCommData != NULL && SmmBootRecordData != NULL) {
>      BootPerformanceDataSize += SmmBootRecordDataSize;
>    }
> 
>    //
>    // Try to allocate the same runtime buffer as last time boot.
> --
> 1.9.5.msysgit.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98
Posted by Dandan Bi 6 years, 9 months ago
The default value of PcdExtFpdtBootRecordPadSize is 0x20000
But the following commit in master update it to 0 by mistake.
SHA-1: 052c98ce246a1ffb0b4c5185a644aa9f902650f7
Subject: MdeModulePkg: Add ResetSystemPei PEIM

This patch is to restore the value.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index ba05859..d1b1af5 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1743,11 +1743,11 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosEntryPointProvideMethod|0x3|UINT32|0x00010069
 
   ## This PCD specifies the additional pad size in FPDT Basic Boot Performance Table for
   #  the extension FPDT boot records received after ReadyToBoot and before ExitBootService.
   # @Prompt Pad size for extension FPDT boot records.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x0|UINT32|0x0001005F
+  gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x20000|UINT32|0x0001005F
 
   ## Indicates if ConIn device are connected on demand.<BR><BR>
   #   TRUE  - ConIn device are not connected during BDS and ReadKeyStroke/ReadKeyStrokeEx produced
   #           by Consplitter should be called before any real key read operation.<BR>
   #   FALSE - ConIn device may be connected normally during BDS.<BR>
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98
Posted by Gao, Liming 6 years, 9 months ago
Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Thursday, March 1, 2018 3:14 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98
> 
> The default value of PcdExtFpdtBootRecordPadSize is 0x20000
> But the following commit in master update it to 0 by mistake.
> SHA-1: 052c98ce246a1ffb0b4c5185a644aa9f902650f7
> Subject: MdeModulePkg: Add ResetSystemPei PEIM
> 
> This patch is to restore the value.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index ba05859..d1b1af5 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1743,11 +1743,11 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosEntryPointProvideMethod|0x3|UINT32|0x00010069
> 
>    ## This PCD specifies the additional pad size in FPDT Basic Boot Performance Table for
>    #  the extension FPDT boot records received after ReadyToBoot and before ExitBootService.
>    # @Prompt Pad size for extension FPDT boot records.
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x0|UINT32|0x0001005F
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x20000|UINT32|0x0001005F
> 
>    ## Indicates if ConIn device are connected on demand.<BR><BR>
>    #   TRUE  - ConIn device are not connected during BDS and ReadKeyStroke/ReadKeyStrokeEx produced
>    #           by Consplitter should be called before any real key read operation.<BR>
>    #   FALSE - ConIn device may be connected normally during BDS.<BR>
> --
> 1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98
Posted by Zeng, Star 6 years, 9 months ago
Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks,
Star
-----Original Message-----
From: Gao, Liming 
Sent: Friday, March 2, 2018 1:23 PM
To: Bi, Dandan <dandan.bi@intel.com>; edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: RE: [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Thursday, March 1, 2018 3:14 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Zeng, Star 
> <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [patch] MdeModulePkg: Fix incorrect commit introduced by 
> commit SHA-1:052c98
> 
> The default value of PcdExtFpdtBootRecordPadSize is 0x20000 But the 
> following commit in master update it to 0 by mistake.
> SHA-1: 052c98ce246a1ffb0b4c5185a644aa9f902650f7
> Subject: MdeModulePkg: Add ResetSystemPei PEIM
> 
> This patch is to restore the value.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec 
> b/MdeModulePkg/MdeModulePkg.dec index ba05859..d1b1af5 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1743,11 +1743,11 @@
>    
> gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosEntryPointProvideMethod|0x3|UI
> NT32|0x00010069
> 
>    ## This PCD specifies the additional pad size in FPDT Basic Boot Performance Table for
>    #  the extension FPDT boot records received after ReadyToBoot and before ExitBootService.
>    # @Prompt Pad size for extension FPDT boot records.
> -  
> gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x0|UINT32|
> 0x0001005F
> +  
> + gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x20000|U
> + INT32|0x0001005F
> 
>    ## Indicates if ConIn device are connected on demand.<BR><BR>
>    #   TRUE  - ConIn device are not connected during BDS and ReadKeyStroke/ReadKeyStrokeEx produced
>    #           by Consplitter should be called before any real key read operation.<BR>
>    #   FALSE - ConIn device may be connected normally during BDS.<BR>
> --
> 1.9.5.msysgit.1

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