public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT
@ 2018-03-01  7:14 Dandan Bi
  2018-03-01  7:14 ` [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98 Dandan Bi
  2018-03-02  5:43 ` [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT Gao, Liming
  0 siblings, 2 replies; 5+ messages in thread
From: Dandan Bi @ 2018-03-01  7:14 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Star Zeng

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



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

* [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98
  2018-03-01  7:14 [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT Dandan Bi
@ 2018-03-01  7:14 ` Dandan Bi
  2018-03-02  5:23   ` Gao, Liming
  2018-03-02  5:43 ` [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT Gao, Liming
  1 sibling, 1 reply; 5+ messages in thread
From: Dandan Bi @ 2018-03-01  7:14 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Star Zeng, Ruiyu Ni

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



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

* Re: [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98
  2018-03-01  7:14 ` [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98 Dandan Bi
@ 2018-03-02  5:23   ` Gao, Liming
  2018-03-02  5:26     ` Zeng, Star
  0 siblings, 1 reply; 5+ messages in thread
From: Gao, Liming @ 2018-03-02  5:23 UTC (permalink / raw)
  To: Bi, Dandan, edk2-devel@lists.01.org; +Cc: Zeng, Star, Ni, Ruiyu

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



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

* Re: [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98
  2018-03-02  5:23   ` Gao, Liming
@ 2018-03-02  5:26     ` Zeng, Star
  0 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2018-03-02  5:26 UTC (permalink / raw)
  To: Gao, Liming, Bi, Dandan, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Zeng, Star

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



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

* Re: [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT
  2018-03-01  7:14 [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT Dandan Bi
  2018-03-01  7:14 ` [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98 Dandan Bi
@ 2018-03-02  5:43 ` Gao, Liming
  1 sibling, 0 replies; 5+ messages in thread
From: Gao, Liming @ 2018-03-02  5:43 UTC (permalink / raw)
  To: Bi, Dandan, edk2-devel@lists.01.org; +Cc: Zeng, Star

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


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

end of thread, other threads:[~2018-03-02  5:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-01  7:14 [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT Dandan Bi
2018-03-01  7:14 ` [patch] MdeModulePkg: Fix incorrect commit introduced by commit SHA-1:052c98 Dandan Bi
2018-03-02  5:23   ` Gao, Liming
2018-03-02  5:26     ` Zeng, Star
2018-03-02  5:43 ` [patch] MdeModulePkg/DxeCorePerfLib: Add status check instead of ASSERT Gao, Liming

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