Re: [PATCH] MdeModulePkg PCD: Allow SkuId to be changed only once

["Gao, Liming" <liming.gao@intel.com>]

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