Re: [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops

["Leif Lindholm" <leif@nuviainc.com>]

On Wed, Mar 04, 2020 at 19:12:37 +0100, Ard Biesheuvel wrote:
> This is a combination of v1 'ArmPkg: eradicate and deprecate by set/way cache
> ops' and v1 'ArmPkg/ArmLib: ASSERT() on misuse of set/way ops'
> 
> As it turns out, there were still some instances of set/way ops left in
> the core code, in ArmMmuLib to be precise.
> 
> This series fixes ArmMmuLib to perform the appropriate cache invalidation
> when populating page tables with the MMU and caches off, allowing us to
> get rid of the cache clean/disable/enable sequences which are incorrect
> and pointless at the same time.
> 
> I have incorporated some ArmLib changes that I posted separately before,
> with the end result being that all uses pf set/way ops are gone from the
> EDK2 core code, and the routines themselves will now ASSERT() when used
> for anything other than managing the caches while the MMU is still off.
> (Note that BeagleBoard in edk2-platforms still relies on this)
> 
> Changes since v1[s]:
> - don't deprecate the set/way ops but make the usable before MMU is enabled
>   only
> - use a more elaborate sequence for invalidating the page table entries on
>   32-bit ARM, to ensure we are compliant with the nooks and crannies of
>   version 7 of the architecture.
> - incorporate patch #1, which was still pending on the list, and is related
>   (its PrePeiCore sibling was already reviewed and merged)
> 
> Ard Biesheuvel (9):
>   ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones
>   ArmPkg/ArmMmuLib ARM: remove dummy constructor
>   ArmPkg/ArmMmuLib ARM: split ArmMmuLibCore.c into core and update code
>   ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries
>   ArmPkg/ArmMmuLib AARCH64: cache-invalidate initial page table entries
>   ArmPkg/ArmLib: move set/way helper functions into private header
>   ArmPkg/ArmLib: clean up library includes
>   ArmPkg/ArmLib: remove bogus protocol declaration
>   ArmPkg/ArmLib: ASSERT on set/way cache ops being used with MMU on

Apart from the minor nit pointed out (which you can fix before
committing) - for the series:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>

Thanks!

>  ArmPkg/Include/Library/ArmLib.h               |  18 -
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c    |  16 +-
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h    |  18 +
>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S |   9 +-
>  ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c          |  17 +-
>  ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h          |  18 +
>  ArmPkg/Library/ArmLib/ArmBaseLib.inf          |   6 +-
>  ArmPkg/Library/ArmLib/ArmLib.c                |   2 -
>  .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |   9 -
>  .../Library/ArmMmuLib/Arm/ArmMmuLibConvert.c  |  32 ++
>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c  | 498 ++----------------
>  .../Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c   | 435 +++++++++++++++
>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf    |   4 +
>  ArmPlatformPkg/PrePi/PeiMPCore.inf            |   1 +
>  ArmPlatformPkg/PrePi/PeiUniCore.inf           |   1 +
>  ArmPlatformPkg/PrePi/PrePi.c                  |   8 +-
>  16 files changed, 591 insertions(+), 501 deletions(-)
>  create mode 100644 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c
>  create mode 100644 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> 
> -- 
> 2.17.1
>