Re: [Patch 00/14] Enable Structure PCD support in edk2

["Zeng, Star" <star.zeng@intel.com>]

Liming,

I have some comments to the patch series about MdeModulePkg changes.

For MdeModulePkg: Update PCD driver to support Dynamic PcdVpdBaseAddress,
Could the if condition adjustment be removed? And how about declaring "extern UINT32 mVpdBaseAddress;" in Service.h instead of Service.c?

For MdeModulePkg: Enable Firmware to retrieve the default setting,
Could this patch be spilt to three by driver update or more patches?
Could the code have more specific name for DATA_DELTA, and etc?
Please update MdeModulePkg.uni for new PCDs PcdSetNvStoreDefaultId and PcdNvStoreDefaultValueBuffer.
Should more comments be added for PcdSetNvStoreDefaultId to state that this PCD should be set at PEI phase?
Should PcdSetNvStoreDefaultIdCallBack use current buffer length instead of max buffer length for NvStoreDefaultValueBuffer?
Please re-check the code logic in ConvertNormalVarStorageToAuthVarStorage() about pad between header, name and data.



Thanks,
Star
-----Original Message-----
From: Gao, Liming 
Sent: Friday, December 1, 2017 10:59 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [Patch 00/14] Enable Structure PCD support in edk2

All code are in https://github.com/tianocore/edk2-staging/tree/StructurePcdV3.
Patches are 544d8e..dd79a7. Unit Test cases are 89e7c7..1b2d19.
The patches will not be sent by mail. Please review them in edk2-staging.

Structure PCD is to support the same C structure/Enum/Macro definition in PCD,
and specify the value for individual fields in a VOID* PCD in DEC/DSC. It
includes the group of the sub features.
[BaseTools] Structure PCD value assignment in DEC/DSC
https://bugzilla.tianocore.org/show_bug.cgi?id=542
[BaseTools] Extended SKU Support - Inheritance
https://bugzilla.tianocore.org/show_bug.cgi?id=543
[BaseTools/PCD] Pcd Database size optimization for multi-SKU
https://bugzilla.tianocore.org/show_bug.cgi?id=546
[BaseTools/PCD/HiiDataBase] Collect DynamicHii PCD value as the default setting
for EFI Variable and HII VarStore.
https://bugzilla.tianocore.org/show_bug.cgi?id=611
[BaseTools] Build report update for Structure PCD value and SKU, DefaultStore
https://bugzilla.tianocore.org/show_bug.cgi?id=706

The main changes are in BaseTools to calculate structure PCD value.
Firmware code Pcd driver, HiiDataBase and Variable driver are updated.
And, PCD database format has been optimized. So, its version is changed
from current 6 to 7. Besides it, others are compatible. There is no impact
on current platform and current usage. If you find any breaking,
please raise it to me.

The detail changes are specified in each tracker and each RFC.

Unit Test:
1) Windows VS2015 Build TestPkg and Nt32Pkg
2) Linux GCC5 Build OvmfPkg
3) Verify Strucutre PCD field value collection
4) Verify Strucutre PCD from the different SKUs
5) Structure DynamicHii PCD is saved as the default EFI variable
6) Structure DynamicHii PCD impacts HII IFR default setting
7) Strucutre PCD field value is shown in build report.
8) Multiple SKU PCD values is shown in build report.

Liming Gao (14):
  BaseTools: Update Makefile to work at absolute path
  BaseTools: Add PcdValueCommon logic into C source CommonLib
  MdeModulePkg: Update PCD driver to support Dynamic PcdVpdBaseAddress
  MdeModulePkg: Enable Firmware to retrieve the default setting
  MdeModulePkg: Update PCD driver to support the optimized PcdDataBase
  MdeModulePkg: Pcd/HiiDatabase Update default setting format with max
    size
  BaseTools: Support Structure PCD value assignment in DEC/DSC
  BaseTools: Collect DynamicHii PCD values and assign it to VPD PCD
    Value
  BaseTools: Support Structure PCD value inherit between the different
    SKUs
  BaseTools: PcdDataBase Optimization for multiple SkuIds
  BaseTools: Report Structure PCD value and SKU, DefaultStore info
  BaseTools: Update NV Default Header format to include the max size
  BaseTools: Optimize VPD PCD value for the different SKUs
  BaseTools: Fixed the issue of Multiple Skus are always disables

 BaseTools/Source/C/Common/GNUmakefile              |    3 +-
 BaseTools/Source/C/Common/Makefile                 |    3 +-
 BaseTools/Source/C/Common/PcdValueCommon.c         |  607 +++++
 BaseTools/Source/C/Common/PcdValueCommon.h         |   78 +
 BaseTools/Source/C/Makefiles/app.makefile          |    2 +-
 BaseTools/Source/C/Makefiles/ms.app                |    4 +-
 BaseTools/Source/Python/AutoGen/AutoGen.py         |  406 ++-
 BaseTools/Source/Python/AutoGen/GenC.py            |   77 +-
 BaseTools/Source/Python/AutoGen/GenMake.py         |    2 +-
 BaseTools/Source/Python/AutoGen/GenPcdDb.py        |  260 +-
 BaseTools/Source/Python/AutoGen/GenVar.py          |  341 +++
 BaseTools/Source/Python/Common/BuildToolError.py   |    3 +
 BaseTools/Source/Python/Common/DataType.py         |    2 +
 BaseTools/Source/Python/Common/GlobalData.py       |    6 +
 BaseTools/Source/Python/Common/Misc.py             |  221 +-
 .../Source/Python/Common/ToolDefClassObject.py     |    1 -
 BaseTools/Source/Python/Common/VpdInfoFile.py      |   11 +-
 .../Source/Python/CommonDataClass/CommonClass.py   |    5 +-
 .../Source/Python/CommonDataClass/DataClass.py     |   11 +
 .../Source/Python/Workspace/BuildClassObject.py    |   66 +
 BaseTools/Source/Python/Workspace/DecBuildData.py  |  463 ++++
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 1961 ++++++++++++++
 BaseTools/Source/Python/Workspace/InfBuildData.py  | 1212 +++++++++
 .../Source/Python/Workspace/MetaFileParser.py      |  311 ++-
 BaseTools/Source/Python/Workspace/MetaFileTable.py |   11 +-
 .../Source/Python/Workspace/WorkspaceCommon.py     |   44 +-
 .../Source/Python/Workspace/WorkspaceDatabase.py   | 2834 +-------------------
 BaseTools/Source/Python/build/BuildReport.py       |  296 +-
 BaseTools/Source/Python/build/build.py             |    2 +
 .../Include/Guid/PcdDataBaseSignatureGuid.h        |   82 +-
 MdeModulePkg/MdeModulePkg.dec                      |   20 +-
 MdeModulePkg/Universal/HiiDatabaseDxe/Database.c   |  470 ++++
 .../Universal/HiiDatabaseDxe/HiiDatabase.h         |    9 +-
 .../Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf    |    1 +
 MdeModulePkg/Universal/PCD/Dxe/Pcd.c               |   29 +-
 MdeModulePkg/Universal/PCD/Dxe/Service.c           |  322 +--
 MdeModulePkg/Universal/PCD/Dxe/Service.h           |   35 +-
 MdeModulePkg/Universal/PCD/Pei/Pcd.c               |  343 ++-
 MdeModulePkg/Universal/PCD/Pei/Pcd.inf             |    5 +
 MdeModulePkg/Universal/PCD/Pei/Service.c           |  150 +-
 MdeModulePkg/Universal/PCD/Pei/Service.h           |   33 +-
 .../Universal/Variable/RuntimeDxe/Variable.c       |  106 +-
 42 files changed, 7036 insertions(+), 3812 deletions(-)
 create mode 100644 BaseTools/Source/C/Common/PcdValueCommon.c
 create mode 100644 BaseTools/Source/C/Common/PcdValueCommon.h
 create mode 100644 BaseTools/Source/Python/AutoGen/GenVar.py
 create mode 100644 BaseTools/Source/Python/Workspace/DecBuildData.py
 create mode 100644 BaseTools/Source/Python/Workspace/DscBuildData.py
 create mode 100644 BaseTools/Source/Python/Workspace/InfBuildData.py

--
2.8.0.windows.1