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

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

Yes, I agree. Only runtime.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, June 12, 2018 8:25 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Zeng, Star
> <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the
> capsule payload to DRAM
> 
> On 12 June 2018 at 17:23, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > Ard
> > Do you think we also need update QueryCapsuleCapabilities() to return
> UNSUPPORTED for CAPSULE_FLAGS_PERSIST_ACROSS_RESET?
> >
> 
> Yes, but only at runtime. I can update the patch if you like.
> 
> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: Tuesday, June 12, 2018 4:23 AM
> >> 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 v3 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/CacheMaintenance.c |
> 70
> >> ++++++++++++++++++++
> >>  MdeModulePkg/Universal/CapsuleRuntimeDxe/CacheMaintenance.c     |
> 39
> >> +++++++++++
> >>  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf  |
> 13
> >> +++-
> >>  MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c       |
> 24
> >> +++++++
> >>  4 files changed, 144 insertions(+), 2 deletions(-)
> >>
> >> diff --git
> >> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CacheMaintenance.c
> >> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CacheMaintenance.c
> >> new file mode 100644
> >> index 000000000000..dc05e345fb8d
> >> --- /dev/null
> >> +++
> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CacheMaintenance.c
> >> @@ -0,0 +1,70 @@
> >> + /** @file
> >> +  Capsule cache maintenance as is required on ARM and AARCH64
> >> +
> >> +  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 <Library/CacheMaintenanceLib.h>
> >> +#include <Library/UefiRuntimeLib.h>
> >> +
> >> +/**
> >> +  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
> >> +
> >> +  @return EFI_SUCCESS       if the operation succeeded.
> >> +          EFI_UNSUPPORTED   if cache maintenance cannot be
> performed
> >> at this
> >> +                            time.
> >> +
> >> +**/
> >> +EFI_STATUS
> >> +EFIAPI
> >> +CapsuleCacheWriteBack (
> >> +  IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
> >> +  )
> >> +{
> >> +  EFI_CAPSULE_BLOCK_DESCRIPTOR    *Desc;
> >> +
> >> +  //
> >> +  // 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.
> >> +  //
> >> +  if (EfiAtRuntime ()) {
> >> +    return EFI_UNSUPPORTED;
> >> +  }
> >> +
> >> +  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);
> >> +
> >> +  return EFI_SUCCESS;
> >> +}
> >> diff --git
> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CacheMaintenance.c
> >> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CacheMaintenance.c
> >> new file mode 100644
> >> index 000000000000..fb7504bb3e1d
> >> --- /dev/null
> >> +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CacheMaintenance.c
> >> @@ -0,0 +1,39 @@
> >> +/** @file
> >> +  Create NULL function for capsule cache maintenance which is only needed
> >> +  on ARM and AARCH64
> >> +
> >> +  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>
> >> +
> >> +/**
> >> +  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
> >> +
> >> +  @return EFI_SUCCESS       if the operation succeeded.
> >> +          EFI_UNSUPPORTED   if cache maintenance cannot be
> performed
> >> at this
> >> +                            time.
> >> +
> >> +**/
> >> +EFI_STATUS
> >> +EFIAPI
> >> +CapsuleCacheWriteBack (
> >> +  IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
> >> +  )
> >> +{
> >> +  return EFI_SUCCESS;
> >> +}
> >> diff --git
> >> a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> >> b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> >> index 9ab04ce1b301..3ceebc5d9646 100644
> >> --- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> >> +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> >> @@ -27,17 +27,23 @@ [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
> >>
> >> -[Sources.Ia32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
> >> +[Sources.Ia32, Sources.IPF, Sources.EBC]
> >>    SaveLongModeContext.c
> >> +  CacheMaintenance.c
> >>
> >>  [Sources.X64]
> >>    X64/SaveLongModeContext.c
> >> +  CacheMaintenance.c
> >> +
> >> +[Sources.ARM, Sources.AARCH64]
> >> +  SaveLongModeContext.c
> >> +  Arm/CacheMaintenance.c
> >>
> >>  [Packages]
> >>    MdePkg/MdePkg.dec
> >> @@ -59,6 +65,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..ee8515adf62f 100644
> >> --- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
> >> +++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
> >> @@ -53,6 +53,25 @@ SaveLongModeContext (
> >>    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
> >> +
> >> +  @return EFI_SUCCESS       if the operation succeeded.
> >> +          EFI_UNSUPPORTED   if cache maintenance cannot be
> performed
> >> at this
> >> +                            time.
> >> +
> >> +**/
> >> +EFI_STATUS
> >> +EFIAPI
> >> +CapsuleCacheWriteBack (
> >> +  IN  EFI_PHYSICAL_ADDRESS    ScatterGatherList
> >> +  );
> >> +
> >>  /**
> >>    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
> >> @@ -214,6 +233,11 @@ UpdateCapsule (
> >>        );
> >>    }
> >>
> >> +  Status = CapsuleCacheWriteBack (ScatterGatherList);
> >> +  if (EFI_ERROR (Status)) {
> >> +    return Status;
> >> +  }
> >> +
> >>    //
> >>    // ScatterGatherList is only referenced if the capsules are defined to
> persist
> >> across
> >>    // system reset. Set its value into NV storage to let pre-boot driver to pick
> it
> >> up
> >> --
> >> 2.17.1
> >