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

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

Ard
Do you think we also need update QueryCapsuleCapabilities() to return UNSUPPORTED for CAPSULE_FLAGS_PERSIST_ACROSS_RESET?

Thank you
Yao Jiewen

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