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::244; helo=mail-io0-x244.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x244.google.com (mail-io0-x244.google.com [IPv6:2607:f8b0:4001:c06::244]) (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 90893210DF5EE for ; Thu, 7 Jun 2018 23:06:34 -0700 (PDT) Received: by mail-io0-x244.google.com with SMTP id t5-v6so14626588ioa.8 for ; Thu, 07 Jun 2018 23:06:34 -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=qS9MKkmN4V4DPEWfo+vghC7C8iOiUpB9Colgwza0aMQ=; b=HeUZIe16E1D+GXnlQp5I655TSAmnkwqu6/dek1D1H3MdDEx4wmdKzvTRiFLaRMfzsS Ed0nkZGzOMRBNQNF0+vKnIZwlt0QW5W3NRfPGtTD7qsw5mf7zM/6awnMrQ9Mz/RcSPCI vMAb81y+a95LuveQ48Kkv5jcwHHIY11jOMYjA= 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=qS9MKkmN4V4DPEWfo+vghC7C8iOiUpB9Colgwza0aMQ=; b=t7qZBupZbW6zltxbFG54WFuDD1Bkxr+8mmIcdOpFPzOl6GWZT1I3iMbsnEW9hAWc6d 1/vcq9oXba9i4JLa4pRC4p1ue/I7y9P8VJJ1QKbFfc0eZkaSAZGckYe4o+YHIB+iadgT qTv8v6b20+1Z1lJtxYmUk0UZIAXBPtWI2a0wt8sn6ua/pCHhjq8lHJrcuzrFYv1JdAC6 /fgpNKOlRuQB38x0/VioP3p1eGRbQ5D0xqj5zL+ADgsPlEp0d0GRz4KWPMBVxrofyp7c 8a6Xilpv/tI7//N09DXmsQB+lMhzavk/QGQMkRB2ufXbdN0+NbYMMDj7igkiuYRTwFYs f9hA== X-Gm-Message-State: APt69E2DLVRvSt5UvpLJ7JZzpro8oCu2qDHs5umI0Ek+iH+y0mg0eVMI QKQt4PqqR9jF0WCjgchkEzK0xFjD7FejFxz8dGD/Kw== X-Google-Smtp-Source: ADUXVKImV2y3lUkzf7Ep6Ni80bJh6XWBtEjJqiQQBl4SCy0UOUusoVjroRb8YywB91ShKip9eds9OxiyucewBW0ee1o= X-Received: by 2002:a6b:32d4:: with SMTP id y203-v6mr4377176ioy.107.1528437993194; Thu, 07 Jun 2018 23:06:33 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Thu, 7 Jun 2018 23:06:32 -0700 (PDT) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BB54B3F@shsmsx102.ccr.corp.intel.com> References: <20180607110812.26778-1-ard.biesheuvel@linaro.org> <20180607110812.26778-2-ard.biesheuvel@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103BB54B3F@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 8 Jun 2018 08:06:32 +0200 Message-ID: To: "Zeng, Star" Cc: "edk2-devel@lists.01.org" , "leif.lindholm@linaro.org" , "Kinney, Michael D" , "Yao, Jiewen" Subject: Re: [PATCH 1/5] 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: Fri, 08 Jun 2018 06:06:34 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On 8 June 2018 at 04:53, Zeng, Star wrote: > I suggest to use goto/adjust code to have one place for both paths to per= form cache maintenance (with comments). > Something like this? @@ -253,6 +254,7 @@ ValidateCapsuleByMemoryResource ( ) { UINTN Index; + BOOLEAN Found; // // Sanity Check @@ -274,19 +276,32 @@ ValidateCapsuleByMemoryResource ( // // No memory resource descriptor reported in HOB list before capsule Coalesce. // - return TRUE; + Found =3D TRUE; + } else { + Found =3D FALSE; } - for (Index =3D 0; MemoryResource[Index].ResourceLength !=3D 0; Index++) = { + for (Index =3D 0; !Found && MemoryResource[Index].ResourceLength !=3D 0; Index++) { if ((Address >=3D MemoryResource[Index].PhysicalStart) && ((Address + Size) <=3D (MemoryResource[Index].PhysicalStart + MemoryResource[Index].ResourceLength))) { DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n", Address, Size, Index, MemoryResource[Index].PhysicalStart, MemoryResource[Index].ResourceLength)); - return TRUE; + Found =3D TRUE; } } + if (Found) { + // + // At this point, we may still be running with the MMU and caches disa= bled, + // and on architectures such as ARM or AARCH64, capsule [meta]data loa= ded + // into memory with the caches on is only guaranteed to be visible to = the + // CPU running with the caches off after performing an explicit writeb= ack. + // + WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size); + return TRUE; + } + DEBUG ((EFI_D_ERROR, "ERROR: Address(0x%lx) Size(0x%lx) not in any MemoryResource\n", Address, Size)); return FALSE; } > > Thanks, > Star > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Thursday, June 7, 2018 7:08 PM > To: edk2-devel@lists.01.org > Cc: leif.lindholm@linaro.org; Kinney, Michael D ; Yao, Jiewen ; Zeng, Star ; = Ard Biesheuvel > Subject: [PATCH 1/5] MdeModulePkg/CapsulePei: clean Dcache before consumi= ng capsule data > > 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 PE= I gets around to coalescing the capsule, the MMU and caches may still be di= sabled, 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 c= lean the caches to memory first. > > Note that this cache maintenance cannot be done during the invocation of = UpdateCapsule(), since the ScatterGatherList structures are only identified= by physical address, and at runtime, the firmware doesn't know whether and= where this memory is mapped, and cache maintenance requires a virtual addr= ess. > > Reviewed-by: Jiewen Yao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 1 + > MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 10 ++++++++= ++ > 2 files changed, 11 insertions(+) > > diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf b/MdeModule= Pkg/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..fb59f338f100 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, EITH= ER EXPRESS OR IMPLIED. > #include > > #include > +#include > #include > #include > #include > @@ -274,6 +275,7 @@ ValidateCapsuleByMemoryResource ( > // > // No memory resource descriptor reported in HOB list before capsule= Coalesce. > // > + WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size); > return TRUE; > } > > @@ -283,6 +285,14 @@ ValidateCapsuleByMemoryResource ( > DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in MemoryResource[= 0x%x] - Start(0x%lx) Length(0x%lx)\n", > Address, Size, > Index, MemoryResource[Index].PhysicalStart, Me= moryResource[Index].ResourceLength)); > + > + // > + // At this point, we may still be running with the MMU and caches = disabled, > + // and on architectures such as ARM or AARCH64, capsule [meta]data= loaded > + // into memory with the caches on is only guaranteed to be visible= to the > + // CPU running with the caches off after performing an explicit wr= iteback. > + // > + WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size); > return TRUE; > } > } > -- > 2.17.0 >