Re: [edk2-platforms][PATCH V1 1/1] Features/Intel/AcpiDebugFeaturePkg: Add feature active PCD

["Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>]

Reviewed-by: Sai Chaganty <rangasai.v.chaganty@intel.com>

-----Original Message-----
From: Kubacki, Michael A 
Sent: Wednesday, December 18, 2019 11:58 PM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2-platforms][PATCH V1 1/1] Features/Intel/AcpiDebugFeaturePkg: Add feature active PCD

Adds a dynamic PCD that specifies whether the feature is active.

This is useful because the feature might be enabled via FeatureFlag PCD PcdAcpiDebugFeatureEnable meaning it is built and included in the flash image but the board might need to control whether the feature is active based on input such as a Setup menu option.

A deactivated feature will be dispatched but simply exit in the entry point.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
 Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec          | 14 +++++++++++++-
 Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugDxe.inf |  5 +++--  Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugSmm.inf |  5 +++--
 Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebug.c      |  4 ++++
 Features/Intel/Debugging/AcpiDebugFeaturePkg/Readme.md                        |  1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
index a74ee3e084..c3bd89fe2b 100644
--- a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
+++ b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.d
+++ ec
@@ -34,6 +34,18 @@
   gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugBufferSize|0x10000|UINT32|0xF0000001
 
 [PcdsDynamic, PcdsDynamicEx]
+  ## This PCD specifies whether the feature is active.
+  #
+  #  The PCD value can be modified at boot time to activate and deactivate the feature functionality.
+  #
+  #  Note that this differs from Pcd<FeatureName>FeatureEnable which 
+ determines whether the feature is included  #  as an advanced feature 
+ in the flash image. If enabled, the feature is then eligible to be activated/deactivated  #  via this PCD.
+  #
+  #  An enabled but deactivated feature will be dispatched but exit in 
+ its entry point without performing any  #  feature actions.
+  
+ gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureActive|TRUE|BOOL
+ EAN|0xD0000001
+
   ## This PCD specifies ACPI debug message buffer address.
   #  The PCD value will be updated during boot time when the buffer is allocated.
-  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugAddress|0|UINT32|0xD0000001
+  
+ gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugAddress|0|UINT32|0xD000
+ 0002
diff --git a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugDxe.inf b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugDxe.inf
index dfcee0a842..b293d83cd9 100644
--- a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugDxe.inf
+++ b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
+++ ebugDxe.inf
@@ -35,8 +35,9 @@
   Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
 
 [Pcd]
-  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugBufferSize  ## CONSUMES
-  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugAddress     ## PRODUCES
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureActive  ## CONSUMES
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugBufferSize     ## CONSUMES
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugAddress        ## PRODUCES
 
 [Sources]
   AcpiDebug.c
diff --git a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugSmm.inf b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugSmm.inf
index 089bb57707..dbb33e4ae2 100644
--- a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugSmm.inf
+++ b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
+++ ebugSmm.inf
@@ -36,8 +36,9 @@
   Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
 
 [Pcd]
-  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugBufferSize  ## CONSUMES
-  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugAddress     ## PRODUCES
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureActive  ## CONSUMES
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugBufferSize     ## CONSUMES
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugAddress        ## PRODUCES
 
 [Sources]
   AcpiDebug.c
diff --git a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebug.c b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebug.c
index 42428d8005..4caeffc7de 100644
--- a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebug.c
+++ b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
+++ ebug.c
@@ -289,6 +289,10 @@ InitializeAcpiDebugDxe (
   EFI_STATUS            Status;
   EFI_EVENT             EndOfDxeEvent;
 
+  if (!PcdGetBool (PcdAcpiDebugFeatureActive)) {
+    return EFI_SUCCESS;
+  }
+
   //
   // Register EndOfDxe notification
   // that point could ensure the Acpi Debug related PCDs initialized.
diff --git a/Features/Intel/Debugging/AcpiDebugFeaturePkg/Readme.md b/Features/Intel/Debugging/AcpiDebugFeaturePkg/Readme.md
index bc0dce9348..bbd7675c11 100644
--- a/Features/Intel/Debugging/AcpiDebugFeaturePkg/Readme.md
+++ b/Features/Intel/Debugging/AcpiDebugFeaturePkg/Readme.md
@@ -78,6 +78,7 @@ instance assigned to `AcpiDebugSmm`.
 
 ## Configuration
 * PcdAcpiDebugEnable - Enables this feature.
+* PcdAcpiDebugFeatureActive - Activates this feature.
 * PcdAcpiDebugAddress - The address of the ACPI debug message buffer.
 * PcdAcpiDebugBufferSize - The size of the ACPI debug message buffer.
 
--
2.16.2.windows.1