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 <[email protected]>; [email protected] > Cc: Bi, Dandan <[email protected]>; Zeng, Star <[email protected]> > 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 <[email protected]> > > Cc: Star Zeng <[email protected]> > > Cc: Jian J Wang <[email protected]> > > Cc: Dandan Bi <[email protected]> > > Two minor comments at below, with them handled, Reviewed-by: Star Zeng > <[email protected]> > > > > --- > > 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 > >
_______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
