* [Patch] MdeModulePkg PCD: Add DynamicEx PcdVpdBaseAddress64 for non SPI platform
@ 2018-11-22 14:32 Liming Gao
2018-11-26 1:27 ` Zeng, Star
0 siblings, 1 reply; 3+ messages in thread
From: Liming Gao @ 2018-11-22 14:32 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Jian J Wang, Dandan Bi
https://bugzilla.tianocore.org/show_bug.cgi?id=1356
Current PcdVpdBaseAddress is 32bit static Pcd. NON SPI platform needs to
configure it as Dynamic PCD. Emulator platform (such as NT32) may set its
value to 64bit address.
To meet with this usage, 64bit DynamicEx PcdVpdBaseAddress64 is introduced.
If its value is not zero, it will be used.
If its value is zero, static PcdVpdBaseAddress will be used.
When NON SPI platform enables VPD PCD, they need to set PcdVpdBaseAddress64.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
---
MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 16 ++++++++++++++++
MdeModulePkg/Universal/PCD/Dxe/Service.c | 3 ++-
MdeModulePkg/Universal/PCD/Pei/Service.c | 15 ++++++++++++++-
MdeModulePkg/MdeModulePkg.dec | 8 ++++++++
MdeModulePkg/Universal/PCD/Dxe/Pcd.inf | 3 ++-
MdeModulePkg/Universal/PCD/Dxe/Service.h | 2 ++
MdeModulePkg/Universal/PCD/Pei/Pcd.inf | 3 ++-
7 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
index f977c7f18e..4e38a3844a 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
@@ -109,6 +109,7 @@ EFI_GET_PCD_INFO_PROTOCOL mEfiGetPcdInfoInstance = {
};
EFI_HANDLE mPcdHandle = NULL;
+UINTN mVpdBaseAddress = 0;
/**
Main entry for PCD DXE driver.
@@ -175,6 +176,21 @@ PcdDxeInit (
&Registration
);
+ //
+ // Cache VpdBaseAddress in entry point for the following usage.
+ //
+
+ //
+ // PcdVpdBaseAddress64 is DynamicEx PCD only. So, DxePcdGet64Ex() is used to get its value.
+ //
+ mVpdBaseAddress = (UINTN) DxePcdGet64Ex (&gEfiMdeModulePkgTokenSpaceGuid, PcdToken (PcdVpdBaseAddress64));
+ if (mVpdBaseAddress == 0) {
+ //
+ // PcdVpdBaseAddress64 is not set, get value from PcdVpdBaseAddress.
+ //
+ mVpdBaseAddress = (UINTN) PcdGet32 (PcdVpdBaseAddress);
+ }
+
return Status;
}
diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.c b/MdeModulePkg/Universal/PCD/Dxe/Service.c
index 0517152366..4b44153e13 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Service.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Service.c
@@ -435,7 +435,8 @@ GetWorker (
switch (LocalTokenNumber & PCD_TYPE_ALL_SET) {
case PCD_TYPE_VPD:
VpdHead = (VPD_HEAD *) ((UINT8 *) PcdDb + Offset);
- RetPtr = (VOID *) ((UINTN) PcdGet32 (PcdVpdBaseAddress) + VpdHead->Offset);
+ ASSERT (mVpdBaseAddress != 0);
+ RetPtr = (VOID *) (mVpdBaseAddress + VpdHead->Offset);
break;
diff --git a/MdeModulePkg/Universal/PCD/Pei/Service.c b/MdeModulePkg/Universal/PCD/Pei/Service.c
index bb4b52baf3..e1613e8af8 100644
--- a/MdeModulePkg/Universal/PCD/Pei/Service.c
+++ b/MdeModulePkg/Universal/PCD/Pei/Service.c
@@ -861,6 +861,7 @@ GetWorker (
UINT32 LocalTokenNumber;
UINT32 LocalTokenCount;
UINT8 *VaraiableDefaultBuffer;
+ UINTN VpdBaseAddress;
//
// TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.
@@ -889,7 +890,19 @@ GetWorker (
{
VPD_HEAD *VpdHead;
VpdHead = (VPD_HEAD *) ((UINT8 *)PeiPcdDb + Offset);
- return (VOID *) ((UINTN) PcdGet32 (PcdVpdBaseAddress) + VpdHead->Offset);
+
+ //
+ // PcdVpdBaseAddress64 is DynamicEx PCD only. So, PeiPcdGet64Ex() is used to get its value.
+ //
+ VpdBaseAddress = (UINTN) PeiPcdGet64Ex (&gEfiMdeModulePkgTokenSpaceGuid, PcdToken (PcdVpdBaseAddress64));
+ if (VpdBaseAddress == 0) {
+ //
+ // PcdVpdBaseAddress64 is not set, get value from PcdVpdBaseAddress.
+ //
+ VpdBaseAddress = (UINTN) PcdGet32 (PcdVpdBaseAddress);
+ }
+ ASSERT (VpdBaseAddress != 0);
+ return (VOID *)(VpdBaseAddress + VpdHead->Offset);
}
case PCD_TYPE_HII|PCD_TYPE_STRING:
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 0abacc1a90..f46a4094ca 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2022,5 +2022,13 @@
# @Prompt NV Storage Default Value Buffer
gEfiMdeModulePkgTokenSpaceGuid.PcdNvStoreDefaultValueBuffer|{0x0}|VOID*|0x00030005
+ ## VPD type PCD allows a developer to point to an absolute physical address PcdVpdBaseAddress
+ # to store PCD value. It will be DynamicExDefault only.
+ # It is used to set VPD region base address. So, it can't be DynamicExVpd PCD. Its value is
+ # required to be accessed in PcdDxe driver entry point. So, its value must be set in PEI phase.
+ # It can't depend on EFI variable service, and can't be DynamicExHii PCD.
+ # @Prompt 64bit VPD base address.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64|0x0|UINT64|0x00030006
+
[UserExtensions.TianoCore."ExtraFiles"]
MdeModulePkgExtra.uni
diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
index 1f41a316bd..4ba78e46a3 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
@@ -73,7 +73,7 @@
#
# c) OEM specificed storage area:
# - The PCD value is stored in OEM specified area which base address is
-# specified by a FixedAtBuild PCD setting - PcdVpdBaseAddress.
+# specified by PCD setting - PcdVpdBaseAddress64 or PcdVpdBaseAddress.
# - The area is read only for PEI and DXE phase.
# - [PcdsDynamicVpd] is used as section name for this type PCD in platform
# DSC file. [PcdsDynamicExVpd] is for dynamicex type PCD.
@@ -344,6 +344,7 @@
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64 ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNvStoreDefaultId ## SOMETIMES_CONSUMES
[Depex]
diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.h b/MdeModulePkg/Universal/PCD/Dxe/Service.h
index ddd5fa471e..3055e30cd1 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Service.h
+++ b/MdeModulePkg/Universal/PCD/Dxe/Service.h
@@ -48,6 +48,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#error "Please make sure the version of PCD DXE Service and the generated PCD DXE Database match."
#endif
+extern UINTN mVpdBaseAddress;
+
/**
Retrieve additional information associated with a PCD token in the default token space.
diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
index 6e28fce8fa..63b125d48a 100644
--- a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
@@ -72,7 +72,7 @@
#
# c) OEM specificed storage area:
# - The PCD value is stored in OEM specified area which base address is
-# specified by a FixedAtBuild PCD setting - PcdVpdBaseAddress.
+# specified by PCD setting - PcdVpdBaseAddress64 or PcdVpdBaseAddress.
# - The area is read only for PEI and DXE phase.
# - [PcdsDynamicVpd] is used as section name for this type PCD in platform
# DSC file. [PcdsDynamicExVpd] is for dynamicex type PCD.
@@ -344,6 +344,7 @@
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64 ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxPeiPcdCallBackNumberPerPcdEntry ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdNvStoreDefaultValueBuffer ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNvStoreDefaultId ## CONSUMES
--
2.13.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch] MdeModulePkg PCD: Add DynamicEx PcdVpdBaseAddress64 for non SPI platform
2018-11-22 14:32 [Patch] MdeModulePkg PCD: Add DynamicEx PcdVpdBaseAddress64 for non SPI platform Liming Gao
@ 2018-11-26 1:27 ` Zeng, Star
2018-11-26 2:44 ` Gao, Liming
0 siblings, 1 reply; 3+ messages in thread
From: Zeng, Star @ 2018-11-26 1:27 UTC (permalink / raw)
To: Liming Gao, edk2-devel; +Cc: Dandan Bi, star.zeng
On 2018/11/22 22:32, Liming Gao wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1356
> Current PcdVpdBaseAddress is 32bit static Pcd. NON SPI platform needs to
> configure it as Dynamic PCD. Emulator platform (such as NT32) may set its
> value to 64bit address.
> To meet with this usage, 64bit DynamicEx PcdVpdBaseAddress64 is introduced.
> If its value is not zero, it will be used.
> If its value is zero, static PcdVpdBaseAddress will be used.
> When NON SPI platform enables VPD PCD, they need to set PcdVpdBaseAddress64.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
Two minor comments at below, with them handled, Reviewed-by: Star Zeng
<star.zeng@intel.com>
> ---
> MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 16 ++++++++++++++++
> MdeModulePkg/Universal/PCD/Dxe/Service.c | 3 ++-
> MdeModulePkg/Universal/PCD/Pei/Service.c | 15 ++++++++++++++-
> MdeModulePkg/MdeModulePkg.dec | 8 ++++++++
> MdeModulePkg/Universal/PCD/Dxe/Pcd.inf | 3 ++-
> MdeModulePkg/Universal/PCD/Dxe/Service.h | 2 ++
> MdeModulePkg/Universal/PCD/Pei/Pcd.inf | 3 ++-
> 7 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> index f977c7f18e..4e38a3844a 100644
> --- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> +++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> @@ -109,6 +109,7 @@ EFI_GET_PCD_INFO_PROTOCOL mEfiGetPcdInfoInstance = {
> };
>
> EFI_HANDLE mPcdHandle = NULL;
> +UINTN mVpdBaseAddress = 0;
>
> /**
> Main entry for PCD DXE driver.
> @@ -175,6 +176,21 @@ PcdDxeInit (
> &Registration
> );
>
> + //
> + // Cache VpdBaseAddress in entry point for the following usage.
> + //
> +
> + //
> + // PcdVpdBaseAddress64 is DynamicEx PCD only. So, DxePcdGet64Ex() is used to get its value.
> + //
> + mVpdBaseAddress = (UINTN) DxePcdGet64Ex (&gEfiMdeModulePkgTokenSpaceGuid, PcdToken (PcdVpdBaseAddress64));
> + if (mVpdBaseAddress == 0) {
> + //
> + // PcdVpdBaseAddress64 is not set, get value from PcdVpdBaseAddress.
> + //
> + mVpdBaseAddress = (UINTN) PcdGet32 (PcdVpdBaseAddress);
> + }
> +
> return Status;
> }
>
> diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.c b/MdeModulePkg/Universal/PCD/Dxe/Service.c
> index 0517152366..4b44153e13 100644
> --- a/MdeModulePkg/Universal/PCD/Dxe/Service.c
> +++ b/MdeModulePkg/Universal/PCD/Dxe/Service.c
> @@ -435,7 +435,8 @@ GetWorker (
> switch (LocalTokenNumber & PCD_TYPE_ALL_SET) {
> case PCD_TYPE_VPD:
> VpdHead = (VPD_HEAD *) ((UINT8 *) PcdDb + Offset);
> - RetPtr = (VOID *) ((UINTN) PcdGet32 (PcdVpdBaseAddress) + VpdHead->Offset);
> + ASSERT (mVpdBaseAddress != 0);
> + RetPtr = (VOID *) (mVpdBaseAddress + VpdHead->Offset);
>
> break;
>
> diff --git a/MdeModulePkg/Universal/PCD/Pei/Service.c b/MdeModulePkg/Universal/PCD/Pei/Service.c
> index bb4b52baf3..e1613e8af8 100644
> --- a/MdeModulePkg/Universal/PCD/Pei/Service.c
> +++ b/MdeModulePkg/Universal/PCD/Pei/Service.c
> @@ -861,6 +861,7 @@ GetWorker (
> UINT32 LocalTokenNumber;
> UINT32 LocalTokenCount;
> UINT8 *VaraiableDefaultBuffer;
> + UINTN VpdBaseAddress;
>
> //
> // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.
> @@ -889,7 +890,19 @@ GetWorker (
> {
> VPD_HEAD *VpdHead;
> VpdHead = (VPD_HEAD *) ((UINT8 *)PeiPcdDb + Offset);
> - return (VOID *) ((UINTN) PcdGet32 (PcdVpdBaseAddress) + VpdHead->Offset);
> +
> + //
> + // PcdVpdBaseAddress64 is DynamicEx PCD only. So, PeiPcdGet64Ex() is used to get its value.
> + //
> + VpdBaseAddress = (UINTN) PeiPcdGet64Ex (&gEfiMdeModulePkgTokenSpaceGuid, PcdToken (PcdVpdBaseAddress64));
> + if (VpdBaseAddress == 0) {
> + //
> + // PcdVpdBaseAddress64 is not set, get value from PcdVpdBaseAddress.
> + //
> + VpdBaseAddress = (UINTN) PcdGet32 (PcdVpdBaseAddress);
> + }
> + ASSERT (VpdBaseAddress != 0);
> + return (VOID *)(VpdBaseAddress + VpdHead->Offset);
> }
>
> case PCD_TYPE_HII|PCD_TYPE_STRING:
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 0abacc1a90..f46a4094ca 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2022,5 +2022,13 @@
> # @Prompt NV Storage Default Value Buffer
> gEfiMdeModulePkgTokenSpaceGuid.PcdNvStoreDefaultValueBuffer|{0x0}|VOID*|0x00030005
>
> + ## VPD type PCD allows a developer to point to an absolute physical address PcdVpdBaseAddress
1. PcdVpdBaseAddress should be PcdVpdBaseAddress64, right?
> + # to store PCD value. It will be DynamicExDefault only.
> + # It is used to set VPD region base address. So, it can't be DynamicExVpd PCD. Its value is
> + # required to be accessed in PcdDxe driver entry point. So, its value must be set in PEI phase.
> + # It can't depend on EFI variable service, and can't be DynamicExHii PCD.
> + # @Prompt 64bit VPD base address.
> + gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64|0x0|UINT64|0x00030006
Please update MdeModulePkg.uni accordingly for this new PCD also.
Thanks,
Star
> +
> [UserExtensions.TianoCore."ExtraFiles"]
> MdeModulePkgExtra.uni
> diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> index 1f41a316bd..4ba78e46a3 100644
> --- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> +++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> @@ -73,7 +73,7 @@
> #
> # c) OEM specificed storage area:
> # - The PCD value is stored in OEM specified area which base address is
> -# specified by a FixedAtBuild PCD setting - PcdVpdBaseAddress.
> +# specified by PCD setting - PcdVpdBaseAddress64 or PcdVpdBaseAddress.
> # - The area is read only for PEI and DXE phase.
> # - [PcdsDynamicVpd] is used as section name for this type PCD in platform
> # DSC file. [PcdsDynamicExVpd] is for dynamicex type PCD.
> @@ -344,6 +344,7 @@
>
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress ## SOMETIMES_CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64 ## SOMETIMES_CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetNvStoreDefaultId ## SOMETIMES_CONSUMES
>
> [Depex]
> diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.h b/MdeModulePkg/Universal/PCD/Dxe/Service.h
> index ddd5fa471e..3055e30cd1 100644
> --- a/MdeModulePkg/Universal/PCD/Dxe/Service.h
> +++ b/MdeModulePkg/Universal/PCD/Dxe/Service.h
> @@ -48,6 +48,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #error "Please make sure the version of PCD DXE Service and the generated PCD DXE Database match."
> #endif
>
> +extern UINTN mVpdBaseAddress;
> +
> /**
> Retrieve additional information associated with a PCD token in the default token space.
>
> diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> index 6e28fce8fa..63b125d48a 100644
> --- a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> +++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> @@ -72,7 +72,7 @@
> #
> # c) OEM specificed storage area:
> # - The PCD value is stored in OEM specified area which base address is
> -# specified by a FixedAtBuild PCD setting - PcdVpdBaseAddress.
> +# specified by PCD setting - PcdVpdBaseAddress64 or PcdVpdBaseAddress.
> # - The area is read only for PEI and DXE phase.
> # - [PcdsDynamicVpd] is used as section name for this type PCD in platform
> # DSC file. [PcdsDynamicExVpd] is for dynamicex type PCD.
> @@ -344,6 +344,7 @@
>
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress ## SOMETIMES_CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64 ## SOMETIMES_CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxPeiPcdCallBackNumberPerPcdEntry ## SOMETIMES_CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdNvStoreDefaultValueBuffer ## SOMETIMES_CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetNvStoreDefaultId ## CONSUMES
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch] MdeModulePkg PCD: Add DynamicEx PcdVpdBaseAddress64 for non SPI platform
2018-11-26 1:27 ` Zeng, Star
@ 2018-11-26 2:44 ` Gao, Liming
0 siblings, 0 replies; 3+ messages in thread
From: Gao, Liming @ 2018-11-26 2:44 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Bi, Dandan
Good catch. I will update the patch.
Thanks
Liming
> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, November 26, 2018 9:28 AM
> To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Bi, Dandan <dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [Patch] MdeModulePkg PCD: Add DynamicEx PcdVpdBaseAddress64 for non SPI platform
>
> On 2018/11/22 22:32, Liming Gao wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1356
> > Current PcdVpdBaseAddress is 32bit static Pcd. NON SPI platform needs to
> > configure it as Dynamic PCD. Emulator platform (such as NT32) may set its
> > value to 64bit address.
> > To meet with this usage, 64bit DynamicEx PcdVpdBaseAddress64 is introduced.
> > If its value is not zero, it will be used.
> > If its value is zero, static PcdVpdBaseAddress will be used.
> > When NON SPI platform enables VPD PCD, they need to set PcdVpdBaseAddress64.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Liming Gao <liming.gao@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Dandan Bi <dandan.bi@intel.com>
>
> Two minor comments at below, with them handled, Reviewed-by: Star Zeng
> <star.zeng@intel.com>
>
>
> > ---
> > MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 16 ++++++++++++++++
> > MdeModulePkg/Universal/PCD/Dxe/Service.c | 3 ++-
> > MdeModulePkg/Universal/PCD/Pei/Service.c | 15 ++++++++++++++-
> > MdeModulePkg/MdeModulePkg.dec | 8 ++++++++
> > MdeModulePkg/Universal/PCD/Dxe/Pcd.inf | 3 ++-
> > MdeModulePkg/Universal/PCD/Dxe/Service.h | 2 ++
> > MdeModulePkg/Universal/PCD/Pei/Pcd.inf | 3 ++-
> > 7 files changed, 46 insertions(+), 4 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> > index f977c7f18e..4e38a3844a 100644
> > --- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> > +++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
> > @@ -109,6 +109,7 @@ EFI_GET_PCD_INFO_PROTOCOL mEfiGetPcdInfoInstance = {
> > };
> >
> > EFI_HANDLE mPcdHandle = NULL;
> > +UINTN mVpdBaseAddress = 0;
> >
> > /**
> > Main entry for PCD DXE driver.
> > @@ -175,6 +176,21 @@ PcdDxeInit (
> > &Registration
> > );
> >
> > + //
> > + // Cache VpdBaseAddress in entry point for the following usage.
> > + //
> > +
> > + //
> > + // PcdVpdBaseAddress64 is DynamicEx PCD only. So, DxePcdGet64Ex() is used to get its value.
> > + //
> > + mVpdBaseAddress = (UINTN) DxePcdGet64Ex (&gEfiMdeModulePkgTokenSpaceGuid, PcdToken (PcdVpdBaseAddress64));
> > + if (mVpdBaseAddress == 0) {
> > + //
> > + // PcdVpdBaseAddress64 is not set, get value from PcdVpdBaseAddress.
> > + //
> > + mVpdBaseAddress = (UINTN) PcdGet32 (PcdVpdBaseAddress);
> > + }
> > +
> > return Status;
> > }
> >
> > diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.c b/MdeModulePkg/Universal/PCD/Dxe/Service.c
> > index 0517152366..4b44153e13 100644
> > --- a/MdeModulePkg/Universal/PCD/Dxe/Service.c
> > +++ b/MdeModulePkg/Universal/PCD/Dxe/Service.c
> > @@ -435,7 +435,8 @@ GetWorker (
> > switch (LocalTokenNumber & PCD_TYPE_ALL_SET) {
> > case PCD_TYPE_VPD:
> > VpdHead = (VPD_HEAD *) ((UINT8 *) PcdDb + Offset);
> > - RetPtr = (VOID *) ((UINTN) PcdGet32 (PcdVpdBaseAddress) + VpdHead->Offset);
> > + ASSERT (mVpdBaseAddress != 0);
> > + RetPtr = (VOID *) (mVpdBaseAddress + VpdHead->Offset);
> >
> > break;
> >
> > diff --git a/MdeModulePkg/Universal/PCD/Pei/Service.c b/MdeModulePkg/Universal/PCD/Pei/Service.c
> > index bb4b52baf3..e1613e8af8 100644
> > --- a/MdeModulePkg/Universal/PCD/Pei/Service.c
> > +++ b/MdeModulePkg/Universal/PCD/Pei/Service.c
> > @@ -861,6 +861,7 @@ GetWorker (
> > UINT32 LocalTokenNumber;
> > UINT32 LocalTokenCount;
> > UINT8 *VaraiableDefaultBuffer;
> > + UINTN VpdBaseAddress;
> >
> > //
> > // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.
> > @@ -889,7 +890,19 @@ GetWorker (
> > {
> > VPD_HEAD *VpdHead;
> > VpdHead = (VPD_HEAD *) ((UINT8 *)PeiPcdDb + Offset);
> > - return (VOID *) ((UINTN) PcdGet32 (PcdVpdBaseAddress) + VpdHead->Offset);
> > +
> > + //
> > + // PcdVpdBaseAddress64 is DynamicEx PCD only. So, PeiPcdGet64Ex() is used to get its value.
> > + //
> > + VpdBaseAddress = (UINTN) PeiPcdGet64Ex (&gEfiMdeModulePkgTokenSpaceGuid, PcdToken (PcdVpdBaseAddress64));
> > + if (VpdBaseAddress == 0) {
> > + //
> > + // PcdVpdBaseAddress64 is not set, get value from PcdVpdBaseAddress.
> > + //
> > + VpdBaseAddress = (UINTN) PcdGet32 (PcdVpdBaseAddress);
> > + }
> > + ASSERT (VpdBaseAddress != 0);
> > + return (VOID *)(VpdBaseAddress + VpdHead->Offset);
> > }
> >
> > case PCD_TYPE_HII|PCD_TYPE_STRING:
> > diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> > index 0abacc1a90..f46a4094ca 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -2022,5 +2022,13 @@
> > # @Prompt NV Storage Default Value Buffer
> > gEfiMdeModulePkgTokenSpaceGuid.PcdNvStoreDefaultValueBuffer|{0x0}|VOID*|0x00030005
> >
> > + ## VPD type PCD allows a developer to point to an absolute physical address PcdVpdBaseAddress
>
> 1. PcdVpdBaseAddress should be PcdVpdBaseAddress64, right?
>
> > + # to store PCD value. It will be DynamicExDefault only.
> > + # It is used to set VPD region base address. So, it can't be DynamicExVpd PCD. Its value is
> > + # required to be accessed in PcdDxe driver entry point. So, its value must be set in PEI phase.
> > + # It can't depend on EFI variable service, and can't be DynamicExHii PCD.
> > + # @Prompt 64bit VPD base address.
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64|0x0|UINT64|0x00030006
>
> Please update MdeModulePkg.uni accordingly for this new PCD also.
>
>
> Thanks,
> Star
>
> > +
> > [UserExtensions.TianoCore."ExtraFiles"]
> > MdeModulePkgExtra.uni
> > diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> > index 1f41a316bd..4ba78e46a3 100644
> > --- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> > +++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> > @@ -73,7 +73,7 @@
> > #
> > # c) OEM specificed storage area:
> > # - The PCD value is stored in OEM specified area which base address is
> > -# specified by a FixedAtBuild PCD setting - PcdVpdBaseAddress.
> > +# specified by PCD setting - PcdVpdBaseAddress64 or PcdVpdBaseAddress.
> > # - The area is read only for PEI and DXE phase.
> > # - [PcdsDynamicVpd] is used as section name for this type PCD in platform
> > # DSC file. [PcdsDynamicExVpd] is for dynamicex type PCD.
> > @@ -344,6 +344,7 @@
> >
> > [Pcd]
> > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress ## SOMETIMES_CONSUMES
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64 ## SOMETIMES_CONSUMES
> > gEfiMdeModulePkgTokenSpaceGuid.PcdSetNvStoreDefaultId ## SOMETIMES_CONSUMES
> >
> > [Depex]
> > diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.h b/MdeModulePkg/Universal/PCD/Dxe/Service.h
> > index ddd5fa471e..3055e30cd1 100644
> > --- a/MdeModulePkg/Universal/PCD/Dxe/Service.h
> > +++ b/MdeModulePkg/Universal/PCD/Dxe/Service.h
> > @@ -48,6 +48,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > #error "Please make sure the version of PCD DXE Service and the generated PCD DXE Database match."
> > #endif
> >
> > +extern UINTN mVpdBaseAddress;
> > +
> > /**
> > Retrieve additional information associated with a PCD token in the default token space.
> >
> > diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> > index 6e28fce8fa..63b125d48a 100644
> > --- a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> > +++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> > @@ -72,7 +72,7 @@
> > #
> > # c) OEM specificed storage area:
> > # - The PCD value is stored in OEM specified area which base address is
> > -# specified by a FixedAtBuild PCD setting - PcdVpdBaseAddress.
> > +# specified by PCD setting - PcdVpdBaseAddress64 or PcdVpdBaseAddress.
> > # - The area is read only for PEI and DXE phase.
> > # - [PcdsDynamicVpd] is used as section name for this type PCD in platform
> > # DSC file. [PcdsDynamicExVpd] is for dynamicex type PCD.
> > @@ -344,6 +344,7 @@
> >
> > [Pcd]
> > gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress ## SOMETIMES_CONSUMES
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress64 ## SOMETIMES_CONSUMES
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxPeiPcdCallBackNumberPerPcdEntry ## SOMETIMES_CONSUMES
> > gEfiMdeModulePkgTokenSpaceGuid.PcdNvStoreDefaultValueBuffer ## SOMETIMES_CONSUMES
> > gEfiMdeModulePkgTokenSpaceGuid.PcdSetNvStoreDefaultId ## CONSUMES
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-26 2:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-22 14:32 [Patch] MdeModulePkg PCD: Add DynamicEx PcdVpdBaseAddress64 for non SPI platform Liming Gao
2018-11-26 1:27 ` Zeng, Star
2018-11-26 2:44 ` Gao, Liming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox