Re: [edk2-devel][edk2-platforms][PATCH V1 3/3] PlatformPayloadFeaturePkg: Update contents to match feature conventions

["Guo Dong" <guo.dong@intel.com>]

Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: Oram, Isaac W <isaac.w.oram@intel.com> 
Sent: Wednesday, August 10, 2022 3:47 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Dong, Guo <guo.dong@intel.com>
Subject: [edk2-devel][edk2-platforms][PATCH V1 3/3] PlatformPayloadFeaturePkg: Update contents to match feature conventions

Rename gPlatformPayloadPkgTokenSpaceGuid to gPlatformPayloadFeaturePkgTokenSpaceGuid

Rename PcdPayloadPackageFeatureEnable to PcdPlatformPayloadFeatureEnable

Update Readme including format improvements

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
---
 .../AdvancedFeaturePkg/AdvancedFeaturePkg.dsc |  2 +-
 .../Include/AdvancedFeatures.dsc              |  2 +-
 .../Include/AdvancedFeaturesPcd.dsc           |  2 +-
 .../AdvancedFeaturePkg/Include/PostMemory.fdf |  2 +-  .../AdvancedFeaturePkg/Include/PreMemory.fdf  |  2 +-  .../PlatformPayloadFeaturePkg/Fvb/FvbSmm.inf  |  2 +-
 .../PlatformPayloadFeaturePkg.dec             |  6 +-
 .../Intel/PlatformPayloadFeaturePkg/Readme.md | 73 ++++++++++---------
 8 files changed, 48 insertions(+), 43 deletions(-)

diff --git a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
index df8e678ae8..236c5570ca 100644
--- a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
+++ b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
@@ -84,7 +84,7 @@
   #
   # Individual features
   #
-  gPlatformPayloadPkgTokenSpaceGuid.PcdPayloadPackageFeatureEnable          |TRUE
+  
+ gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdPlatformPayloadFeatureEnab
+ le  |TRUE
 
 #
 # PCD that are required to be set by the build target should be configured here for test purposes diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc
index 6a80546592..804aab89bc 100644
--- a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc
+++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc
@@ -79,6 +79,6 @@
 #
 # Individual features
 #
-!if gPlatformPayloadPkgTokenSpaceGuid.PcdPayloadPackageFeatureEnable == TRUE
+!if 
+gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdPlatformPayloadFeatureEnabl
+e == TRUE
   !include PlatformPayloadFeaturePkg/Include/PlatformPayloadFeature.dsc
 !endif
diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
index 08f258a48c..0af5a8bfbc 100644
--- a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
+++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
@@ -82,7 +82,7 @@
   gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable      |FALSE
   gVirtualKeyboardFeaturePkgTokenSpaceGuid.PcdVirtualKeyboardFeatureEnable  |FALSE
 
-  gPlatformPayloadPkgTokenSpaceGuid.PcdPayloadPackageFeatureEnable          |FALSE
+  
+ gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdPlatformPayloadFeatureEnab
+ le  |FALSE
 
 #
 # There seems to be some build parsing odd behavior that requires this PCD to be specified even though diff --git a/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf b/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf
index 66032ba117..349bdcc491 100644
--- a/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf
+++ b/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf
@@ -80,6 +80,6 @@
 #
 # Individual features
 #
-!if gPlatformPayloadPkgTokenSpaceGuid.PcdPayloadPackageFeatureEnable == TRUE
+!if 
+gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdPlatformPayloadFeatureEnabl
+e == TRUE
   !include PlatformPayloadFeaturePkg/Include/PostMemory.fdf
 !endif
diff --git a/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf b/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf
index 9cd6de833b..b991a5aabf 100644
--- a/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf
+++ b/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf
@@ -80,6 +80,6 @@
 #
 # Individual features
 #
-!if gPlatformPayloadPkgTokenSpaceGuid.PcdPayloadPackageFeatureEnable == TRUE
+!if 
+gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdPlatformPayloadFeatureEnabl
+e == TRUE
   !include PlatformPayloadFeaturePkg/Include/PreMemory.fdf
 !endif
diff --git a/Features/Intel/PlatformPayloadFeaturePkg/Fvb/FvbSmm.inf b/Features/Intel/PlatformPayloadFeaturePkg/Fvb/FvbSmm.inf
index f973465cba..a692d9bf41 100644
--- a/Features/Intel/PlatformPayloadFeaturePkg/Fvb/FvbSmm.inf
+++ b/Features/Intel/PlatformPayloadFeaturePkg/Fvb/FvbSmm.inf
@@ -60,7 +60,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
-  gPlatformPayloadPkgTokenSpaceGuid.PcdNvsDataFile
+  gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdNvsDataFile
 
 [Depex]
   TRUE
diff --git a/Features/Intel/PlatformPayloadFeaturePkg/PlatformPayloadFeaturePkg.dec b/Features/Intel/PlatformPayloadFeaturePkg/PlatformPayloadFeaturePkg.dec
index 78d36f12eb..f02c5f70ca 100644
--- a/Features/Intel/PlatformPayloadFeaturePkg/PlatformPayloadFeaturePkg.dec
+++ b/Features/Intel/PlatformPayloadFeaturePkg/PlatformPayloadFeaturePkg
+++ .dec
@@ -21,7 +21,7 @@
   #
   ## Defines the token space for the Payload platform Package PCDs.
   #
-  gPlatformPayloadPkgTokenSpaceGuid  = {0xa375ddc6, 0x74fb, 0x4de1, {0x82, 0x6d, 0xa3, 0x10, 0xa3, 0x3b, 0x89, 0x77}}
+  gPlatformPayloadFeaturePkgTokenSpaceGuid  = {0xa375ddc6, 0x74fb, 
+ 0x4de1, {0x82, 0x6d, 0xa3, 0x10, 0xa3, 0x3b, 0x89, 0x77}}
 
   #
   # SPI variable support
@@ -41,9 +41,9 @@
 [PcdsFeatureFlag]
   ## Feature enabling control
   # @Prompt Enable payload platform FV common for most Intel platforms
-  gPlatformPayloadPkgTokenSpaceGuid.PcdPayloadPackageFeatureEnable|FALSE|BOOLEAN|0x00000001
+  
+ gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdPlatformPayloadFeatureEnab
+ le|FALSE|BOOLEAN|0x00000001
 
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## FFS filename to find the default variable initial data file.
   # @Prompt FFS Name of variable initial data file
-  gPlatformPayloadPkgTokenSpaceGuid.PcdNvsDataFile |{ 0x1a, 0xf1, 0xb1, 0xae, 0x42, 0xcc, 0xcf, 0x4e, 0xac, 0x60, 0xdb, 0xab, 0xf6, 0xca, 0x69, 0xe6 }|VOID*|0x00000025
+  gPlatformPayloadFeaturePkgTokenSpaceGuid.PcdNvsDataFile |{ 0x1a, 
+ 0xf1, 0xb1, 0xae, 0x42, 0xcc, 0xcf, 0x4e, 0xac, 0x60, 0xdb, 0xab, 
+ 0xf6, 0xca, 0x69, 0xe6 }|VOID*|0x00000025
diff --git a/Features/Intel/PlatformPayloadFeaturePkg/Readme.md b/Features/Intel/PlatformPayloadFeaturePkg/Readme.md
index 1067196388..9cdb26285c 100644
--- a/Features/Intel/PlatformPayloadFeaturePkg/Readme.md
+++ b/Features/Intel/PlatformPayloadFeaturePkg/Readme.md
@@ -19,31 +19,45 @@ universal payload specification. And a Platform Payload could be built from Plat  to provide Intel platform specific features (e.g. SPI module, PCH SMM) in FV/FD format.
 This platform payload could be inserted into universal UEFI payload as an ELF section  to generate a full-feature payload.
+
 ## Firmware Volumes
-* FvPlatformPayload
+* PlatformPayload.fv
 
 ## Build Flows
-use windows host as example to generate a full-feature payload:
+Use Windows host as example to generate a full-feature payload:
 
-** Setup the build env
+* Setup the build env
+<pre>
 set WORKSPACE=c:\payload
-set PACKAGES_PATH=%WORKSPACE%\edk2;%WORKSPACE%\edk2-platforms\Features\Intel;
-    %WORKSPACE%\edk2-platforms\Platform\Intel
+set 
+PACKAGES_PATH=%WORKSPACE%\edk2;%WORKSPACE%\edk2-platforms\Features\Inte
+l;%WORKSPACE%\edk2-platforms\Platform\Intel
 edk2\edksetup.bat
-
-** Build universal UEFI payload with platform Payload
+</pre>
+* Build universal UEFI payload with platform payload <pre>
 python edk2-platforms\Features\Intel\PlatformPayloadFeaturePkg\PlatformPayloadFeaturePkg.py -t VS2019 -D SMM_SUPPORT=TRUE -DVARIABLE_SUPPORT=NONE -D SMM_VARIABLE=TRUE -or
+</pre>
+* Build universal UEFI payload then build the platform payload and 
+patch it to the universal UEFI payload <pre>
 python edk2\UefiPayloadPkg\UniversalPayloadBuild.py -t VS2019 -D SMM_SUPPORT=TRUE -DVARIABLE_SUPPORT=NONE  python edk2-platforms\Features\Intel\PlatformPayloadFeaturePkg\PlatformPayloadFeaturePkg.py -t VS2019 -D SMM_VARIABLE=TRUE -s
+</pre>
 
-  If build success, the final UEFI payload is at Build\UefiPayloadPkgX64\UniversalPayload.elf.
+If build succeeds, the final UEFI payload is at 
+<B>Build\UefiPayloadPkgX64\UniversalPayload.elf</B>
 
+Note that standalone feature package build works with "-a X64" or "-a IA32 -a X64" but "-a IA32" is not supported.
+Note that this does not patch the universal UEFI payload, it only creates a PlatformPayload.fv.
+<pre>
+build -p PlatformPayloadFeaturePkg/PlatformPayloadFeaturePkg.dsc -a X64 
+-a IA32 </pre>
 ## Features
 
-1. Modules
-Currently only SMM veriable feature is available.
+1. SMM variable feature configuration
+  * PcdPlatformPayloadFeatureEnable - Enables this feature.
+  * SMM_VARIABLE - TRUE enables the SPI SMM variable feature implementation.
+
+2. Modules: Currently only SMM variable feature is available.
 Several build macros are used as below for SMM variable feature modules.
+<pre>
 !if $(SMM_VARIABLE) == TRUE
   ## PchSmiDispatchSmm
   ## FvbSmm
@@ -51,11 +65,16 @@ Several build macros are used as below for SMM variable feature modules.
   ## VariableSmm
   ## VariableSmmRuntimeDxe
 !endif
+</pre>
+
+3. Data Flows
 
-2. Data Flows
 SMM variable:
+
 The interface with bootloader are defined in PlatformPayloadFeaturePkg\Include\Guid
+
 SpiFlashInfoGuid.h    -- SPI related information for SPI flash operation.
+
 NvVariableInfoGuid.h  -- Variable related information for SPI NV variables.
 
 ## Control Flows
@@ -63,33 +82,19 @@ EDK2 DXE/SMM core from universal UEFI payload would dispatch all the modules  from this platform payload.
 
 ## Test Point Results
-*_TODO_*
-The test(s) that can verify porting is complete for the feature.
-
-Each feature must describe at least one test point to verify the feature is successful. If the test point is not -implemented, this should be stated.
+None implemented
 
 ## Functional Exit Criteria
-*_TODO_*
-The testable functionality for the feature.
-
-This section should provide an ordered list of criteria that a board integrator can reference to ensure the feature is -functional on their board.
+Boot to UEFI shell and verify variable functionality over resets
 
 ## Feature Enabling Checklist
-*_TODO_*
-An ordered list of required activities to achieve desired functionality for the feature.
+Verify configuration of PcdPlatformPayloadFeatureEnable and 
+SMM_VARIABLE Boot to UEFI Shell Update a variable Reset the system and 
+verify variable remains updated
 
 ## Performance Impact
-A general expectation for the impact on overall boot performance due to using this feature.
-
-This section is expected to provide guidance on:
-* How to estimate performance impact due to the feature
-* How to measure performance impact of the feature
-* How to manage performance impact of the feature
+Minimal expected
 
 ## Common Optimizations
-*_TODO_*
-Common size or performance tuning options for this feature.
-
-This section is recommended but not required. If not used, the contents should be left empty.
+None
--
2.36.1.windows.1