public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/4] MdeModulePkg ArmPkg: support for persistent capsules and progress reporting
@ 2018-06-12 11:23 Ard Biesheuvel
  2018-06-12 11:23 ` [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM Ard Biesheuvel
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-12 11:23 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, star.zeng, jiewen.yao, michael.d.kinney,
	Ard Biesheuvel

This is the delta of code required to implement PersistAcrossReset on ARM
systems, and to wire up the capsule handling routines in a way that makes
the new progress reporting code do something meaningful on such platforms.

Changes since v2:
- move cache handling from CapsulePei to CapsuleRuntimeDxe, and make it ARM only
- drop patch to change ProcessCapsules() logic in DxeCapsuleLibFmp; instead,
  the platform BDS code is modified to perform the ProcessCapsuleImage()
  call directly

Changes since v1:
- incorporate Star's feedback (#1, #2)
- add Leif's ack (#4)

Patch #1 ensures that the capsule data which is preserved in DRAM across
a reboot is written back to main memory before attempting to access it
with the caches off.

Patch #2 updates DxeCapsuleLibFmp so it does not pass down the progress
indication callback if its own attempt to invoke it has already failed.

Patch #3 updates ArmPkg's generic PlatformBootManagerLib implementation
to only call ProcessCapsules() after the [potentially non-trusted]
console is up and running, to ensure that firmware update progress can
be reported to the user.

Patch #4 modifies ArmSmcPsciResetSystemLib to emulate a proper warm reboot
by reentering PEI with interrupts, MMU and caches enabled. This works
around the lack of an architected warm reboot in most current implementations.
(The PSCI spec does cover warm reboot, but it was added recently and most
secure firmware implementations haven't caught up yet)

Ard Biesheuvel (4):
  MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM
  MdeModulePkg/DxeCapsuleLibFmp: pass progress callback only if it works
  ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once
  ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot

 ArmPkg/ArmPkg.dec                             |  4 +
 .../ArmSmcPsciResetSystemLib.c                | 21 ++++-
 .../ArmSmcPsciResetSystemLib.inf              |  9 ++
 .../PlatformBootManagerLib/PlatformBm.c       | 87 +++++++++++++------
 .../PlatformBootManagerLib.inf                |  1 +
 .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 13 ++-
 .../CapsuleRuntimeDxe/Arm/CacheMaintenance.c  | 70 +++++++++++++++
 .../CapsuleRuntimeDxe/CacheMaintenance.c      | 39 +++++++++
 .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf   | 13 ++-
 .../CapsuleRuntimeDxe/CapsuleService.c        | 24 +++++
 10 files changed, 247 insertions(+), 34 deletions(-)
 create mode 100644 MdeModulePkg/Universal/CapsuleRuntimeDxe/Arm/CacheMaintenance.c
 create mode 100644 MdeModulePkg/Universal/CapsuleRuntimeDxe/CacheMaintenance.c

-- 
2.17.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM
  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 ` Ard Biesheuvel
  2018-06-12 15:23   ` Yao, Jiewen
  2018-06-12 11:23 ` [PATCH v3 2/4] MdeModulePkg/DxeCapsuleLibFmp: pass progress callback only if it works Ard Biesheuvel
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-12 11:23 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, star.zeng, jiewen.yao, michael.d.kinney,
	Ard Biesheuvel

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/4] MdeModulePkg/DxeCapsuleLibFmp: pass progress callback only if it works
  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 11:23 ` Ard Biesheuvel
  2018-06-12 11:23 ` [PATCH v3 3/4] ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once Ard Biesheuvel
  2018-06-12 11:23 ` [PATCH v3 4/4] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot Ard Biesheuvel
  3 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-12 11:23 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, star.zeng, jiewen.yao, michael.d.kinney,
	Ard Biesheuvel

If the first call to UpdateImageProgress() fails, there is no point
in passing a pointer to it to Fmp->SetImage(), since it is highly
unlikely to succeed on any subsequent calls.

This permits the FMP implementation to fall back to an alternate means
of providing feedback to the user, e.g., via the console.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index f0226eafa576..ab41df0eb0a4 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -841,6 +841,7 @@ SetFmpImageData (
   UINT8                                         *Image;
   VOID                                          *VendorCode;
   CHAR16                                        *AbortReason;
+  EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS ProgressCallback;
 
   Status = gBS->HandleProtocol(
                   Handle,
@@ -892,7 +893,11 @@ SetFmpImageData (
   //
   // Before calling SetImage(), reset the progress bar to 0%
   //
-  UpdateImageProgress (0);
+  ProgressCallback = UpdateImageProgress;
+  Status = UpdateImageProgress (0);
+  if (EFI_ERROR (Status)) {
+    ProgressCallback = NULL;
+  }
 
   Status = Fmp->SetImage(
                   Fmp,
@@ -900,13 +905,15 @@ SetFmpImageData (
                   Image,                                  // Image
                   ImageHeader->UpdateImageSize,           // ImageSize
                   VendorCode,                             // VendorCode
-                  UpdateImageProgress,                    // Progress
+                  ProgressCallback,                       // Progress
                   &AbortReason                            // AbortReason
                   );
   //
   // Set the progress bar to 100% after returning from SetImage()
   //
-  UpdateImageProgress (100);
+  if (ProgressCallback != NULL) {
+    UpdateImageProgress (100);
+  }
 
   DEBUG((DEBUG_INFO, "Fmp->SetImage - %r\n", Status));
   if (AbortReason != NULL) {
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/4] ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once
  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 11:23 ` [PATCH v3 2/4] MdeModulePkg/DxeCapsuleLibFmp: pass progress callback only if it works Ard Biesheuvel
@ 2018-06-12 11:23 ` Ard Biesheuvel
  2018-06-12 12:25   ` Leif Lindholm
  2018-06-12 11:23 ` [PATCH v3 4/4] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot Ard Biesheuvel
  3 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-12 11:23 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, star.zeng, jiewen.yao, michael.d.kinney,
	Ard Biesheuvel

ARM platforms have no restriction on when a system firmware update
capsule can be applied, and so it is not necessary to call
ProcessCapsules() twice. So let's drop the first invocation that
occurs before EndOfDxe, and rewrite the second call so that all
capsule updates will be applied when the console is up and able to
provide progress feedback.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 87 ++++++++++++++------
 ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 +
 2 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 3456a71fbb9c..7c21cce5960b 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -24,6 +24,7 @@
 #include <Library/PcdLib.h>
 #include <Library/UefiBootManagerLib.h>
 #include <Library/UefiLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
 #include <Protocol/DevicePath.h>
 #include <Protocol/EsrtManagement.h>
 #include <Protocol/GraphicsOutput.h>
@@ -553,21 +554,6 @@ PlatformBootManagerBeforeConsole (
   VOID
   )
 {
-  EFI_STATUS                    Status;
-  ESRT_MANAGEMENT_PROTOCOL      *EsrtManagement;
-
-  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
-    DEBUG ((DEBUG_INFO, "ProcessCapsules Before EndOfDxe ......\n"));
-    Status = ProcessCapsules ();
-    DEBUG ((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
-  } else {
-    Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
-                    (VOID **)&EsrtManagement);
-    if (!EFI_ERROR (Status)) {
-      EsrtManagement->SyncEsrtFmp ();
-    }
-  }
-
   //
   // Signal EndOfDxe PI Event
   //
@@ -618,6 +604,57 @@ PlatformBootManagerBeforeConsole (
   PlatformRegisterOptionsAndKeys ();
 }
 
+STATIC
+VOID
+HandleCapsules (
+  VOID
+  )
+{
+  ESRT_MANAGEMENT_PROTOCOL    *EsrtManagement;
+  EFI_PEI_HOB_POINTERS        HobPointer;
+  EFI_CAPSULE_HEADER          *CapsuleHeader;
+  BOOLEAN                     NeedReset;
+  EFI_STATUS                  Status;
+
+  DEBUG ((DEBUG_INFO, "%a: processing capsules ...\n", __FUNCTION__));
+
+  Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
+                  (VOID **)&EsrtManagement);
+  if (!EFI_ERROR (Status)) {
+    EsrtManagement->SyncEsrtFmp ();
+  }
+
+  //
+  // Find all capsule images from hob
+  //
+  HobPointer.Raw = GetHobList ();
+  NeedReset = FALSE;
+  while ((HobPointer.Raw = GetNextHob (EFI_HOB_TYPE_UEFI_CAPSULE,
+                             HobPointer.Raw)) != NULL) {
+    CapsuleHeader = (VOID *)(UINTN)HobPointer.Capsule->BaseAddress;
+
+    Status = ProcessCapsuleImage (CapsuleHeader);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: failed to process capsule %p - %r\n",
+        __FUNCTION__, CapsuleHeader, Status));
+      return;
+    }
+    if ((CapsuleHeader->Flags & CAPSULE_FLAGS_INITIATE_RESET) != 0) {
+      NeedReset = TRUE;
+    }
+    HobPointer.Raw = GET_NEXT_HOB (HobPointer);
+  }
+
+  if (NeedReset) {
+      DEBUG ((DEBUG_WARN, "%a: capsule update successful, resetting ...\n",
+        __FUNCTION__));
+
+      gRT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL);
+      CpuDeadLoop();
+  }
+}
+
+
 #define VERSION_STRING_PREFIX    L"Tianocore/EDK2 firmware version "
 
 /**
@@ -637,7 +674,6 @@ PlatformBootManagerAfterConsole (
   VOID
   )
 {
-  ESRT_MANAGEMENT_PROTOCOL      *EsrtManagement;
   EFI_STATUS                    Status;
   EFI_GRAPHICS_OUTPUT_PROTOCOL  *GraphicsOutput;
   UINTN                         FirmwareVerLength;
@@ -675,17 +711,14 @@ PlatformBootManagerAfterConsole (
   //
   EfiBootManagerConnectAll ();
 
-  Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
-                  (VOID **)&EsrtManagement);
-  if (!EFI_ERROR (Status)) {
-    EsrtManagement->SyncEsrtFmp ();
-  }
-
-  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
-    DEBUG((DEBUG_INFO, "ProcessCapsules After EndOfDxe ......\n"));
-    Status = ProcessCapsules ();
-    DEBUG((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
-  }
+  //
+  // On ARM, there is currently no reason to use the phased capsule
+  // update approach where some capsules are dispatched before EndOfDxe
+  // and some are dispatched after. So just handle all capsules here,
+  // when the console is up and we can actually give the user some
+  // feedback about what is going on.
+  //
+  HandleCapsules ();
 
   //
   // Enumerate all possible boot options.
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index e8cbb10dabdd..28d606d5c329 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -55,6 +55,7 @@ [LibraryClasses]
   UefiBootManagerLib
   UefiBootServicesTableLib
   UefiLib
+  UefiRuntimeServicesTableLib
 
 [FeaturePcd]
   gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 4/4] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot
  2018-06-12 11:23 [PATCH v3 0/4] MdeModulePkg ArmPkg: support for persistent capsules and progress reporting Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2018-06-12 11:23 ` [PATCH v3 3/4] ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once Ard Biesheuvel
@ 2018-06-12 11:23 ` Ard Biesheuvel
  3 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-12 11:23 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, star.zeng, jiewen.yao, michael.d.kinney,
	Ard Biesheuvel

Implement ResetSystemLib's EnterS3WithImmediateWake() routine using
a jump back to the PEI entry point with interrupts and MMU+caches
disabled. This is only possible at boot time, when we are sure that
the current CPU is the only one up and running. Also, it depends on
the platform whether the PEI code is preserved in memory (it may be
copied to DRAM rather than execute in place), so also add a feature
PCD to selectively enable this feature.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 ArmPkg/ArmPkg.dec                                                    |  4 ++++
 ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c   | 21 ++++++++++++++++++--
 ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf |  9 +++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index debe066b6f7b..3aa229fe2ec9 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -85,6 +85,10 @@ [PcdsFeatureFlag.common]
   # Define if the GICv3 controller should use the GICv2 legacy
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
 
+  # Whether to implement warm reboot for capsule update using a jump back to the
+  # PEI entry point with caches and interrupts disabled.
+  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|FALSE|BOOLEAN|0x0000001F
+
 [PcdsFeatureFlag.ARM]
   # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
   # TRUE may be appropriate to fix performance problems if you don't care about
diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
index d6d26bce5009..10ceafd14d5d 100644
--- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
+++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
@@ -15,10 +15,13 @@
 
 #include <PiDxe.h>
 
+#include <Library/ArmMmuLib.h>
+#include <Library/ArmSmcLib.h>
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 #include <Library/ResetSystemLib.h>
-#include <Library/ArmSmcLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeLib.h>
 
 #include <IndustryStandard/ArmStdSmc.h>
 
@@ -89,7 +92,21 @@ EnterS3WithImmediateWake (
   VOID
   )
 {
-  // Not implemented
+  VOID (*Reset)(VOID);
+
+  if (FeaturePcdGet (PcdArmReenterPeiForCapsuleWarmReboot) &&
+      !EfiAtRuntime ()) {
+    //
+    // At boot time, we are the only core running, so we can implement the
+    // immediate wake (which is used by capsule update) by disabling the MMU
+    // and interrupts, and jumping to the PEI entry point.
+    //
+    Reset = (VOID (*)(VOID))(UINTN)FixedPcdGet64 (PcdFvBaseAddress);
+
+    gBS->RaiseTPL (TPL_HIGH_LEVEL);
+    ArmDisableMmu ();
+    Reset ();
+  }
 }
 
 /**
diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
index 5a1ee976e5bc..19021cd1e8b6 100644
--- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
+++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
@@ -30,6 +30,15 @@ [Packages]
   MdePkg/MdePkg.dec
 
 [LibraryClasses]
+  ArmMmuLib
   ArmSmcLib
   BaseLib
   DebugLib
+  UefiBootServicesTableLib
+  UefiRuntimeLib
+
+[FeaturePcd]
+  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFvBaseAddress
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/4] ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2018-06-12 12:25 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, star.zeng, jiewen.yao, michael.d.kinney

On Tue, Jun 12, 2018 at 01:23:28PM +0200, Ard Biesheuvel wrote:
> ARM platforms have no restriction on when a system firmware update
> capsule can be applied, and so it is not necessary to call
> ProcessCapsules() twice. So let's drop the first invocation that
> occurs before EndOfDxe, and rewrite the second call so that all
> capsule updates will be applied when the console is up and able to
> provide progress feedback.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I thought I acked this one last time around? Perhaps you wanted it
again after the discussion. Anyway:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 87 ++++++++++++++------
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 +
>  2 files changed, 61 insertions(+), 27 deletions(-)
> 
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 3456a71fbb9c..7c21cce5960b 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -24,6 +24,7 @@
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootManagerLib.h>
>  #include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
>  #include <Protocol/DevicePath.h>
>  #include <Protocol/EsrtManagement.h>
>  #include <Protocol/GraphicsOutput.h>
> @@ -553,21 +554,6 @@ PlatformBootManagerBeforeConsole (
>    VOID
>    )
>  {
> -  EFI_STATUS                    Status;
> -  ESRT_MANAGEMENT_PROTOCOL      *EsrtManagement;
> -
> -  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
> -    DEBUG ((DEBUG_INFO, "ProcessCapsules Before EndOfDxe ......\n"));
> -    Status = ProcessCapsules ();
> -    DEBUG ((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
> -  } else {
> -    Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
> -                    (VOID **)&EsrtManagement);
> -    if (!EFI_ERROR (Status)) {
> -      EsrtManagement->SyncEsrtFmp ();
> -    }
> -  }
> -
>    //
>    // Signal EndOfDxe PI Event
>    //
> @@ -618,6 +604,57 @@ PlatformBootManagerBeforeConsole (
>    PlatformRegisterOptionsAndKeys ();
>  }
>  
> +STATIC
> +VOID
> +HandleCapsules (
> +  VOID
> +  )
> +{
> +  ESRT_MANAGEMENT_PROTOCOL    *EsrtManagement;
> +  EFI_PEI_HOB_POINTERS        HobPointer;
> +  EFI_CAPSULE_HEADER          *CapsuleHeader;
> +  BOOLEAN                     NeedReset;
> +  EFI_STATUS                  Status;
> +
> +  DEBUG ((DEBUG_INFO, "%a: processing capsules ...\n", __FUNCTION__));
> +
> +  Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
> +                  (VOID **)&EsrtManagement);
> +  if (!EFI_ERROR (Status)) {
> +    EsrtManagement->SyncEsrtFmp ();
> +  }
> +
> +  //
> +  // Find all capsule images from hob
> +  //
> +  HobPointer.Raw = GetHobList ();
> +  NeedReset = FALSE;
> +  while ((HobPointer.Raw = GetNextHob (EFI_HOB_TYPE_UEFI_CAPSULE,
> +                             HobPointer.Raw)) != NULL) {
> +    CapsuleHeader = (VOID *)(UINTN)HobPointer.Capsule->BaseAddress;
> +
> +    Status = ProcessCapsuleImage (CapsuleHeader);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "%a: failed to process capsule %p - %r\n",
> +        __FUNCTION__, CapsuleHeader, Status));
> +      return;
> +    }
> +    if ((CapsuleHeader->Flags & CAPSULE_FLAGS_INITIATE_RESET) != 0) {
> +      NeedReset = TRUE;
> +    }
> +    HobPointer.Raw = GET_NEXT_HOB (HobPointer);
> +  }
> +
> +  if (NeedReset) {
> +      DEBUG ((DEBUG_WARN, "%a: capsule update successful, resetting ...\n",
> +        __FUNCTION__));
> +
> +      gRT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL);
> +      CpuDeadLoop();
> +  }
> +}
> +
> +
>  #define VERSION_STRING_PREFIX    L"Tianocore/EDK2 firmware version "
>  
>  /**
> @@ -637,7 +674,6 @@ PlatformBootManagerAfterConsole (
>    VOID
>    )
>  {
> -  ESRT_MANAGEMENT_PROTOCOL      *EsrtManagement;
>    EFI_STATUS                    Status;
>    EFI_GRAPHICS_OUTPUT_PROTOCOL  *GraphicsOutput;
>    UINTN                         FirmwareVerLength;
> @@ -675,17 +711,14 @@ PlatformBootManagerAfterConsole (
>    //
>    EfiBootManagerConnectAll ();
>  
> -  Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
> -                  (VOID **)&EsrtManagement);
> -  if (!EFI_ERROR (Status)) {
> -    EsrtManagement->SyncEsrtFmp ();
> -  }
> -
> -  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
> -    DEBUG((DEBUG_INFO, "ProcessCapsules After EndOfDxe ......\n"));
> -    Status = ProcessCapsules ();
> -    DEBUG((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
> -  }
> +  //
> +  // On ARM, there is currently no reason to use the phased capsule
> +  // update approach where some capsules are dispatched before EndOfDxe
> +  // and some are dispatched after. So just handle all capsules here,
> +  // when the console is up and we can actually give the user some
> +  // feedback about what is going on.
> +  //
> +  HandleCapsules ();
>  
>    //
>    // Enumerate all possible boot options.
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index e8cbb10dabdd..28d606d5c329 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -55,6 +55,7 @@ [LibraryClasses]
>    UefiBootManagerLib
>    UefiBootServicesTableLib
>    UefiLib
> +  UefiRuntimeServicesTableLib
>  
>  [FeaturePcd]
>    gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/4] ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once
  2018-06-12 12:25   ` Leif Lindholm
@ 2018-06-12 12:26     ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-12 12:26 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: edk2-devel@lists.01.org, Zeng, Star, Yao, Jiewen,
	Kinney, Michael D

On 12 June 2018 at 14:25, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Jun 12, 2018 at 01:23:28PM +0200, Ard Biesheuvel wrote:
>> ARM platforms have no restriction on when a system firmware update
>> capsule can be applied, and so it is not necessary to call
>> ProcessCapsules() twice. So let's drop the first invocation that
>> occurs before EndOfDxe, and rewrite the second call so that all
>> capsule updates will be applied when the console is up and able to
>> provide progress feedback.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> I thought I acked this one last time around? Perhaps you wanted it
> again after the discussion.

It has been substantially modified.

> Anyway:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks.

>> ---
>>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 87 ++++++++++++++------
>>  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  1 +
>>  2 files changed, 61 insertions(+), 27 deletions(-)
>>
>> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
>> index 3456a71fbb9c..7c21cce5960b 100644
>> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
>> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
>> @@ -24,6 +24,7 @@
>>  #include <Library/PcdLib.h>
>>  #include <Library/UefiBootManagerLib.h>
>>  #include <Library/UefiLib.h>
>> +#include <Library/UefiRuntimeServicesTableLib.h>
>>  #include <Protocol/DevicePath.h>
>>  #include <Protocol/EsrtManagement.h>
>>  #include <Protocol/GraphicsOutput.h>
>> @@ -553,21 +554,6 @@ PlatformBootManagerBeforeConsole (
>>    VOID
>>    )
>>  {
>> -  EFI_STATUS                    Status;
>> -  ESRT_MANAGEMENT_PROTOCOL      *EsrtManagement;
>> -
>> -  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
>> -    DEBUG ((DEBUG_INFO, "ProcessCapsules Before EndOfDxe ......\n"));
>> -    Status = ProcessCapsules ();
>> -    DEBUG ((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
>> -  } else {
>> -    Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
>> -                    (VOID **)&EsrtManagement);
>> -    if (!EFI_ERROR (Status)) {
>> -      EsrtManagement->SyncEsrtFmp ();
>> -    }
>> -  }
>> -
>>    //
>>    // Signal EndOfDxe PI Event
>>    //
>> @@ -618,6 +604,57 @@ PlatformBootManagerBeforeConsole (
>>    PlatformRegisterOptionsAndKeys ();
>>  }
>>
>> +STATIC
>> +VOID
>> +HandleCapsules (
>> +  VOID
>> +  )
>> +{
>> +  ESRT_MANAGEMENT_PROTOCOL    *EsrtManagement;
>> +  EFI_PEI_HOB_POINTERS        HobPointer;
>> +  EFI_CAPSULE_HEADER          *CapsuleHeader;
>> +  BOOLEAN                     NeedReset;
>> +  EFI_STATUS                  Status;
>> +
>> +  DEBUG ((DEBUG_INFO, "%a: processing capsules ...\n", __FUNCTION__));
>> +
>> +  Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
>> +                  (VOID **)&EsrtManagement);
>> +  if (!EFI_ERROR (Status)) {
>> +    EsrtManagement->SyncEsrtFmp ();
>> +  }
>> +
>> +  //
>> +  // Find all capsule images from hob
>> +  //
>> +  HobPointer.Raw = GetHobList ();
>> +  NeedReset = FALSE;
>> +  while ((HobPointer.Raw = GetNextHob (EFI_HOB_TYPE_UEFI_CAPSULE,
>> +                             HobPointer.Raw)) != NULL) {
>> +    CapsuleHeader = (VOID *)(UINTN)HobPointer.Capsule->BaseAddress;
>> +
>> +    Status = ProcessCapsuleImage (CapsuleHeader);
>> +    if (EFI_ERROR (Status)) {
>> +      DEBUG ((DEBUG_ERROR, "%a: failed to process capsule %p - %r\n",
>> +        __FUNCTION__, CapsuleHeader, Status));
>> +      return;
>> +    }
>> +    if ((CapsuleHeader->Flags & CAPSULE_FLAGS_INITIATE_RESET) != 0) {
>> +      NeedReset = TRUE;
>> +    }
>> +    HobPointer.Raw = GET_NEXT_HOB (HobPointer);
>> +  }
>> +
>> +  if (NeedReset) {
>> +      DEBUG ((DEBUG_WARN, "%a: capsule update successful, resetting ...\n",
>> +        __FUNCTION__));
>> +
>> +      gRT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL);
>> +      CpuDeadLoop();
>> +  }
>> +}
>> +
>> +
>>  #define VERSION_STRING_PREFIX    L"Tianocore/EDK2 firmware version "
>>
>>  /**
>> @@ -637,7 +674,6 @@ PlatformBootManagerAfterConsole (
>>    VOID
>>    )
>>  {
>> -  ESRT_MANAGEMENT_PROTOCOL      *EsrtManagement;
>>    EFI_STATUS                    Status;
>>    EFI_GRAPHICS_OUTPUT_PROTOCOL  *GraphicsOutput;
>>    UINTN                         FirmwareVerLength;
>> @@ -675,17 +711,14 @@ PlatformBootManagerAfterConsole (
>>    //
>>    EfiBootManagerConnectAll ();
>>
>> -  Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL,
>> -                  (VOID **)&EsrtManagement);
>> -  if (!EFI_ERROR (Status)) {
>> -    EsrtManagement->SyncEsrtFmp ();
>> -  }
>> -
>> -  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
>> -    DEBUG((DEBUG_INFO, "ProcessCapsules After EndOfDxe ......\n"));
>> -    Status = ProcessCapsules ();
>> -    DEBUG((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
>> -  }
>> +  //
>> +  // On ARM, there is currently no reason to use the phased capsule
>> +  // update approach where some capsules are dispatched before EndOfDxe
>> +  // and some are dispatched after. So just handle all capsules here,
>> +  // when the console is up and we can actually give the user some
>> +  // feedback about what is going on.
>> +  //
>> +  HandleCapsules ();
>>
>>    //
>>    // Enumerate all possible boot options.
>> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>> index e8cbb10dabdd..28d606d5c329 100644
>> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>> @@ -55,6 +55,7 @@ [LibraryClasses]
>>    UefiBootManagerLib
>>    UefiBootServicesTableLib
>>    UefiLib
>> +  UefiRuntimeServicesTableLib
>>
>>  [FeaturePcd]
>>    gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport
>> --
>> 2.17.1
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Yao, Jiewen @ 2018-06-12 15:23 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org
  Cc: leif.lindholm@linaro.org, Zeng, Star, Kinney, Michael D

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM
  2018-06-12 15:23   ` Yao, Jiewen
@ 2018-06-12 15:24     ` Ard Biesheuvel
  2018-06-12 16:27       ` Yao, Jiewen
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-12 15:24 UTC (permalink / raw)
  To: Yao, Jiewen
  Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org, Zeng, Star,
	Kinney, Michael D

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
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM
  2018-06-12 15:24     ` Ard Biesheuvel
@ 2018-06-12 16:27       ` Yao, Jiewen
  0 siblings, 0 replies; 10+ messages in thread
From: Yao, Jiewen @ 2018-06-12 16:27 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org, Zeng, Star,
	Kinney, Michael D

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-06-12 16:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox