[edk2] [RFC] PCD: Database size optimization for multi-SKU

Star Zeng posted 1 patch 6 years, 11 months ago
Failed in applying to current master (apply log)
.../Include/Guid/PcdDataBaseSignatureGuid.h        | 32 +++++++++++++++++-----
1 file changed, 25 insertions(+), 7 deletions(-)
[edk2] [RFC] PCD: Database size optimization for multi-SKU
Posted by Star Zeng 6 years, 11 months ago
- Requirement
Reduce PCD database size for multi-SKU case to save image size at build time and memory resource at boot time.


- Current limitation
When multiple SKUs are enabled, the full set of PCD values for all SKUs will be generated into PCD database,
and HOB will be created for the PCD database, the HOB will consume much memory resource when many SKUs are specified,
but the HOB length and pre-memory phase memory resource are limited.


- Proposal
BaseTools can generate the optimized PCD database to save the image size at build time,
the optimized PCD database layout will be like below, the PCD database will be composed of the full
default SKU data(PCD_DATABASE_INIT) and the non-default SKU delta data(PCD_DATABASE_SKU_DELTA).
PCD driver will build HOB to store the full default SKU data, and patch HOB data based on
non-default SKU delta data for the SKU set by SetSku(), it can save memory resource at boot time.

//
// PCD database layout:
// +---------------------------------+
// | PCD_DATABASE_INIT (DEFAULT SKU) |
// +---------------------------------+
// | PCD_DATABASE_SKU_DELTA (SKU A)  |
// +---------------------------------+
// | PCD_DATABASE_SKU_DELTA (SKU B)  |
// +---------------------------------+
// | ......                          |
// +---------------------------------+
//

The patch below shows the detailed PCD database format change.

BaseTools, PCD database and driver updates are needed for this proposal.
BaseTools can even be intelligent to generate smallest SKU delta based on the PCD values for different SKUs and the relationship between SKUs.
For single SKU (default) case, this proposal is expected to have no impact.
Note: For multi-SKU case, PCD database format will be changed, so PCD driver will have to be updated together with BaseTools.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Tim Lewis <tim.lewis@insyde.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>

---
 .../Include/Guid/PcdDataBaseSignatureGuid.h        | 32 +++++++++++++++++-----
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
index d2e848800b75..0069823458cd 100644
--- a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
+++ b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
@@ -62,11 +62,6 @@ typedef struct  {
 } DYNAMICEX_MAPPING;
 
 typedef struct {
-  UINT32  SkuDataStartOffset;   // Offset(with DATUM TYPE info) from the PCD_DB.
-  UINT32  SkuIdTableOffset;     // Offset from the PCD_DB.
-} SKU_HEAD;
-
-typedef struct {
   UINT32  StringIndex;          // Offset in String Table in units of UINT8.
   UINT32  DefaultValueOffset;   // Offset of the Default Value.
   UINT16  GuidTableIndex;       // Offset in Guid Table in units of GUID.
@@ -115,7 +110,6 @@ typedef struct {
     // Padding is needed to keep necessary alignment
     //
     //SKU_ID                         SkuIdTable[];            // SkuIds system supports.
-    //SKU_ID                         SkuIndexTable[];         // SkuIds for each PCD with SKU enable.
     //UINT64                         ValueUint64[];
     //UINT32                         ValueUint32[];
     //VPD_HEAD                       VpdHead[];               // VPD Offset
@@ -125,7 +119,6 @@ typedef struct {
     //STRING_HEAD                    StringHead[];            // String PCD
     //PCD_NAME_INDEX                 PcdNameTable[];          // PCD name index info. It can be accessed by the PcdNameTableOffset.
     //VARIABLE_HEAD                  VariableHead[];          // HII PCD
-    //SKU_HEAD                       SkuHead[];               // Store SKU info for each PCD with SKU enable.
     //UINT8                          StringTable[];           // String for String PCD value and HII PCD Variable Name. It can be accessed by StringTableOffset.
     //SIZE_INFO                      SizeTable[];             // MaxSize and CurSize for String PCD. It can be accessed by SizeTableOffset.
     //UINT16                         ValueUint16[];
@@ -134,6 +127,31 @@ typedef struct {
 
 } PCD_DATABASE_INIT;
 
+typedef struct {
+  UINT32    Offset:24;
+  UINT32    Data:8;
+} PCD_DATABASE_SKU_DELTA_DATA;
+
+typedef struct {
+  SKU_ID    SkuId;
+  SKU_ID    SkuIdCompared;
+  UINT32    Length;
+  //PCD_DATABASE_SKU_DELTA_DATA   DeltaData[]
+} PCD_DATABASE_SKU_DELTA;
+
+//
+// PCD database layout:
+// +---------------------------------+
+// | PCD_DATABASE_INIT (DEFAULT SKU) |
+// +---------------------------------+
+// | PCD_DATABASE_SKU_DELTA (SKU A)  |
+// +---------------------------------+
+// | PCD_DATABASE_SKU_DELTA (SKU B)  |
+// +---------------------------------+
+// | ......                          |
+// +---------------------------------+
+//
+
 //
 // PEI and DXE Pcd driver use the same PCD database
 //
-- 
2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC] PCD: Database size optimization for multi-SKU
Posted by Tim Lewis 6 years, 11 months ago
Star-



One more requirement that I propose is the the format update be detectable easily by inspection of the binary image. We have tools that dump and manipulate the PCD database and the revision or version number allows us to support multiple core versions. I think this is already your intent and I want to make it an explicit requirement.



Other than that, I think this is a much needed change for multi-sku firmware images.



Thanks,

Tim



Sent from my Windows 10 phone



From: Star Zeng<mailto:star.zeng@intel.com>
Sent: Tuesday, May 16, 2017 7:33 AM
To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Star Zeng<mailto:star.zeng@intel.com>; Michael Kinney<mailto:michael.d.kinney@intel.com>; Liming Gao<mailto:liming.gao@intel.com>; Tim Lewis<mailto:tim.lewis@insyde.com>; Yonghong Zhu<mailto:yonghong.zhu@intel.com>
Subject: [RFC] PCD: Database size optimization for multi-SKU



- Requirement
Reduce PCD database size for multi-SKU case to save image size at build time and memory resource at boot time.


- Current limitation
When multiple SKUs are enabled, the full set of PCD values for all SKUs will be generated into PCD database,
and HOB will be created for the PCD database, the HOB will consume much memory resource when many SKUs are specified,
but the HOB length and pre-memory phase memory resource are limited.


- Proposal
BaseTools can generate the optimized PCD database to save the image size at build time,
the optimized PCD database layout will be like below, the PCD database will be composed of the full
default SKU data(PCD_DATABASE_INIT) and the non-default SKU delta data(PCD_DATABASE_SKU_DELTA).
PCD driver will build HOB to store the full default SKU data, and patch HOB data based on
non-default SKU delta data for the SKU set by SetSku(), it can save memory resource at boot time.

//
// PCD database layout:
// +---------------------------------+
// | PCD_DATABASE_INIT (DEFAULT SKU) |
// +---------------------------------+
// | PCD_DATABASE_SKU_DELTA (SKU A)  |
// +---------------------------------+
// | PCD_DATABASE_SKU_DELTA (SKU B)  |
// +---------------------------------+
// | ......                          |
// +---------------------------------+
//

The patch below shows the detailed PCD database format change.

BaseTools, PCD database and driver updates are needed for this proposal.
BaseTools can even be intelligent to generate smallest SKU delta based on the PCD values for different SKUs and the relationship between SKUs.
For single SKU (default) case, this proposal is expected to have no impact.
Note: For multi-SKU case, PCD database format will be changed, so PCD driver will have to be updated together with BaseTools.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Tim Lewis <tim.lewis@insyde.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>

---
 .../Include/Guid/PcdDataBaseSignatureGuid.h        | 32 +++++++++++++++++-----
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
index d2e848800b75..0069823458cd 100644
--- a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
+++ b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
@@ -62,11 +62,6 @@ typedef struct  {
 } DYNAMICEX_MAPPING;

 typedef struct {
-  UINT32  SkuDataStartOffset;   // Offset(with DATUM TYPE info) from the PCD_DB.
-  UINT32  SkuIdTableOffset;     // Offset from the PCD_DB.
-} SKU_HEAD;
-
-typedef struct {
   UINT32  StringIndex;          // Offset in String Table in units of UINT8.
   UINT32  DefaultValueOffset;   // Offset of the Default Value.
   UINT16  GuidTableIndex;       // Offset in Guid Table in units of GUID.
@@ -115,7 +110,6 @@ typedef struct {
     // Padding is needed to keep necessary alignment
     //
     //SKU_ID                         SkuIdTable[];            // SkuIds system supports.
-    //SKU_ID                         SkuIndexTable[];         // SkuIds for each PCD with SKU enable.
     //UINT64                         ValueUint64[];
     //UINT32                         ValueUint32[];
     //VPD_HEAD                       VpdHead[];               // VPD Offset
@@ -125,7 +119,6 @@ typedef struct {
     //STRING_HEAD                    StringHead[];            // String PCD
     //PCD_NAME_INDEX                 PcdNameTable[];          // PCD name index info. It can be accessed by the PcdNameTableOffset.
     //VARIABLE_HEAD                  VariableHead[];          // HII PCD
-    //SKU_HEAD                       SkuHead[];               // Store SKU info for each PCD with SKU enable.
     //UINT8                          StringTable[];           // String for String PCD value and HII PCD Variable Name. It can be accessed by StringTableOffset.
     //SIZE_INFO                      SizeTable[];             // MaxSize and CurSize for String PCD. It can be accessed by SizeTableOffset.
     //UINT16                         ValueUint16[];
@@ -134,6 +127,31 @@ typedef struct {

 } PCD_DATABASE_INIT;

+typedef struct {
+  UINT32    Offset:24;
+  UINT32    Data:8;
+} PCD_DATABASE_SKU_DELTA_DATA;
+
+typedef struct {
+  SKU_ID    SkuId;
+  SKU_ID    SkuIdCompared;
+  UINT32    Length;
+  //PCD_DATABASE_SKU_DELTA_DATA   DeltaData[]
+} PCD_DATABASE_SKU_DELTA;
+
+//
+// PCD database layout:
+// +---------------------------------+
+// | PCD_DATABASE_INIT (DEFAULT SKU) |
+// +---------------------------------+
+// | PCD_DATABASE_SKU_DELTA (SKU A)  |
+// +---------------------------------+
+// | PCD_DATABASE_SKU_DELTA (SKU B)  |
+// +---------------------------------+
+// | ......                          |
+// +---------------------------------+
+//
+
 //
 // PEI and DXE Pcd driver use the same PCD database
 //
--
2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC] PCD: Database size optimization for multi-SKU
Posted by Zeng, Star 6 years, 11 months ago
Tim,

Yes, right. The PCD_DATABASE_INIT.BuildVersion will be increased for the PCD database format change to support multi-SKU. That is the rule we maintain the PCD database. :)


Thanks,
Star
From: Tim Lewis [mailto:tim.lewis@insyde.com]
Sent: Wednesday, May 17, 2017 2:04 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: RE: [RFC] PCD: Database size optimization for multi-SKU


Star-



One more requirement that I propose is the the format update be detectable easily by inspection of the binary image. We have tools that dump and manipulate the PCD database and the revision or version number allows us to support multiple core versions. I think this is already your intent and I want to make it an explicit requirement.



Other than that, I think this is a much needed change for multi-sku firmware images.



Thanks,

Tim



Sent from my Windows 10 phone



From: Star Zeng<mailto:star.zeng@intel.com>
Sent: Tuesday, May 16, 2017 7:33 AM
To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Star Zeng<mailto:star.zeng@intel.com>; Michael Kinney<mailto:michael.d.kinney@intel.com>; Liming Gao<mailto:liming.gao@intel.com>; Tim Lewis<mailto:tim.lewis@insyde.com>; Yonghong Zhu<mailto:yonghong.zhu@intel.com>
Subject: [RFC] PCD: Database size optimization for multi-SKU


- Requirement
Reduce PCD database size for multi-SKU case to save image size at build time and memory resource at boot time.


- Current limitation
When multiple SKUs are enabled, the full set of PCD values for all SKUs will be generated into PCD database,
and HOB will be created for the PCD database, the HOB will consume much memory resource when many SKUs are specified,
but the HOB length and pre-memory phase memory resource are limited.


- Proposal
BaseTools can generate the optimized PCD database to save the image size at build time,
the optimized PCD database layout will be like below, the PCD database will be composed of the full
default SKU data(PCD_DATABASE_INIT) and the non-default SKU delta data(PCD_DATABASE_SKU_DELTA).
PCD driver will build HOB to store the full default SKU data, and patch HOB data based on
non-default SKU delta data for the SKU set by SetSku(), it can save memory resource at boot time.

//
// PCD database layout:
// +---------------------------------+
// | PCD_DATABASE_INIT (DEFAULT SKU) |
// +---------------------------------+
// | PCD_DATABASE_SKU_DELTA (SKU A)  |
// +---------------------------------+
// | PCD_DATABASE_SKU_DELTA (SKU B)  |
// +---------------------------------+
// | ......                          |
// +---------------------------------+
//

The patch below shows the detailed PCD database format change.

BaseTools, PCD database and driver updates are needed for this proposal.
BaseTools can even be intelligent to generate smallest SKU delta based on the PCD values for different SKUs and the relationship between SKUs.
For single SKU (default) case, this proposal is expected to have no impact.
Note: For multi-SKU case, PCD database format will be changed, so PCD driver will have to be updated together with BaseTools.

Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Cc: Tim Lewis <tim.lewis@insyde.com<mailto:tim.lewis@insyde.com>>
Cc: Yonghong Zhu <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>>

---
 .../Include/Guid/PcdDataBaseSignatureGuid.h        | 32 +++++++++++++++++-----
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
index d2e848800b75..0069823458cd 100644
--- a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
+++ b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
@@ -62,11 +62,6 @@ typedef struct  {
 } DYNAMICEX_MAPPING;

 typedef struct {
-  UINT32  SkuDataStartOffset;   // Offset(with DATUM TYPE info) from the PCD_DB.
-  UINT32  SkuIdTableOffset;     // Offset from the PCD_DB.
-} SKU_HEAD;
-
-typedef struct {
   UINT32  StringIndex;          // Offset in String Table in units of UINT8.
   UINT32  DefaultValueOffset;   // Offset of the Default Value.
   UINT16  GuidTableIndex;       // Offset in Guid Table in units of GUID.
@@ -115,7 +110,6 @@ typedef struct {
     // Padding is needed to keep necessary alignment
     //
     //SKU_ID                         SkuIdTable[];            // SkuIds system supports.
-    //SKU_ID                         SkuIndexTable[];         // SkuIds for each PCD with SKU enable.
     //UINT64                         ValueUint64[];
     //UINT32                         ValueUint32[];
     //VPD_HEAD                       VpdHead[];               // VPD Offset
@@ -125,7 +119,6 @@ typedef struct {
     //STRING_HEAD                    StringHead[];            // String PCD
     //PCD_NAME_INDEX                 PcdNameTable[];          // PCD name index info. It can be accessed by the PcdNameTableOffset.
     //VARIABLE_HEAD                  VariableHead[];          // HII PCD
-    //SKU_HEAD                       SkuHead[];               // Store SKU info for each PCD with SKU enable.
     //UINT8                          StringTable[];           // String for String PCD value and HII PCD Variable Name. It can be accessed by StringTableOffset.
     //SIZE_INFO                      SizeTable[];             // MaxSize and CurSize for String PCD. It can be accessed by SizeTableOffset.
     //UINT16                         ValueUint16[];
@@ -134,6 +127,31 @@ typedef struct {

 } PCD_DATABASE_INIT;

+typedef struct {
+  UINT32    Offset:24;
+  UINT32    Data:8;
+} PCD_DATABASE_SKU_DELTA_DATA;
+
+typedef struct {
+  SKU_ID    SkuId;
+  SKU_ID    SkuIdCompared;
+  UINT32    Length;
+  //PCD_DATABASE_SKU_DELTA_DATA   DeltaData[]
+} PCD_DATABASE_SKU_DELTA;
+
+//
+// PCD database layout:
+// +---------------------------------+
+// | PCD_DATABASE_INIT (DEFAULT SKU) |
+// +---------------------------------+
+// | PCD_DATABASE_SKU_DELTA (SKU A)  |
+// +---------------------------------+
+// | PCD_DATABASE_SKU_DELTA (SKU B)  |
+// +---------------------------------+
+// | ......                          |
+// +---------------------------------+
+//
+
 //
 // PEI and DXE Pcd driver use the same PCD database
 //
--
2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel