On 05/29/15 14:33, Ard Biesheuvel wrote:
> Instead of inferring the GIC revision from the CPU id registers
> and the presence/availability of the system register interface
> upon each invocation, move the logic to a constructor and cache
> the result.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
> ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 23 ++++++++++--
> ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 23 ++++++++++--
> ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf | 3 +-
> 3 files changed, 40 insertions(+), 9 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> index 0e0fa3b9f33e..9853c7ba8566 100644
> --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> @@ -15,9 +15,11 @@
> #include <Library/ArmLib.h>
> #include <Library/ArmGicLib.h>
>
> -ARM_GIC_ARCH_REVISION
> +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision;
> +
> +RETURN_STATUS
> EFIAPI
> -ArmGicGetSupportedArchRevision (
> +ArmGicArchLibInitialize (
> VOID
> )
> {
> @@ -43,9 +45,22 @@ ArmGicGetSupportedArchRevision (
> IccSre = ArmGicV3GetControlSystemRegisterEnable ();
> }
> if (IccSre & ICC_SRE_EL2_SRE) {
> - return ARM_GIC_ARCH_REVISION_3;
> + mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
> + goto Done;
> }
> }
>
> - return ARM_GIC_ARCH_REVISION_2;
> + mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
> +
> +Done:
> + return RETURN_SUCCESS;
> +}
> +
> +ARM_GIC_ARCH_REVISION
> +EFIAPI
> +ArmGicGetSupportedArchRevision (
> + VOID
> + )
> +{
> + return mGicArchRevision;
> }
> diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> index f256de704631..f8822a224580 100644
> --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> @@ -15,9 +15,11 @@
> #include <Library/ArmLib.h>
> #include <Library/ArmGicLib.h>
>
> -ARM_GIC_ARCH_REVISION
> +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision;
> +
> +RETURN_STATUS
> EFIAPI
> -ArmGicGetSupportedArchRevision (
> +ArmGicArchLibInitialize (
> VOID
> )
> {
> @@ -43,9 +45,22 @@ ArmGicGetSupportedArchRevision (
> IccSre = ArmGicV3GetControlSystemRegisterEnable ();
> }
> if (IccSre & ICC_SRE_EL2_SRE) {
> - return ARM_GIC_ARCH_REVISION_3;
> + mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
> + goto Done;
> }
> }
>
> - return ARM_GIC_ARCH_REVISION_2;
> + mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
> +
> +Done:
> + return RETURN_SUCCESS;
> +}
> +
> +ARM_GIC_ARCH_REVISION
> +EFIAPI
> +ArmGicGetSupportedArchRevision (
> + VOID
> + )
> +{
> + return mGicArchRevision;
> }
> diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> index d71b2adc3027..7dbcb08f50d6 100644
> --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> +++ b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> @@ -17,7 +17,8 @@
> FILE_GUID = cd67f41a-26e9-4482-90c9-a9aff803382a
> MODULE_TYPE = BASE
> VERSION_STRING = 1.0
> - LIBRARY_CLASS = ArmGicArchLib
> + LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER
> UEFI_APPLICATION
> + CONSTRUCTOR = ArmGicArchLibInitialize
>
> [Sources.ARM]
> Arm/ArmGicArchLib.c
>
I trust that you rebuilt all of the DSCs that use this (now restricted)
library instance via their default library class resolutions, and there
were no errors (ie. no other referring module types than spelled out
here) :)
Code-wise, it looks fine.
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
I think I'm finished reviewing the still pending patches. At the end of
this series, we have three instances for the library class:
ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf:
LIBRARY_CLASS = ArmGicArchLib|SEC
ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf:
LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION
ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf:
LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION
The first one (without caching) is used for SEC modules in:
- ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
- ArmPlatformPkg/ArmPlatformPkg.dsc
- ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
- ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
The second one (with caching in a static variable) is used as the
default resolution in:
- ArmPkg/ArmPkg.dsc
- ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
- ArmPlatformPkg/ArmPlatformPkg.dsc
- ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
- ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
The third one (also with caching in a static variable, but taken from a
PCD, not probed from hardware) is used as the default resolution in:
- ArmVirtPkg/ArmVirt.dsc.inc
Looks good!
Thanks
Laszlo
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
