public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Yao, Jiewen" <jiewen.yao@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	 "leif.lindholm@linaro.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
Date: Tue, 12 Jun 2018 17:24:44 +0200	[thread overview]
Message-ID: <CAKv+Gu-DVNEVscfeMHEqpckyhb+GiPVzHAwBp9ZPut+VqJ2=9g@mail.gmail.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AC40191@shsmsx102.ccr.corp.intel.com>

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
>


  reply	other threads:[~2018-06-12 15:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-12 11:23 [PATCH v3 0/4] MdeModulePkg ArmPkg: support for persistent capsules and progress reporting Ard Biesheuvel
2018-06-12 11:23 ` [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM Ard Biesheuvel
2018-06-12 15:23   ` Yao, Jiewen
2018-06-12 15:24     ` Ard Biesheuvel [this message]
2018-06-12 16:27       ` Yao, Jiewen
2018-06-12 11:23 ` [PATCH v3 2/4] MdeModulePkg/DxeCapsuleLibFmp: pass progress callback only if it works Ard Biesheuvel
2018-06-12 11:23 ` [PATCH v3 3/4] ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once Ard Biesheuvel
2018-06-12 12:25   ` Leif Lindholm
2018-06-12 12:26     ` Ard Biesheuvel
2018-06-12 11:23 ` [PATCH v3 4/4] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu-DVNEVscfeMHEqpckyhb+GiPVzHAwBp9ZPut+VqJ2=9g@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox