From: Ryan Harkin <ryan.harkin@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [PATCH] ArmPkg/ArmLib: remove indirection layer from timer register accessors
Date: Fri, 20 Jan 2017 14:32:00 +0000 [thread overview]
Message-ID: <CAD0U-h+QDPR3tyf8GNLV3XHNW5s2MkddpX0ma4zEN44ev8QVMA@mail.gmail.com> (raw)
In-Reply-To: <1484913996-5062-1-git-send-email-ard.biesheuvel@linaro.org>
Hi Ard,
On 20 January 2017 at 12:06, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> The generic timer support libraries call the actual system register
> accessor function via a single pair of functions ArmArchTimerReadReg()
> and ArmArchTimerWriteReg(), which take an enum to argument to identify
> the register, and return output values by pointer reference.
>
> Since these functions are never called with a non-immediate argument,
> we can simply replace each invocation with the underlying system register
> accessor instead. This is mostly functionally equivalent, with the
> exception of the bounds check for the enum (which is pointless given the
> fact that we never pass a variable), the check for the presence of the
> architected timer (which only makes sense for ARMv7, but is highly unlikely
> to vary between platforms that are similar enough to run the same firmware
> image), and a check for enum values that refer to the HYP view of the timer,
> which we never referred to anywhere in the code in the first place.
>
> So get rid of the middle man, and update the ArmGenericTimerPhyCounterLib
> and ArmGenericTimerVirtCounterLib implementations to call the system
> register accessors directly.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Are there any other patches needed to get this working?
I've just applied it to the head of EDK2 [1] and I get this error when
building for TC2, FVP and Juno:
Building ... /linaro/platforms/uefi/edk2/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
[AARCH64]
/linaro/platforms/uefi/edk2/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c:
In function 'ArmGenericTimerSetTimerFreq':
/linaro/platforms/uefi/edk2/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c:51:19:
error: 'CntFrq' undeclared (first use in this function)
ArmWriteCntFrq (CntFrq);
^
/linaro/platforms/uefi/edk2/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c:51:19:
note: each undeclared identifier is reported only once for each
function it appears in
GNUmakefile:304: recipe for target
'/linaro/platforms/uefi/edk2/Build/ArmVExpress-FVP-AArch64/DEBUG_GCC5/AARCH64/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib/OUTPUT/ArmGenericTimerPhyCounterLib.obj'
failed
make: *** [/linaro/platforms/uefi/edk2/Build/ArmVExpress-FVP-AArch64/DEBUG_GCC5/AARCH64/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib/OUTPUT/ArmGenericTimerPhyCounterLib.obj]
Error 1
Cheers,
Ryan.
> ---
> ArmPkg/Drivers/TimerDxe/TimerDxe.c | 1 -
> ArmPkg/Include/Chipset/AArch64.h | 1 -
> ArmPkg/Include/Chipset/ArmArchTimer.h | 139 ----------------
> ArmPkg/Include/Chipset/ArmV7.h | 1 -
> ArmPkg/Include/Library/ArmArchTimer.h | 55 -------
> ArmPkg/Include/Library/ArmLib.h | 128 +++++++++++++++
> ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c | 42 ++---
> ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c | 42 ++---
> ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c | 167 --------------------
> ArmPkg/Library/ArmLib/Arm/ArmV7ArchTimer.c | 167 --------------------
> ArmPkg/Library/ArmLib/ArmBaseLib.inf | 2 -
> 11 files changed, 156 insertions(+), 589 deletions(-)
>
> diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> index 1169d426b255..2416c90f5545 100644
> --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> @@ -25,7 +25,6 @@
> #include <Library/PcdLib.h>
> #include <Library/IoLib.h>
> #include <Library/ArmGenericTimerCounterLib.h>
> -#include <Library/ArmArchTimer.h>
>
> #include <Protocol/Timer.h>
> #include <Protocol/HardwareInterrupt.h>
> diff --git a/ArmPkg/Include/Chipset/AArch64.h b/ArmPkg/Include/Chipset/AArch64.h
> index e94c9290c582..9aecb1df81e0 100644
> --- a/ArmPkg/Include/Chipset/AArch64.h
> +++ b/ArmPkg/Include/Chipset/AArch64.h
> @@ -17,7 +17,6 @@
> #define __AARCH64_H__
>
> #include <Chipset/AArch64Mmu.h>
> -#include <Chipset/ArmArchTimer.h>
>
> // ARM Interrupt ID in Exception Table
> #define ARM_ARCH_EXCEPTION_IRQ EXCEPT_AARCH64_IRQ
> diff --git a/ArmPkg/Include/Chipset/ArmArchTimer.h b/ArmPkg/Include/Chipset/ArmArchTimer.h
> deleted file mode 100644
> index 1caad3d3367a..000000000000
> --- a/ArmPkg/Include/Chipset/ArmArchTimer.h
> +++ /dev/null
> @@ -1,139 +0,0 @@
> -/** @file
> -*
> -* Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -*
> -* This program and the accompanying materials
> -* are licensed and made available under the terms and conditions of the BSD License
> -* which accompanies this distribution. The full text of the license may be found at
> -* http://opensource.org/licenses/bsd-license.php
> -*
> -* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -*
> -**/
> -
> -#ifndef __ARM_ARCH_TIMER_H_
> -#define __ARM_ARCH_TIMER_H_
> -
> -UINTN
> -EFIAPI
> -ArmReadCntFrq (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntFrq (
> - UINTN FreqInHz
> - );
> -
> -UINT64
> -EFIAPI
> -ArmReadCntPct (
> - VOID
> - );
> -
> -UINTN
> -EFIAPI
> -ArmReadCntkCtl (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntkCtl (
> - UINTN Val
> - );
> -
> -UINTN
> -EFIAPI
> -ArmReadCntpTval (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntpTval (
> - UINTN Val
> - );
> -
> -UINTN
> -EFIAPI
> -ArmReadCntpCtl (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntpCtl (
> - UINTN Val
> - );
> -
> -UINTN
> -EFIAPI
> -ArmReadCntvTval (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntvTval (
> - UINTN Val
> - );
> -
> -UINTN
> -EFIAPI
> -ArmReadCntvCtl (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntvCtl (
> - UINTN Val
> - );
> -
> -UINT64
> -EFIAPI
> -ArmReadCntvCt (
> - VOID
> - );
> -
> -UINT64
> -EFIAPI
> -ArmReadCntpCval (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntpCval (
> - UINT64 Val
> - );
> -
> -UINT64
> -EFIAPI
> -ArmReadCntvCval (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntvCval (
> - UINT64 Val
> - );
> -
> -UINT64
> -EFIAPI
> -ArmReadCntvOff (
> - VOID
> - );
> -
> -VOID
> -EFIAPI
> -ArmWriteCntvOff (
> - UINT64 Val
> - );
> -
> -#endif // __ARM_ARCH_TIMER_H_
> -
> diff --git a/ArmPkg/Include/Chipset/ArmV7.h b/ArmPkg/Include/Chipset/ArmV7.h
> index 4fb06636e0b0..ee4ac4374bd3 100644
> --- a/ArmPkg/Include/Chipset/ArmV7.h
> +++ b/ArmPkg/Include/Chipset/ArmV7.h
> @@ -17,7 +17,6 @@
> #define __ARM_V7_H__
>
> #include <Chipset/ArmV7Mmu.h>
> -#include <Chipset/ArmArchTimer.h>
>
> // ARM Interrupt ID in Exception Table
> #define ARM_ARCH_EXCEPTION_IRQ EXCEPT_ARM_IRQ
> diff --git a/ArmPkg/Include/Library/ArmArchTimer.h b/ArmPkg/Include/Library/ArmArchTimer.h
> deleted file mode 100644
> index 876c1f65c525..000000000000
> --- a/ArmPkg/Include/Library/ArmArchTimer.h
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -/** @file
> -
> - Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
> -
> - This program and the accompanying materials
> - are licensed and made available under the terms and conditions of the BSD License
> - which accompanies this distribution. The full text of the license may be found at
> - http://opensource.org/licenses/bsd-license.php
> -
> - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -
> -**/
> -
> -#ifndef __ARM_ARCH_TIMER_H__
> -#define __ARM_ARCH_TIMER_H__
> -
> -#define ARM_ARCH_TIMER_ENABLE (1 << 0)
> -#define ARM_ARCH_TIMER_IMASK (1 << 1)
> -#define ARM_ARCH_TIMER_ISTATUS (1 << 2)
> -
> -typedef enum {
> - CntFrq = 0,
> - CntPct,
> - CntkCtl,
> - CntpTval,
> - CntpCtl,
> - CntvTval,
> - CntvCtl,
> - CntvCt,
> - CntpCval,
> - CntvCval,
> - CntvOff,
> - CnthCtl,
> - CnthpTval,
> - CnthpCtl,
> - CnthpCval,
> - RegMaximum
> -} ARM_ARCH_TIMER_REGS;
> -
> -VOID
> -EFIAPI
> -ArmArchTimerReadReg (
> - IN ARM_ARCH_TIMER_REGS Reg,
> - OUT VOID *DstBuf
> - );
> -
> -VOID
> -EFIAPI
> -ArmArchTimerWriteReg (
> - IN ARM_ARCH_TIMER_REGS Reg,
> - IN VOID *SrcBuf
> - );
> -
> -#endif // __ARM_ARCH_TIMER_H__
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 40d97e09b566..19501efa991f 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -587,4 +587,132 @@ ArmUnsetCpuActlrBit (
> IN UINTN Bits
> );
>
> +//
> +// Accessors for the architected generic timer registers
> +//
> +
> +#define ARM_ARCH_TIMER_ENABLE (1 << 0)
> +#define ARM_ARCH_TIMER_IMASK (1 << 1)
> +#define ARM_ARCH_TIMER_ISTATUS (1 << 2)
> +
> +UINTN
> +EFIAPI
> +ArmReadCntFrq (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntFrq (
> + UINTN FreqInHz
> + );
> +
> +UINT64
> +EFIAPI
> +ArmReadCntPct (
> + VOID
> + );
> +
> +UINTN
> +EFIAPI
> +ArmReadCntkCtl (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntkCtl (
> + UINTN Val
> + );
> +
> +UINTN
> +EFIAPI
> +ArmReadCntpTval (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntpTval (
> + UINTN Val
> + );
> +
> +UINTN
> +EFIAPI
> +ArmReadCntpCtl (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntpCtl (
> + UINTN Val
> + );
> +
> +UINTN
> +EFIAPI
> +ArmReadCntvTval (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntvTval (
> + UINTN Val
> + );
> +
> +UINTN
> +EFIAPI
> +ArmReadCntvCtl (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntvCtl (
> + UINTN Val
> + );
> +
> +UINT64
> +EFIAPI
> +ArmReadCntvCt (
> + VOID
> + );
> +
> +UINT64
> +EFIAPI
> +ArmReadCntpCval (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntpCval (
> + UINT64 Val
> + );
> +
> +UINT64
> +EFIAPI
> +ArmReadCntvCval (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntvCval (
> + UINT64 Val
> + );
> +
> +UINT64
> +EFIAPI
> +ArmReadCntvOff (
> + VOID
> + );
> +
> +VOID
> +EFIAPI
> +ArmWriteCntvOff (
> + UINT64 Val
> + );
> +
> #endif // __ARM_LIB__
> diff --git a/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c b/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c
> index 826827fb0916..268b06e035bf 100644
> --- a/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c
> +++ b/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c
> @@ -14,7 +14,7 @@
> **/
>
> #include <Library/ArmGenericTimerCounterLib.h>
> -#include <Library/ArmArchTimer.h>
> +#include <Library/ArmLib.h>
>
> VOID
> EFIAPI
> @@ -24,9 +24,9 @@ ArmGenericTimerEnableTimer (
> {
> UINTN TimerCtrlReg;
>
> - ArmArchTimerReadReg (CntpCtl, (VOID *)&TimerCtrlReg);
> + TimerCtrlReg = ArmReadCntpCtl ();
> TimerCtrlReg |= ARM_ARCH_TIMER_ENABLE;
> - ArmArchTimerWriteReg (CntpCtl, (VOID *)&TimerCtrlReg);
> + ArmWriteCntpCtl (TimerCtrlReg);
> }
>
> VOID
> @@ -37,9 +37,9 @@ ArmGenericTimerDisableTimer (
> {
> UINTN TimerCtrlReg;
>
> - ArmArchTimerReadReg (CntpCtl, (VOID *)&TimerCtrlReg);
> + TimerCtrlReg = ArmReadCntpCtl ();
> TimerCtrlReg &= ~ARM_ARCH_TIMER_ENABLE;
> - ArmArchTimerWriteReg (CntpCtl, (VOID *)&TimerCtrlReg);
> + ArmWriteCntpCtl (TimerCtrlReg);
> }
>
> VOID
> @@ -48,7 +48,7 @@ ArmGenericTimerSetTimerFreq (
> IN UINTN FreqInHz
> )
> {
> - ArmArchTimerWriteReg (CntFrq, (VOID *)&FreqInHz);
> + ArmWriteCntFrq (CntFrq);
> }
>
> UINTN
> @@ -57,9 +57,7 @@ ArmGenericTimerGetTimerFreq (
> VOID
> )
> {
> - UINTN ArchTimerFreq = 0;
> - ArmArchTimerReadReg (CntFrq, (VOID *)&ArchTimerFreq);
> - return ArchTimerFreq;
> + return ArmReadCntFrq ();
> }
>
> UINTN
> @@ -68,10 +66,7 @@ ArmGenericTimerGetTimerVal (
> VOID
> )
> {
> - UINTN ArchTimerValue;
> - ArmArchTimerReadReg (CntpTval, (VOID *)&ArchTimerValue);
> -
> - return ArchTimerValue;
> + return ArmReadCntpTval ();
> }
>
>
> @@ -81,7 +76,7 @@ ArmGenericTimerSetTimerVal (
> IN UINTN Value
> )
> {
> - ArmArchTimerWriteReg (CntpTval, (VOID *)&Value);
> + ArmWriteCntpTval (Value);
> }
>
> UINT64
> @@ -90,10 +85,7 @@ ArmGenericTimerGetSystemCount (
> VOID
> )
> {
> - UINT64 SystemCount;
> - ArmArchTimerReadReg (CntPct, (VOID *)&SystemCount);
> -
> - return SystemCount;
> + return ArmReadCntPct ();
> }
>
> UINTN
> @@ -102,10 +94,7 @@ ArmGenericTimerGetTimerCtrlReg (
> VOID
> )
> {
> - UINTN Value;
> - ArmArchTimerReadReg (CntpCtl, (VOID *)&Value);
> -
> - return Value;
> + return ArmReadCntpCtl ();
> }
>
> VOID
> @@ -114,7 +103,7 @@ ArmGenericTimerSetTimerCtrlReg (
> UINTN Value
> )
> {
> - ArmArchTimerWriteReg (CntpCtl, (VOID *)&Value);
> + ArmWriteCntpCtl (Value);
> }
>
> UINT64
> @@ -123,10 +112,7 @@ ArmGenericTimerGetCompareVal (
> VOID
> )
> {
> - UINT64 Value;
> - ArmArchTimerReadReg (CntpCval, (VOID *)&Value);
> -
> - return Value;
> + return ArmReadCntpCval ();
> }
>
> VOID
> @@ -135,5 +121,5 @@ ArmGenericTimerSetCompareVal (
> IN UINT64 Value
> )
> {
> - ArmArchTimerWriteReg (CntpCval, (VOID *)&Value);
> + ArmWriteCntpCval (Value);
> }
> diff --git a/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c b/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c
> index f99c8525b900..69a4ceb62db6 100644
> --- a/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c
> +++ b/ArmPkg/Library/ArmGenericTimerVirtCounterLib/ArmGenericTimerVirtCounterLib.c
> @@ -14,7 +14,7 @@
> **/
>
> #include <Library/ArmGenericTimerCounterLib.h>
> -#include <Library/ArmArchTimer.h>
> +#include <Library/ArmLib.h>
>
> VOID
> EFIAPI
> @@ -24,7 +24,7 @@ ArmGenericTimerEnableTimer (
> {
> UINTN TimerCtrlReg;
>
> - ArmArchTimerReadReg (CntvCtl, (VOID *)&TimerCtrlReg);
> + TimerCtrlReg = ArmReadCntvCtl ();
> TimerCtrlReg |= ARM_ARCH_TIMER_ENABLE;
>
> //
> @@ -36,7 +36,7 @@ ArmGenericTimerEnableTimer (
> // leaving this in once KVM gets fixed.
> //
> TimerCtrlReg &= ~ARM_ARCH_TIMER_IMASK;
> - ArmArchTimerWriteReg (CntvCtl, (VOID *)&TimerCtrlReg);
> + ArmWriteCntvCtl (TimerCtrlReg);
> }
>
> VOID
> @@ -47,9 +47,9 @@ ArmGenericTimerDisableTimer (
> {
> UINTN TimerCtrlReg;
>
> - ArmArchTimerReadReg (CntvCtl, (VOID *)&TimerCtrlReg);
> + TimerCtrlReg = ArmReadCntvCtl ();
> TimerCtrlReg &= ~ARM_ARCH_TIMER_ENABLE;
> - ArmArchTimerWriteReg (CntvCtl, (VOID *)&TimerCtrlReg);
> + ArmWriteCntvCtl (TimerCtrlReg);
> }
>
> VOID
> @@ -58,7 +58,7 @@ ArmGenericTimerSetTimerFreq (
> IN UINTN FreqInHz
> )
> {
> - ArmArchTimerWriteReg (CntFrq, (VOID *)&FreqInHz);
> + ArmWriteCntFrq (FreqInHz);
> }
>
> UINTN
> @@ -67,9 +67,7 @@ ArmGenericTimerGetTimerFreq (
> VOID
> )
> {
> - UINTN ArchTimerFreq = 0;
> - ArmArchTimerReadReg (CntFrq, (VOID *)&ArchTimerFreq);
> - return ArchTimerFreq;
> + return ArmReadCntFrq ();
> }
>
> UINTN
> @@ -78,10 +76,7 @@ ArmGenericTimerGetTimerVal (
> VOID
> )
> {
> - UINTN ArchTimerValue;
> - ArmArchTimerReadReg (CntvTval, (VOID *)&ArchTimerValue);
> -
> - return ArchTimerValue;
> + return ArmReadCntvTval ();
> }
>
>
> @@ -91,7 +86,7 @@ ArmGenericTimerSetTimerVal (
> IN UINTN Value
> )
> {
> - ArmArchTimerWriteReg (CntvTval, (VOID *)&Value);
> + ArmWriteCntvTval (Value);
> }
>
> UINT64
> @@ -100,10 +95,7 @@ ArmGenericTimerGetSystemCount (
> VOID
> )
> {
> - UINT64 SystemCount;
> - ArmArchTimerReadReg (CntvCt, (VOID *)&SystemCount);
> -
> - return SystemCount;
> + return ArmReadCntvCt ();
> }
>
> UINTN
> @@ -112,10 +104,7 @@ ArmGenericTimerGetTimerCtrlReg (
> VOID
> )
> {
> - UINTN Value;
> - ArmArchTimerReadReg (CntvCtl, (VOID *)&Value);
> -
> - return Value;
> + return ArmReadCntvCtl ();
> }
>
> VOID
> @@ -124,7 +113,7 @@ ArmGenericTimerSetTimerCtrlReg (
> UINTN Value
> )
> {
> - ArmArchTimerWriteReg (CntvCtl, (VOID *)&Value);
> + ArmWriteCntvCtl (Value);
> }
>
> UINT64
> @@ -133,10 +122,7 @@ ArmGenericTimerGetCompareVal (
> VOID
> )
> {
> - UINT64 Value;
> - ArmArchTimerReadReg (CntvCval, (VOID *)&Value);
> -
> - return Value;
> + return ArmReadCntvCval ();
> }
>
> VOID
> @@ -145,5 +131,5 @@ ArmGenericTimerSetCompareVal (
> IN UINT64 Value
> )
> {
> - ArmArchTimerWriteReg (CntvCval, (VOID *)&Value);
> + ArmWriteCntvCval (Value);
> }
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c b/ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c
> deleted file mode 100644
> index 31286eefff30..000000000000
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64ArchTimer.c
> +++ /dev/null
> @@ -1,167 +0,0 @@
> -/** @file
> -*
> -* Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -*
> -* This program and the accompanying materials
> -* are licensed and made available under the terms and conditions of the BSD License
> -* which accompanies this distribution. The full text of the license may be found at
> -* http://opensource.org/licenses/bsd-license.php
> -*
> -* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -*
> -**/
> -
> -#include <Uefi.h>
> -#include <Chipset/AArch64.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/ArmLib.h>
> -#include <Library/BaseLib.h>
> -#include <Library/DebugLib.h>
> -#include "AArch64Lib.h"
> -#include "ArmLibPrivate.h"
> -#include <Library/ArmArchTimer.h>
> -
> -VOID
> -EFIAPI
> -ArmArchTimerReadReg (
> - IN ARM_ARCH_TIMER_REGS Reg,
> - OUT VOID *DstBuf
> - )
> -{
> - // Check if the Generic/Architecture timer is implemented
> - if (ArmIsArchTimerImplemented ()) {
> -
> - switch (Reg) {
> -
> - case CntFrq:
> - *((UINTN *)DstBuf) = ArmReadCntFrq ();
> - break;
> -
> - case CntPct:
> - *((UINT64 *)DstBuf) = ArmReadCntPct ();
> - break;
> -
> - case CntkCtl:
> - *((UINTN *)DstBuf) = ArmReadCntkCtl();
> - break;
> -
> - case CntpTval:
> - *((UINTN *)DstBuf) = ArmReadCntpTval ();
> - break;
> -
> - case CntpCtl:
> - *((UINTN *)DstBuf) = ArmReadCntpCtl ();
> - break;
> -
> - case CntvTval:
> - *((UINTN *)DstBuf) = ArmReadCntvTval ();
> - break;
> -
> - case CntvCtl:
> - *((UINTN *)DstBuf) = ArmReadCntvCtl ();
> - break;
> -
> - case CntvCt:
> - *((UINT64 *)DstBuf) = ArmReadCntvCt ();
> - break;
> -
> - case CntpCval:
> - *((UINT64 *)DstBuf) = ArmReadCntpCval ();
> - break;
> -
> - case CntvCval:
> - *((UINT64 *)DstBuf) = ArmReadCntvCval ();
> - break;
> -
> - case CntvOff:
> - *((UINT64 *)DstBuf) = ArmReadCntvOff ();
> - break;
> -
> - case CnthCtl:
> - case CnthpTval:
> - case CnthpCtl:
> - case CnthpCval:
> - DEBUG ((EFI_D_ERROR, "The register is related to Hypervisor Mode. Can't perform requested operation\n "));
> - break;
> -
> - default:
> - DEBUG ((EFI_D_ERROR, "Unknown ARM Generic Timer register %x. \n ", Reg));
> - }
> - } else {
> - DEBUG ((EFI_D_ERROR, "Attempt to read ARM Generic Timer registers. But ARM Generic Timer extension is not implemented \n "));
> - ASSERT (0);
> - }
> -}
> -
> -VOID
> -EFIAPI
> -ArmArchTimerWriteReg (
> - IN ARM_ARCH_TIMER_REGS Reg,
> - IN VOID *SrcBuf
> - )
> -{
> - // Check if the Generic/Architecture timer is implemented
> - if (ArmIsArchTimerImplemented ()) {
> -
> - switch (Reg) {
> -
> - case CntFrq:
> - ArmWriteCntFrq (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntPct:
> - DEBUG ((EFI_D_ERROR, "Can't write to Read Only Register: CNTPCT \n"));
> - break;
> -
> - case CntkCtl:
> - ArmWriteCntkCtl (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntpTval:
> - ArmWriteCntpTval (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntpCtl:
> - ArmWriteCntpCtl (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntvTval:
> - ArmWriteCntvTval (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntvCtl:
> - ArmWriteCntvCtl (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntvCt:
> - DEBUG ((EFI_D_ERROR, "Can't write to Read Only Register: CNTVCT \n"));
> - break;
> -
> - case CntpCval:
> - ArmWriteCntpCval (*((UINT64 *)SrcBuf) );
> - break;
> -
> - case CntvCval:
> - ArmWriteCntvCval (*((UINT64 *)SrcBuf) );
> - break;
> -
> - case CntvOff:
> - ArmWriteCntvOff (*((UINT64 *)SrcBuf));
> - break;
> -
> - case CnthCtl:
> - case CnthpTval:
> - case CnthpCtl:
> - case CnthpCval:
> - DEBUG ((EFI_D_ERROR, "The register is related to Hypervisor Mode. Can't perform requested operation\n "));
> - break;
> -
> - default:
> - DEBUG ((EFI_D_ERROR, "Unknown ARM Generic Timer register %x. \n ", Reg));
> - }
> - } else {
> - DEBUG ((EFI_D_ERROR, "Attempt to write to ARM Generic Timer registers. But ARM Generic Timer extension is not implemented \n "));
> - ASSERT (0);
> - }
> -}
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7ArchTimer.c b/ArmPkg/Library/ArmLib/Arm/ArmV7ArchTimer.c
> deleted file mode 100644
> index c0b3a9ed5d24..000000000000
> --- a/ArmPkg/Library/ArmLib/Arm/ArmV7ArchTimer.c
> +++ /dev/null
> @@ -1,167 +0,0 @@
> -/** @file
> -*
> -* Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
> -*
> -* This program and the accompanying materials
> -* are licensed and made available under the terms and conditions of the BSD License
> -* which accompanies this distribution. The full text of the license may be found at
> -* http://opensource.org/licenses/bsd-license.php
> -*
> -* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -*
> -**/
> -
> -#include <Uefi.h>
> -#include <Chipset/ArmV7.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/ArmLib.h>
> -#include <Library/BaseLib.h>
> -#include <Library/DebugLib.h>
> -#include "ArmV7Lib.h"
> -#include "ArmLibPrivate.h"
> -#include <Library/ArmArchTimer.h>
> -
> -VOID
> -EFIAPI
> -ArmArchTimerReadReg (
> - IN ARM_ARCH_TIMER_REGS Reg,
> - OUT VOID *DstBuf
> - )
> -{
> - // Check if the Generic/Architecture timer is implemented
> - if (ArmIsArchTimerImplemented ()) {
> - switch (Reg) {
> - case CntFrq:
> - *((UINTN *)DstBuf) = ArmReadCntFrq ();
> - return;
> -
> - case CntPct:
> - *((UINT64 *)DstBuf) = ArmReadCntPct ();
> - return;
> -
> - case CntkCtl:
> - *((UINTN *)DstBuf) = ArmReadCntkCtl();
> - return;
> -
> - case CntpTval:
> - *((UINTN *)DstBuf) = ArmReadCntpTval ();
> - return;
> -
> - case CntpCtl:
> - *((UINTN *)DstBuf) = ArmReadCntpCtl ();
> - return;
> -
> - case CntvTval:
> - *((UINTN *)DstBuf) = ArmReadCntvTval ();
> - return;
> -
> - case CntvCtl:
> - *((UINTN *)DstBuf) = ArmReadCntvCtl ();
> - return;
> -
> - case CntvCt:
> - *((UINT64 *)DstBuf) = ArmReadCntvCt ();
> - return;
> -
> - case CntpCval:
> - *((UINT64 *)DstBuf) = ArmReadCntpCval ();
> - return;
> -
> - case CntvCval:
> - *((UINT64 *)DstBuf) = ArmReadCntvCval ();
> - return;
> -
> - case CntvOff:
> - *((UINT64 *)DstBuf) = ArmReadCntvOff ();
> - return;
> -
> - case CnthCtl:
> - case CnthpTval:
> - case CnthpCtl:
> - case CnthpCval:
> - DEBUG ((EFI_D_ERROR, "The register is related to Hypervisor Mode. Can't perform requested operation\n "));
> - break;
> -
> - default:
> - DEBUG ((EFI_D_ERROR, "Unknown ARM Generic Timer register %x. \n ", Reg));
> - }
> - } else {
> - DEBUG ((EFI_D_ERROR, "Attempt to read ARM Generic Timer registers. But ARM Generic Timer extension is not implemented \n "));
> - ASSERT (0);
> - }
> -
> - *((UINT64 *)DstBuf) = 0;
> -}
> -
> -VOID
> -EFIAPI
> -ArmArchTimerWriteReg (
> - IN ARM_ARCH_TIMER_REGS Reg,
> - IN VOID *SrcBuf
> - )
> -{
> - // Check if the Generic/Architecture timer is implemented
> - if (ArmIsArchTimerImplemented ()) {
> -
> - switch (Reg) {
> -
> - case CntFrq:
> - ArmWriteCntFrq (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntPct:
> - DEBUG ((EFI_D_ERROR, "Can't write to Read Only Register: CNTPCT \n"));
> - break;
> -
> - case CntkCtl:
> - ArmWriteCntkCtl (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntpTval:
> - ArmWriteCntpTval (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntpCtl:
> - ArmWriteCntpCtl (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntvTval:
> - ArmWriteCntvTval (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntvCtl:
> - ArmWriteCntvCtl (*((UINTN *)SrcBuf));
> - break;
> -
> - case CntvCt:
> - DEBUG ((EFI_D_ERROR, "Can't write to Read Only Register: CNTVCT \n"));
> - break;
> -
> - case CntpCval:
> - ArmWriteCntpCval (*((UINT64 *)SrcBuf) );
> - break;
> -
> - case CntvCval:
> - ArmWriteCntvCval (*((UINT64 *)SrcBuf) );
> - break;
> -
> - case CntvOff:
> - ArmWriteCntvOff (*((UINT64 *)SrcBuf));
> - break;
> -
> - case CnthCtl:
> - case CnthpTval:
> - case CnthpCtl:
> - case CnthpCval:
> - DEBUG ((EFI_D_ERROR, "The register is related to Hypervisor Mode. Can't perform requested operation\n "));
> - break;
> -
> - default:
> - DEBUG ((EFI_D_ERROR, "Unknown ARM Generic Timer register %x. \n ", Reg));
> - }
> - } else {
> - DEBUG ((EFI_D_ERROR, "Attempt to write to ARM Generic Timer registers. But ARM Generic Timer extension is not implemented \n "));
> - ASSERT (0);
> - }
> -}
> diff --git a/ArmPkg/Library/ArmLib/ArmBaseLib.inf b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> index 05a585343cda..79e17be6d411 100644
> --- a/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +++ b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> @@ -27,7 +27,6 @@ [Sources]
>
> [Sources.ARM]
> Arm/ArmV7Lib.c
> - Arm/ArmV7ArchTimer.c
>
> Arm/ArmLibSupport.S | GCC
> Arm/ArmLibSupportV7.S | GCC
> @@ -41,7 +40,6 @@ [Sources.ARM]
>
> [Sources.AARCH64]
> AArch64/AArch64Lib.c
> - AArch64/AArch64ArchTimer.c
>
> AArch64/ArmLibSupport.S
> AArch64/ArmLibSupportV8.S
> --
> 2.7.4
>
[1] https://github.com/tianocore/edk2/commit/90d1f671cdad43fa80ba295b3f8d1133d68229df
90d1f67 2017-01-20 ArmPlatformPkg/NorFlashDxe: Change Flash memory
attributes before writes [Achin Gupta]
next prev parent reply other threads:[~2017-01-20 14:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-20 12:06 [PATCH] ArmPkg/ArmLib: remove indirection layer from timer register accessors Ard Biesheuvel
2017-01-20 14:21 ` Leif Lindholm
2017-01-20 14:32 ` Ryan Harkin [this message]
2017-01-20 14:35 ` Ard Biesheuvel
2017-01-20 15:50 ` Ryan Harkin
2017-01-20 15:51 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAD0U-h+QDPR3tyf8GNLV3XHNW5s2MkddpX0ma4zEN44ev8QVMA@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox