From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::231; helo=mail-io0-x231.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x231.google.com (mail-io0-x231.google.com [IPv6:2607:f8b0:4001:c06::231]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id CDB81211E19DD for ; Tue, 12 Jun 2018 08:24:45 -0700 (PDT) Received: by mail-io0-x231.google.com with SMTP id l19-v6so28518644ioj.5 for ; Tue, 12 Jun 2018 08:24:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=oxwVbY5hgrVAeK/YBSjqPyB4MSr0YLD+ciZDWyJOnr0=; b=RDcL9JXZZuxQrl+7g5flQK4fRnYoAPpTSLwUcxf4Tdc+wxFqSpexDUVMK6aAVC0CNk aSJLxrrhkGSoXPgdeHEfiXaAczKxCeNPY5jMnbmv1bCB2AzQgLCGShzK93k+A5E8nzTk cqYERgteA8TNS32taRWD6Wvy9pPtPd+sQHydo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=oxwVbY5hgrVAeK/YBSjqPyB4MSr0YLD+ciZDWyJOnr0=; b=rLGYs4IpEgf0nkxdoCTAocj0P3ufqISH4wAAC/G04FixvtrC1qzFBwR2I2UT/6cXuN KIG6GkstOyWRf9sTqM2PZN4hv2Gr4JDrOsafmh85AT+EbkALouLuaX8Jtg4EYr0JjTxu UJ5BfnWfS4shjX0TJQ+TA5ZjIeJNq4C/iC/p0sQ2ScLWGUcG+RxxRxx4R+Ao0oI3JzKL B+uMHhp6IG6Me574cedLQJPPofRBnuYoSVXxWDteihrHJQ8QoKg4HcopQgElqnsjuF/Y khtkHIdoOIILyyyJdC5OqdZqyJHiECIDKE8/zZ861eZaK8dkEZLFj8inypDxFgAG8A/N jbaA== X-Gm-Message-State: APt69E2a6IpATqpNMXiB2n1UmoWcpVG6AZ1ZVjYXm8ChYMFjuWSNzTC+ AgHJ/4Jolzza+efuhRjvFP0iQPe0vWCy/KaoQem5lQ== X-Google-Smtp-Source: ADUXVKLhW4MjU5esNG80D0duvj0ukDkPS4HgO5CYmgDcgwgHGns1bf2kI64QrDVO+bHgVJidpxwe4TzdQ8Q6XkCZiXk= X-Received: by 2002:a6b:520d:: with SMTP id g13-v6mr1010218iob.60.1528817085038; Tue, 12 Jun 2018 08:24:45 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Tue, 12 Jun 2018 08:24:44 -0700 (PDT) In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AC40191@shsmsx102.ccr.corp.intel.com> References: <20180612112329.664-1-ard.biesheuvel@linaro.org> <20180612112329.664-2-ard.biesheuvel@linaro.org> <74D8A39837DF1E4DA445A8C0B3885C503AC40191@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Tue, 12 Jun 2018 17:24:44 +0200 Message-ID: To: "Yao, Jiewen" Cc: "edk2-devel@lists.01.org" , "leif.lindholm@linaro.org" , "Zeng, Star" , "Kinney, Michael D" Subject: Re: [PATCH v3 1/4] MdeModulePkg/CapsuleRuntimeDxe: clean the capsule payload to DRAM X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jun 2018 15:24:46 -0000 Content-Type: text/plain; charset="UTF-8" On 12 June 2018 at 17:23, Yao, Jiewen 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 ; Yao, Jiewen >> ; Kinney, Michael D ; Ard >> Biesheuvel >> 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 >> --- >> 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.
>> + >> + 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 >> + >> +#include >> +#include >> + >> +/** >> + 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.
>> + >> + 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 >> + >> +/** >> + 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 >