public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Guo Dong" <guo.dong@intel.com>
To: "Liu, Zhiguang" <zhiguang.liu@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Ma, Maurice" <maurice.ma@intel.com>,
	"You, Benjamin" <benjamin.you@intel.com>
Subject: Re: [Patch V2 07/12] UefiPayloadPkg: Fix up UPL Pcd database
Date: Wed, 23 Jun 2021 23:45:38 +0000	[thread overview]
Message-ID: <BYAPR11MB36223E6CC43B28DF2AAA23639E089@BYAPR11MB3622.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210623025235.3311-8-zhiguang.liu@intel.com>


Reviewed-by: Guo Dong <guo.dong@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Tuesday, June 22, 2021 7:53 PM
> To: devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
> <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: [Patch V2 07/12] UefiPayloadPkg: Fix up UPL Pcd database
> 
> V1:
> Edk2 bootloader will pass the pei pcd database, and UPL also contain a
> PCD database.
> Dxe PCD driver has the assumption that the two PCD database can be
> catenated and the local token number should be successive。
> This patch will manually fix up the UPL PCD database to meet that
> assumption.
> V2:
> Update the function comments
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  UefiPayloadPkg/UefiPayloadEntry/LoadDxeCore.c             | 19
> ++++++++++++-------
>  UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h        | 45
> +++++++++++++++++++++++++++++++++++++++++++++
>  UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c   | 57
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf |  2 ++
>  UefiPayloadPkg/UefiPayloadPkg.dec                         |  2 ++
>  UefiPayloadPkg/UefiPayloadPkg.dsc                         |  1 +
>  6 files changed, 119 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/LoadDxeCore.c
> b/UefiPayloadPkg/UefiPayloadEntry/LoadDxeCore.c
> index f5d70c59f8..0b6cb47cd0 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/LoadDxeCore.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/LoadDxeCore.c
> @@ -114,20 +114,23 @@ LoadPeCoffImage (
>  }
> 
> 
> 
>  /**
> 
> -  This function searchs a given file type within a valid FV.
> 
> +  This function searchs a given file type with a given Guid within a valid FV.
> 
> +  If input Guid is NULL, will locate the first section having the given file type
> 
> 
> 
>    @param FvHeader        A pointer to firmware volume header that contains
> the set of files
> 
>                           to be searched.
> 
>    @param FileType        File type to be searched.
> 
> +  @param Guid            Will ignore if it is NULL.
> 
>    @param FileHeader      A pointer to the discovered file, if successful.
> 
> 
> 
>    @retval EFI_SUCCESS    Successfully found FileType
> 
>    @retval EFI_NOT_FOUND  File type can't be found.
> 
>  **/
> 
>  EFI_STATUS
> 
> -FvFindFile (
> 
> +FvFindFileByTypeGuid (
> 
>    IN  EFI_FIRMWARE_VOLUME_HEADER  *FvHeader,
> 
>    IN  EFI_FV_FILETYPE             FileType,
> 
> +  IN  EFI_GUID                    *Guid           OPTIONAL,
> 
>    OUT EFI_FFS_FILE_HEADER         **FileHeader
> 
>    )
> 
>  {
> 
> @@ -171,8 +174,10 @@ FvFindFile (
>      // Look for file type
> 
>      //
> 
>      if (File->Type == FileType) {
> 
> -      *FileHeader = File;
> 
> -      return EFI_SUCCESS;
> 
> +      if (Guid == NULL || CompareGuid(&File->Name, Guid)) {
> 
> +        *FileHeader = File;
> 
> +        return EFI_SUCCESS;
> 
> +      }
> 
>      }
> 
>    }
> 
> 
> 
> @@ -266,7 +271,7 @@ LoadDxeCore (
>    //
> 
>    // DXE FV is inside Payload FV. Here find DXE FV from Payload FV
> 
>    //
> 
> -  Status = FvFindFile (PayloadFv,
> EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE, &FileHeader);
> 
> +  Status = FvFindFileByTypeGuid (PayloadFv,
> EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE, NULL, &FileHeader);
> 
>    if (EFI_ERROR (Status)) {
> 
>      return Status;
> 
>    }
> 
> @@ -283,7 +288,7 @@ LoadDxeCore (
>    //
> 
>    // Find DXE core file from DXE FV
> 
>    //
> 
> -  Status = FvFindFile (DxeCoreFv, EFI_FV_FILETYPE_DXE_CORE, &FileHeader);
> 
> +  Status = FvFindFileByTypeGuid (DxeCoreFv, EFI_FV_FILETYPE_DXE_CORE,
> NULL, &FileHeader);
> 
>    if (EFI_ERROR (Status)) {
> 
>      return Status;
> 
>    }
> 
> @@ -330,7 +335,7 @@ UniversalLoadDxeCore (
>    //
> 
>    // Find DXE core file from DXE FV
> 
>    //
> 
> -  Status = FvFindFile (DxeFv, EFI_FV_FILETYPE_DXE_CORE, &FileHeader);
> 
> +  Status = FvFindFileByTypeGuid (DxeFv, EFI_FV_FILETYPE_DXE_CORE, NULL,
> &FileHeader);
> 
>    if (EFI_ERROR (Status)) {
> 
>      return Status;
> 
>    }
> 
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> index dff7b6b87f..331724c687 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> @@ -35,6 +35,7 @@
>  #include <UniversalPayload/AcpiTable.h>
> 
>  #include <UniversalPayload/UniversalPayload.h>
> 
>  #include <UniversalPayload/ExtraData.h>
> 
> +#include <Guid/PcdDataBaseSignatureGuid.h>
> 
> 
> 
>  #define LEGACY_8259_MASK_REGISTER_MASTER  0x21
> 
>  #define LEGACY_8259_MASK_REGISTER_SLAVE   0xA1
> 
> @@ -159,4 +160,48 @@ HandOffToDxeCore (
>    IN EFI_PEI_HOB_POINTERS   HobList
> 
>    );
> 
> 
> 
> +EFI_STATUS
> 
> +FixUpPcdDatabase (
> 
> +  IN  EFI_FIRMWARE_VOLUME_HEADER *DxeFv
> 
> +  );
> 
> +
> 
> +/**
> 
> +  This function searchs a given section type within a valid FFS file.
> 
> +
> 
> +  @param  FileHeader            A pointer to the file header that contains the set
> of sections to
> 
> +                                be searched.
> 
> +  @param  SearchType            The value of the section type to search.
> 
> +  @param  SectionData           A pointer to the discovered section, if
> successful.
> 
> +
> 
> +  @retval EFI_SUCCESS           The section was found.
> 
> +  @retval EFI_NOT_FOUND         The section was not found.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +FileFindSection (
> 
> +  IN EFI_FFS_FILE_HEADER        *FileHeader,
> 
> +  IN EFI_SECTION_TYPE           SectionType,
> 
> +  OUT VOID                      **SectionData
> 
> +  );
> 
> +
> 
> +/**
> 
> +  This function searchs a given file type with a given Guid within a valid FV.
> 
> +  If input Guid is NULL, will locate the first section having the given file type
> 
> +
> 
> +  @param FvHeader        A pointer to firmware volume header that contains
> the set of files
> 
> +                         to be searched.
> 
> +  @param FileType        File type to be searched.
> 
> +  @param Guid            Will ignore if it is NULL.
> 
> +  @param FileHeader      A pointer to the discovered file, if successful.
> 
> +
> 
> +  @retval EFI_SUCCESS    Successfully found FileType
> 
> +  @retval EFI_NOT_FOUND  File type can't be found.
> 
> +**/
> 
> +EFI_STATUS
> 
> +FvFindFileByTypeGuid (
> 
> +  IN  EFI_FIRMWARE_VOLUME_HEADER  *FvHeader,
> 
> +  IN  EFI_FV_FILETYPE             FileType,
> 
> +  IN  EFI_GUID                    *Guid           OPTIONAL,
> 
> +  OUT EFI_FFS_FILE_HEADER         **FileHeader
> 
> +  );
> 
>  #endif
> 
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
> b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
> index 9d59454486..b8c1317df0 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
> @@ -25,6 +25,62 @@
> 
> 
>  extern VOID  *mHobList;
> 
> 
> 
> +/**
> 
> +  Some bootloader may pass a pcd database, and UPL also contain a PCD
> database.
> 
> +  Dxe PCD driver has the assumption that the two PCD database can be
> catenated and
> 
> +  the local token number should be successive。
> 
> +  This function will fix up the UPL PCD database to meet that assumption.
> 
> +
> 
> +  @param[in]   DxeFv         The FV where to find the Universal PCD database.
> 
> +
> 
> +  @retval EFI_SUCCESS        If it completed successfully.
> 
> +  @retval other              Failed to fix up.
> 
> +**/
> 
> +EFI_STATUS
> 
> +FixUpPcdDatabase (
> 
> +  IN  EFI_FIRMWARE_VOLUME_HEADER *DxeFv
> 
> +  )
> 
> +{
> 
> +  EFI_STATUS                  Status;
> 
> +  EFI_FFS_FILE_HEADER         *FileHeader;
> 
> +  VOID                        *PcdRawData;
> 
> +  PEI_PCD_DATABASE            *PeiDatabase;
> 
> +  PEI_PCD_DATABASE            *UplDatabase;
> 
> +  EFI_HOB_GUID_TYPE           *GuidHob;
> 
> +  DYNAMICEX_MAPPING           *ExMapTable;
> 
> +  UINTN                       Index;
> 
> +
> 
> +  GuidHob = GetFirstGuidHob (&gPcdDataBaseHobGuid);
> 
> +  if (GuidHob == NULL) {
> 
> +    //
> 
> +    // No fix-up is needed.
> 
> +    //
> 
> +    return EFI_SUCCESS;
> 
> +  }
> 
> +  PeiDatabase = (PEI_PCD_DATABASE *) GET_GUID_HOB_DATA (GuidHob);
> 
> +  DEBUG ((DEBUG_INFO, "Find the Pei PCD data base, the total local token
> number is %d\n", PeiDatabase->LocalTokenCount));
> 
> +
> 
> +  Status = FvFindFileByTypeGuid (DxeFv, EFI_FV_FILETYPE_DRIVER,
> PcdGetPtr (PcdPcdDriverFile), &FileHeader);
> 
> +  ASSERT_EFI_ERROR (Status);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    return Status;
> 
> +  }
> 
> +  Status = FileFindSection (FileHeader, EFI_SECTION_RAW, &PcdRawData);
> 
> +  ASSERT_EFI_ERROR (Status);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  UplDatabase = (PEI_PCD_DATABASE *) PcdRawData;
> 
> +  ExMapTable  = (DYNAMICEX_MAPPING *) (UINTN) ((UINTN) PcdRawData
> + UplDatabase->ExMapTableOffset);
> 
> +
> 
> +  for (Index = 0; Index < UplDatabase->ExTokenCount; Index++) {
> 
> +    ExMapTable[Index].TokenNumber += PeiDatabase->LocalTokenCount;
> 
> +  }
> 
> +  DEBUG ((DEBUG_INFO, "Fix up UPL PCD database successfully\n"));
> 
> +  return EFI_SUCCESS;
> 
> +}
> 
> +
> 
>  /**
> 
>    Add HOB into HOB list
> 
> 
> 
> @@ -332,6 +388,7 @@ _ModuleEntryPoint (
>    Status = BuildHobs (BootloaderParameter, &DxeFv);
> 
>    ASSERT_EFI_ERROR (Status);
> 
> 
> 
> +  FixUpPcdDatabase (DxeFv);
> 
>    Status = UniversalLoadDxeCore (DxeFv, &DxeCoreEntryPoint);
> 
>    ASSERT_EFI_ERROR (Status);
> 
> 
> 
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> index 77cd25aafd..76d7e4791c 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> @@ -63,6 +63,7 @@
>    gEfiAcpiTableGuid
> 
>    gUefiSerialPortInfoGuid
> 
>    gUniversalPayloadExtraDataGuid
> 
> +  gPcdDataBaseHobGuid
> 
> 
> 
>  [FeaturePcd.IA32]
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode      ##
> CONSUMES
> 
> @@ -72,6 +73,7 @@
> 
> 
> 
> 
>  [Pcd.IA32,Pcd.X64]
> 
> +  gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable                      ##
> SOMETIMES_CONSUMES
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrM
> ask    ## CONSUMES
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask
> ## CONSUMES
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec
> b/UefiPayloadPkg/UefiPayloadPkg.dec
> index 105e1f5a1c..d84f560995 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dec
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dec
> @@ -72,3 +72,5 @@
> gUefiPayloadPkgTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
> |0x80|UINT32|0x
> 
> 
>  # Size of the region used by UEFI in permanent memory
> 
> 
> gUefiPayloadPkgTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x020
> 00000|UINT32|0x00000017
> 
> +
> 
> +gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile|{ 0x57, 0x72, 0xcf, 0x80,
> 0xab, 0x87, 0xf9, 0x47, 0xa3, 0xfe, 0xD5, 0x0B, 0x76, 0xd8, 0x95,
> 0x41 }|VOID*|0x00000018
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc
> b/UefiPayloadPkg/UefiPayloadPkg.dsc
> index d8277efccd..e3d669a6d6 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
> @@ -298,6 +298,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21,
> 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66,
> 0x23, 0x31 }
> 
> 
> 
> +  gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile|{ 0x57, 0x72, 0xcf,
> 0x80, 0xab, 0x87, 0xf9, 0x47, 0xa3, 0xfe, 0xD5, 0x0B, 0x76, 0xd8, 0x95, 0x41 }
> 
> 
> 
>  !if $(SOURCE_DEBUG_ENABLE)
> 
> 
> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x
> 2
> 
> --
> 2.30.0.windows.2


  reply	other threads:[~2021-06-23 23:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-23  2:52 [Patch V2 00/12] Enable Universal Payload in UefiPayloadPkg Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 01/12] UefiPayloadPkg: Add HobLib for UniversalPayload Zhiguang Liu
2021-06-23 23:45   ` Guo Dong
2021-06-23  2:52 ` [Patch V2 02/12] MdeModulePkg: Add new structure for the Universal Payload Serial Port Info Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 03/12] UefiPayloadPkg: Add a separate PlatformHookLib for Universal Payload Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 04/12] UefiPayloadPkg: Update the function definition of HobConstructor Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 05/12] UefiPayloadPkg: Create separate Payload Entry for UniversalPayload Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 06/12] UefiPayloadPkg: Get and enter DxeCore for Universal Payload Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 07/12] UefiPayloadPkg: Fix up UPL Pcd database Zhiguang Liu
2021-06-23 23:45   ` Guo Dong [this message]
2021-06-23  2:52 ` [Patch V2 08/12] UefiPayloadPkg: Include UniversalPayLoad modules in UefiPayloadPkg.dsc Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 09/12] UefiPayloadPkg: Remove assert when reserve MMIO/IO resource for devices Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 10/12] UefiPayloadPkg: Add macro to disable some drivers Zhiguang Liu
2021-06-23 23:48   ` Guo Dong
2021-06-23  2:52 ` [Patch V2 11/12] UefiPayloadPkg: Add PcdInstallAcpiSdtProtocol feature in UefiPayloadPkg Zhiguang Liu
2021-06-23  2:52 ` [Patch V2 12/12] UefiPayloadPkg: Add PcdResetOnMemoryTypeInformationChange " Zhiguang Liu
     [not found] ` <168B167444E9F647.3239@groups.io>
2021-06-23  8:50   ` [edk2-devel] [Patch V2 10/12] UefiPayloadPkg: Add macro to disable some drivers Zhiguang Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR11MB36223E6CC43B28DF2AAA23639E089@BYAPR11MB3622.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox