* [PATCH] MdeModulePkg PCD: Allow SkuId to be changed only once
@ 2017-03-14 11:58 Star Zeng
2017-03-14 14:29 ` Gao, Liming
0 siblings, 1 reply; 2+ messages in thread
From: Star Zeng @ 2017-03-14 11:58 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Liming Gao, Michael Kinney, Feng Tian
Current PI spec has no clear description about whether the
SkuId could be changed multiple times or not during one boot.
If the SkuId could be changed multiple times during one boot,
different modules may get inconsistent PCD values.
And DynamicHii PCD maps to UEFI variable, once one DynamicHii
PCD(UEFI variable) is set for one SkuId, then the PCD value
will be always from UEFI variable but not PCD database, even
the SkuId is set to other value.
This patch is to update PCD drivers to allow SkuId to be
changed only once during one boot.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 27 +++++++++++++++++++++++----
MdeModulePkg/Universal/PCD/Pei/Pcd.c | 28 ++++++++++++++++++++++++----
2 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
index 1bb9098c04cf..9d710bbf1fb7 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
@@ -3,7 +3,7 @@
produce the implementation of native PCD protocol and EFI_PCD_PROTOCOL defined in
PI 1.4a Vol3.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -272,19 +272,38 @@ DxePcdSetSku (
SKU_ID *SkuIdTable;
UINTN Index;
+ if (SkuId == mPcdDatabase.DxeDb->SystemSkuId) {
+ //
+ // The input SKU Id is equal to current SKU Id, return directly.
+ //
+ return;
+ }
+
+ if (mPcdDatabase.DxeDb->SystemSkuId != (SKU_ID) 0) {
+ DEBUG ((DEBUG_ERROR, "PcdDxe - The SKU Id could be changed only once."));
+ DEBUG ((
+ DEBUG_ERROR,
+ "PcdDxe - The SKU Id was set to 0x%lx already, it could not be set to 0x%lx any more.",
+ mPcdDatabase.DxeDb->SystemSkuId,
+ (SKU_ID) SkuId
+ ));
+ ASSERT (FALSE);
+ return;
+ }
+
SkuIdTable = (SKU_ID *) ((UINT8 *) mPcdDatabase.DxeDb + mPcdDatabase.DxeDb->SkuIdTableOffset);
for (Index = 0; Index < SkuIdTable[0]; Index++) {
if (SkuId == SkuIdTable[Index + 1]) {
+ DEBUG ((EFI_D_INFO, "PcdDxe - Set current SKU Id to 0x%lx.\n", (SKU_ID) SkuId));
mPcdDatabase.DxeDb->SystemSkuId = (SKU_ID) SkuId;
return;
}
}
//
- // Invalid input SkuId, the default SKU Id will be used for the system.
+ // Invalid input SkuId, the default SKU Id will be still used for the system.
//
- DEBUG ((EFI_D_INFO, "PcdDxe - Invalid input SkuId, the default SKU Id will be used.\n"));
- mPcdDatabase.DxeDb->SystemSkuId = (SKU_ID) 0;
+ DEBUG ((EFI_D_INFO, "PcdDxe - Invalid input SkuId, the default SKU Id will be still used.\n"));
return;
}
diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.c b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
index 668860b61c6c..a3f7337ceca3 100644
--- a/MdeModulePkg/Universal/PCD/Pei/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
@@ -1,7 +1,7 @@
/** @file
All Pcd Ppi services are implemented here.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -262,19 +262,39 @@ PeiPcdSetSku (
UINTN Index;
PeiPcdDb = GetPcdDatabase();
+
+ if (SkuId == PeiPcdDb->SystemSkuId) {
+ //
+ // The input SKU Id is equal to current SKU Id, return directly.
+ //
+ return;
+ }
+
+ if (PeiPcdDb->SystemSkuId != (SKU_ID) 0) {
+ DEBUG ((DEBUG_ERROR, "PcdPei - The SKU Id could be changed only once."));
+ DEBUG ((
+ DEBUG_ERROR,
+ "PcdPei - The SKU Id was set to 0x%lx already, it could not be set to 0x%lx any more.",
+ PeiPcdDb->SystemSkuId,
+ (SKU_ID) SkuId
+ ));
+ ASSERT (FALSE);
+ return;
+ }
+
SkuIdTable = (SKU_ID *) ((UINT8 *) PeiPcdDb + PeiPcdDb->SkuIdTableOffset);
for (Index = 0; Index < SkuIdTable[0]; Index++) {
if (SkuId == SkuIdTable[Index + 1]) {
+ DEBUG ((EFI_D_INFO, "PcdPei - Set current SKU Id to 0x%lx.\n", (SKU_ID) SkuId));
PeiPcdDb->SystemSkuId = (SKU_ID) SkuId;
return;
}
}
//
- // Invalid input SkuId, the default SKU Id will be used for the system.
+ // Invalid input SkuId, the default SKU Id will be still used for the system.
//
- DEBUG ((EFI_D_INFO, "PcdPei - Invalid input SkuId, the default SKU Id will be used.\n"));
- PeiPcdDb->SystemSkuId = (SKU_ID) 0;
+ DEBUG ((EFI_D_INFO, "PcdPei - Invalid input SkuId, the default SKU Id will be still used.\n"));
return;
}
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] MdeModulePkg PCD: Allow SkuId to be changed only once
2017-03-14 11:58 [PATCH] MdeModulePkg PCD: Allow SkuId to be changed only once Star Zeng
@ 2017-03-14 14:29 ` Gao, Liming
0 siblings, 0 replies; 2+ messages in thread
From: Gao, Liming @ 2017-03-14 14:29 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Tian, Feng
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, March 14, 2017 7:58 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Tian,
> Feng <feng.tian@intel.com>
> Subject: [PATCH] MdeModulePkg PCD: Allow SkuId to be changed only once
>
> Current PI spec has no clear description about whether the
> SkuId could be changed multiple times or not during one boot.
>
> If the SkuId could be changed multiple times during one boot,
> different modules may get inconsistent PCD values.
> And DynamicHii PCD maps to UEFI variable, once one DynamicHii
> PCD(UEFI variable) is set for one SkuId, then the PCD value
> will be always from UEFI variable but not PCD database, even
> the SkuId is set to other value.
>
> This patch is to update PCD drivers to allow SkuId to be
> changed only once during one boot.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 27 +++++++++++++++++++++++----
> MdeModulePkg/Universal/PCD/Pei/Pcd.c | 28 ++++++++++++++++++++++++----
> 2 files changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> index 1bb9098c04cf..9d710bbf1fb7 100644
> --- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> +++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> @@ -3,7 +3,7 @@
> produce the implementation of native PCD protocol and EFI_PCD_PROTOCOL defined in
> PI 1.4a Vol3.
>
> -Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -272,19 +272,38 @@ DxePcdSetSku (
> SKU_ID *SkuIdTable;
> UINTN Index;
>
> + if (SkuId == mPcdDatabase.DxeDb->SystemSkuId) {
> + //
> + // The input SKU Id is equal to current SKU Id, return directly.
> + //
> + return;
> + }
> +
> + if (mPcdDatabase.DxeDb->SystemSkuId != (SKU_ID) 0) {
> + DEBUG ((DEBUG_ERROR, "PcdDxe - The SKU Id could be changed only once."));
> + DEBUG ((
> + DEBUG_ERROR,
> + "PcdDxe - The SKU Id was set to 0x%lx already, it could not be set to 0x%lx any more.",
> + mPcdDatabase.DxeDb->SystemSkuId,
> + (SKU_ID) SkuId
> + ));
> + ASSERT (FALSE);
> + return;
> + }
> +
> SkuIdTable = (SKU_ID *) ((UINT8 *) mPcdDatabase.DxeDb + mPcdDatabase.DxeDb->SkuIdTableOffset);
> for (Index = 0; Index < SkuIdTable[0]; Index++) {
> if (SkuId == SkuIdTable[Index + 1]) {
> + DEBUG ((EFI_D_INFO, "PcdDxe - Set current SKU Id to 0x%lx.\n", (SKU_ID) SkuId));
> mPcdDatabase.DxeDb->SystemSkuId = (SKU_ID) SkuId;
> return;
> }
> }
>
> //
> - // Invalid input SkuId, the default SKU Id will be used for the system.
> + // Invalid input SkuId, the default SKU Id will be still used for the system.
> //
> - DEBUG ((EFI_D_INFO, "PcdDxe - Invalid input SkuId, the default SKU Id will be used.\n"));
> - mPcdDatabase.DxeDb->SystemSkuId = (SKU_ID) 0;
> + DEBUG ((EFI_D_INFO, "PcdDxe - Invalid input SkuId, the default SKU Id will be still used.\n"));
> return;
> }
>
> diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.c b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
> index 668860b61c6c..a3f7337ceca3 100644
> --- a/MdeModulePkg/Universal/PCD/Pei/Pcd.c
> +++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
> @@ -1,7 +1,7 @@
> /** @file
> All Pcd Ppi services are implemented here.
>
> -Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -262,19 +262,39 @@ PeiPcdSetSku (
> UINTN Index;
>
> PeiPcdDb = GetPcdDatabase();
> +
> + if (SkuId == PeiPcdDb->SystemSkuId) {
> + //
> + // The input SKU Id is equal to current SKU Id, return directly.
> + //
> + return;
> + }
> +
> + if (PeiPcdDb->SystemSkuId != (SKU_ID) 0) {
> + DEBUG ((DEBUG_ERROR, "PcdPei - The SKU Id could be changed only once."));
> + DEBUG ((
> + DEBUG_ERROR,
> + "PcdPei - The SKU Id was set to 0x%lx already, it could not be set to 0x%lx any more.",
> + PeiPcdDb->SystemSkuId,
> + (SKU_ID) SkuId
> + ));
> + ASSERT (FALSE);
> + return;
> + }
> +
> SkuIdTable = (SKU_ID *) ((UINT8 *) PeiPcdDb + PeiPcdDb->SkuIdTableOffset);
> for (Index = 0; Index < SkuIdTable[0]; Index++) {
> if (SkuId == SkuIdTable[Index + 1]) {
> + DEBUG ((EFI_D_INFO, "PcdPei - Set current SKU Id to 0x%lx.\n", (SKU_ID) SkuId));
> PeiPcdDb->SystemSkuId = (SKU_ID) SkuId;
> return;
> }
> }
>
> //
> - // Invalid input SkuId, the default SKU Id will be used for the system.
> + // Invalid input SkuId, the default SKU Id will be still used for the system.
> //
> - DEBUG ((EFI_D_INFO, "PcdPei - Invalid input SkuId, the default SKU Id will be used.\n"));
> - PeiPcdDb->SystemSkuId = (SKU_ID) 0;
> + DEBUG ((EFI_D_INFO, "PcdPei - Invalid input SkuId, the default SKU Id will be still used.\n"));
> return;
> }
>
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-14 14:29 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-14 11:58 [PATCH] MdeModulePkg PCD: Allow SkuId to be changed only once Star Zeng
2017-03-14 14:29 ` Gao, Liming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox