public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
@ 2020-06-17  7:35 Tan, Ming
  2020-06-17 14:42 ` Wang, Jian J
  0 siblings, 1 reply; 7+ messages in thread
From: Tan, Ming @ 2020-06-17  7:35 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Hao A Wu

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2786

In order to support enable/disable report status code through memory
or serial dynamic, change the following PCDs from [PcdsFeatureFlag] to
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]:
  PcdStatusCodeUseSerial
  PcdStatusCodeUseMemory
The original plaforms can use PcdsFixedAtBuild in .dsc files to save size.
Currently do not remove the old pcd type to keep compatible.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Ming Tan <ming.tan@intel.com>
---
V5: Do not remove th old pcd type to keep compatible, and change to standalone patch.
V4: No change for this 1/4 patch, just modify the 2-4/4 patchs.
V3: Split one patch to several patchs, each Pkg has one patch.
V2: Change the new type from [PcdsDynamic] to [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
    And set to PcdsFixedAtBuild in the original platform .dsc files.

 MdeModulePkg/MdeModulePkg.dec                    | 13 +++++++++++++
 .../StatusCodeHandler/Pei/StatusCodeHandlerPei.c |  6 +++---
 .../Pei/StatusCodeHandlerPei.inf                 |  6 ++----
 .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c     | 16 ++++++++--------
 .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf   |  6 ++----
 .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.c | 10 +++++-----
 .../Smm/StatusCodeHandlerSmm.inf                 |  6 ++----
 7 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 4f44af694862..bd369d1f230e 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2001,6 +2001,19 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   # @Prompt TCG Platform Firmware Profile revision.
   gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision|0|UINT32|0x00010077
 
+  ## Indicates if StatusCode is reported via Serial port.<BR><BR>
+  #   TRUE  - Reports StatusCode via Serial port.<BR>
+  #   FALSE - Does not report StatusCode via Serial port.<BR>
+  # @Prompt Enable StatusCode via Serial port.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE|BOOLEAN|0x00010022
+
+  ## Indicates if StatusCode is stored in memory.
+  #  The memory is boot time memory in PEI Phase and is runtime memory in DXE Phase.<BR><BR>
+  #   TRUE  - Stores StatusCode in memory.<BR>
+  #   FALSE - Does not store StatusCode in memory.<BR>
+  # @Prompt Enable StatusCode via memory.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE|BOOLEAN|0x00010023
+
 [PcdsPatchableInModule]
   ## Specify memory size with page number for PEI code when
   #  Loading Module at Fixed Address feature is enabled.
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c
index 1b07f92f3ce8..9b2ea4ee84d9 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c
@@ -2,7 +2,7 @@
   Report Status Code Handler PEIM which produces general handlers and hook them
   onto the PEI status code router.
 
-  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -45,13 +45,13 @@ StatusCodeHandlerPeiEntry (
   // If enable UseSerial, then initialize serial port.
   // if enable UseMemory, then initialize memory status code worker.
   //
-  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
+  if (PcdGetBool (PcdStatusCodeUseSerial)) {
     Status = SerialPortInitialize();
     ASSERT_EFI_ERROR (Status);
     Status = RscHandlerPpi->Register (SerialStatusCodeReportWorker);
     ASSERT_EFI_ERROR (Status);
   }
-  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
+  if (PcdGetBool (PcdStatusCodeUseMemory)) {
     Status = MemoryStatusCodeInitializeWorker ();
     ASSERT_EFI_ERROR (Status);
     Status = RscHandlerPpi->Register (MemoryStatusCodeReportWorker);
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
index 8aef9af34a05..64380ddfaccc 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
+++ b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
@@ -53,11 +53,9 @@ [Guids]
 [Ppis]
   gEfiPeiRscHandlerPpiGuid                      ## CONSUMES
 
-[FeaturePcd]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
-
 [Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1|gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory    ## SOMETIMES_CONSUMES
 
 [Depex]
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c
index 79cc48fa55a4..a8c0fe5b7149 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c
@@ -2,7 +2,7 @@
   Status Code Handler Driver which produces general handlers and hook them
   onto the DXE status code router.
 
-  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -29,7 +29,7 @@ UnregisterBootTimeHandlers (
   IN VOID             *Context
   )
 {
-  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
+  if (PcdGetBool (PcdStatusCodeUseSerial)) {
     mRscHandlerProtocol->Unregister (SerialStatusCodeReportWorker);
   }
 }
@@ -80,14 +80,14 @@ InitializationDispatcherWorker (
   // If enable UseSerial, then initialize serial port.
   // if enable UseRuntimeMemory, then initialize runtime memory status code worker.
   //
-  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
+  if (PcdGetBool (PcdStatusCodeUseSerial)) {
     //
     // Call Serial Port Lib API to initialize serial port.
     //
     Status = SerialPortInitialize ();
     ASSERT_EFI_ERROR (Status);
   }
-  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
+  if (PcdGetBool (PcdStatusCodeUseMemory)) {
     Status = RtMemoryStatusCodeInitializeWorker ();
     ASSERT_EFI_ERROR (Status);
   }
@@ -115,7 +115,7 @@ InitializationDispatcherWorker (
         //
         // Dispatch records to devices based on feature flag.
         //
-        if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
+        if (PcdGetBool (PcdStatusCodeUseSerial)) {
           SerialStatusCodeReportWorker (
             Record[Index].CodeType,
             Record[Index].Value,
@@ -124,7 +124,7 @@ InitializationDispatcherWorker (
             NULL
             );
         }
-        if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
+        if (PcdGetBool (PcdStatusCodeUseMemory)) {
           RtMemoryStatusCodeReportWorker (
             Record[Index].CodeType,
             Record[Index].Value,
@@ -171,10 +171,10 @@ StatusCodeHandlerRuntimeDxeEntry (
   //
   InitializationDispatcherWorker ();
 
-  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
+  if (PcdGetBool (PcdStatusCodeUseSerial)) {
     mRscHandlerProtocol->Register (SerialStatusCodeReportWorker, TPL_HIGH_LEVEL);
   }
-  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
+  if (PcdGetBool (PcdStatusCodeUseMemory)) {
     mRscHandlerProtocol->Register (RtMemoryStatusCodeReportWorker, TPL_HIGH_LEVEL);
   }
 
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
index d74c2a55dcaf..faadfd9578fe 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
@@ -58,12 +58,10 @@ [Guids]
 [Protocols]
   gEfiRscHandlerProtocolGuid                    ## CONSUMES
 
-[FeaturePcd]
+[Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeReplayIn  ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
-
-[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize |128| gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory   ## SOMETIMES_CONSUMES
 
 [Depex]
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.c b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.c
index f54991ed3f67..20271571ded4 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.c
@@ -2,7 +2,7 @@
   Status Code Handler Driver which produces general handlers and hook them
   onto the SMM status code router.
 
-  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -28,14 +28,14 @@ InitializationDispatcherWorker (
   // If enable UseSerial, then initialize serial port.
   // if enable UseRuntimeMemory, then initialize runtime memory status code worker.
   //
-  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
+  if (PcdGetBool (PcdStatusCodeUseSerial)) {
     //
     // Call Serial Port Lib API to initialize serial port.
     //
     Status = SerialPortInitialize ();
     ASSERT_EFI_ERROR (Status);
   }
-  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
+  if (PcdGetBool (PcdStatusCodeUseMemory)) {
     Status = MemoryStatusCodeInitializeWorker ();
     ASSERT_EFI_ERROR (Status);
   }
@@ -73,10 +73,10 @@ StatusCodeHandlerSmmEntry (
   //
   InitializationDispatcherWorker ();
 
-  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
+  if (PcdGetBool (PcdStatusCodeUseSerial)) {
     mRscHandlerProtocol->Register (SerialStatusCodeReportWorker);
   }
-  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
+  if (PcdGetBool (PcdStatusCodeUseMemory)) {
     mRscHandlerProtocol->Register (MemoryStatusCodeReportWorker);
   }
 
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf
index 47d0545f9591..4e24d87e55d1 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf
+++ b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf
@@ -53,11 +53,9 @@ [Guids]
 [Protocols]
   gEfiSmmRscHandlerProtocolGuid                 ## CONSUMES
 
-[FeaturePcd]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
-
 [Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize |128| gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory   ## SOMETIMES_CONSUMES
 
 [Depex]
-- 
2.24.0.windows.2


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

* Re: [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
  2020-06-17  7:35 [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code Tan, Ming
@ 2020-06-17 14:42 ` Wang, Jian J
  2020-06-17 16:47   ` [edk2-devel] " Michael D Kinney
  0 siblings, 1 reply; 7+ messages in thread
From: Wang, Jian J @ 2020-06-17 14:42 UTC (permalink / raw)
  To: Tan, Ming, devel@edk2.groups.io; +Cc: Wu, Hao A

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----Original Message-----
> From: Tan, Ming <ming.tan@intel.com>
> Sent: Wednesday, June 17, 2020 3:36 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2786
> 
> In order to support enable/disable report status code through memory
> or serial dynamic, change the following PCDs from [PcdsFeatureFlag] to
> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]:
>   PcdStatusCodeUseSerial
>   PcdStatusCodeUseMemory
> The original plaforms can use PcdsFixedAtBuild in .dsc files to save size.
> Currently do not remove the old pcd type to keep compatible.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Ming Tan <ming.tan@intel.com>
> ---
> V5: Do not remove th old pcd type to keep compatible, and change to
> standalone patch.
> V4: No change for this 1/4 patch, just modify the 2-4/4 patchs.
> V3: Split one patch to several patchs, each Pkg has one patch.
> V2: Change the new type from [PcdsDynamic] to [PcdsFixedAtBuild,
> PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>     And set to PcdsFixedAtBuild in the original platform .dsc files.
> 
>  MdeModulePkg/MdeModulePkg.dec                    | 13 +++++++++++++
>  .../StatusCodeHandler/Pei/StatusCodeHandlerPei.c |  6 +++---
>  .../Pei/StatusCodeHandlerPei.inf                 |  6 ++----
>  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c     | 16 ++++++++--------
>  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf   |  6 ++----
>  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.c | 10 +++++-----
>  .../Smm/StatusCodeHandlerSmm.inf                 |  6 ++----
>  7 files changed, 35 insertions(+), 28 deletions(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 4f44af694862..bd369d1f230e 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2001,6 +2001,19 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic, PcdsDynamicEx]
>    # @Prompt TCG Platform Firmware Profile revision.
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision|0|UINT3
> 2|0x00010077
> 
> +  ## Indicates if StatusCode is reported via Serial port.<BR><BR>
> +  #   TRUE  - Reports StatusCode via Serial port.<BR>
> +  #   FALSE - Does not report StatusCode via Serial port.<BR>
> +  # @Prompt Enable StatusCode via Serial port.
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE|BOOLEAN|
> 0x00010022
> +
> +  ## Indicates if StatusCode is stored in memory.
> +  #  The memory is boot time memory in PEI Phase and is runtime memory in
> DXE Phase.<BR><BR>
> +  #   TRUE  - Stores StatusCode in memory.<BR>
> +  #   FALSE - Does not store StatusCode in memory.<BR>
> +  # @Prompt Enable StatusCode via memory.
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE|BOOLEA
> N|0x00010023
> +
>  [PcdsPatchableInModule]
>    ## Specify memory size with page number for PEI code when
>    #  Loading Module at Fixed Address feature is enabled.
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c
> index 1b07f92f3ce8..9b2ea4ee84d9 100644
> --- a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.c
> @@ -2,7 +2,7 @@
>    Report Status Code Handler PEIM which produces general handlers and hook
> them
>    onto the PEI status code router.
> 
> -  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -45,13 +45,13 @@ StatusCodeHandlerPeiEntry (
>    // If enable UseSerial, then initialize serial port.
>    // if enable UseMemory, then initialize memory status code worker.
>    //
> -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
>      Status = SerialPortInitialize();
>      ASSERT_EFI_ERROR (Status);
>      Status = RscHandlerPpi->Register (SerialStatusCodeReportWorker);
>      ASSERT_EFI_ERROR (Status);
>    }
> -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
>      Status = MemoryStatusCodeInitializeWorker ();
>      ASSERT_EFI_ERROR (Status);
>      Status = RscHandlerPpi->Register (MemoryStatusCodeReportWorker);
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
> index 8aef9af34a05..64380ddfaccc 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
> @@ -53,11 +53,9 @@ [Guids]
>  [Ppis]
>    gEfiPeiRscHandlerPpiGuid                      ## CONSUMES
> 
> -[FeaturePcd]
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> -
>  [Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> CONSUMES
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1|gEfiMdeMod
> ulePkgTokenSpaceGuid.PcdStatusCodeUseMemory    ##
> SOMETIMES_CONSUMES
> 
>  [Depex]
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.c
> index 79cc48fa55a4..a8c0fe5b7149 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.c
> @@ -2,7 +2,7 @@
>    Status Code Handler Driver which produces general handlers and hook them
>    onto the DXE status code router.
> 
> -  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -29,7 +29,7 @@ UnregisterBootTimeHandlers (
>    IN VOID             *Context
>    )
>  {
> -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
>      mRscHandlerProtocol->Unregister (SerialStatusCodeReportWorker);
>    }
>  }
> @@ -80,14 +80,14 @@ InitializationDispatcherWorker (
>    // If enable UseSerial, then initialize serial port.
>    // if enable UseRuntimeMemory, then initialize runtime memory status code
> worker.
>    //
> -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
>      //
>      // Call Serial Port Lib API to initialize serial port.
>      //
>      Status = SerialPortInitialize ();
>      ASSERT_EFI_ERROR (Status);
>    }
> -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
>      Status = RtMemoryStatusCodeInitializeWorker ();
>      ASSERT_EFI_ERROR (Status);
>    }
> @@ -115,7 +115,7 @@ InitializationDispatcherWorker (
>          //
>          // Dispatch records to devices based on feature flag.
>          //
> -        if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> +        if (PcdGetBool (PcdStatusCodeUseSerial)) {
>            SerialStatusCodeReportWorker (
>              Record[Index].CodeType,
>              Record[Index].Value,
> @@ -124,7 +124,7 @@ InitializationDispatcherWorker (
>              NULL
>              );
>          }
> -        if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> +        if (PcdGetBool (PcdStatusCodeUseMemory)) {
>            RtMemoryStatusCodeReportWorker (
>              Record[Index].CodeType,
>              Record[Index].Value,
> @@ -171,10 +171,10 @@ StatusCodeHandlerRuntimeDxeEntry (
>    //
>    InitializationDispatcherWorker ();
> 
> -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
>      mRscHandlerProtocol->Register (SerialStatusCodeReportWorker,
> TPL_HIGH_LEVEL);
>    }
> -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
>      mRscHandlerProtocol->Register (RtMemoryStatusCodeReportWorker,
> TPL_HIGH_LEVEL);
>    }
> 
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.inf
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.inf
> index d74c2a55dcaf..faadfd9578fe 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.inf
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandl
> erRuntimeDxe.inf
> @@ -58,12 +58,10 @@ [Guids]
>  [Protocols]
>    gEfiRscHandlerProtocolGuid                    ## CONSUMES
> 
> -[FeaturePcd]
> +[Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeReplayIn  ## CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> -
> -[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize |128|
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory   ##
> SOMETIMES_CONSUMES
> 
>  [Depex]
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.
> c
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.
> c
> index f54991ed3f67..20271571ded4 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.
> c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.
> c
> @@ -2,7 +2,7 @@
>    Status Code Handler Driver which produces general handlers and hook them
>    onto the SMM status code router.
> 
> -  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -28,14 +28,14 @@ InitializationDispatcherWorker (
>    // If enable UseSerial, then initialize serial port.
>    // if enable UseRuntimeMemory, then initialize runtime memory status code
> worker.
>    //
> -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
>      //
>      // Call Serial Port Lib API to initialize serial port.
>      //
>      Status = SerialPortInitialize ();
>      ASSERT_EFI_ERROR (Status);
>    }
> -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
>      Status = MemoryStatusCodeInitializeWorker ();
>      ASSERT_EFI_ERROR (Status);
>    }
> @@ -73,10 +73,10 @@ StatusCodeHandlerSmmEntry (
>    //
>    InitializationDispatcherWorker ();
> 
> -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
>      mRscHandlerProtocol->Register (SerialStatusCodeReportWorker);
>    }
> -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
>      mRscHandlerProtocol->Register (MemoryStatusCodeReportWorker);
>    }
> 
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.i
> nf
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.i
> nf
> index 47d0545f9591..4e24d87e55d1 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.i
> nf
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.i
> nf
> @@ -53,11 +53,9 @@ [Guids]
>  [Protocols]
>    gEfiSmmRscHandlerProtocolGuid                 ## CONSUMES
> 
> -[FeaturePcd]
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> -
>  [Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize |128|
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory   ##
> SOMETIMES_CONSUMES
> 
>  [Depex]
> --
> 2.24.0.windows.2


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

* Re: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
  2020-06-17 14:42 ` Wang, Jian J
@ 2020-06-17 16:47   ` Michael D Kinney
  2020-06-18  0:40     ` Tan, Ming
  2020-06-18  0:57     ` Wang, Jian J
  0 siblings, 2 replies; 7+ messages in thread
From: Michael D Kinney @ 2020-06-17 16:47 UTC (permalink / raw)
  To: devel@edk2.groups.io, Wang, Jian J, Tan, Ming, Kinney, Michael D
  Cc: Wu, Hao A

Ming,

With this change, are the same PCDs listed as both
a Feature Flag PCD and a non-Feature Flag PCD?

That should not be allowed, and if that passes the
build, then the build tools should flag that as an
error condition.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Wang, Jian J
> Sent: Wednesday, June 17, 2020 7:43 AM
> To: Tan, Ming <ming.tan@intel.com>; devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v5 1/1]
> MdeModulePkg.dec: Change PCDs for status code.
> 
> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> 
> Regards,
> Jian
> 
> > -----Original Message-----
> > From: Tan, Ming <ming.tan@intel.com>
> > Sent: Wednesday, June 17, 2020 3:36 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>
> > Subject: [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs
> for status code.
> >
> > REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2786
> >
> > In order to support enable/disable report status code
> through memory
> > or serial dynamic, change the following PCDs from
> [PcdsFeatureFlag] to
> > [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
> PcdsDynamicEx]:
> >   PcdStatusCodeUseSerial
> >   PcdStatusCodeUseMemory
> > The original plaforms can use PcdsFixedAtBuild in .dsc
> files to save size.
> > Currently do not remove the old pcd type to keep
> compatible.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Signed-off-by: Ming Tan <ming.tan@intel.com>
> > ---
> > V5: Do not remove th old pcd type to keep compatible,
> and change to
> > standalone patch.
> > V4: No change for this 1/4 patch, just modify the 2-
> 4/4 patchs.
> > V3: Split one patch to several patchs, each Pkg has
> one patch.
> > V2: Change the new type from [PcdsDynamic] to
> [PcdsFixedAtBuild,
> > PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> >     And set to PcdsFixedAtBuild in the original
> platform .dsc files.
> >
> >  MdeModulePkg/MdeModulePkg.dec                    | 13
> +++++++++++++
> >  .../StatusCodeHandler/Pei/StatusCodeHandlerPei.c |  6
> +++---
> >  .../Pei/StatusCodeHandlerPei.inf                 |  6
> ++----
> >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c     | 16
> ++++++++--------
> >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf   |  6
> ++----
> >  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.c | 10
> +++++-----
> >  .../Smm/StatusCodeHandlerSmm.inf                 |  6
> ++----
> >  7 files changed, 35 insertions(+), 28 deletions(-)
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec
> > index 4f44af694862..bd369d1f230e 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -2001,6 +2001,19 @@ [PcdsFixedAtBuild,
> PcdsPatchableInModule,
> > PcdsDynamic, PcdsDynamicEx]
> >    # @Prompt TCG Platform Firmware Profile revision.
> >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevis
> ion|0|UINT3
> > 2|0x00010077
> >
> > +  ## Indicates if StatusCode is reported via Serial
> port.<BR><BR>
> > +  #   TRUE  - Reports StatusCode via Serial port.<BR>
> > +  #   FALSE - Does not report StatusCode via Serial
> port.<BR>
> > +  # @Prompt Enable StatusCode via Serial port.
> > +
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TR
> UE|BOOLEAN|
> > 0x00010022
> > +
> > +  ## Indicates if StatusCode is stored in memory.
> > +  #  The memory is boot time memory in PEI Phase and
> is runtime memory in
> > DXE Phase.<BR><BR>
> > +  #   TRUE  - Stores StatusCode in memory.<BR>
> > +  #   FALSE - Does not store StatusCode in
> memory.<BR>
> > +  # @Prompt Enable StatusCode via memory.
> > +
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FA
> LSE|BOOLEA
> > N|0x00010023
> > +
> >  [PcdsPatchableInModule]
> >    ## Specify memory size with page number for PEI
> code when
> >    #  Loading Module at Fixed Address feature is
> enabled.
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> > index 1b07f92f3ce8..9b2ea4ee84d9 100644
> > ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> > @@ -2,7 +2,7 @@
> >    Report Status Code Handler PEIM which produces
> general handlers and hook
> > them
> >    onto the PEI status code router.
> >
> > -  Copyright (c) 2009 - 2018, Intel Corporation. All
> rights reserved.<BR>
> > +  Copyright (c) 2009 - 2020, Intel Corporation. All
> rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -45,13 +45,13 @@ StatusCodeHandlerPeiEntry (
> >    // If enable UseSerial, then initialize serial
> port.
> >    // if enable UseMemory, then initialize memory
> status code worker.
> >    //
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      Status = SerialPortInitialize();
> >      ASSERT_EFI_ERROR (Status);
> >      Status = RscHandlerPpi->Register
> (SerialStatusCodeReportWorker);
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      Status = MemoryStatusCodeInitializeWorker ();
> >      ASSERT_EFI_ERROR (Status);
> >      Status = RscHandlerPpi->Register
> (MemoryStatusCodeReportWorker);
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> > index 8aef9af34a05..64380ddfaccc 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> > @@ -53,11 +53,9 @@ [Guids]
> >  [Ppis]
> >    gEfiPeiRscHandlerPpiGuid                      ##
> CONSUMES
> >
> > -[FeaturePcd]
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> CONSUMES
> > -
> >  [Pcd]
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> CONSUMES
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> |gEfiMdeMod
> > ulePkgTokenSpaceGuid.PcdStatusCodeUseMemory    ##
> > SOMETIMES_CONSUMES
> >
> >  [Depex]
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> > index 79cc48fa55a4..a8c0fe5b7149 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> > @@ -2,7 +2,7 @@
> >    Status Code Handler Driver which produces general
> handlers and hook them
> >    onto the DXE status code router.
> >
> > -  Copyright (c) 2006 - 2019, Intel Corporation. All
> rights reserved.<BR>
> > +  Copyright (c) 2006 - 2020, Intel Corporation. All
> rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -29,7 +29,7 @@ UnregisterBootTimeHandlers (
> >    IN VOID             *Context
> >    )
> >  {
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      mRscHandlerProtocol->Unregister
> (SerialStatusCodeReportWorker);
> >    }
> >  }
> > @@ -80,14 +80,14 @@ InitializationDispatcherWorker (
> >    // If enable UseSerial, then initialize serial
> port.
> >    // if enable UseRuntimeMemory, then initialize
> runtime memory status code
> > worker.
> >    //
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      //
> >      // Call Serial Port Lib API to initialize serial
> port.
> >      //
> >      Status = SerialPortInitialize ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      Status = RtMemoryStatusCodeInitializeWorker ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > @@ -115,7 +115,7 @@ InitializationDispatcherWorker (
> >          //
> >          // Dispatch records to devices based on
> feature flag.
> >          //
> > -        if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +        if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >            SerialStatusCodeReportWorker (
> >              Record[Index].CodeType,
> >              Record[Index].Value,
> > @@ -124,7 +124,7 @@ InitializationDispatcherWorker (
> >              NULL
> >              );
> >          }
> > -        if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +        if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >            RtMemoryStatusCodeReportWorker (
> >              Record[Index].CodeType,
> >              Record[Index].Value,
> > @@ -171,10 +171,10 @@ StatusCodeHandlerRuntimeDxeEntry
> (
> >    //
> >    InitializationDispatcherWorker ();
> >
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      mRscHandlerProtocol->Register
> (SerialStatusCodeReportWorker,
> > TPL_HIGH_LEVEL);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      mRscHandlerProtocol->Register
> (RtMemoryStatusCodeReportWorker,
> > TPL_HIGH_LEVEL);
> >    }
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> > index d74c2a55dcaf..faadfd9578fe 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> > @@ -58,12 +58,10 @@ [Guids]
> >  [Protocols]
> >    gEfiRscHandlerProtocolGuid                    ##
> CONSUMES
> >
> > -[FeaturePcd]
> > +[Pcd]
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeReplayIn  ##
> CONSUMES
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> CONSUMES
> > -
> > -[Pcd]
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> |128|
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> ##
> > SOMETIMES_CONSUMES
> >
> >  [Depex]
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> > index f54991ed3f67..20271571ded4 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> > @@ -2,7 +2,7 @@
> >    Status Code Handler Driver which produces general
> handlers and hook them
> >    onto the SMM status code router.
> >
> > -  Copyright (c) 2009 - 2018, Intel Corporation. All
> rights reserved.<BR>
> > +  Copyright (c) 2009 - 2020, Intel Corporation. All
> rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -28,14 +28,14 @@ InitializationDispatcherWorker (
> >    // If enable UseSerial, then initialize serial
> port.
> >    // if enable UseRuntimeMemory, then initialize
> runtime memory status code
> > worker.
> >    //
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      //
> >      // Call Serial Port Lib API to initialize serial
> port.
> >      //
> >      Status = SerialPortInitialize ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      Status = MemoryStatusCodeInitializeWorker ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > @@ -73,10 +73,10 @@ StatusCodeHandlerSmmEntry (
> >    //
> >    InitializationDispatcherWorker ();
> >
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      mRscHandlerProtocol->Register
> (SerialStatusCodeReportWorker);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      mRscHandlerProtocol->Register
> (MemoryStatusCodeReportWorker);
> >    }
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> > index 47d0545f9591..4e24d87e55d1 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> > @@ -53,11 +53,9 @@ [Guids]
> >  [Protocols]
> >    gEfiSmmRscHandlerProtocolGuid                 ##
> CONSUMES
> >
> > -[FeaturePcd]
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> CONSUMES
> > -
> >  [Pcd]
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> CONSUMES
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> |128|
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> ##
> > SOMETIMES_CONSUMES
> >
> >  [Depex]
> > --
> > 2.24.0.windows.2
> 
> 
> 


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

* Re: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
  2020-06-17 16:47   ` [edk2-devel] " Michael D Kinney
@ 2020-06-18  0:40     ` Tan, Ming
  2020-06-18  0:57     ` Wang, Jian J
  1 sibling, 0 replies; 7+ messages in thread
From: Tan, Ming @ 2020-06-18  0:40 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Wang, Jian J; +Cc: Wu, Hao A

Michael:
  I verify it can pass the local build.
  Use this solution, can keep compatible, need not remove the setting in platforms at first.
  I will remove the Feature Flag PCD at last, after the platforms change the PCD type.

  BR/Tan Ming.

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Thursday, June 18, 2020 12:47 AM
To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Tan, Ming <ming.tan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Wu, Hao A <hao.a.wu@intel.com>
Subject: RE: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.

Ming,

With this change, are the same PCDs listed as both a Feature Flag PCD and a non-Feature Flag PCD?

That should not be allowed, and if that passes the build, then the build tools should flag that as an error condition.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wang, 
> Jian J
> Sent: Wednesday, June 17, 2020 7:43 AM
> To: Tan, Ming <ming.tan@intel.com>; devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v5 1/1]
> MdeModulePkg.dec: Change PCDs for status code.
> 
> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> 
> Regards,
> Jian
> 
> > -----Original Message-----
> > From: Tan, Ming <ming.tan@intel.com>
> > Sent: Wednesday, June 17, 2020 3:36 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>
> > Subject: [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs
> for status code.
> >
> > REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2786
> >
> > In order to support enable/disable report status code
> through memory
> > or serial dynamic, change the following PCDs from
> [PcdsFeatureFlag] to
> > [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
> PcdsDynamicEx]:
> >   PcdStatusCodeUseSerial
> >   PcdStatusCodeUseMemory
> > The original plaforms can use PcdsFixedAtBuild in .dsc
> files to save size.
> > Currently do not remove the old pcd type to keep
> compatible.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Signed-off-by: Ming Tan <ming.tan@intel.com>
> > ---
> > V5: Do not remove th old pcd type to keep compatible,
> and change to
> > standalone patch.
> > V4: No change for this 1/4 patch, just modify the 2-
> 4/4 patchs.
> > V3: Split one patch to several patchs, each Pkg has
> one patch.
> > V2: Change the new type from [PcdsDynamic] to
> [PcdsFixedAtBuild,
> > PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> >     And set to PcdsFixedAtBuild in the original
> platform .dsc files.
> >
> >  MdeModulePkg/MdeModulePkg.dec                    | 13
> +++++++++++++
> >  .../StatusCodeHandler/Pei/StatusCodeHandlerPei.c |  6
> +++---
> >  .../Pei/StatusCodeHandlerPei.inf                 |  6
> ++----
> >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c     | 16
> ++++++++--------
> >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf   |  6
> ++----
> >  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.c | 10
> +++++-----
> >  .../Smm/StatusCodeHandlerSmm.inf                 |  6
> ++----
> >  7 files changed, 35 insertions(+), 28 deletions(-)
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec 
> > b/MdeModulePkg/MdeModulePkg.dec index 4f44af694862..bd369d1f230e 
> > 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -2001,6 +2001,19 @@ [PcdsFixedAtBuild,
> PcdsPatchableInModule,
> > PcdsDynamic, PcdsDynamicEx]
> >    # @Prompt TCG Platform Firmware Profile revision.
> >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevis
> ion|0|UINT3
> > 2|0x00010077
> >
> > +  ## Indicates if StatusCode is reported via Serial
> port.<BR><BR>
> > +  #   TRUE  - Reports StatusCode via Serial port.<BR>
> > +  #   FALSE - Does not report StatusCode via Serial
> port.<BR>
> > +  # @Prompt Enable StatusCode via Serial port.
> > +
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TR
> UE|BOOLEAN|
> > 0x00010022
> > +
> > +  ## Indicates if StatusCode is stored in memory.
> > +  #  The memory is boot time memory in PEI Phase and
> is runtime memory in
> > DXE Phase.<BR><BR>
> > +  #   TRUE  - Stores StatusCode in memory.<BR>
> > +  #   FALSE - Does not store StatusCode in
> memory.<BR>
> > +  # @Prompt Enable StatusCode via memory.
> > +
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FA
> LSE|BOOLEA
> > N|0x00010023
> > +
> >  [PcdsPatchableInModule]
> >    ## Specify memory size with page number for PEI
> code when
> >    #  Loading Module at Fixed Address feature is
> enabled.
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> > index 1b07f92f3ce8..9b2ea4ee84d9 100644
> > ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.c
> > @@ -2,7 +2,7 @@
> >    Report Status Code Handler PEIM which produces
> general handlers and hook
> > them
> >    onto the PEI status code router.
> >
> > -  Copyright (c) 2009 - 2018, Intel Corporation. All
> rights reserved.<BR>
> > +  Copyright (c) 2009 - 2020, Intel Corporation. All
> rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -45,13 +45,13 @@ StatusCodeHandlerPeiEntry (
> >    // If enable UseSerial, then initialize serial
> port.
> >    // if enable UseMemory, then initialize memory
> status code worker.
> >    //
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      Status = SerialPortInitialize();
> >      ASSERT_EFI_ERROR (Status);
> >      Status = RscHandlerPpi->Register
> (SerialStatusCodeReportWorker);
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      Status = MemoryStatusCodeInitializeWorker ();
> >      ASSERT_EFI_ERROR (Status);
> >      Status = RscHandlerPpi->Register
> (MemoryStatusCodeReportWorker);
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> > index 8aef9af34a05..64380ddfaccc 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> eHandlerPei.inf
> > @@ -53,11 +53,9 @@ [Guids]
> >  [Ppis]
> >    gEfiPeiRscHandlerPpiGuid                      ##
> CONSUMES
> >
> > -[FeaturePcd]
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> > -
> >  [Pcd]
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> |gEfiMdeMod
> > ulePkgTokenSpaceGuid.PcdStatusCodeUseMemory    ##
> > SOMETIMES_CONSUMES
> >
> >  [Depex]
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> > index 79cc48fa55a4..a8c0fe5b7149 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.c
> > @@ -2,7 +2,7 @@
> >    Status Code Handler Driver which produces general
> handlers and hook them
> >    onto the DXE status code router.
> >
> > -  Copyright (c) 2006 - 2019, Intel Corporation. All
> rights reserved.<BR>
> > +  Copyright (c) 2006 - 2020, Intel Corporation. All
> rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -29,7 +29,7 @@ UnregisterBootTimeHandlers (
> >    IN VOID             *Context
> >    )
> >  {
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      mRscHandlerProtocol->Unregister
> (SerialStatusCodeReportWorker);
> >    }
> >  }
> > @@ -80,14 +80,14 @@ InitializationDispatcherWorker (
> >    // If enable UseSerial, then initialize serial
> port.
> >    // if enable UseRuntimeMemory, then initialize
> runtime memory status code
> > worker.
> >    //
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      //
> >      // Call Serial Port Lib API to initialize serial
> port.
> >      //
> >      Status = SerialPortInitialize ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      Status = RtMemoryStatusCodeInitializeWorker ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > @@ -115,7 +115,7 @@ InitializationDispatcherWorker (
> >          //
> >          // Dispatch records to devices based on
> feature flag.
> >          //
> > -        if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +        if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >            SerialStatusCodeReportWorker (
> >              Record[Index].CodeType,
> >              Record[Index].Value,
> > @@ -124,7 +124,7 @@ InitializationDispatcherWorker (
> >              NULL
> >              );
> >          }
> > -        if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +        if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >            RtMemoryStatusCodeReportWorker (
> >              Record[Index].CodeType,
> >              Record[Index].Value,
> > @@ -171,10 +171,10 @@ StatusCodeHandlerRuntimeDxeEntry
> (
> >    //
> >    InitializationDispatcherWorker ();
> >
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      mRscHandlerProtocol->Register
> (SerialStatusCodeReportWorker,
> > TPL_HIGH_LEVEL);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      mRscHandlerProtocol->Register
> (RtMemoryStatusCodeReportWorker,
> > TPL_HIGH_LEVEL);
> >    }
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> > index d74c2a55dcaf..faadfd9578fe 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> atusCodeHandl
> > erRuntimeDxe.inf
> > @@ -58,12 +58,10 @@ [Guids]
> >  [Protocols]
> >    gEfiRscHandlerProtocolGuid                    ##
> CONSUMES
> >
> > -[FeaturePcd]
> > +[Pcd]
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeReplayIn  ## CONSUMES
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> > -
> > -[Pcd]
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> |128|
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> ##
> > SOMETIMES_CONSUMES
> >
> >  [Depex]
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> > index f54991ed3f67..20271571ded4 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.
> > c
> > @@ -2,7 +2,7 @@
> >    Status Code Handler Driver which produces general
> handlers and hook them
> >    onto the SMM status code router.
> >
> > -  Copyright (c) 2009 - 2018, Intel Corporation. All
> rights reserved.<BR>
> > +  Copyright (c) 2009 - 2020, Intel Corporation. All
> rights reserved.<BR>
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -28,14 +28,14 @@ InitializationDispatcherWorker (
> >    // If enable UseSerial, then initialize serial
> port.
> >    // if enable UseRuntimeMemory, then initialize
> runtime memory status code
> > worker.
> >    //
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      //
> >      // Call Serial Port Lib API to initialize serial
> port.
> >      //
> >      Status = SerialPortInitialize ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      Status = MemoryStatusCodeInitializeWorker ();
> >      ASSERT_EFI_ERROR (Status);
> >    }
> > @@ -73,10 +73,10 @@ StatusCodeHandlerSmmEntry (
> >    //
> >    InitializationDispatcherWorker ();
> >
> > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> >      mRscHandlerProtocol->Register
> (SerialStatusCodeReportWorker);
> >    }
> > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> >      mRscHandlerProtocol->Register
> (MemoryStatusCodeReportWorker);
> >    }
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> > index 47d0545f9591..4e24d87e55d1 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> eHandlerSmm.i
> > nf
> > @@ -53,11 +53,9 @@ [Guids]
> >  [Protocols]
> >    gEfiSmmRscHandlerProtocolGuid                 ##
> CONSUMES
> >
> > -[FeaturePcd]
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> > -
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> > -
> >  [Pcd]
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ## CONSUMES
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > CONSUMES
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> |128|
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> ##
> > SOMETIMES_CONSUMES
> >
> >  [Depex]
> > --
> > 2.24.0.windows.2
> 
> 
> 


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

* Re: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
  2020-06-17 16:47   ` [edk2-devel] " Michael D Kinney
  2020-06-18  0:40     ` Tan, Ming
@ 2020-06-18  0:57     ` Wang, Jian J
  2020-06-18  1:46       ` Michael D Kinney
  1 sibling, 1 reply; 7+ messages in thread
From: Wang, Jian J @ 2020-06-18  0:57 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Tan, Ming; +Cc: Wu, Hao A

Mike,

I tried to build it. There's no warnings. And it's free to change type to any of them.
Why's there such limitation? FeatureFlag looks the same as FixedAtBuild to me.

Regards,
Jian

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Thursday, June 18, 2020 12:47 AM
> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Tan, Ming
> <ming.tan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: RE: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for
> status code.
> 
> Ming,
> 
> With this change, are the same PCDs listed as both
> a Feature Flag PCD and a non-Feature Flag PCD?
> 
> That should not be allowed, and if that passes the
> build, then the build tools should flag that as an
> error condition.
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> > Behalf Of Wang, Jian J
> > Sent: Wednesday, June 17, 2020 7:43 AM
> > To: Tan, Ming <ming.tan@intel.com>; devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v5 1/1]
> > MdeModulePkg.dec: Change PCDs for status code.
> >
> > Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> >
> > Regards,
> > Jian
> >
> > > -----Original Message-----
> > > From: Tan, Ming <ming.tan@intel.com>
> > > Sent: Wednesday, June 17, 2020 3:36 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>
> > > Subject: [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs
> > for status code.
> > >
> > > REF:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2786
> > >
> > > In order to support enable/disable report status code
> > through memory
> > > or serial dynamic, change the following PCDs from
> > [PcdsFeatureFlag] to
> > > [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
> > PcdsDynamicEx]:
> > >   PcdStatusCodeUseSerial
> > >   PcdStatusCodeUseMemory
> > > The original plaforms can use PcdsFixedAtBuild in .dsc
> > files to save size.
> > > Currently do not remove the old pcd type to keep
> > compatible.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Signed-off-by: Ming Tan <ming.tan@intel.com>
> > > ---
> > > V5: Do not remove th old pcd type to keep compatible,
> > and change to
> > > standalone patch.
> > > V4: No change for this 1/4 patch, just modify the 2-
> > 4/4 patchs.
> > > V3: Split one patch to several patchs, each Pkg has
> > one patch.
> > > V2: Change the new type from [PcdsDynamic] to
> > [PcdsFixedAtBuild,
> > > PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> > >     And set to PcdsFixedAtBuild in the original
> > platform .dsc files.
> > >
> > >  MdeModulePkg/MdeModulePkg.dec                    | 13
> > +++++++++++++
> > >  .../StatusCodeHandler/Pei/StatusCodeHandlerPei.c |  6
> > +++---
> > >  .../Pei/StatusCodeHandlerPei.inf                 |  6
> > ++----
> > >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c     | 16
> > ++++++++--------
> > >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf   |  6
> > ++----
> > >  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.c | 10
> > +++++-----
> > >  .../Smm/StatusCodeHandlerSmm.inf                 |  6
> > ++----
> > >  7 files changed, 35 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > b/MdeModulePkg/MdeModulePkg.dec
> > > index 4f44af694862..bd369d1f230e 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -2001,6 +2001,19 @@ [PcdsFixedAtBuild,
> > PcdsPatchableInModule,
> > > PcdsDynamic, PcdsDynamicEx]
> > >    # @Prompt TCG Platform Firmware Profile revision.
> > >
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevis
> > ion|0|UINT3
> > > 2|0x00010077
> > >
> > > +  ## Indicates if StatusCode is reported via Serial
> > port.<BR><BR>
> > > +  #   TRUE  - Reports StatusCode via Serial port.<BR>
> > > +  #   FALSE - Does not report StatusCode via Serial
> > port.<BR>
> > > +  # @Prompt Enable StatusCode via Serial port.
> > > +
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TR
> > UE|BOOLEAN|
> > > 0x00010022
> > > +
> > > +  ## Indicates if StatusCode is stored in memory.
> > > +  #  The memory is boot time memory in PEI Phase and
> > is runtime memory in
> > > DXE Phase.<BR><BR>
> > > +  #   TRUE  - Stores StatusCode in memory.<BR>
> > > +  #   FALSE - Does not store StatusCode in
> > memory.<BR>
> > > +  # @Prompt Enable StatusCode via memory.
> > > +
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FA
> > LSE|BOOLEA
> > > N|0x00010023
> > > +
> > >  [PcdsPatchableInModule]
> > >    ## Specify memory size with page number for PEI
> > code when
> > >    #  Loading Module at Fixed Address feature is
> > enabled.
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.c
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.c
> > > index 1b07f92f3ce8..9b2ea4ee84d9 100644
> > > ---
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.c
> > > +++
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.c
> > > @@ -2,7 +2,7 @@
> > >    Report Status Code Handler PEIM which produces
> > general handlers and hook
> > > them
> > >    onto the PEI status code router.
> > >
> > > -  Copyright (c) 2009 - 2018, Intel Corporation. All
> > rights reserved.<BR>
> > > +  Copyright (c) 2009 - 2020, Intel Corporation. All
> > rights reserved.<BR>
> > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -45,13 +45,13 @@ StatusCodeHandlerPeiEntry (
> > >    // If enable UseSerial, then initialize serial
> > port.
> > >    // if enable UseMemory, then initialize memory
> > status code worker.
> > >    //
> > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > >      Status = SerialPortInitialize();
> > >      ASSERT_EFI_ERROR (Status);
> > >      Status = RscHandlerPpi->Register
> > (SerialStatusCodeReportWorker);
> > >      ASSERT_EFI_ERROR (Status);
> > >    }
> > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > >      Status = MemoryStatusCodeInitializeWorker ();
> > >      ASSERT_EFI_ERROR (Status);
> > >      Status = RscHandlerPpi->Register
> > (MemoryStatusCodeReportWorker);
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.inf
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.inf
> > > index 8aef9af34a05..64380ddfaccc 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.inf
> > > +++
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > eHandlerPei.inf
> > > @@ -53,11 +53,9 @@ [Guids]
> > >  [Ppis]
> > >    gEfiPeiRscHandlerPpiGuid                      ##
> > CONSUMES
> > >
> > > -[FeaturePcd]
> > > -
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > CONSUMES
> > > -
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > CONSUMES
> > > -
> > >  [Pcd]
> > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > CONSUMES
> > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > CONSUMES
> > >
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> > |gEfiMdeMod
> > > ulePkgTokenSpaceGuid.PcdStatusCodeUseMemory    ##
> > > SOMETIMES_CONSUMES
> > >
> > >  [Depex]
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.c
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.c
> > > index 79cc48fa55a4..a8c0fe5b7149 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.c
> > > +++
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.c
> > > @@ -2,7 +2,7 @@
> > >    Status Code Handler Driver which produces general
> > handlers and hook them
> > >    onto the DXE status code router.
> > >
> > > -  Copyright (c) 2006 - 2019, Intel Corporation. All
> > rights reserved.<BR>
> > > +  Copyright (c) 2006 - 2020, Intel Corporation. All
> > rights reserved.<BR>
> > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -29,7 +29,7 @@ UnregisterBootTimeHandlers (
> > >    IN VOID             *Context
> > >    )
> > >  {
> > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > >      mRscHandlerProtocol->Unregister
> > (SerialStatusCodeReportWorker);
> > >    }
> > >  }
> > > @@ -80,14 +80,14 @@ InitializationDispatcherWorker (
> > >    // If enable UseSerial, then initialize serial
> > port.
> > >    // if enable UseRuntimeMemory, then initialize
> > runtime memory status code
> > > worker.
> > >    //
> > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > >      //
> > >      // Call Serial Port Lib API to initialize serial
> > port.
> > >      //
> > >      Status = SerialPortInitialize ();
> > >      ASSERT_EFI_ERROR (Status);
> > >    }
> > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > >      Status = RtMemoryStatusCodeInitializeWorker ();
> > >      ASSERT_EFI_ERROR (Status);
> > >    }
> > > @@ -115,7 +115,7 @@ InitializationDispatcherWorker (
> > >          //
> > >          // Dispatch records to devices based on
> > feature flag.
> > >          //
> > > -        if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > +        if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > >            SerialStatusCodeReportWorker (
> > >              Record[Index].CodeType,
> > >              Record[Index].Value,
> > > @@ -124,7 +124,7 @@ InitializationDispatcherWorker (
> > >              NULL
> > >              );
> > >          }
> > > -        if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > +        if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > >            RtMemoryStatusCodeReportWorker (
> > >              Record[Index].CodeType,
> > >              Record[Index].Value,
> > > @@ -171,10 +171,10 @@ StatusCodeHandlerRuntimeDxeEntry
> > (
> > >    //
> > >    InitializationDispatcherWorker ();
> > >
> > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > >      mRscHandlerProtocol->Register
> > (SerialStatusCodeReportWorker,
> > > TPL_HIGH_LEVEL);
> > >    }
> > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > >      mRscHandlerProtocol->Register
> > (RtMemoryStatusCodeReportWorker,
> > > TPL_HIGH_LEVEL);
> > >    }
> > >
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.inf
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.inf
> > > index d74c2a55dcaf..faadfd9578fe 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.inf
> > > +++
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > atusCodeHandl
> > > erRuntimeDxe.inf
> > > @@ -58,12 +58,10 @@ [Guids]
> > >  [Protocols]
> > >    gEfiRscHandlerProtocolGuid                    ##
> > CONSUMES
> > >
> > > -[FeaturePcd]
> > > +[Pcd]
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeReplayIn  ##
> > CONSUMES
> > > -
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > CONSUMES
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > CONSUMES
> > > -
> > > -[Pcd]
> > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > CONSUMES
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> > |128|
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> > ##
> > > SOMETIMES_CONSUMES
> > >
> > >  [Depex]
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.
> > > c
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.
> > > c
> > > index f54991ed3f67..20271571ded4 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.
> > > c
> > > +++
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.
> > > c
> > > @@ -2,7 +2,7 @@
> > >    Status Code Handler Driver which produces general
> > handlers and hook them
> > >    onto the SMM status code router.
> > >
> > > -  Copyright (c) 2009 - 2018, Intel Corporation. All
> > rights reserved.<BR>
> > > +  Copyright (c) 2009 - 2020, Intel Corporation. All
> > rights reserved.<BR>
> > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -28,14 +28,14 @@ InitializationDispatcherWorker (
> > >    // If enable UseSerial, then initialize serial
> > port.
> > >    // if enable UseRuntimeMemory, then initialize
> > runtime memory status code
> > > worker.
> > >    //
> > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > >      //
> > >      // Call Serial Port Lib API to initialize serial
> > port.
> > >      //
> > >      Status = SerialPortInitialize ();
> > >      ASSERT_EFI_ERROR (Status);
> > >    }
> > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > >      Status = MemoryStatusCodeInitializeWorker ();
> > >      ASSERT_EFI_ERROR (Status);
> > >    }
> > > @@ -73,10 +73,10 @@ StatusCodeHandlerSmmEntry (
> > >    //
> > >    InitializationDispatcherWorker ();
> > >
> > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > >      mRscHandlerProtocol->Register
> > (SerialStatusCodeReportWorker);
> > >    }
> > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > >      mRscHandlerProtocol->Register
> > (MemoryStatusCodeReportWorker);
> > >    }
> > >
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.i
> > > nf
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.i
> > > nf
> > > index 47d0545f9591..4e24d87e55d1 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.i
> > > nf
> > > +++
> > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > eHandlerSmm.i
> > > nf
> > > @@ -53,11 +53,9 @@ [Guids]
> > >  [Protocols]
> > >    gEfiSmmRscHandlerProtocolGuid                 ##
> > CONSUMES
> > >
> > > -[FeaturePcd]
> > > -
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > CONSUMES
> > > -
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > CONSUMES
> > > -
> > >  [Pcd]
> > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > CONSUMES
> > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > CONSUMES
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> > |128|
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> > ##
> > > SOMETIMES_CONSUMES
> > >
> > >  [Depex]
> > > --
> > > 2.24.0.windows.2
> >
> >
> > 


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

* Re: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
  2020-06-18  0:57     ` Wang, Jian J
@ 2020-06-18  1:46       ` Michael D Kinney
  2020-06-18  2:25         ` Wang, Jian J
  0 siblings, 1 reply; 7+ messages in thread
From: Michael D Kinney @ 2020-06-18  1:46 UTC (permalink / raw)
  To: Wang, Jian J, devel@edk2.groups.io, Tan, Ming, Kinney, Michael D
  Cc: Wu, Hao A

Jian,

There are few rules listed in the EDK II DEC File Format
Specification that apply here:

https://edk2-docs.gitbook.io/edk-ii-dec-specification/3_edk_ii_dec_file_format/310_pcd_sections

* PCD entries may be put into any or all PCD section types
  except PcdsFeatureFlag sections.

* PCDs listed in PcdsFeatureFlag sections must only be listed
  in PcdsFeatureFlag sections.

* The PcdsFeatureFlag PcdType may not be mixed with any
  other PcdType elements in the section header. The following
  example is NOT VALID:

    [PcdsFeatureFlag.IA32, PcdsFixedAtBuild.IA32]

Best regards,

Mike

> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Wednesday, June 17, 2020 5:58 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; Tan, Ming <ming.tan@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: RE: [edk2-devel] [PATCH v5 1/1]
> MdeModulePkg.dec: Change PCDs for status code.
> 
> Mike,
> 
> I tried to build it. There's no warnings. And it's free
> to change type to any of them.
> Why's there such limitation? FeatureFlag looks the same
> as FixedAtBuild to me.
> 
> Regards,
> Jian
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Thursday, June 18, 2020 12:47 AM
> > To: devel@edk2.groups.io; Wang, Jian J
> <jian.j.wang@intel.com>; Tan, Ming
> > <ming.tan@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v5 1/1]
> MdeModulePkg.dec: Change PCDs for
> > status code.
> >
> > Ming,
> >
> > With this change, are the same PCDs listed as both
> > a Feature Flag PCD and a non-Feature Flag PCD?
> >
> > That should not be allowed, and if that passes the
> > build, then the build tools should flag that as an
> > error condition.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> > > Behalf Of Wang, Jian J
> > > Sent: Wednesday, June 17, 2020 7:43 AM
> > > To: Tan, Ming <ming.tan@intel.com>;
> devel@edk2.groups.io
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v5 1/1]
> > > MdeModulePkg.dec: Change PCDs for status code.
> > >
> > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> > >
> > > Regards,
> > > Jian
> > >
> > > > -----Original Message-----
> > > > From: Tan, Ming <ming.tan@intel.com>
> > > > Sent: Wednesday, June 17, 2020 3:36 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao
> A
> > > <hao.a.wu@intel.com>
> > > > Subject: [PATCH v5 1/1] MdeModulePkg.dec: Change
> PCDs
> > > for status code.
> > > >
> > > > REF:
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2786
> > > >
> > > > In order to support enable/disable report status
> code
> > > through memory
> > > > or serial dynamic, change the following PCDs from
> > > [PcdsFeatureFlag] to
> > > > [PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic,
> > > PcdsDynamicEx]:
> > > >   PcdStatusCodeUseSerial
> > > >   PcdStatusCodeUseMemory
> > > > The original plaforms can use PcdsFixedAtBuild in
> .dsc
> > > files to save size.
> > > > Currently do not remove the old pcd type to keep
> > > compatible.
> > > >
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > Signed-off-by: Ming Tan <ming.tan@intel.com>
> > > > ---
> > > > V5: Do not remove th old pcd type to keep
> compatible,
> > > and change to
> > > > standalone patch.
> > > > V4: No change for this 1/4 patch, just modify the
> 2-
> > > 4/4 patchs.
> > > > V3: Split one patch to several patchs, each Pkg
> has
> > > one patch.
> > > > V2: Change the new type from [PcdsDynamic] to
> > > [PcdsFixedAtBuild,
> > > > PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> > > >     And set to PcdsFixedAtBuild in the original
> > > platform .dsc files.
> > > >
> > > >  MdeModulePkg/MdeModulePkg.dec
> | 13
> > > +++++++++++++
> > > >  .../StatusCodeHandler/Pei/StatusCodeHandlerPei.c
> |  6
> > > +++---
> > > >  .../Pei/StatusCodeHandlerPei.inf
> |  6
> > > ++----
> > > >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c
> | 16
> > > ++++++++--------
> > > >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
> |  6
> > > ++----
> > > >  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.c
> | 10
> > > +++++-----
> > > >  .../Smm/StatusCodeHandlerSmm.inf
> |  6
> > > ++----
> > > >  7 files changed, 35 insertions(+), 28 deletions(-
> )
> > > >
> > > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > > b/MdeModulePkg/MdeModulePkg.dec
> > > > index 4f44af694862..bd369d1f230e 100644
> > > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > > @@ -2001,6 +2001,19 @@ [PcdsFixedAtBuild,
> > > PcdsPatchableInModule,
> > > > PcdsDynamic, PcdsDynamicEx]
> > > >    # @Prompt TCG Platform Firmware Profile
> revision.
> > > >
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevis
> > > ion|0|UINT3
> > > > 2|0x00010077
> > > >
> > > > +  ## Indicates if StatusCode is reported via
> Serial
> > > port.<BR><BR>
> > > > +  #   TRUE  - Reports StatusCode via Serial
> port.<BR>
> > > > +  #   FALSE - Does not report StatusCode via
> Serial
> > > port.<BR>
> > > > +  # @Prompt Enable StatusCode via Serial port.
> > > > +
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TR
> > > UE|BOOLEAN|
> > > > 0x00010022
> > > > +
> > > > +  ## Indicates if StatusCode is stored in memory.
> > > > +  #  The memory is boot time memory in PEI Phase
> and
> > > is runtime memory in
> > > > DXE Phase.<BR><BR>
> > > > +  #   TRUE  - Stores StatusCode in memory.<BR>
> > > > +  #   FALSE - Does not store StatusCode in
> > > memory.<BR>
> > > > +  # @Prompt Enable StatusCode via memory.
> > > > +
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FA
> > > LSE|BOOLEA
> > > > N|0x00010023
> > > > +
> > > >  [PcdsPatchableInModule]
> > > >    ## Specify memory size with page number for PEI
> > > code when
> > > >    #  Loading Module at Fixed Address feature is
> > > enabled.
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.c
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.c
> > > > index 1b07f92f3ce8..9b2ea4ee84d9 100644
> > > > ---
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.c
> > > > +++
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.c
> > > > @@ -2,7 +2,7 @@
> > > >    Report Status Code Handler PEIM which produces
> > > general handlers and hook
> > > > them
> > > >    onto the PEI status code router.
> > > >
> > > > -  Copyright (c) 2009 - 2018, Intel Corporation.
> All
> > > rights reserved.<BR>
> > > > +  Copyright (c) 2009 - 2020, Intel Corporation.
> All
> > > rights reserved.<BR>
> > > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >  **/
> > > > @@ -45,13 +45,13 @@ StatusCodeHandlerPeiEntry (
> > > >    // If enable UseSerial, then initialize serial
> > > port.
> > > >    // if enable UseMemory, then initialize memory
> > > status code worker.
> > > >    //
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > >      Status = SerialPortInitialize();
> > > >      ASSERT_EFI_ERROR (Status);
> > > >      Status = RscHandlerPpi->Register
> > > (SerialStatusCodeReportWorker);
> > > >      ASSERT_EFI_ERROR (Status);
> > > >    }
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > >      Status = MemoryStatusCodeInitializeWorker ();
> > > >      ASSERT_EFI_ERROR (Status);
> > > >      Status = RscHandlerPpi->Register
> > > (MemoryStatusCodeReportWorker);
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.inf
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.inf
> > > > index 8aef9af34a05..64380ddfaccc 100644
> > > > ---
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.inf
> > > > +++
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > eHandlerPei.inf
> > > > @@ -53,11 +53,9 @@ [Guids]
> > > >  [Ppis]
> > > >    gEfiPeiRscHandlerPpiGuid
> ##
> > > CONSUMES
> > > >
> > > > -[FeaturePcd]
> > > > -
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > CONSUMES
> > > > -
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > CONSUMES
> > > > -
> > > >  [Pcd]
> > > > +
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > CONSUMES
> > > > +
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > CONSUMES
> > > >
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> > > |gEfiMdeMod
> > > > ulePkgTokenSpaceGuid.PcdStatusCodeUseMemory    ##
> > > > SOMETIMES_CONSUMES
> > > >
> > > >  [Depex]
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.c
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.c
> > > > index 79cc48fa55a4..a8c0fe5b7149 100644
> > > > ---
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.c
> > > > +++
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.c
> > > > @@ -2,7 +2,7 @@
> > > >    Status Code Handler Driver which produces
> general
> > > handlers and hook them
> > > >    onto the DXE status code router.
> > > >
> > > > -  Copyright (c) 2006 - 2019, Intel Corporation.
> All
> > > rights reserved.<BR>
> > > > +  Copyright (c) 2006 - 2020, Intel Corporation.
> All
> > > rights reserved.<BR>
> > > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >  **/
> > > > @@ -29,7 +29,7 @@ UnregisterBootTimeHandlers (
> > > >    IN VOID             *Context
> > > >    )
> > > >  {
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > >      mRscHandlerProtocol->Unregister
> > > (SerialStatusCodeReportWorker);
> > > >    }
> > > >  }
> > > > @@ -80,14 +80,14 @@ InitializationDispatcherWorker
> (
> > > >    // If enable UseSerial, then initialize serial
> > > port.
> > > >    // if enable UseRuntimeMemory, then initialize
> > > runtime memory status code
> > > > worker.
> > > >    //
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > >      //
> > > >      // Call Serial Port Lib API to initialize
> serial
> > > port.
> > > >      //
> > > >      Status = SerialPortInitialize ();
> > > >      ASSERT_EFI_ERROR (Status);
> > > >    }
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > >      Status = RtMemoryStatusCodeInitializeWorker
> ();
> > > >      ASSERT_EFI_ERROR (Status);
> > > >    }
> > > > @@ -115,7 +115,7 @@ InitializationDispatcherWorker
> (
> > > >          //
> > > >          // Dispatch records to devices based on
> > > feature flag.
> > > >          //
> > > > -        if (FeaturePcdGet
> (PcdStatusCodeUseSerial)) {
> > > > +        if (PcdGetBool (PcdStatusCodeUseSerial))
> {
> > > >            SerialStatusCodeReportWorker (
> > > >              Record[Index].CodeType,
> > > >              Record[Index].Value,
> > > > @@ -124,7 +124,7 @@ InitializationDispatcherWorker
> (
> > > >              NULL
> > > >              );
> > > >          }
> > > > -        if (FeaturePcdGet
> (PcdStatusCodeUseMemory)) {
> > > > +        if (PcdGetBool (PcdStatusCodeUseMemory))
> {
> > > >            RtMemoryStatusCodeReportWorker (
> > > >              Record[Index].CodeType,
> > > >              Record[Index].Value,
> > > > @@ -171,10 +171,10 @@
> StatusCodeHandlerRuntimeDxeEntry
> > > (
> > > >    //
> > > >    InitializationDispatcherWorker ();
> > > >
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > >      mRscHandlerProtocol->Register
> > > (SerialStatusCodeReportWorker,
> > > > TPL_HIGH_LEVEL);
> > > >    }
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > >      mRscHandlerProtocol->Register
> > > (RtMemoryStatusCodeReportWorker,
> > > > TPL_HIGH_LEVEL);
> > > >    }
> > > >
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.inf
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.inf
> > > > index d74c2a55dcaf..faadfd9578fe 100644
> > > > ---
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.inf
> > > > +++
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > atusCodeHandl
> > > > erRuntimeDxe.inf
> > > > @@ -58,12 +58,10 @@ [Guids]
> > > >  [Protocols]
> > > >    gEfiRscHandlerProtocolGuid
> ##
> > > CONSUMES
> > > >
> > > > -[FeaturePcd]
> > > > +[Pcd]
> > > >
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeReplayIn
> ##
> > > CONSUMES
> > > > -
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > CONSUMES
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > CONSUMES
> > > > -
> > > > -[Pcd]
> > > > +
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > CONSUMES
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> > > |128|
> > > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> > > ##
> > > > SOMETIMES_CONSUMES
> > > >
> > > >  [Depex]
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.
> > > > c
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.
> > > > c
> > > > index f54991ed3f67..20271571ded4 100644
> > > > ---
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.
> > > > c
> > > > +++
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.
> > > > c
> > > > @@ -2,7 +2,7 @@
> > > >    Status Code Handler Driver which produces
> general
> > > handlers and hook them
> > > >    onto the SMM status code router.
> > > >
> > > > -  Copyright (c) 2009 - 2018, Intel Corporation.
> All
> > > rights reserved.<BR>
> > > > +  Copyright (c) 2009 - 2020, Intel Corporation.
> All
> > > rights reserved.<BR>
> > > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >  **/
> > > > @@ -28,14 +28,14 @@ InitializationDispatcherWorker
> (
> > > >    // If enable UseSerial, then initialize serial
> > > port.
> > > >    // if enable UseRuntimeMemory, then initialize
> > > runtime memory status code
> > > > worker.
> > > >    //
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > >      //
> > > >      // Call Serial Port Lib API to initialize
> serial
> > > port.
> > > >      //
> > > >      Status = SerialPortInitialize ();
> > > >      ASSERT_EFI_ERROR (Status);
> > > >    }
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > >      Status = MemoryStatusCodeInitializeWorker ();
> > > >      ASSERT_EFI_ERROR (Status);
> > > >    }
> > > > @@ -73,10 +73,10 @@ StatusCodeHandlerSmmEntry (
> > > >    //
> > > >    InitializationDispatcherWorker ();
> > > >
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > >      mRscHandlerProtocol->Register
> > > (SerialStatusCodeReportWorker);
> > > >    }
> > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > >      mRscHandlerProtocol->Register
> > > (MemoryStatusCodeReportWorker);
> > > >    }
> > > >
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.i
> > > > nf
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.i
> > > > nf
> > > > index 47d0545f9591..4e24d87e55d1 100644
> > > > ---
> > > >
> > >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.i
> > > > nf
> > > > +++
> > > >
> > >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > eHandlerSmm.i
> > > > nf
> > > > @@ -53,11 +53,9 @@ [Guids]
> > > >  [Protocols]
> > > >    gEfiSmmRscHandlerProtocolGuid
> ##
> > > CONSUMES
> > > >
> > > > -[FeaturePcd]
> > > > -
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > CONSUMES
> > > > -
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > CONSUMES
> > > > -
> > > >  [Pcd]
> > > > +
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > CONSUMES
> > > > +
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > CONSUMES
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> > > |128|
> > > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> > > ##
> > > > SOMETIMES_CONSUMES
> > > >
> > > >  [Depex]
> > > > --
> > > > 2.24.0.windows.2
> > >
> > >
> > > 


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

* Re: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code.
  2020-06-18  1:46       ` Michael D Kinney
@ 2020-06-18  2:25         ` Wang, Jian J
  0 siblings, 0 replies; 7+ messages in thread
From: Wang, Jian J @ 2020-06-18  2:25 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Tan, Ming; +Cc: Wu, Hao A

Ming,

Sorry for previous comment. Although build supports it, the spec doesn't allow such
usage. Please revert to v4. Please keep R-B for v4.

Regards,
Jian

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Thursday, June 18, 2020 9:47 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Tan, Ming
> <ming.tan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: RE: [edk2-devel] [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for
> status code.
> 
> Jian,
> 
> There are few rules listed in the EDK II DEC File Format
> Specification that apply here:
> 
> https://edk2-docs.gitbook.io/edk-ii-dec-
> specification/3_edk_ii_dec_file_format/310_pcd_sections
> 
> * PCD entries may be put into any or all PCD section types
>   except PcdsFeatureFlag sections.
> 
> * PCDs listed in PcdsFeatureFlag sections must only be listed
>   in PcdsFeatureFlag sections.
> 
> * The PcdsFeatureFlag PcdType may not be mixed with any
>   other PcdType elements in the section header. The following
>   example is NOT VALID:
> 
>     [PcdsFeatureFlag.IA32, PcdsFixedAtBuild.IA32]
> 
> Best regards,
> 
> Mike
> 
> > -----Original Message-----
> > From: Wang, Jian J <jian.j.wang@intel.com>
> > Sent: Wednesday, June 17, 2020 5:58 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io; Tan, Ming <ming.tan@intel.com>
> > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v5 1/1]
> > MdeModulePkg.dec: Change PCDs for status code.
> >
> > Mike,
> >
> > I tried to build it. There's no warnings. And it's free
> > to change type to any of them.
> > Why's there such limitation? FeatureFlag looks the same
> > as FixedAtBuild to me.
> >
> > Regards,
> > Jian
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Thursday, June 18, 2020 12:47 AM
> > > To: devel@edk2.groups.io; Wang, Jian J
> > <jian.j.wang@intel.com>; Tan, Ming
> > > <ming.tan@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH v5 1/1]
> > MdeModulePkg.dec: Change PCDs for
> > > status code.
> > >
> > > Ming,
> > >
> > > With this change, are the same PCDs listed as both
> > > a Feature Flag PCD and a non-Feature Flag PCD?
> > >
> > > That should not be allowed, and if that passes the
> > > build, then the build tools should flag that as an
> > > error condition.
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> > > > Behalf Of Wang, Jian J
> > > > Sent: Wednesday, June 17, 2020 7:43 AM
> > > > To: Tan, Ming <ming.tan@intel.com>;
> > devel@edk2.groups.io
> > > > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > > > Subject: Re: [edk2-devel] [PATCH v5 1/1]
> > > > MdeModulePkg.dec: Change PCDs for status code.
> > > >
> > > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> > > >
> > > > Regards,
> > > > Jian
> > > >
> > > > > -----Original Message-----
> > > > > From: Tan, Ming <ming.tan@intel.com>
> > > > > Sent: Wednesday, June 17, 2020 3:36 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao
> > A
> > > > <hao.a.wu@intel.com>
> > > > > Subject: [PATCH v5 1/1] MdeModulePkg.dec: Change
> > PCDs
> > > > for status code.
> > > > >
> > > > > REF:
> > > > https://bugzilla.tianocore.org/show_bug.cgi?id=2786
> > > > >
> > > > > In order to support enable/disable report status
> > code
> > > > through memory
> > > > > or serial dynamic, change the following PCDs from
> > > > [PcdsFeatureFlag] to
> > > > > [PcdsFixedAtBuild, PcdsPatchableInModule,
> > PcdsDynamic,
> > > > PcdsDynamicEx]:
> > > > >   PcdStatusCodeUseSerial
> > > > >   PcdStatusCodeUseMemory
> > > > > The original plaforms can use PcdsFixedAtBuild in
> > .dsc
> > > > files to save size.
> > > > > Currently do not remove the old pcd type to keep
> > > > compatible.
> > > > >
> > > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > > Signed-off-by: Ming Tan <ming.tan@intel.com>
> > > > > ---
> > > > > V5: Do not remove th old pcd type to keep
> > compatible,
> > > > and change to
> > > > > standalone patch.
> > > > > V4: No change for this 1/4 patch, just modify the
> > 2-
> > > > 4/4 patchs.
> > > > > V3: Split one patch to several patchs, each Pkg
> > has
> > > > one patch.
> > > > > V2: Change the new type from [PcdsDynamic] to
> > > > [PcdsFixedAtBuild,
> > > > > PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> > > > >     And set to PcdsFixedAtBuild in the original
> > > > platform .dsc files.
> > > > >
> > > > >  MdeModulePkg/MdeModulePkg.dec
> > | 13
> > > > +++++++++++++
> > > > >  .../StatusCodeHandler/Pei/StatusCodeHandlerPei.c
> > |  6
> > > > +++---
> > > > >  .../Pei/StatusCodeHandlerPei.inf
> > |  6
> > > > ++----
> > > > >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c
> > | 16
> > > > ++++++++--------
> > > > >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
> > |  6
> > > > ++----
> > > > >  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.c
> > | 10
> > > > +++++-----
> > > > >  .../Smm/StatusCodeHandlerSmm.inf
> > |  6
> > > > ++----
> > > > >  7 files changed, 35 insertions(+), 28 deletions(-
> > )
> > > > >
> > > > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > > > b/MdeModulePkg/MdeModulePkg.dec
> > > > > index 4f44af694862..bd369d1f230e 100644
> > > > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > > > @@ -2001,6 +2001,19 @@ [PcdsFixedAtBuild,
> > > > PcdsPatchableInModule,
> > > > > PcdsDynamic, PcdsDynamicEx]
> > > > >    # @Prompt TCG Platform Firmware Profile
> > revision.
> > > > >
> > > > >
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevis
> > > > ion|0|UINT3
> > > > > 2|0x00010077
> > > > >
> > > > > +  ## Indicates if StatusCode is reported via
> > Serial
> > > > port.<BR><BR>
> > > > > +  #   TRUE  - Reports StatusCode via Serial
> > port.<BR>
> > > > > +  #   FALSE - Does not report StatusCode via
> > Serial
> > > > port.<BR>
> > > > > +  # @Prompt Enable StatusCode via Serial port.
> > > > > +
> > > > >
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TR
> > > > UE|BOOLEAN|
> > > > > 0x00010022
> > > > > +
> > > > > +  ## Indicates if StatusCode is stored in memory.
> > > > > +  #  The memory is boot time memory in PEI Phase
> > and
> > > > is runtime memory in
> > > > > DXE Phase.<BR><BR>
> > > > > +  #   TRUE  - Stores StatusCode in memory.<BR>
> > > > > +  #   FALSE - Does not store StatusCode in
> > > > memory.<BR>
> > > > > +  # @Prompt Enable StatusCode via memory.
> > > > > +
> > > > >
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FA
> > > > LSE|BOOLEA
> > > > > N|0x00010023
> > > > > +
> > > > >  [PcdsPatchableInModule]
> > > > >    ## Specify memory size with page number for PEI
> > > > code when
> > > > >    #  Loading Module at Fixed Address feature is
> > > > enabled.
> > > > > diff --git
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.c
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.c
> > > > > index 1b07f92f3ce8..9b2ea4ee84d9 100644
> > > > > ---
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.c
> > > > > +++
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.c
> > > > > @@ -2,7 +2,7 @@
> > > > >    Report Status Code Handler PEIM which produces
> > > > general handlers and hook
> > > > > them
> > > > >    onto the PEI status code router.
> > > > >
> > > > > -  Copyright (c) 2009 - 2018, Intel Corporation.
> > All
> > > > rights reserved.<BR>
> > > > > +  Copyright (c) 2009 - 2020, Intel Corporation.
> > All
> > > > rights reserved.<BR>
> > > > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > >  **/
> > > > > @@ -45,13 +45,13 @@ StatusCodeHandlerPeiEntry (
> > > > >    // If enable UseSerial, then initialize serial
> > > > port.
> > > > >    // if enable UseMemory, then initialize memory
> > > > status code worker.
> > > > >    //
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > > >      Status = SerialPortInitialize();
> > > > >      ASSERT_EFI_ERROR (Status);
> > > > >      Status = RscHandlerPpi->Register
> > > > (SerialStatusCodeReportWorker);
> > > > >      ASSERT_EFI_ERROR (Status);
> > > > >    }
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > > >      Status = MemoryStatusCodeInitializeWorker ();
> > > > >      ASSERT_EFI_ERROR (Status);
> > > > >      Status = RscHandlerPpi->Register
> > > > (MemoryStatusCodeReportWorker);
> > > > > diff --git
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.inf
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.inf
> > > > > index 8aef9af34a05..64380ddfaccc 100644
> > > > > ---
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.inf
> > > > > +++
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCod
> > > > eHandlerPei.inf
> > > > > @@ -53,11 +53,9 @@ [Guids]
> > > > >  [Ppis]
> > > > >    gEfiPeiRscHandlerPpiGuid
> > ##
> > > > CONSUMES
> > > > >
> > > > > -[FeaturePcd]
> > > > > -
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > > CONSUMES
> > > > > -
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > > CONSUMES
> > > > > -
> > > > >  [Pcd]
> > > > > +
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > > CONSUMES
> > > > > +
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > > CONSUMES
> > > > >
> > > > >
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> > > > |gEfiMdeMod
> > > > > ulePkgTokenSpaceGuid.PcdStatusCodeUseMemory    ##
> > > > > SOMETIMES_CONSUMES
> > > > >
> > > > >  [Depex]
> > > > > diff --git
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.c
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.c
> > > > > index 79cc48fa55a4..a8c0fe5b7149 100644
> > > > > ---
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.c
> > > > > +++
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.c
> > > > > @@ -2,7 +2,7 @@
> > > > >    Status Code Handler Driver which produces
> > general
> > > > handlers and hook them
> > > > >    onto the DXE status code router.
> > > > >
> > > > > -  Copyright (c) 2006 - 2019, Intel Corporation.
> > All
> > > > rights reserved.<BR>
> > > > > +  Copyright (c) 2006 - 2020, Intel Corporation.
> > All
> > > > rights reserved.<BR>
> > > > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > >  **/
> > > > > @@ -29,7 +29,7 @@ UnregisterBootTimeHandlers (
> > > > >    IN VOID             *Context
> > > > >    )
> > > > >  {
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > > >      mRscHandlerProtocol->Unregister
> > > > (SerialStatusCodeReportWorker);
> > > > >    }
> > > > >  }
> > > > > @@ -80,14 +80,14 @@ InitializationDispatcherWorker
> > (
> > > > >    // If enable UseSerial, then initialize serial
> > > > port.
> > > > >    // if enable UseRuntimeMemory, then initialize
> > > > runtime memory status code
> > > > > worker.
> > > > >    //
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > > >      //
> > > > >      // Call Serial Port Lib API to initialize
> > serial
> > > > port.
> > > > >      //
> > > > >      Status = SerialPortInitialize ();
> > > > >      ASSERT_EFI_ERROR (Status);
> > > > >    }
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > > >      Status = RtMemoryStatusCodeInitializeWorker
> > ();
> > > > >      ASSERT_EFI_ERROR (Status);
> > > > >    }
> > > > > @@ -115,7 +115,7 @@ InitializationDispatcherWorker
> > (
> > > > >          //
> > > > >          // Dispatch records to devices based on
> > > > feature flag.
> > > > >          //
> > > > > -        if (FeaturePcdGet
> > (PcdStatusCodeUseSerial)) {
> > > > > +        if (PcdGetBool (PcdStatusCodeUseSerial))
> > {
> > > > >            SerialStatusCodeReportWorker (
> > > > >              Record[Index].CodeType,
> > > > >              Record[Index].Value,
> > > > > @@ -124,7 +124,7 @@ InitializationDispatcherWorker
> > (
> > > > >              NULL
> > > > >              );
> > > > >          }
> > > > > -        if (FeaturePcdGet
> > (PcdStatusCodeUseMemory)) {
> > > > > +        if (PcdGetBool (PcdStatusCodeUseMemory))
> > {
> > > > >            RtMemoryStatusCodeReportWorker (
> > > > >              Record[Index].CodeType,
> > > > >              Record[Index].Value,
> > > > > @@ -171,10 +171,10 @@
> > StatusCodeHandlerRuntimeDxeEntry
> > > > (
> > > > >    //
> > > > >    InitializationDispatcherWorker ();
> > > > >
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > > >      mRscHandlerProtocol->Register
> > > > (SerialStatusCodeReportWorker,
> > > > > TPL_HIGH_LEVEL);
> > > > >    }
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > > >      mRscHandlerProtocol->Register
> > > > (RtMemoryStatusCodeReportWorker,
> > > > > TPL_HIGH_LEVEL);
> > > > >    }
> > > > >
> > > > > diff --git
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.inf
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.inf
> > > > > index d74c2a55dcaf..faadfd9578fe 100644
> > > > > ---
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.inf
> > > > > +++
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/St
> > > > atusCodeHandl
> > > > > erRuntimeDxe.inf
> > > > > @@ -58,12 +58,10 @@ [Guids]
> > > > >  [Protocols]
> > > > >    gEfiRscHandlerProtocolGuid
> > ##
> > > > CONSUMES
> > > > >
> > > > > -[FeaturePcd]
> > > > > +[Pcd]
> > > > >
> > > > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeReplayIn
> > ##
> > > > CONSUMES
> > > > > -
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > > CONSUMES
> > > > >
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > > CONSUMES
> > > > > -
> > > > > -[Pcd]
> > > > > +
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > > CONSUMES
> > > > >
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> > > > |128|
> > > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> > > > ##
> > > > > SOMETIMES_CONSUMES
> > > > >
> > > > >  [Depex]
> > > > > diff --git
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.
> > > > > c
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.
> > > > > c
> > > > > index f54991ed3f67..20271571ded4 100644
> > > > > ---
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.
> > > > > c
> > > > > +++
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.
> > > > > c
> > > > > @@ -2,7 +2,7 @@
> > > > >    Status Code Handler Driver which produces
> > general
> > > > handlers and hook them
> > > > >    onto the SMM status code router.
> > > > >
> > > > > -  Copyright (c) 2009 - 2018, Intel Corporation.
> > All
> > > > rights reserved.<BR>
> > > > > +  Copyright (c) 2009 - 2020, Intel Corporation.
> > All
> > > > rights reserved.<BR>
> > > > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > >  **/
> > > > > @@ -28,14 +28,14 @@ InitializationDispatcherWorker
> > (
> > > > >    // If enable UseSerial, then initialize serial
> > > > port.
> > > > >    // if enable UseRuntimeMemory, then initialize
> > > > runtime memory status code
> > > > > worker.
> > > > >    //
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > > >      //
> > > > >      // Call Serial Port Lib API to initialize
> > serial
> > > > port.
> > > > >      //
> > > > >      Status = SerialPortInitialize ();
> > > > >      ASSERT_EFI_ERROR (Status);
> > > > >    }
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > > >      Status = MemoryStatusCodeInitializeWorker ();
> > > > >      ASSERT_EFI_ERROR (Status);
> > > > >    }
> > > > > @@ -73,10 +73,10 @@ StatusCodeHandlerSmmEntry (
> > > > >    //
> > > > >    InitializationDispatcherWorker ();
> > > > >
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseSerial)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseSerial)) {
> > > > >      mRscHandlerProtocol->Register
> > > > (SerialStatusCodeReportWorker);
> > > > >    }
> > > > > -  if (FeaturePcdGet (PcdStatusCodeUseMemory)) {
> > > > > +  if (PcdGetBool (PcdStatusCodeUseMemory)) {
> > > > >      mRscHandlerProtocol->Register
> > > > (MemoryStatusCodeReportWorker);
> > > > >    }
> > > > >
> > > > > diff --git
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.i
> > > > > nf
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.i
> > > > > nf
> > > > > index 47d0545f9591..4e24d87e55d1 100644
> > > > > ---
> > > > >
> > > >
> > a/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.i
> > > > > nf
> > > > > +++
> > > > >
> > > >
> > b/MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCod
> > > > eHandlerSmm.i
> > > > > nf
> > > > > @@ -53,11 +53,9 @@ [Guids]
> > > > >  [Protocols]
> > > > >    gEfiSmmRscHandlerProtocolGuid
> > ##
> > > > CONSUMES
> > > > >
> > > > > -[FeaturePcd]
> > > > > -
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > > CONSUMES
> > > > > -
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > > CONSUMES
> > > > > -
> > > > >  [Pcd]
> > > > > +
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial ##
> > > > CONSUMES
> > > > > +
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory ##
> > > > > CONSUMES
> > > > >
> > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize
> > > > |128|
> > > > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory
> > > > ##
> > > > > SOMETIMES_CONSUMES
> > > > >
> > > > >  [Depex]
> > > > > --
> > > > > 2.24.0.windows.2
> > > >
> > > >
> > > > 


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

end of thread, other threads:[~2020-06-18  2:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-17  7:35 [PATCH v5 1/1] MdeModulePkg.dec: Change PCDs for status code Tan, Ming
2020-06-17 14:42 ` Wang, Jian J
2020-06-17 16:47   ` [edk2-devel] " Michael D Kinney
2020-06-18  0:40     ` Tan, Ming
2020-06-18  0:57     ` Wang, Jian J
2020-06-18  1:46       ` Michael D Kinney
2020-06-18  2:25         ` Wang, Jian J

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