Re: [edk2-devel][Patch v2 6/7] MdeModulePkg/DxeIpl: Support Capsule On Disk.

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----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 6/7] MdeModulePkg/DxeIpl: Support Capsule
> On Disk.
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1852
> 
> If Capsule On Disk mode, call Capsule On Disk Load PPI to load
> capsules. When it fails, still goes to Firmware Update boot path.
> BDS will clear corresponding indicator and reboot later on.
> 
> 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/Core/DxeIplPeim/DxeIpl.h   |  3 ++-
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 20 ++++++++++--------
>  MdeModulePkg/Core/DxeIplPeim/DxeLoad.c  | 37
> ++++++++++++++++++++++++++++++++-
>  3 files changed, 49 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> index 063fefb414..90b5b5b211 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> @@ -1,10 +1,10 @@
>  /** @file
>    Master header file for DxeIpl PEIM. All source files in this module should
>    include this file for common definitions.
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
>  #ifndef __PEI_DXEIPL_H__
> @@ -19,10 +19,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Ppi/FirmwareVolumeInfo.h>
>  #include <Ppi/GuidedSectionExtraction.h>
>  #include <Ppi/LoadFile.h>
>  #include <Ppi/S3Resume2.h>
>  #include <Ppi/RecoveryModule.h>
> +#include <Ppi/CapsuleOnDisk.h>
>  #include <Ppi/VectorHandoffInfo.h>
> 
>  #include <Guid/MemoryTypeInformation.h>
>  #include <Guid/MemoryAllocationHob.h>
>  #include <Guid/FirmwareFileSystem2.h>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 62bb3f3077..ff036d8688 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -3,11 +3,11 @@
>  #
>  #  This module produces a special PPI named the DXE Initial Program Load
> (IPL)
>  #  PPI to discover and dispatch the DXE Foundation and components that are
>  #  needed to run the DXE Foundation.
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -74,21 +74,23 @@
> 
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>    ArmMmuLib
> 
>  [Ppis]
> -  gEfiDxeIplPpiGuid                 ## PRODUCES
> -  gEfiPeiDecompressPpiGuid          ## PRODUCES
> -  gEfiEndOfPeiSignalPpiGuid         ## SOMETIMES_PRODUCES # Not produced
> on S3 boot path
> -  gEfiPeiReadOnlyVariable2PpiGuid   ## SOMETIMES_CONSUMES
> -  gEfiPeiLoadFilePpiGuid            ## SOMETIMES_CONSUMES
> -  gEfiPeiS3Resume2PpiGuid           ## SOMETIMES_CONSUMES # Consumed
> on S3 boot path
> -  gEfiPeiRecoveryModulePpiGuid      ## SOMETIMES_CONSUMES #
> Consumed on recovery boot path
> +  gEfiDxeIplPpiGuid                      ## PRODUCES
> +  gEfiPeiDecompressPpiGuid               ## PRODUCES
> +  gEfiEndOfPeiSignalPpiGuid              ## SOMETIMES_PRODUCES # Not
> produced on S3 boot path
> +  gEfiPeiReadOnlyVariable2PpiGuid        ## SOMETIMES_CONSUMES
> +  gEfiPeiLoadFilePpiGuid                 ## SOMETIMES_CONSUMES
> +  gEfiPeiS3Resume2PpiGuid                ## SOMETIMES_CONSUMES #
> Consumed on S3 boot path
> +  gEfiPeiRecoveryModulePpiGuid           ## SOMETIMES_CONSUMES #
> Consumed on recovery boot path
>    ## SOMETIMES_CONSUMES
>    ## UNDEFINED # HOB
>    gEfiVectorHandoffInfoPpiGuid
> -  gEfiPeiMemoryDiscoveredPpiGuid    ## SOMETIMES_CONSUMES
> +  gEfiPeiMemoryDiscoveredPpiGuid         ## SOMETIMES_CONSUMES
> +  gEfiPeiBootInCapsuleOnDiskModePpiGuid  ## SOMETIMES_CONSUMES
> +  gEdkiiPeiCapsuleOnDiskPpiGuid          ## SOMETIMES_CONSUMES #
> Consumed on firmware update boot path
> 
>  [Guids]
>    ## SOMETIMES_CONSUMES ## Variable:L"MemoryTypeInformation"
>    ## SOMETIMES_PRODUCES ## HOB
>    gEfiMemoryTypeInformationGuid
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> index c6e5b83309..9dc2d4485f 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> @@ -1,11 +1,11 @@
>  /** @file
>    Last PEIM.
>    Responsibility of this module is to load the DXE Core from a Firmware
> Volume.
> 
>  Copyright (c) 2016 HP Development Company, L.P.
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
>  #include "DxeIpl.h"
> @@ -263,17 +263,38 @@ DxeLoadCore (
>    UINTN                                     Instance;
>    UINT32                                    AuthenticationState;
>    UINTN                                     DataSize;
>    EFI_PEI_S3_RESUME2_PPI                    *S3Resume;
>    EFI_PEI_RECOVERY_MODULE_PPI               *PeiRecovery;
> +  EFI_PEI_CAPSULE_ON_DISK_PPI               *PeiCapsuleOnDisk;
>    EFI_MEMORY_TYPE_INFORMATION
> MemoryData[EfiMaxMemoryType + 1];
> +  VOID                                      *CapsuleOnDiskModePpi;
> +  BOOLEAN                                   IsCapsuleOnDiskMode;
> +
> +  IsCapsuleOnDiskMode = FALSE;
> 
>    //
>    // if in S3 Resume, restore configure
>    //
>    BootMode = GetBootModeHob ();
> 
> +  //
> +  // If Capsule On Disk mode, call storage stack to read Capsule Relocation
> file
> +  // IoMmmu is highly recommmended to enable before reading
> +  //
> +  if (BootMode == BOOT_ON_FLASH_UPDATE) {
> +    Status = PeiServicesLocatePpi (
> +               &gEfiPeiBootInCapsuleOnDiskModePpiGuid,
> +               0,
> +               NULL,
> +               &CapsuleOnDiskModePpi
> +               );
> +    if (!EFI_ERROR(Status)) {
> +      IsCapsuleOnDiskMode = TRUE;
> +    }
> +  }
> +


Suggest to merge the above logic with the below 

  else if (IsCapsuleOnDiskMode) {...}

chunk of codes together.

In such case, the logic seems a bit cleaner and 'IsCapsuleOnDiskMode' can
also be dropped.

Best Regards,
Hao Wu


>    if (BootMode == BOOT_ON_S3_RESUME) {
>      Status = PeiServicesLocatePpi (
>                 &gEfiPeiS3Resume2PpiGuid,
>                 0,
>                 NULL,
> @@ -328,10 +349,24 @@ DxeLoadCore (
>      }
>      REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> (EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_PC_CAPSULE_START));
>      //
>      // Now should have a HOB with the DXE core
>      //
> +  } else if (IsCapsuleOnDiskMode) {
> +    Status = PeiServicesLocatePpi (
> +               &gEdkiiPeiCapsuleOnDiskPpiGuid,
> +               0,
> +               NULL,
> +               (VOID **) &PeiCapsuleOnDisk
> +               );
> +
> +    //
> +    // Whether failed, still goes to Firmware Update boot path. BDS will clear
> corresponding indicator and reboot later on
> +    //
> +    if (!EFI_ERROR (Status)) {
> +      Status = PeiCapsuleOnDisk->LoadCapsuleOnDisk (PeiServices,
> PeiCapsuleOnDisk);
> +    }
>    }
> 
>    if (GetFirstGuidHob ((CONST EFI_GUID
> *)&gEfiMemoryTypeInformationGuid) == NULL) {
>      //
>      // Don't build GuidHob if GuidHob has been installed.
> --
> 2.16.2.windows.1
> 
> 
>