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

["Xu, Wei6" <wei6.xu@intel.com>]

> > +  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.

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,,Uefi
> 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
> >
> >
> >