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:c0b::243; helo=mail-it0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x243.google.com (mail-it0-x243.google.com [IPv6:2607:f8b0:4001:c0b::243]) (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 6A3442119927C for ; Wed, 6 Jun 2018 23:00:03 -0700 (PDT) Received: by mail-it0-x243.google.com with SMTP id k17-v6so1303304ita.0 for ; Wed, 06 Jun 2018 23:00:03 -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:content-transfer-encoding; bh=B1yF8WEot4kBIIz+ndd0CoD3hw1/5oVRlBOED6Xzxx4=; b=FMOdNinIXi0d/3Bcc1sYjHjb3Ye10WJWvTcxMW1cBGBwrFDLvjiXjTxd6GNpULS5d1 BOMS4K0AOQ1StiiX4yCc7HcjfiKl5PW5ebPu7iZ1ffKtHPENjGnmXrsnW0BbNytkPfPr fRI8hvbiVGjLLVuAwCi+UorPJcl3VUJCYZSaU= 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:content-transfer-encoding; bh=B1yF8WEot4kBIIz+ndd0CoD3hw1/5oVRlBOED6Xzxx4=; b=PnMzOTbezyJzFHr2CqXf9Q4QInQt6Plp3jsB5peUe5M57co+NMKsxb348c0yrYYEbd pdwJft9ZWgtn71GL6txAC1ikuufrJin4OWIU5vuoMWB5SOiE3ftjLuCSXesKl1KKqHdS y47+Py11zTeEZmddhXJ89ogkLSV1CMg0TPrF1dHo3vF8NgWvFNg9EeUwAs3guU+A1lEu 88KYCYLCBMkSyhKCpOVByG3sYIV72h+IoZN+Au9mzxXuaqQFmzLaaZkoXSI/WV1wH5ad qs8WUCuAhEgpzs3gA/+9SPeV1MyQDV2TQ16roLOBtcZgwkH/z3VVuy4BOllM4gj7npcF SGvg== X-Gm-Message-State: APt69E2P56ybRjIu25wC2hYmAc+MsXsznlov9XP3Iq5dYnPNErjZ2Hju LIj+Ma0cTZpECgcp0zHZB4SpT6g3gKd8kZ8gjvwcpg== X-Google-Smtp-Source: ADUXVKKrAI4ALK9EMRpkPS0ddpqjjUHEtPnrAAak6QUc1W7rN6Wu8ZpXfFYVg+0n6xZ72Fk/yPRiF3p8KDMiUeWl5m4= X-Received: by 2002:a24:298e:: with SMTP id p136-v6mr562547itp.42.1528351202318; Wed, 06 Jun 2018 23:00:02 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Wed, 6 Jun 2018 23:00:01 -0700 (PDT) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BB54543@shsmsx102.ccr.corp.intel.com> References: <20180606095235.20822-1-ard.biesheuvel@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103BB542D7@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103BB54543@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Thu, 7 Jun 2018 08:00:01 +0200 Message-ID: To: "Zeng, Star" Cc: "Ni, Ruiyu" , "edk2-devel@lists.01.org" , Leif Lindholm , "Yao, Jiewen" , "Gao, Liming" , "Kinney, Michael D" Subject: Re: [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data 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: Thu, 07 Jun 2018 06:00:03 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On 7 June 2018 at 07:41, Zeng, Star wrote: > Thanks, got the point. > > It seems vague that who to ensure the cache coherency. MMU? Caller of Upd= ateCapsule? UpdateCapsule? Consumer of capsule data? > Unfortunately, since the spec does not mention it at all, we cannot rely on the caller of UpdateCapsule() to ensure this. That would require a spec change, and break backward compatibility with older revisions. The main issue here is that ARM does not provide the means to clean the entire cache, the only architectural method is clean cacheline by virtual address. So that leaves: - UpdateCapsule - problematic because it may be called at runtime and the firmware has no means of translating the physical addresses - ResetSystem - same as above: it is a runtime service, and so it cannot rely on a mapping to exist for those physical addresses - SEC - lacks the information about where the capsule resides - CapsulePei - already extracts the information about the capsule address in memory, and can perform the cache maintenance right before consuming the data. So unless anyone has any other suggestions, I think this approach is the only feasible one. If there are any concerns about adding this for architectures, I can look into refactoring CapsulePei and add it only for ARM/AARCH64. Thanks, Ard. > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ar= d Biesheuvel > Sent: Thursday, June 7, 2018 12:50 PM > To: Zeng, Star > Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Leif Lindhol= m ; Yao, Jiewen ; Gao, Limi= ng ; Kinney, Michael D > Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before = consuming capsule data > > On 7 June 2018 at 03:37, Zeng, Star wrote: >> Hi Ard, >> >> The input parameter CapsuleHeaderArray of UpdateCapsule has the virtual = address. >> > > It has the virtual address of the capsules yes. But how about the data st= ructures passed as the ScatterGatherList? > > >> CapsuleHeaderArray >> Virtual pointer to an array of virtual pointers to the capsules being >> passed into update capsule. Each capsules is assumed to stored in >> contiguous virtual memory. The capsules in the CapsuleHeaderArray must >> be the same capsules as the ScatterGatherList. The CapsuleHeaderArray >> must have the >> >> >> Thanks, >> Star >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> Ard Biesheuvel >> Sent: Wednesday, June 6, 2018 8:10 PM >> To: edk2-devel@lists.01.org >> Cc: Ni, Ruiyu ; Ard Biesheuvel >> ; Gao, Liming ; Yao, >> Jiewen ; Leif Lindholm >> ; Kinney, Michael D >> ; Zeng, Star >> Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache >> before consuming capsule data >> >> On 6 June 2018 at 11:52, Ard Biesheuvel wrot= e: >>> 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 >>> may read stale data if we don't clean the caches to memory first. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ard Biesheuvel >> >> Leif asked me to include a note why this cannot be done when >> UpdateCapsule() is called. >> >> >> """ >> Note that this cache maintenance cannot be done during the invocation of= UpdateCapsule(), since the ScatterGatherList structures are only identifie= d by physical address, and at runtime, the firmware doesn't know whether an= d where this memory is mapped, and cache maintenance requires a virtual add= ress. >> """ >> >>> --- >>> MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 1 + >>> MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 4 ++++ >>> 2 files changed, 5 insertions(+) >>> >>> diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf >>> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf >>> index c54bc21a95a8..594e110d1f8a 100644 >>> --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf >>> +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf >>> @@ -48,6 +48,7 @@ [Packages] >>> >>> [LibraryClasses] >>> BaseLib >>> + CacheMaintenanceLib >>> HobLib >>> BaseMemoryLib >>> PeiServicesLib >>> diff --git >>> a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c >>> b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c >>> index 3e7054cd38a9..1730f925adc5 100644 >>> --- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c >>> +++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c >>> @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EI= THER EXPRESS OR IMPLIED. >>> #include >>> >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -283,6 +284,9 @@ ValidateCapsuleByMemoryResource ( >>> DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in MemoryResourc= e[0x%x] - Start(0x%lx) Length(0x%lx)\n", >>> Address, Size, >>> Index, >>> MemoryResource[Index].PhysicalStart, >>> MemoryResource[Index].ResourceLength)); >>> + >>> + WriteBackDataCacheRange ((VOID *)(UINTN)Address, Size); >>> + >>> return TRUE; >>> } >>> } >>> -- >>> 2.17.0 >>> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel