Re: [PATCH v4 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Friday, June 15, 2018 3:52 AM
> To: Zeng, Star <star.zeng@intel.com>
> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Yao, Jiewen
> <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v4 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the
> capsule payload to DRAM
> 
> On 14 June 2018 at 02:54, Zeng, Star <star.zeng@intel.com> wrote:
> > With 'EFIAPI' removed from IsPersistAcrossResetCapsuleSupported and
> CapsuleCacheWriteBack definitions, Reviewed-by: Star Zeng
> <star.zeng@intel.com>.
> >
> > You can wait a little more time in case Jiewen/Mike has comments.
> >
> 
> Thank you Star.
> 
> I will push these by the end of today unless anyone objects.
> 
> 
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Wednesday, June 13, 2018 4:09 PM
> > To: edk2-devel@lists.01.org
> > Cc: leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>
> > Subject: [PATCH v4 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the
> capsule payload to DRAM
> >
> > When capsule updates are staged for processing after a warm reboot, they are
> copied into memory with the MMU and caches enabled. When the capsule PEI
> gets around to coalescing the capsule, the MMU and caches may still be disabled,
> and so on architectures where uncached accesses are incoherent with the caches
> (such as ARM and AARCH64), we need to ensure that the data passed into
> UpdateCapsule() is written back to main memory before performing the warm
> reboot.
> >
> > Unfortunately, on ARM, the only type of cache maintenance instructions that
> are suitable for this purpose operate on virtual addresses only, and given that the
> UpdateCapsule() prototype includes the physical address of a linked list of
> scatter/gather data structures that are mapped at an address that is unknown to
> the firmware (and may not even be mapped at all when UpdateCapsule() is
> invoked), we can only perform this cache maintenance at boot time. Fortunately,
> both Windows and Linux only invoke UpdateCapsule() before calling
> ExitBootServices(), so this is not a problem in practice.
> >
> > In the future, we may propose adding a secure firmware service that permits
> performing the cache maintenance at OS runtime, in which case this code may
> be enhanced to call that service if available. For now, we just fail any
> UpdateCapsule() calls performed at OS runtime on ARM.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CapsuleReset.c    | 77
> ++++++++++++++++++++
> >  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c        | 51
> +++++++++++++
> >  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf | 14
> +++-
> >  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c      | 33
> ++-------
> >  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h      | 73
> +++++++++++++++++++
> >  5 files changed, 219 insertions(+), 29 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CapsuleReset.c
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CapsuleReset.c
> > new file mode 100644
> > index 000000000000..7e0ca06ce7d0
> > --- /dev/null
> > +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CapsuleReset.c
> > @@ -0,0 +1,77 @@
> > + /** @file
> > +  ARM implementation of architecture specific routines related to
> > + PersistAcrossReset capsules
> > +
> > +  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > +
> > +  This program and the accompanying materials are licensed and made
> > + available  under the terms and conditions of the BSD License which
> > + accompanies this  distribution.  The full text of the license may be
> > + found at  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include "CapsuleService.h"
> > +
> > +#include <Library/CacheMaintenanceLib.h>
> > +
> > +/**
> > +  Whether the platform supports capsules that persist across reset.
> > +Note that
> > +  some platforms only support such capsules at boot time.
> > +
> > +  @return TRUE  if a PersistAcrossReset capsule may be passed to
> UpdateCapsule()
> > +                at this time
> > +          FALSE otherwise
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +IsPersistAcrossResetCapsuleSupported (
> > +  VOID
> > +  )
> > +{
> > +  //
> > +  // ARM requires the capsule payload to be cleaned to the point of
> > +coherency
> > +  // (PoC), but only permits doing so using cache maintenance
> > +instructions that
> > +  // operate on virtual addresses. Since at runtime, we don't know the
> > +virtual
> > +  // addresses of the data structures that make up the scatter/gather
> > +list, we
> > +  // cannot perform the maintenance, and all we can do is give up.
> > +  //
> > +  return FeaturePcdGet (PcdSupportUpdateCapsuleReset) && !EfiAtRuntime
> > +(); }
> > +
> > +/**
> > +  Writes Back a range of data cache lines covering a set of capsules in
> memory.
> > +
> > +  Writes Back the data cache lines specified by ScatterGatherList.
> > +
> > +  @param  ScatterGatherList Physical address of the data structure that
> > +                            describes a set of capsules in memory
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +CapsuleCacheWriteBack (
> > +  IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
> > +  )
> > +{
> > +  EFI_CAPSULE_BLOCK_DESCRIPTOR    *Desc;
> > +
> > +  Desc = (EFI_CAPSULE_BLOCK_DESCRIPTOR *)(UINTN)ScatterGatherList;
> do
> > + {
> > +    WriteBackDataCacheRange (Desc, sizeof *Desc);
> > +
> > +    if (Desc->Length > 0) {
> > +      WriteBackDataCacheRange ((VOID *)(UINTN)Desc->Union.DataBlock,
> > +                               Desc->Length
> > +                               );
> > +      Desc++;
> > +    } else if (Desc->Union.ContinuationPointer > 0) {
> > +      Desc = (EFI_CAPSULE_BLOCK_DESCRIPTOR
> *)(UINTN)Desc->Union.ContinuationPointer;
> > +    }
> > +  } while (Desc->Length > 0 || Desc->Union.ContinuationPointer > 0);
> > +
> > +  WriteBackDataCacheRange (Desc, sizeof *Desc); }
> > diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c
> > new file mode 100644
> > index 000000000000..09616999e3f8
> > --- /dev/null
> > +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleReset.c
> > @@ -0,0 +1,51 @@
> > +/** @file
> > +  Default implementation of architecture specific routines related to
> > +  PersistAcrossReset capsules
> > +
> > +  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > +
> > +  This program and the accompanying materials are licensed and made
> > + available  under the terms and conditions of the BSD License which
> > + accompanies this  distribution.  The full text of the license may be
> > + found at  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include "CapsuleService.h"
> > +
> > +/**
> > +  Whether the platform supports capsules that persist across reset.
> > +Note that
> > +  some platforms only support such capsules at boot time.
> > +
> > +  @return TRUE  if a PersistAcrossReset capsule may be passed to
> UpdateCapsule()
> > +                at this time
> > +          FALSE otherwise
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +IsPersistAcrossResetCapsuleSupported (
> > +  VOID
> > +  )
> > +{
> > +  return FeaturePcdGet (PcdSupportUpdateCapsuleReset); }
> > +
> > +/**
> > +  Writes Back a range of data cache lines covering a set of capsules in
> memory.
> > +
> > +  Writes Back the data cache lines specified by ScatterGatherList.
> > +
> > +  @param  ScatterGatherList Physical address of the data structure that
> > +                            describes a set of capsules in memory
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +CapsuleCacheWriteBack (
> > +  IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
> > +  )
> > +{
> > +}
> > diff --git
> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> > index 9ab04ce1b301..43a29ee22948 100644
> > --- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> > +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> > @@ -27,17 +27,24 @@ [Defines]
> >  #
> >  # The following information is for reference only and not required by the
> build tools.
> >  #
> > -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> > +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC ARM AARCH64
> >  #
> >
> >  [Sources]
> >    CapsuleService.c
> > +  CapsuleService.h
> >
> > -[Sources.Ia32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
> > +[Sources.Ia32, Sources.IPF, Sources.EBC]
> >    SaveLongModeContext.c
> > +  CapsuleReset.c
> >
> >  [Sources.X64]
> >    X64/SaveLongModeContext.c
> > +  CapsuleReset.c
> > +
> > +[Sources.ARM, Sources.AARCH64]
> > +  SaveLongModeContext.c
> > +  Arm/CapsuleReset.c
> >
> >  [Packages]
> >    MdePkg/MdePkg.dec
> > @@ -59,6 +66,9 @@ [LibraryClasses.X64]
> >    UefiLib
> >    BaseMemoryLib
> >
> > +[LibraryClasses.ARM, LibraryClasses.AARCH64]
> > +  CacheMaintenanceLib
> > +
> >  [Guids]
> >    ## SOMETIMES_PRODUCES   ## Variable:L"CapsuleUpdateData" #
> (Process across reset capsule image) for capsule updated data
> >    ## SOMETIMES_PRODUCES   ## Variable:L"CapsuleLongModeBuffer" #
> The long mode buffer used by IA32 Capsule PEIM to call X64 CapsuleCoalesce
> code to handle >4GB capsule blocks
> > diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
> > index 216798d1617e..23fd6d59c59e 100644
> > --- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
> > +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
> > @@ -15,22 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> >
> >  **/
> >
> > -#include <Uefi.h>
> > -
> > -#include <Protocol/Capsule.h>
> > -#include <Guid/CapsuleVendor.h>
> > -#include <Guid/FmpCapsule.h>
> > -
> > -#include <Library/DebugLib.h>
> > -#include <Library/PcdLib.h>
> > -#include <Library/CapsuleLib.h>
> > -#include <Library/UefiDriverEntryPoint.h> -#include
> <Library/UefiBootServicesTableLib.h>
> > -#include <Library/UefiRuntimeServicesTableLib.h>
> > -#include <Library/UefiRuntimeLib.h>
> > -#include <Library/BaseLib.h>
> > -#include <Library/PrintLib.h>
> > -#include <Library/BaseMemoryLib.h>
> > +#include "CapsuleService.h"
> > +
> >  //
> >  // Handle for the installation of Capsule Architecture Protocol.
> >  //
> > @@ -44,15 +30,6 @@ UINTN       mTimes      = 0;
> >  UINT32      mMaxSizePopulateCapsule     = 0;
> >  UINT32      mMaxSizeNonPopulateCapsule  = 0;
> >
> > -/**
> > -  Create the variable to save the base address of page table and stack
> > -  for transferring into long mode in IA32 PEI.
> > -**/
> > -VOID
> > -SaveLongModeContext (
> > -  VOID
> > -  );
> > -
> >  /**
> >    Passes capsules to the firmware with both virtual and physical mapping.
> Depending on the intended
> >    consumption, the firmware may process the capsule immediately. If the
> payload should persist @@ -194,10 +171,12 @@ UpdateCapsule (
> >    //
> >    // Check if the platform supports update capsule across a system reset
> >    //
> > -  if (!FeaturePcdGet(PcdSupportUpdateCapsuleReset)) {
> > +  if (!IsPersistAcrossResetCapsuleSupported ()) {
> >      return EFI_UNSUPPORTED;
> >    }
> >
> > +  CapsuleCacheWriteBack (ScatterGatherList);
> > +
> >    //
> >    // Construct variable name CapsuleUpdateData, CapsuleUpdateData1,
> CapsuleUpdateData2...
> >    // if user calls UpdateCapsule multiple times.
> > @@ -344,7 +323,7 @@ QueryCapsuleCapabilities (
> >      //
> >      //Check if the platform supports update capsule across a system reset
> >      //
> > -    if (!FeaturePcdGet(PcdSupportUpdateCapsuleReset)) {
> > +    if (!IsPersistAcrossResetCapsuleSupported ()) {
> >        return EFI_UNSUPPORTED;
> >      }
> >      *ResetType = EfiResetWarm;
> > diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h
> > new file mode 100644
> > index 000000000000..85aafc144b41
> > --- /dev/null
> > +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h
> > @@ -0,0 +1,73 @@
> > +/** @file
> > +  Capsule Runtime Driver produces two UEFI capsule runtime services.
> > +  (UpdateCapsule, QueryCapsuleCapabilities)
> > +  It installs the Capsule Architectural Protocol defined in PI1.0a to
> > +signify
> > +  the capsule runtime services are ready.
> > +
> > +  Copyright (c) 2006 - 2017, Intel Corporation. All rights
> > + reserved.<BR>  Copyright (c) 2018, Linaro, Ltd. All rights
> > + reserved.<BR>
> > +
> > +  This program and the accompanying materials are licensed and made
> > + available  under the terms and conditions of the BSD License which
> > + accompanies this  distribution.  The full text of the license may be
> > + found at  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include <Uefi.h>
> > +
> > +#include <Protocol/Capsule.h>
> > +#include <Guid/CapsuleVendor.h>
> > +#include <Guid/FmpCapsule.h>
> > +
> > +#include <Library/DebugLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/CapsuleLib.h>
> > +#include <Library/UefiDriverEntryPoint.h> #include
> > +<Library/UefiBootServicesTableLib.h>
> > +#include <Library/UefiRuntimeServicesTableLib.h>
> > +#include <Library/UefiRuntimeLib.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/PrintLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +
> > +/**
> > +  Create the variable to save the base address of page table and stack
> > +  for transferring into long mode in IA32 PEI.
> > +**/
> > +VOID
> > +SaveLongModeContext (
> > +  VOID
> > +  );
> > +
> > +/**
> > +  Whether the platform supports capsules that persist across reset.
> > +Note that
> > +  some platforms only support such capsules at boot time.
> > +
> > +  @return TRUE  if a PersistAcrossReset capsule may be passed to
> UpdateCapsule()
> > +                at this time
> > +          FALSE otherwise
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +IsPersistAcrossResetCapsuleSupported (
> > +  VOID
> > +  );
> > +
> > +/**
> > +  Writes Back a range of data cache lines covering a set of capsules in
> memory.
> > +
> > +  Writes Back the data cache lines specified by ScatterGatherList.
> > +
> > +  @param  ScatterGatherList Physical address of the data structure that
> > +                            describes a set of capsules in memory
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +CapsuleCacheWriteBack (
> > +  IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
> > +  );
> > --
> > 2.17.1
> >