| Message ID | 1484913996-5062-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 734bd6cc41097bde7cc7d54084a042ff9b0ca0f5 |
| Headers | show |
On Fri, Jan 20, 2017 at 12:06:36PM +0000, Ard Biesheuvel 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. This looks like useful cleanup and removed duplication. > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > 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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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] _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 20 January 2017 at 14:32, Ryan Harkin <ryan.harkin@linaro.org> wrote: > 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 > Oops! Poor testing on my part: that should be FreqInHz not CntFrq _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 20 January 2017 at 14:35, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 20 January 2017 at 14:32, Ryan Harkin <ryan.harkin@linaro.org> wrote: >> 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 >> > > > Oops! Poor testing on my part: that should be FreqInHz not CntFrq Naughty! ;-) Okay, so with that change, it works on TC2, FVP Foundation & AEMv8 and Juno R0/R1/R2: Tested-by: Ryan Harkin <ryan.harkin@linaro.org> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 20 January 2017 at 15:50, Ryan Harkin <ryan.harkin@linaro.org> wrote: > On 20 January 2017 at 14:35, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> On 20 January 2017 at 14:32, Ryan Harkin <ryan.harkin@linaro.org> wrote: >>> 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 >>> >> >> >> Oops! Poor testing on my part: that should be FreqInHz not CntFrq > > Naughty! ;-) > > Okay, so with that change, it works on TC2, FVP Foundation & AEMv8 and > Juno R0/R1/R2: > > Tested-by: Ryan Harkin <ryan.harkin@linaro.org> Splendid! Thanks _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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
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> --- 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(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel