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::241; helo=mail-io0-x241.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x241.google.com (mail-io0-x241.google.com [IPv6:2607:f8b0:4001:c06::241]) (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 52394210DF77B for ; Thu, 7 Jun 2018 23:24:59 -0700 (PDT) Received: by mail-io0-x241.google.com with SMTP id g7-v6so14644762ioh.11 for ; Thu, 07 Jun 2018 23:24:59 -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=xSeHvPlCyX5GpZNHcsvS8V0UcL4lyfSUG67friMDS8A=; b=iPW4oAA2ymJTmJyRp1g1kpnyzLdpPRcRi0jT65d+JnARU5PSilmmc4A1UW6mZBaFi1 rgzjZ1gg1ej2eE3OOwFwXQrmkoXJTN7zcJy+o7rh+d1tzX1Y7jceSRRtzn1gFP6uqVpg dqEQUGrf2djTkmEPQGul+iELEhc03h5UPOWxU= 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=xSeHvPlCyX5GpZNHcsvS8V0UcL4lyfSUG67friMDS8A=; b=Xk7LlVVtmzpOJ/BFkmn32QIDJN47vP2nqbCu+UFdbGo4W/eOS54PSJe8oSDjoMZqoJ Ir+j/aIGqG+wcSnJ7+R5JOsAK6ojGer+0/iSTtY2nnTB6rGy69U1PizkIELvWjJoI6Tt C7JVqqQUHO8bCkjLahWVqXAwajp4xeDG+cSKDf6qMChZ8oXnKhhGz6FKfXUnYZVbUvxw f+GHRfqMFA3sk6adS9UQ8PzSXk5s+wQNEoHOlgpBjF9K3LXarJdtAq6eKK7GGQ0rthQN 1d4ljK1ujUUOV/59bUda0GFVVZgh8m3f7VCOld4HkzSZBoGej7VDEW+j/FE27uJU+K8V Mvdg== X-Gm-Message-State: APt69E3GyxBFJMBYE2qPrlBsnGo9jnJCIVDDio9xxlm4ISGqgQ3Jonbr I7kBNSiXGqf9wpNGesfEaf3KxqiqnYGaHeGvyEekPw== X-Google-Smtp-Source: ADUXVKK/rP9IzJoucia85gZIeWcD9sXB5VddWhEuxGl/NaqHPKllcFt9zK2Hz1iYAmk11UMgoZqS5fdUkr4/qO1cHbQ= X-Received: by 2002:a6b:520d:: with SMTP id g13-v6mr2973035iob.60.1528439099294; Thu, 07 Jun 2018 23:24:59 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Thu, 7 Jun 2018 23:24:58 -0700 (PDT) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BB54D3A@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> <0C09AFA07DD0434D9E2A0C6AEB0483103BB54D3A@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 8 Jun 2018 08:24:58 +0200 Message-ID: To: "Zeng, Star" Cc: "Kinney, Michael D" , "edk2-devel@lists.01.org" , "Yao, Jiewen" , "leif.lindholm@linaro.org" 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:25:00 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On 8 June 2018 at 08:21, Zeng, Star wrote: > My thought is like below, FYR. > Thanks. That works for me. I will update the patch. > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > 8bf218e00d8bd5c4f01c83f3d16c636140d32fda > .../Universal/CapsulePei/Common/CapsuleCoalesce.c | 37 +++++++++++++++-= ------ > 1 file changed, 26 insertions(+), 11 deletions(-) > > diff --git a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c b= /MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c > index 3e7054cd38a9..da047034c988 100644 > --- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c > +++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c > @@ -253,6 +253,7 @@ ValidateCapsuleByMemoryResource ( > ) > { > UINTN Index; > + BOOLEAN Valid; > > // > // Sanity Check > @@ -270,25 +271,39 @@ ValidateCapsuleByMemoryResource ( > return FALSE; > } > > + Valid =3D FALSE; > if (MemoryResource =3D=3D NULL) { > // > // No memory resource descriptor reported in HOB list before capsule= Coalesce. > // > - return TRUE; > + Valid =3D TRUE; > + } else { > + for (Index =3D 0; 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 MemoryResourc= e[0x%x] - Start(0x%lx) Length(0x%lx)\n", > + Address, Size, > + Index, MemoryResource[Index].PhysicalStart, = MemoryResource[Index].ResourceLength)); > + Valid =3D TRUE; > + break; > + } > + } > + if (!Valid) { > + DEBUG ((EFI_D_ERROR, "ERROR: Address(0x%lx) Size(0x%lx) not in any= MemoryResource\n", Address, Size)); > + } > } > > - for (Index =3D 0; MemoryResource[Index].ResourceLength !=3D 0; Index++= ) { > - if ((Address >=3D MemoryResource[Index].PhysicalStart) && > - ((Address + Size) <=3D (MemoryResource[Index].PhysicalStart + Me= moryResource[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, Me= moryResource[Index].ResourceLength)); > - return TRUE; > - } > + if (Valid) { > + // > + // At this point, we may still be running with the MMU and caches di= sabled, > + // and on architectures such as ARM or AARCH64, capsule [meta]data l= oaded > + // into memory with the caches on is only guaranteed to be visible t= o the > + // CPU running with the caches off after performing an explicit writ= eback. > + // > + WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size); > } > > - DEBUG ((EFI_D_ERROR, "ERROR: Address(0x%lx) Size(0x%lx) not in any Mem= oryResource\n", Address, Size)); > - return FALSE; > + return Valid; > } > > /** > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > > > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ar= d Biesheuvel > Sent: Friday, June 8, 2018 2:07 PM > To: Zeng, Star > Cc: Kinney, Michael D ; edk2-devel@lists.01.o= rg; Yao, Jiewen ; leif.lindholm@linaro.org > Subject: Re: [edk2] [PATCH 1/5] MdeModulePkg/CapsulePei: clean Dcache bef= ore consuming capsule data > > 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 pe= rform 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 di= sabled, > + // and on architectures such as ARM or AARCH64, capsule [meta]data l= oaded > + // into memory with the caches on is only guaranteed to be visible t= o the > + // CPU running with the caches off after performing an explicit writ= eback. > + // > + WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size); > + return TRUE; > + } > + > DEBUG ((EFI_D_ERROR, "ERROR: Address(0x%lx) Size(0x%lx) not in any Mem= oryResource\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 >> consuming 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 P= EI gets around to coalescing the capsule, the MMU and caches may still be d= isabled, and so on architectures where uncached accesses are incoherent wit= h the caches (such as ARM and AARCH64), we may read stale data if we don't = clean the caches to memory first. >> >> 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. >> >> 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/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..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, EIT= HER EXPRESS OR IMPLIED. >> #include >> >> #include >> +#include >> #include >> #include >> #include >> @@ -274,6 +275,7 @@ ValidateCapsuleByMemoryResource ( >> // >> // No memory resource descriptor reported in HOB list before capsul= e 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, >> MemoryResource[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]dat= a loaded >> + // into memory with the caches on is only guaranteed to be visibl= e to the >> + // CPU running with the caches off after performing an explicit w= riteback. >> + // >> + WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size); >> return TRUE; >> } >> } >> -- >> 2.17.0 >> > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel