Re: [PATCH 0/9] ArmPkg: Start cleaning up ID register handling

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

On Fri, Dec 18, 2020 at 16:02:07 +0100, Ard Biesheuvel wrote:
> On 12/18/20 3:16 PM, Leif Lindholm wrote:
> > As discussed in https://edk2.groups.io/g/devel/topic/78784065,
> > the ARM and AArch64 ports were using an unfortunate overloading
> > of accessor functions for the ID registers which describe the
> > presence and versions of features implemented in a CPU.
> > 
> > This makes no sense, since the layout, and indeed the names of
> > the registers for the different architectural states differ.
> > 
> > This set replaces the use of the current accessor functions
> > with some helper functions in ArmLib. It also moves the
> > accessor function prototypes into private headers so they
> > can no longer be called from outside ArmLib
> > 
> > As a pure side effect, we're able to throw away half of
> > ArmGicArchLib/ArmGicArchSecLib.
> > 
> > Note that some additional cleanup can also be done in the ARM
> > portion of ArmMmuLib (which implements its own direct accessor
> > for ID_MMFR0), and internally in ArmLib where some assembly
> > code does its own feature checks, which could be moved to C
> > helpers.
> > 
> > Furthermore, it might be useful to move the ID register field
> > definitions to a private header.
> > 
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > 
> > Leif Lindholm (9):
> >   ArmPkg/ArmLib: add ArmHasGicSystemRegisters () helper function
> >   ArmPkg: use ID register helper for ArmGicArch(Sec)Lib
> >   ArmPkg: remove duplicated ARM/AArch64 ArmGicArchLib sources
> >   ArmPkg: remove duplicated ARM/AArch64 ArmGicArchSecLib sources
> >   ArmPkg: add ArmHasSecurityExtensions () helper function
> >   ArmPkg: use helper to check for Security extensions in ArmArchTimerLib
> >   ArmPkg/ArmLib: delete AArch64 version of ArmReadIdPfr1
> >   ArmPkg/ArmLib: rename AArch64 variant of ArmReadIdPfr0
> >   ArmPkg/ArmLib: move ArmReadIdPfr0/1 into private header ArmV7Lib.h
> > 
> 
> Thanks for getting started on this.
> 
> For the series,
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Thanks!

Pushed as 6573ae8c8575..e2bfd172e4b9 (#1244).

> >  ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf                |  7 +--
> >  ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf          |  7 +--
> >  ArmPkg/Include/Library/ArmLib.h                               | 47 +++++++++++----
> >  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h                    |  6 ++
> >  ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h                          | 12 ++++
> >  ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c              |  2 +-
> >  ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c              | 60 --------------------
> >  ArmPkg/Library/ArmGicArchLib/{AArch64 => }/ArmGicArchLib.c    |  2 +-
> >  ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c           | 45 ---------------
> >  ArmPkg/Library/ArmGicArchSecLib/{AArch64 => }/ArmGicArchLib.c |  2 +-
> >  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c                    | 16 ++++++
> >  ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c                          | 31 ++++++++++
> >  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S                | 13 +----
> >  13 files changed, 110 insertions(+), 140 deletions(-)
> >  delete mode 100644 ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> >  rename ArmPkg/Library/ArmGicArchLib/{AArch64 => }/ArmGicArchLib.c (94%)
> >  delete mode 100644 ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
> >  rename ArmPkg/Library/ArmGicArchSecLib/{AArch64 => }/ArmGicArchLib.c (94%)
> > 
>