[PATCH 0/2] Initialize CNTHCTL

[PATCH 0/2] Initialize CNTHCTL

[evan.lloyd]

From: Evan <evan.lloyd@arm.com>

A pair of minor patches to correct an omission.
This patch set enables timer access as required by the UEFI
specification.
The first patch adds accessors for the register to ArmPkg,
the second fixes the error in ArmPlatofrmPkg.
They are in different packages, but dependent.
As the responsible adults involved are the same, I hope that is
acceptable.

The code can be examined at:
https://github.com/EvanLloyd/tianocore/tree/initialize_cnthctl_v1


Sami Mujawar (2):
  ArmPkg: Add CNTHCTL_EL2 support functions
  ArmPlatformPkg: Timer access for non-secure EL1/0

 ArmPkg/Include/Chipset/AArch64.h               | 12 +++++++++++-
 ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c       |  9 ++++++++-
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 10 ++++++++++
 ArmPlatformPkg/PrePeiCore/AArch64/Helper.S     |  9 ++++++++-
 4 files changed, 37 insertions(+), 3 deletions(-)

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

[PATCH 1/2] ArmPkg: Add CNTHCTL_EL2 support functions

[evan.lloyd]

From: Sami Mujawar <sami.mujawar@arm.com>

Added helper functions for reading and writing the
CNTHCTL_EL2 register.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
---
 ArmPkg/Include/Chipset/AArch64.h               | 12 +++++++++++-
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 10 ++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Include/Chipset/AArch64.h b/ArmPkg/Include/Chipset/AArch64.h
index cebfc5da426a44bb97505ed9c1da3b2b2e35a0cc..76c2cc735ef3196273b28c7a804048d8e1f6d123 100644
--- a/ArmPkg/Include/Chipset/AArch64.h
+++ b/ArmPkg/Include/Chipset/AArch64.h
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
-  Copyright (c) 2011 - 2015, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2011 - 2017, 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
@@ -235,4 +235,14 @@ ArmWriteCptr (
   IN  UINT64 Cptr
   );
 
+UINT32
+ArmReadCntHctl (
+  VOID
+  );
+
+VOID
+ArmWriteCntHctl (
+  IN UINT32 CntHctl
+  );
+
 #endif // __AARCH64_H__
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index 6e8074a4868979d2fa3b850e1e588862179f829a..dde6a756528f3abf1bd5a142448e42122a9bd8fa 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -480,4 +480,14 @@ ASM_FUNC(ArmReadCurrentEL)
   mrs   x0, CurrentEL
   ret
 
+// UINT32 ArmReadCntHctl(VOID)
+ASM_FUNC(ArmReadCntHctl)
+  mrs   x0, cnthctl_el2
+  ret
+
+// VOID ArmWriteCntHctl(UINT32 CntHctl)
+ASM_FUNC(ArmWriteCntHctl)
+  msr   cnthctl_el2, x0
+  ret
+
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

[PATCH 2/2] ArmPlatformPkg: Timer access for non-secure EL1/0

[evan.lloyd]

From: Sami Mujawar <sami.mujawar@arm.com>

According to Section 2.3.6 of the "UEFI Specification 2.6 Errata A";
the primary CPU must be configured such that 'Timer access must be
provided to non-secure EL1 and EL0 by setting bits EL1PCTEN and
EL1PCEN in register CNTHCTL_EL2.'

This commit adds this missing set-up to the PrePi and PrePeiCore
modules.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
---
 ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c   | 9 ++++++++-
 ArmPlatformPkg/PrePeiCore/AArch64/Helper.S | 9 ++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c b/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c
index 217986107e44185c2c6e11ca8a417fad659cde2f..4da590805aaae4016952c0e5e9ce90c1897e37c4 100644
--- a/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c
+++ b/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+*  Copyright (c) 2011-2017, 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
@@ -29,5 +29,12 @@ ArchInitialize (
   if (ArmReadCurrentEL () == AARCH64_EL2) {
     // Trap General Exceptions. All exceptions that would be routed to EL1 are routed to EL2
     ArmWriteHcr (ARM_HCR_TGE);
+
+    /* Enable Timer access for non-secure EL1 and EL0
+       The cnthctl_el2 register bits are architecturally
+       UNKNOWN on reset.
+       Disable event stream as it is not in use at this stage
+    */
+    ArmWriteCntHctl (CNTHCTL_EL2_EL1PCTEN | CNTHCTL_EL2_EL1PCEN);
   }
 }
diff --git a/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S b/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S
index 5f35484b1259f8b85c370de2cde945db2b799c13..b4f35b7ff5d389ecf61c025f85bb6cf8fcce793d 100644
--- a/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S
+++ b/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S
@@ -1,5 +1,5 @@
 #========================================================================================
-#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+#  Copyright (c) 2011-2017, 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
@@ -36,6 +36,13 @@ ASM_FUNC(SetupExceptionLevel2)
 
    msr     cptr_el2, xzr          // Disable copro traps to EL2
 
+   // Enable Timer access for non-secure EL1 and EL0
+   // The cnthctl_el2 register bits are architecturally
+   // UNKNOWN on reset.
+   // Disable event stream as it is not in use at this stage
+   mov     x0, #(CNTHCTL_EL2_EL1PCTEN | CNTHCTL_EL2_EL1PCEN)
+   msr     cnthctl_el2, x0
+
    ret
 
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH 0/2] Initialize CNTHCTL

[Leif Lindholm]

Hi Evan,

Apologies for delay, was off sick last week.

On Tue, May 16, 2017 at 11:10:43AM +0100, evan.lloyd@arm.com wrote:
> From: Evan <evan.lloyd@arm.com>
> 
> A pair of minor patches to correct an omission.
> This patch set enables timer access as required by the UEFI
> specification.
> The first patch adds accessors for the register to ArmPkg,
> the second fixes the error in ArmPlatofrmPkg.
> They are in different packages, but dependent.
> As the responsible adults involved are the same, I hope that is
> acceptable.

More than acceptable, it's preferable (and means we don't break
bisect).

This looks like a clear fix - thanks.

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Pushed as 7320b8e..0e07733.

> The code can be examined at:
> https://github.com/EvanLloyd/tianocore/tree/initialize_cnthctl_v1
> 
> 
> Sami Mujawar (2):
>   ArmPkg: Add CNTHCTL_EL2 support functions
>   ArmPlatformPkg: Timer access for non-secure EL1/0
> 
>  ArmPkg/Include/Chipset/AArch64.h               | 12 +++++++++++-
>  ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c       |  9 ++++++++-
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 10 ++++++++++
>  ArmPlatformPkg/PrePeiCore/AArch64/Helper.S     |  9 ++++++++-
>  4 files changed, 37 insertions(+), 3 deletions(-)
> 
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>