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

["Chiu, Chasel" <chasel.chiu@intel.com>]

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kubacki,
> Michael A
> Sent: Thursday, December 19, 2019 3: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-devel] [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/AcpiDe
> bugDxe.inf |  5 +++--
> Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDe
> bugSmm.inf |  5 +++--
> 
> Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDe
> bug.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|UI
> NT32|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|B
> OOL
> + 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|0x
> D0000001
> +
> +
> gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugAddress|0|UINT32|0x
> D000
> + 0002
> diff --git
> a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
> ebugDxe.inf
> b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
> ebugDxe.inf
> index dfcee0a842..b293d83cd9 100644
> ---
> a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
> ebugDxe.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/AcpiD
> ebugSmm.inf
> b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
> ebugSmm.inf
> index 089bb57707..dbb33e4ae2 100644
> ---
> a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
> ebugSmm.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/AcpiD
> ebug.c
> b/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
> ebug.c
> index 42428d8005..4caeffc7de 100644
> ---
> a/Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiD
> ebug.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
> 
> 
>