[edk2-devel] [PATCH 0/9] ArmPkg: Start cleaning up ID register handling

Leif Lindholm posted 9 patches 3 years, 4 months ago
Failed in applying to current master (apply log)
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%)
[edk2-devel] [PATCH 0/9] ArmPkg: Start cleaning up ID register handling
Posted by Leif Lindholm 3 years, 4 months ago
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

 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%)

-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69196): https://edk2.groups.io/g/devel/message/69196
Mute This Topic: https://groups.io/mt/79061630/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] ArmPkg: Start cleaning up ID register handling
Posted by Ard Biesheuvel 3 years, 4 months ago
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>

>  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%)
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69207): https://edk2.groups.io/g/devel/message/69207
Mute This Topic: https://groups.io/mt/79061630/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] ArmPkg: Start cleaning up ID register handling
Posted by Leif Lindholm 3 years, 4 months ago
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%)
> > 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69208): https://edk2.groups.io/g/devel/message/69208
Mute This Topic: https://groups.io/mt/79061630/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-