Re: [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

["Zeng, Star" <star.zeng@intel.com>]

Hi Ard,

The input parameter CapsuleHeaderArray of UpdateCapsule has the virtual address.

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 <ruiyu.ni@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gao, Liming <liming.gao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

On 6 June 2018 at 11:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 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 <ard.biesheuvel@linaro.org>

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 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 address.
"""

> ---
>  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, EITHER EXPRESS OR IMPLIED.
>  #include <Guid/CapsuleVendor.h>
>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/CacheMaintenanceLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/PrintLib.h>
>  #include <Library/BaseLib.h>
> @@ -283,6 +284,9 @@ 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));
> +
> +      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