Re: [edk2-devel][Patch v2 3/7] MdeModulePkg: Add CapsuleOnDiskLoadPei PEIM.

["Ni, Ray" <ray.ni@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Xu,
> Wei6
> Sent: Wednesday, June 19, 2019 4:41 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: Re: [edk2-devel][Patch v2 3/7] MdeModulePkg: Add
> CapsuleOnDiskLoadPei PEIM.
> 
> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  FileNameSize = PcdGetSize (PcdCoDRelocationFileName);  Status =
> > > + PcdSetPtrS (PcdRecoveryFileName, &FileNameSize, (VOID *)
> > > PcdGetPtr(PcdCoDRelocationFileName));
> >
> >
> > Buffer for 'PcdRecoveryFileName' may not be big enough to hold the
> > content in 'PcdCoDRelocationFileName'.
> >
> > I think there might be a chance for the above PcdSetPtrS() call to fail.
> >
> 
> 
> Thanks a lot for the comments.
> Yes, 'PcdRecoveryFileName' should be larger than
> 'PcdCoDRelocationFileName'.
> I think no need to update the code, since these two PCDs are fixed during
> build time.
> I will update the description of 'PcdCoDRelocationFileName' to mention: it
> must be smaller than 'PcdRecoveryFileName', otherwise failure may occur.

But your code doesn't check the status of PcdSetPtrS().

> 
> Do you have comments about it?
> Thanks again.
> 
> 
> BR,
> Wei
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Wednesday, June 12, 2019 3:49 PM
> > To: devel@edk2.groups.io; Xu, Wei6 <wei6.xu@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> > <chao.b.zhang@intel.com>
> > Subject: RE: [edk2-devel][Patch v2 3/7] MdeModulePkg: Add
> > CapsuleOnDiskLoadPei PEIM.
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> > > Of Xu,
> > > Wei6
> > > Sent: Wednesday, June 05, 2019 11:42 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wang, Jian J; Wu, Hao A; Zhang, Chao B; Xu, Wei6
> > > Subject: [edk2-devel][Patch v2 3/7] MdeModulePkg: Add
> > > CapsuleOnDiskLoadPei PEIM.
> > >
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1852
> > >
> > > This module provides PPI to load Capsule On Disk temp relocation
> > > file from Root Directory file system, retrieve the capsules from the
> > > temp file and create capsule hobs for these capsules.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Chao B Zhang <chao.b.zhang@intel.com>
> > > Signed-off-by: Wei6 Xu <wei6.xu@intel.com>
> > > ---
> > >  MdeModulePkg/MdeModulePkg.dsc                      |   4 +
> > >  .../CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.c    | 442
> > > +++++++++++++++++++++
> > >  .../CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.inf  |  64 +++
> > > .../CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.uni  |  15 +
> > >  .../CapsuleOnDiskLoadPeiExtra.uni                  |  14 +
> > >  5 files changed, 539 insertions(+)
> > >  create mode 100644
> > >
> >
> MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.c
> > >  create mode 100644
> > >
> >
> MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.in
> > > f
> > >  create mode 100644
> > >
> >
> MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.u
> > > ni
> > >  create mode 100644
> > >
> >
> MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPeiEx
> > > tra.uni
> >
> > Since this a new module, could you help to follow the recommendation
> > in
> > https://edk2.groups.io/g/devel/message/39655?p=,,,20,0,0,0::Created,,U
> > efi
> > DebugLibStdErr,20,2,0,31318888
> >
> > to add/update 'static' (lower case) for global variables/functions
> > whose scope is limited within a single file?
> >
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > b/MdeModulePkg/MdeModulePkg.dsc index 995fd805e1..615edddbcc
> > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > @@ -197,10 +197,13 @@
> > >    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x06
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule|0x0
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizePopulateCapsule|0x0
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxPeiPerformanceLogEntries|28
> > >
> > > +[PcdsDynamicExDefault]
> > > +
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdRecoveryFileName|L"FVMAIN.FV"
> > > +
> > >  [Components]
> > >    MdeModulePkg/Application/HelloWorld/HelloWorld.inf
> > >    MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> > >
> MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.inf
> > >
> > > @@ -315,10 +318,11 @@
> > >
> > >
> >
> NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMainte
> > > nanceManagerUiLib.inf
> > >    }
> > >
> > >
> >
> MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManager
> > > Dxe.inf
> > >
> > >
> >
> MdeModulePkg/Universal/BootManagerPolicyDxe/BootManagerPolicyDxe.i
> > > nf
> > >    MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > > +
> > >
> >
> MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.in
> > > f
> > >
> MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> > >
> > MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> > >    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> > >
> > >
> >
> MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleD
> > > xe.inf
> > >
> > >
> >
> MdeModulePkg/Universal/Console/GraphicsOutputDxe/GraphicsOutputDx
> > > e.inf
> > > diff --git
> > >
> >
> a/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > c
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > c
> > > new file mode 100644
> > > index 0000000000..40d25f3d3b
> > > --- /dev/null
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > c
> > > @@ -0,0 +1,442 @@
> > > +/** @file
> > > +  Recovery module.
> > > +
> > > +  Caution: This module requires additional review when modified.
> > > +  This module will have external input - Capsule-on-Disk Temp
> > > + Relocation
> > > image.
> > > +  This external input must be validated carefully to avoid security
> > > + issue like  buffer overflow, integer overflow.
> > > +
> > > +  RetrieveRelocatedCapsule() will receive untrusted input and do
> > > + basic
> > > validation.
> > > +
> > > +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +//
> > > +// The package level header files this module uses // #include
> > > +<Uefi.h> #include <PiPei.h>
> > > +
> > > +//
> > > +// The protocols, PPI and GUID defintions for this module //
> > > +#include <Ppi/MasterBootMode.h> #include
> <Ppi/FirmwareVolumeInfo.h>
> > #include
> > > +<Ppi/ReadOnlyVariable2.h> #include <Ppi/Capsule.h> #include
> > > +<Ppi/CapsuleOnDisk.h> #include <Ppi/DeviceRecoveryModule.h>
> > > +
> > > +#include <Guid/FirmwareFileSystem2.h> // // The Library classes
> > > +this module consumes // #include <Library/DebugLib.h> #include
> > > +<Library/PeimEntryPoint.h> #include <Library/PeiServicesLib.h>
> > > +#include <Library/HobLib.h> #include <Library/BaseMemoryLib.h>
> > > +#include <Library/MemoryAllocationLib.h> #include
> > > +<Library/PcdLib.h> #include <Library/CapsuleLib.h> #include
> > > +<Library/ReportStatusCodeLib.h>
> > > +
> > > +/**
> > > +  Loads a DXE capsule from some media into memory and updates the
> > HOB
> > > table
> > > +  with the DXE firmware volume information.
> > > +
> > > +  @param[in]  PeiServices   General-purpose services that are available
> to
> > > every PEIM.
> > > +  @param[in]  This          Indicates the EFI_PEI_RECOVERY_MODULE_PPI
> > > instance.
> > > +
> > > +  @retval EFI_SUCCESS        The capsule was loaded correctly.
> > > +  @retval EFI_DEVICE_ERROR   A device error occurred.
> > > +  @retval EFI_NOT_FOUND      A recovery DXE capsule cannot be found.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +LoadCapsuleOnDisk (
> > > +  IN EFI_PEI_SERVICES              **PeiServices,
> > > +  IN EFI_PEI_CAPSULE_ON_DISK_PPI   *This
> > > +  );
> > > +
> > > +EFI_PEI_CAPSULE_ON_DISK_PPI mCapsuleOnDiskPpi = {
> > > +  LoadCapsuleOnDisk
> > > +};
> > > +
> > > +EFI_PEI_PPI_DESCRIPTOR mCapsuleOnDiskPpiList = {
> > > +  (EFI_PEI_PPI_DESCRIPTOR_PPI |
> > > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> > > +  &gEdkiiPeiCapsuleOnDiskPpiGuid,
> > > +  &mCapsuleOnDiskPpi
> > > +};
> > > +
> > > +/**
> > > +  Determine if capsule comes from memory by checking Capsule PPI.
> > > +
> > > +  @param[in]  PeiServices General purpose services available to
> > > + every
> > PEIM.
> > > +
> > > +  @retval TRUE   Capsule comes from memory.
> > > +  @retval FALSE  No capsule comes from memory.
> > > +
> > > +**/
> > > +STATIC
> > > +BOOLEAN
> > > +CheckCapsuleFromRam (
> > > +  IN CONST EFI_PEI_SERVICES          **PeiServices
> > > +  )
> > > +{
> > > +  EFI_STATUS              Status;
> > > +  PEI_CAPSULE_PPI         *Capsule;
> > > +
> > > +  Status = PeiServicesLocatePpi (
> > > +             &gPeiCapsulePpiGuid,
> >
> >
> > Suggest to use gEfiPeiCapsulePpiGuid here.
> > gPeiCapsulePpiGuid is kept for compatibility before PI Version 1.4.
> >
> >
> > > +             0,
> > > +             NULL,
> > > +             (VOID **) &Capsule
> > > +             );
> > > +  if (!EFI_ERROR(Status)) {
> > > +    Status = Capsule->CheckCapsuleUpdate ((EFI_PEI_SERVICES
> > > **)PeiServices);
> > > +    if (!EFI_ERROR(Status)) {
> > > +      return TRUE;
> > > +    }
> > > +  }
> > > +
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Determine if it is a Capsule On Disk mode.
> > > +
> > > +  @retval TRUE         Capsule On Disk mode.
> > > +  @retval FALSE        Not capsule On Disk mode.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +IsCapsuleOnDiskMode (
> > > +  VOID
> > > +  )
> > > +{
> > > +  EFI_STATUS                      Status;
> > > +  UINTN                           Size;
> > > +  EFI_PEI_READ_ONLY_VARIABLE2_PPI *PPIVariableServices;
> > > +  BOOLEAN                         CodRelocInfo;
> > > +
> > > +  Status = PeiServicesLocatePpi (
> > > +             &gEfiPeiReadOnlyVariable2PpiGuid,
> > > +             0,
> > > +             NULL,
> > > +             (VOID **) &PPIVariableServices
> > > +             );
> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  Size = sizeof (BOOLEAN);
> > > +  Status = PPIVariableServices->GetVariable (
> > > +                                  PPIVariableServices,
> > > +                                  COD_RELOCATION_INFO_VAR_NAME,
> > > +                                  &gEfiCapsuleVendorGuid,
> > > +                                  NULL,
> > > +                                  &Size,
> > > +                                  &CodRelocInfo
> > > +                                  );
> > > +
> > > +  if (EFI_ERROR (Status) || Size != sizeof(BOOLEAN) || CodRelocInfo
> > > + != TRUE)
> >
> >
> > For 'CodRelocInfo != TRUE', variable of BOOLEAN type can be directly
> > used in the 'if' statement without comparing with 'TRUE' or 'FALSE'.
> >
> >
> > > {
> > > +    DEBUG (( DEBUG_ERROR, "Error Get CodRelocationInfo variable
> > > + %r!\n",
> > > Status));
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  return TRUE;
> > > +}
> > > +
> > > +/**
> > > +  Gets capsule images from relocated capsule buffer.
> > > +  Create Capsule hob for each Capsule.
> > > +
> > > +  Caution: This function may receive untrusted input.
> > > +  Capsule-on-Disk Temp Relocation image is external input, so this
> > > + function  will validate Capsule-on-Disk Temp Relocation image to
> > > + make sure the
> > > content
> > > +  is read within the buffer.
> > > +
> > > +  @param[in]  RelocCapsuleBuf        Buffer pointer to the relocated
> capsule.
> > > +  @param[in]  RelocCapsuleTotalSize  Total size of the relocated capsule.
> > > +
> > > +  @retval EFI_SUCCESS     Succeed to get capsules and create hob.
> > > +  @retval Others          Fail to get capsules and create hob.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +RetrieveRelocatedCapsule (
> > > +  IN UINT8                *RelocCapsuleBuf,
> > > +  IN UINTN                RelocCapsuleTotalSize
> > > +  )
> > > +{
> > > +  EFI_STATUS               Status;
> > > +  UINTN                    Index;
> > > +  UINT8                    *CapsuleDataBufEnd;
> > > +  UINT8                    *CapsulePtr;
> > > +  UINT32                   CapsuleSize;
> > > +  UINT64                   TotalImageSize;
> > > +  UINTN                    CapsuleNum;
> > > +
> > > +  CapsuleNum = 0;
> > > +
> > > +  //
> > > +  // Temp file contains at least 2 capsule (including 1 capsule
> > > + name
> > > + capsule)
> > > & 1 UINT64
> > > +  //
> > > +  if (RelocCapsuleTotalSize < sizeof(UINT64) +
> > > + sizeof(EFI_CAPSULE_HEADER)
> > > * 2) {
> > > +    return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > > +  CopyMem(&TotalImageSize, RelocCapsuleBuf, sizeof(UINT64));
> > > +
> > > +  DEBUG ((DEBUG_INFO, "ProcessRelocatedCapsule CapsuleBuf %x
> > > TotalCapSize %lx\n",
> > > +                      RelocCapsuleBuf, TotalImageSize));
> > > +
> > > +  RelocCapsuleBuf += sizeof(UINT64);
> > > +
> > > +  //
> > > +  // TempCaspule file length check
> > > +  //
> > > +  if (MAX_ADDRESS - TotalImageSize <= sizeof(UINT64) ||
> > > +      (UINT64)RelocCapsuleTotalSize != TotalImageSize + sizeof(UINT64)
> ||
> > > +      (UINTN)(MAX_ADDRESS -
> > > (PHYSICAL_ADDRESS)(UINTN)RelocCapsuleBuf) <= TotalImageSize) {
> > > +    return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > > +  CapsuleDataBufEnd = RelocCapsuleBuf + TotalImageSize;
> > > +
> > > +  //
> > > +  // TempCapsule file integrity check over Capsule Header to ensure
> > > + no data
> > > corruption in NV Var & Relocation storage
> > > +  //
> > > +  CapsulePtr = RelocCapsuleBuf;
> > > +
> > > +  while (CapsulePtr < CapsuleDataBufEnd) {
> > > +    if ((CapsuleDataBufEnd - CapsulePtr) < sizeof(EFI_CAPSULE_HEADER)
> ||
> > > +        ((EFI_CAPSULE_HEADER *)CapsulePtr)->CapsuleImageSize <
> > > sizeof(EFI_CAPSULE_HEADER) ||
> > > +        (UINTN)(MAX_ADDRESS - (PHYSICAL_ADDRESS)(UINTN)CapsulePtr)
> > > + <
> > > ((EFI_CAPSULE_HEADER *)CapsulePtr)->CapsuleImageSize
> > > +        ) {
> > > +      break;
> > > +    }
> > > +    CapsulePtr += ((EFI_CAPSULE_HEADER *)CapsulePtr)-
> > >CapsuleImageSize;
> > > +    CapsuleNum ++;
> > > +  }
> > > +
> > > +  if (CapsulePtr != CapsuleDataBufEnd) {
> > > +    Status = EFI_INVALID_PARAMETER;
> > > +    goto EXIT;
> > > +  }
> > > +
> > > +  //
> > > +  // Capsule count must be less than PcdCapsuleMax, avoid building
> > > + too
> > > many CvHobs to occupy all the free space in HobList.
> > > +  //
> > > +  if (CapsuleNum > PcdGet16 (PcdCapsuleMax)) {
> > > +    Status = EFI_INVALID_PARAMETER;
> > > +    goto EXIT;
> > > +  }
> > > +
> > > +  //
> > > +  // Re-iterate the capsule buffer to create Capsule hob & Capsule
> > > + Name Str
> > > Hob for each Capsule saved in relocated capsule file
> > > +  //
> > > +  CapsulePtr = RelocCapsuleBuf;
> > > +  Index      = 0;
> > > +  while (CapsulePtr < CapsuleDataBufEnd) {
> > > +    CapsuleSize = ((EFI_CAPSULE_HEADER *)CapsulePtr)-
> > >CapsuleImageSize;
> > > +    BuildCvHob ((EFI_PHYSICAL_ADDRESS)(UINTN)CapsulePtr,
> > > + CapsuleSize);
> > > +
> > > +    DEBUG((DEBUG_INFO, "Capsule saved in address %x size %x\n",
> > > CapsulePtr, CapsuleSize));
> > > +
> > > +    CapsulePtr += CapsuleSize;
> > > +    Index++;
> > > +  }
> > > +
> > > +EXIT:
> > > +
> > > +  return Status;
> > > +}
> > > +
> > > +/**
> > > +  Recovery module entrypoint
> > > +
> > > +  @param[in] FileHandle   Handle of the file being invoked.
> > > +  @param[in] PeiServices  Describes the list of possible PEI Services.
> > > +
> > > +  @return EFI_SUCCESS Recovery module is initialized.
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +InitializeCapsuleOnDiskLoad (
> > > +  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> > > +  IN CONST EFI_PEI_SERVICES     **PeiServices
> > > +  )
> > > +{
> > > +  EFI_STATUS  Status;
> > > +  UINTN       BootMode;
> > > +  UINTN       FileNameSize;
> > > +
> > > +  BootMode = GetBootModeHob();
> > > +  ASSERT(BootMode == BOOT_ON_FLASH_UPDATE);
> > > +
> > > +  //
> > > +  // If there are capsules provisioned in memory, quit.
> > > +  // Only one capsule resource is accept, CapsuleOnRam's priority
> > > + is higher
> > > than CapsuleOnDisk.
> > > +  //
> > > +  if (CheckCapsuleFromRam(PeiServices)) {
> > > +    DEBUG((DEBUG_ERROR, "Capsule On Memory Detected! Quit.\n"));
> > > +    return EFI_ABORTED;
> > > +  }
> > > +
> > > +  DEBUG_CODE (
> > > +   VOID *CapsuleOnDiskModePpi;
> > > +
> > > +  if (!IsCapsuleOnDiskMode()){
> > > +    return EFI_NOT_FOUND;
> > > +  }
> > > +
> > > +  //
> > > +  // Check Capsule On Disk Relocation flag. If exists, load capsule
> > > + & create
> > > Capsule Hob
> > > +  //
> > > +  Status = PeiServicesLocatePpi (
> > > +             &gEfiPeiBootInCapsuleOnDiskModePpiGuid,
> > > +             0,
> > > +             NULL,
> > > +             (VOID **)&CapsuleOnDiskModePpi
> > > +             );
> > > +    if (EFI_ERROR(Status)) {
> > > +      DEBUG((DEBUG_ERROR, "Locate CapsuleOnDiskModePpi
> error %x\n",
> > > Status));
> > > +      return Status;
> > > +    }
> > > +  );
> > > +
> > > +  Status = (**PeiServices).InstallPpi (PeiServices,
> > > + &mCapsuleOnDiskPpiList);
> >
> >
> > Minor one, suggest to directly use PeiServicesInstallPpi().
> >
> >
> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  FileNameSize = PcdGetSize (PcdCoDRelocationFileName);  Status =
> > > + PcdSetPtrS (PcdRecoveryFileName, &FileNameSize, (VOID *)
> > > PcdGetPtr(PcdCoDRelocationFileName));
> >
> >
> > Buffer for 'PcdRecoveryFileName' may not be big enough to hold the
> > content in 'PcdCoDRelocationFileName'.
> >
> > I think there might be a chance for the above PcdSetPtrS() call to fail.
> >
> >
> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  return Status;
> > > +}
> > > +
> > > +/**
> > > +  Loads a DXE capsule from some media into memory and updates the
> > HOB
> > > table
> > > +  with the DXE firmware volume information.
> > > +
> > > +  @param[in]  PeiServices   General-purpose services that are available
> to
> > > every PEIM.
> > > +  @param[in]  This          Indicates the EFI_PEI_RECOVERY_MODULE_PPI
> > > instance.
> > > +
> > > +  @retval EFI_SUCCESS        The capsule was loaded correctly.
> > > +  @retval EFI_DEVICE_ERROR   A device error occurred.
> > > +  @retval EFI_NOT_FOUND      A recovery DXE capsule cannot be found.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +LoadCapsuleOnDisk (
> > > +  IN EFI_PEI_SERVICES                     **PeiServices,
> > > +  IN EFI_PEI_CAPSULE_ON_DISK_PPI          *This
> > > +  )
> > > +{
> > > +  EFI_STATUS                          Status;
> > > +  EFI_PEI_DEVICE_RECOVERY_MODULE_PPI  *DeviceRecoveryPpi;
> > > +  UINTN                               NumberRecoveryCapsules;
> > > +  UINTN                               Instance;
> > > +  UINTN                               CapsuleInstance;
> > > +  UINTN                               CapsuleSize;
> > > +  EFI_GUID                            CapsuleType;
> > > +  VOID                                *CapsuleBuffer;
> > > +
> > > +  DEBUG ((DEBUG_INFO | DEBUG_LOAD, "Load Capsule On Disk
> > Entry\n"));
> > > +
> > > +  for (Instance = 0; ; Instance++) {
> > > +    Status = PeiServicesLocatePpi (
> > > +               &gEfiPeiDeviceRecoveryModulePpiGuid,
> > > +               Instance,
> > > +               NULL,
> > > +               (VOID **)&DeviceRecoveryPpi
> > > +               );
> > > +    DEBUG ((DEBUG_INFO, "LoadCapsuleOnDisk - LocateRecoveryPpi (%d)
> > -
> > >  %r\n", Instance, Status));
> > > +    if (EFI_ERROR (Status)) {
> > > +      if (Instance == 0) {
> > > +        REPORT_STATUS_CODE (
> > > +          EFI_ERROR_CODE | EFI_ERROR_MAJOR,
> > > +          (EFI_SOFTWARE_PEI_MODULE |
> > > EFI_SW_PEI_EC_RECOVERY_PPI_NOT_FOUND)
> > > +          );
> > > +      }
> > > +      break;
> > > +    }
> > > +    NumberRecoveryCapsules = 0;
> > > +    Status = DeviceRecoveryPpi->GetNumberRecoveryCapsules (
> > > +                                  (EFI_PEI_SERVICES **)PeiServices,
> > > +                                  DeviceRecoveryPpi,
> > > +                                  &NumberRecoveryCapsules
> > > +                                  );
> > > +    DEBUG ((DEBUG_INFO, "LoadCapsuleOnDisk -
> > > GetNumberRecoveryCapsules (%d) - %r\n", NumberRecoveryCapsules,
> > > Status));
> > > +    if (EFI_ERROR (Status)) {
> > > +      continue;
> > > +    }
> > > +
> > > +    for (CapsuleInstance = 1; CapsuleInstance <=
> > > + NumberRecoveryCapsules;
> > > CapsuleInstance++) {
> > > +      CapsuleSize = 0;
> > > +      Status = DeviceRecoveryPpi->GetRecoveryCapsuleInfo (
> > > +                                    (EFI_PEI_SERVICES **)PeiServices,
> > > +                                    DeviceRecoveryPpi,
> > > +                                    CapsuleInstance,
> > > +                                    &CapsuleSize,
> > > +                                    &CapsuleType
> > > +                                    );
> > > +      DEBUG ((DEBUG_INFO, "LoadCapsuleOnDisk -
> > GetRecoveryCapsuleInfo
> > > (%d - %x) - %r\n", CapsuleInstance, CapsuleSize, Status));
> > > +      if (EFI_ERROR (Status)) {
> > > +        break;
> > > +      }
> > > +
> > > +      //
> > > +      // Allocate the memory so that it gets preserved into DXE.
> > > +      // Capsule is special because it may need to populate to system table
> > > +      //
> > > +      CapsuleBuffer = AllocateRuntimePages (EFI_SIZE_TO_PAGES
> > > (CapsuleSize));
> > > +
> > > +      if (CapsuleBuffer == NULL) {
> > > +        DEBUG ((DEBUG_ERROR, "LoadCapsuleOnDisk -
> > > + AllocateRuntimePages
> > > fail\n"));
> > > +        continue;
> > > +      }
> > > +
> > > +      Status = DeviceRecoveryPpi->LoadRecoveryCapsule (
> > > +                                    (EFI_PEI_SERVICES **)PeiServices,
> > > +                                    DeviceRecoveryPpi,
> > > +                                    CapsuleInstance,
> > > +                                    CapsuleBuffer
> > > +                                    );
> > > +      DEBUG ((DEBUG_INFO, "LoadCapsuleOnDisk - LoadRecoveryCapsule
> > > (%d) - %r\n", CapsuleInstance, Status));
> > > +      if (EFI_ERROR (Status)) {
> > > +        FreePages (CapsuleBuffer, EFI_SIZE_TO_PAGES(CapsuleSize));
> > > +        break;
> > > +      }
> > > +
> > > +      //
> > > +      // Capsule Update Mode, Split relocated Capsule buffer into
> > > + different
> > > capsule vehical hobs.
> > > +      //
> > > +      Status = RetrieveRelocatedCapsule(CapsuleBuffer,
> > > + CapsuleSize);
> > > +
> > > +      break;
> > > +    }
> > > +
> > > +    if (EFI_ERROR (Status)) {
> > > +      REPORT_STATUS_CODE (
> > > +        EFI_ERROR_CODE | EFI_ERROR_MAJOR,
> > > +        (EFI_SOFTWARE_PEI_MODULE |
> > > EFI_SW_PEI_EC_NO_RECOVERY_CAPSULE)
> > > +        );
> > > +    }
> > > +
> > > +    return Status;
> > > +  }
> > > +
> > > +  //
> > > +  // Any attack against GPT, Relocation Info Variable or temp
> > > + relocation file
> > > will result in no Capsule HOB and return EFI_NOT_FOUND.
> > > +  // After flow to DXE phase. since no capsule hob is detected.
> > > + Platform will
> > > clear Info flag and force restart.
> > > +  // No volunerability will be exposed  //
> > > +
> > > +  return EFI_NOT_FOUND;
> > > +}
> > > diff --git
> > >
> >
> a/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > inf
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > inf
> > > new file mode 100644
> > > index 0000000000..4af07440b7
> > > --- /dev/null
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > inf
> > > @@ -0,0 +1,64 @@
> > > +## @file
> > > +# Load Capsule on Disk module.
> > > +#
> > > +# Load Capsule On Disk from Root Directory file system. Create CV
> > > +hob # based on temporary Capsule On Disk file.
> > > +#
> > > +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> #
> > > +#
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> > > +
> > > +[Defines]
> > > +  INF_VERSION                    = 0x00010005
> > > +  BASE_NAME                      = CapsuleOnDiskLoadPei
> > > +  MODULE_UNI_FILE                = CapsuleOnDiskLoadPei.uni
> > > +  FILE_GUID                      = 8ADEDF9E-2EC8-40fb-AE56-B76D90225D2D
> > > +  MODULE_TYPE                    = PEIM
> > > +  VERSION_STRING                 = 1.0
> > > +  ENTRY_POINT                    = InitializeCapsuleOnDiskLoad
> > > +
> > > +#
> > > +# The following information is for reference only and not required
> > > +by the
> > > build tools.
> > > +#
> > > +#  VALID_ARCHITECTURES           = IA32 X64 EBC
> > > +#
> > > +
> > > +[Sources]
> > > +  CapsuleOnDiskLoadPei.c
> > > +
> > > +[Packages]
> > > +  MdePkg/MdePkg.dec
> > > +  MdeModulePkg/MdeModulePkg.dec
> > > +
> > > +[LibraryClasses]
> > > +  PeimEntryPoint
> > > +  DebugLib
> > > +  HobLib
> > > +  BaseMemoryLib
> > > +  MemoryAllocationLib
> > > +  ReportStatusCodeLib
> > > +
> > > +[Ppis]
> > > +  gEdkiiPeiCapsuleOnDiskPpiGuid           ## PRODUCES
> > > +  gEfiPeiReadOnlyVariable2PpiGuid         ## CONSUMES
> > > +  gEfiPeiBootInCapsuleOnDiskModePpiGuid   ##
> SOMETIMES_CONSUMES
> > > +  gEfiPeiDeviceRecoveryModulePpiGuid      ## CONSUMES
> > > +  gPeiCapsulePpiGuid                      ## CONSUMES
> >
> >
> > Suggest to use gEfiPeiCapsulePpiGuid here.
> > gPeiCapsulePpiGuid is kept for compatibility before PI Version 1.4.
> >
> >
> > > +
> > > +[Guids]
> > > +  gEfiCapsuleVendorGuid                   ## SOMETIMES_CONSUMES ##
> > Variable
> > > L"CodRelocationInfo"
> > > +
> > > +[Pcd]
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdCoDRelocationFileName
> > > ## CONSUMES
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax                            ##
> > > CONSUMES
> > > +
> > > +[PcdEx]
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdRecoveryFileName
> > ##
> > > PRODUCES
> > > +
> > > +[depex]
> >
> >
> > Minor comment:
> > [depex] -> [Depex]
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > > +  gEfiPeiBootInCapsuleOnD
> > iskModePpiGuid
> > > +
> > > +[UserExtensions.TianoCore."ExtraFiles"]
> > > +  CapsuleOnDiskLoadPeiExtra.uni
> > > diff --git
> > >
> >
> a/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > uni
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > uni
> > > new file mode 100644
> > > index 0000000000..c3eae6a5c2
> > > --- /dev/null
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei.
> > > uni
> > > @@ -0,0 +1,15 @@
> > > +// /** @file
> > > +// Caspule On Disk Load module.
> > > +//
> > > +// Load Capsule On Disk and build CV hob.
> > > +//
> > > +// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > > +// // SPDX-License-Identifier: BSD-2-Clause-Patent // // **/
> > > +
> > > +
> > > +#string STR_MODULE_ABSTRACT             #language en-US "Caspule On
> Disk
> > > Load module."
> > > +
> > > +#string STR_MODULE_DESCRIPTION          #language en-US "Load
> Capsule
> > > On Disk and build CV hob."
> > > diff --git
> > >
> >
> a/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei
> > > Extra.uni
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei
> > > Extra.uni
> > > new file mode 100644
> > > index 0000000000..81034f6294
> > > --- /dev/null
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/CapsuleOnDiskLoadPei/CapsuleOnDiskLoadPei
> > > Extra.uni
> > > @@ -0,0 +1,14 @@
> > > +// /** @file
> > > +// CapsuleOnDiskLoadPei Localized Strings and Content // //
> > > +Copyright
> > > +(c) 2019, Intel Corporation. All rights reserved.<BR> // //
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent // // **/
> > > +
> > > +#string STR_PROPERTIES_MODULE_NAME
> > > +#language en-US
> > > +"CapsuleOnDiskLoad PEI Driver"
> > > +
> > > +
> > > --
> > > 2.16.2.windows.1
> > >
> > >
> > >
> 
> 
>