* [PATCH 1/9] ArmPkg/ArmLib: add ArmHasGicSystemRegisters () helper function
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 2/9] ArmPkg: use ID register helper for ArmGicArch(Sec)Lib Leif Lindholm
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
Create a helper function to eliminate direct feature register reading,
which gets messy in code shared between ARM/AArch64.
Returns BOOLEAN True if the CPU implements the GIC System Register
Interface (any version), otherwise returns BOOL False.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Include/Library/ArmLib.h | 18 ++++++++++++++++++
ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 16 ++++++++++++++++
ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 16 ++++++++++++++++
3 files changed, 50 insertions(+)
diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 5a27b7c2fc27..8a364f2ca96f 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -2,6 +2,7 @@
Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2020, NUVIA Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -715,4 +716,21 @@ ArmGetPhysicalAddressBits (
VOID
);
+
+///
+/// ID Register Helper functions
+///
+
+/**
+ Check whether the CPU supports the GIC system register interface (any version)
+
+ @return Whether GIC System Register Interface is supported
+
+**/
+BOOLEAN
+EFIAPI
+ArmHasGicSystemRegisters (
+ VOID
+ );
+
#endif // __ARM_LIB__
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
index 3fbd591192e2..5b10eb33c97d 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
@@ -2,6 +2,7 @@
Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
Portions copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2020, NUVIA Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -71,3 +72,18 @@ ArmCleanDataCache (
ArmDataSynchronizationBarrier ();
AArch64DataCacheOperation (ArmCleanDataCacheEntryBySetWay);
}
+
+/**
+ Check whether the CPU supports the GIC system register interface (any version)
+
+ @return Whether GIC System Register Interface is supported
+
+**/
+BOOLEAN
+EFIAPI
+ArmHasGicSystemRegisters (
+ VOID
+ )
+{
+ return ((ArmReadIdPfr0 () & AARCH64_PFR0_GIC) != 0);
+}
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
index 2c4a23e1a1b2..3faada3a6539 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
+++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
@@ -2,6 +2,7 @@
Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
+ Copyright (c) 2020, NUVIA Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -71,3 +72,18 @@ ArmCleanDataCache (
ArmDataSynchronizationBarrier ();
ArmV7DataCacheOperation (ArmCleanDataCacheEntryBySetWay);
}
+
+/**
+ Check whether the CPU supports the GIC system register interface (any version)
+
+ @return Whether GIC System Register Interface is supported
+
+**/
+BOOLEAN
+EFIAPI
+ArmHasGicSystemRegisters (
+ VOID
+ )
+{
+ return ((ArmReadIdPfr1 () & ARM_PFR1_GIC) != 0);
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/9] ArmPkg: use ID register helper for ArmGicArch(Sec)Lib
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
2020-12-18 14:16 ` [PATCH 1/9] ArmPkg/ArmLib: add ArmHasGicSystemRegisters () helper function Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 3/9] ArmPkg: remove duplicated ARM/AArch64 ArmGicArchLib sources Leif Lindholm
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
Use ArmHasGicSystemRegisters () instead of direct ID register tests.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 2 +-
ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 2 +-
ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c | 2 +-
ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
index 4086a294dafd..6fd69658e0e5 100644
--- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
+++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
@@ -25,7 +25,7 @@ ArmGicArchLibInitialize (
// feature is implemented on the CPU. This is also convenient as our GICv3
// driver requires SRE. If only Memory mapped access is available we try to
// drive the GIC as a v2.
- if (ArmReadIdPfr0 () & AARCH64_PFR0_GIC) {
+ if (ArmHasGicSystemRegisters ()) {
// Make sure System Register access is enabled (SRE). This depends on the
// higher privilege level giving us permission, otherwise we will either
// cause an exception here, or the write doesn't stick in which case we need
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
index 222d8059825d..7e7e46e69faa 100644
--- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
+++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
@@ -25,7 +25,7 @@ ArmGicArchLibInitialize (
// feature is implemented on the CPU. This is also convenient as our GICv3
// driver requires SRE. If only Memory mapped access is available we try to
// drive the GIC as a v2.
- if (ArmReadIdPfr1 () & ARM_PFR1_GIC) {
+ if (ArmHasGicSystemRegisters ()) {
// Make sure System Register access is enabled (SRE). This depends on the
// higher privilege level giving us permission, otherwise we will either
// cause an exception here, or the write doesn't stick in which case we need
diff --git a/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
index 4f2479e70c74..ca81951b2b2b 100644
--- a/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
+++ b/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
@@ -23,7 +23,7 @@ ArmGicGetSupportedArchRevision (
// feature is implemented on the CPU. This is also convenient as our GICv3
// driver requires SRE. If only Memory mapped access is available we try to
// drive the GIC as a v2.
- if (ArmReadIdPfr0 () & AARCH64_PFR0_GIC) {
+ if (ArmHasGicSystemRegisters ()) {
// Make sure System Register access is enabled (SRE). This depends on the
// higher privilege level giving us permission, otherwise we will either
// cause an exception here, or the write doesn't stick in which case we need
diff --git a/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
index 8e1baeee201a..2373ca409a17 100644
--- a/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
+++ b/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
@@ -23,7 +23,7 @@ ArmGicGetSupportedArchRevision (
// feature is implemented on the CPU. This is also convenient as our GICv3
// driver requires SRE. If only Memory mapped access is available we try to
// drive the GIC as a v2.
- if (ArmReadIdPfr1 () & ARM_PFR1_GIC) {
+ if (ArmHasGicSystemRegisters ()) {
// Make sure System Register access is enabled (SRE). This depends on the
// higher privilege level giving us permission, otherwise we will either
// cause an exception here, or the write doesn't stick in which case we need
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/9] ArmPkg: remove duplicated ARM/AArch64 ArmGicArchLib sources
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
2020-12-18 14:16 ` [PATCH 1/9] ArmPkg/ArmLib: add ArmHasGicSystemRegisters () helper function Leif Lindholm
2020-12-18 14:16 ` [PATCH 2/9] ArmPkg: use ID register helper for ArmGicArch(Sec)Lib Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 4/9] ArmPkg: remove duplicated ARM/AArch64 ArmGicArchSecLib sources Leif Lindholm
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
The ID register access was the only difference between them, so
after switching to the ArmHasGicSystemRegisters () helper, there
is no longer any need to have separate ARM/AArch64 source files
for ArmGicArchLib, so unify them and drop the subdirectories.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf | 7 +--
ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 60 --------------------
ArmPkg/Library/ArmGicArchLib/{AArch64 => }/ArmGicArchLib.c | 0
3 files changed, 2 insertions(+), 65 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
index 92ac11c2f5e5..bedddff93955 100644
--- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
+++ b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
@@ -14,11 +14,8 @@ [Defines]
LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION
CONSTRUCTOR = ArmGicArchLibInitialize
-[Sources.ARM]
- Arm/ArmGicArchLib.c
-
-[Sources.AARCH64]
- AArch64/ArmGicArchLib.c
+[Sources]
+ ArmGicArchLib.c
[Packages]
MdePkg/MdePkg.dec
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
deleted file mode 100644
index 7e7e46e69faa..000000000000
--- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/** @file
-*
-* Copyright (c) 2014, ARM Limited. All rights reserved.
-*
-* SPDX-License-Identifier: BSD-2-Clause-Patent
-*
-**/
-
-#include <Library/ArmLib.h>
-#include <Library/ArmGicLib.h>
-
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision;
-
-RETURN_STATUS
-EFIAPI
-ArmGicArchLibInitialize (
- VOID
- )
-{
- UINT32 IccSre;
-
- // Ideally we would like to use the GICC IIDR Architecture version here, but
- // this does not seem to be very reliable as the implementation could easily
- // get it wrong. It is more reliable to check if the GICv3 System Register
- // feature is implemented on the CPU. This is also convenient as our GICv3
- // driver requires SRE. If only Memory mapped access is available we try to
- // drive the GIC as a v2.
- if (ArmHasGicSystemRegisters ()) {
- // Make sure System Register access is enabled (SRE). This depends on the
- // higher privilege level giving us permission, otherwise we will either
- // cause an exception here, or the write doesn't stick in which case we need
- // to fall back to the GICv2 MMIO interface.
- // Note: We do not need to set ICC_SRE_EL2.Enable because the OS is started
- // at the same exception level.
- // It is the OS responsibility to set this bit.
- IccSre = ArmGicV3GetControlSystemRegisterEnable ();
- if (!(IccSre & ICC_SRE_EL2_SRE)) {
- ArmGicV3SetControlSystemRegisterEnable (IccSre| ICC_SRE_EL2_SRE);
- IccSre = ArmGicV3GetControlSystemRegisterEnable ();
- }
- if (IccSre & ICC_SRE_EL2_SRE) {
- mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
- goto Done;
- }
- }
-
- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-
-Done:
- return RETURN_SUCCESS;
-}
-
-ARM_GIC_ARCH_REVISION
-EFIAPI
-ArmGicGetSupportedArchRevision (
- VOID
- )
-{
- return mGicArchRevision;
-}
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.c
similarity index 100%
rename from ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
rename to ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.c
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/9] ArmPkg: remove duplicated ARM/AArch64 ArmGicArchSecLib sources
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
` (2 preceding siblings ...)
2020-12-18 14:16 ` [PATCH 3/9] ArmPkg: remove duplicated ARM/AArch64 ArmGicArchLib sources Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 5/9] ArmPkg: add ArmHasSecurityExtensions () helper function Leif Lindholm
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
The ID register access was the only difference between them, so
after switching to the ArmHasGicSystemRegisters () helper, there
is no longer any need to have separate ARM/AArch64 source files
for ArmGicArchSecLib, so unify them and drop the subdirectories.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf | 7 +--
ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c | 45 --------------------
ArmPkg/Library/ArmGicArchSecLib/{AArch64 => }/ArmGicArchLib.c | 0
3 files changed, 2 insertions(+), 50 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
index 1a1179a98012..ccebabe451cc 100644
--- a/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
+++ b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
@@ -13,11 +13,8 @@ [Defines]
VERSION_STRING = 1.0
LIBRARY_CLASS = ArmGicArchLib|SEC
-[Sources.ARM]
- Arm/ArmGicArchLib.c
-
-[Sources.AARCH64]
- AArch64/ArmGicArchLib.c
+[Sources]
+ ArmGicArchLib.c
[Packages]
MdePkg/MdePkg.dec
diff --git a/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
deleted file mode 100644
index 2373ca409a17..000000000000
--- a/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
+++ /dev/null
@@ -1,45 +0,0 @@
-/** @file
-*
-* Copyright (c) 2014, ARM Limited. All rights reserved.
-*
-* SPDX-License-Identifier: BSD-2-Clause-Patent
-*
-**/
-
-#include <Library/ArmLib.h>
-#include <Library/ArmGicLib.h>
-
-ARM_GIC_ARCH_REVISION
-EFIAPI
-ArmGicGetSupportedArchRevision (
- VOID
- )
-{
- UINT32 IccSre;
-
- // Ideally we would like to use the GICC IIDR Architecture version here, but
- // this does not seem to be very reliable as the implementation could easily
- // get it wrong. It is more reliable to check if the GICv3 System Register
- // feature is implemented on the CPU. This is also convenient as our GICv3
- // driver requires SRE. If only Memory mapped access is available we try to
- // drive the GIC as a v2.
- if (ArmHasGicSystemRegisters ()) {
- // Make sure System Register access is enabled (SRE). This depends on the
- // higher privilege level giving us permission, otherwise we will either
- // cause an exception here, or the write doesn't stick in which case we need
- // to fall back to the GICv2 MMIO interface.
- // Note: We do not need to set ICC_SRE_EL2.Enable because the OS is started
- // at the same exception level.
- // It is the OS responsibility to set this bit.
- IccSre = ArmGicV3GetControlSystemRegisterEnable ();
- if (!(IccSre & ICC_SRE_EL2_SRE)) {
- ArmGicV3SetControlSystemRegisterEnable (IccSre| ICC_SRE_EL2_SRE);
- IccSre = ArmGicV3GetControlSystemRegisterEnable ();
- }
- if (IccSre & ICC_SRE_EL2_SRE) {
- return ARM_GIC_ARCH_REVISION_3;
- }
- }
-
- return ARM_GIC_ARCH_REVISION_2;
-}
diff --git a/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchLib.c
similarity index 100%
rename from ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
rename to ArmPkg/Library/ArmGicArchSecLib/ArmGicArchLib.c
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/9] ArmPkg: add ArmHasSecurityExtensions () helper function
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
` (3 preceding siblings ...)
2020-12-18 14:16 ` [PATCH 4/9] ArmPkg: remove duplicated ARM/AArch64 ArmGicArchSecLib sources Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 6/9] ArmPkg: use helper to check for Security extensions in ArmArchTimerLib Leif Lindholm
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
Create a helper function to eliminate direct feature register reading.
Returns BOOLEAN True if the CPU implements the Security extensions,
otherwise returns BOOL False.
This function is only implemented for ARM, not AArch64.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Include/Library/ArmLib.h | 17 +++++++++++++++++
ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 15 +++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 8a364f2ca96f..c2ed72112b73 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -733,4 +733,21 @@ ArmHasGicSystemRegisters (
VOID
);
+#ifdef MDE_CPU_ARM
+///
+/// AArch32-only ID Register Helper functions
+///
+/**
+ Check whether the CPU supports the Security extensions
+
+ @return Whether the Security extensions are implemented
+
+**/
+BOOLEAN
+EFIAPI
+ArmHasSecurityExtensions (
+ VOID
+ );
+#endif // MDE_CPU_ARM
+
#endif // __ARM_LIB__
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
index 3faada3a6539..9f81a7223732 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
+++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
@@ -87,3 +87,18 @@ ArmHasGicSystemRegisters (
{
return ((ArmReadIdPfr1 () & ARM_PFR1_GIC) != 0);
}
+
+/**
+ Check whether the CPU supports the Security extensions
+
+ @return Whether the Security extensions are implemented
+
+**/
+BOOLEAN
+EFIAPI
+ArmHasSecurityExtensions (
+ VOID
+ )
+{
+ return ((ArmReadIdPfr1 () & ARM_PFR1_SEC) != 0);
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/9] ArmPkg: use helper to check for Security extensions in ArmArchTimerLib
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
` (4 preceding siblings ...)
2020-12-18 14:16 ` [PATCH 5/9] ArmPkg: add ArmHasSecurityExtensions () helper function Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 7/9] ArmPkg/ArmLib: delete AArch64 version of ArmReadIdPfr1 Leif Lindholm
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
Use the helper ArmHasSecurityExtensions () instead of accessing
ID_PFR1 directly. Only affects ARM build.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c
index 7c698fe471f3..24d9dae4e660 100644
--- a/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c
+++ b/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c
@@ -56,7 +56,7 @@ TimerConstructor (
// If the security extension is not implemented, set Timer Frequency
// here.
//
- if ((ArmReadIdPfr1 () & ARM_PFR1_SEC) == 0x0) {
+ if (ArmHasSecurityExtensions ()) {
ArmGenericTimerSetTimerFreq (PcdGet32 (PcdArmArchTimerFreqInHz));
}
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/9] ArmPkg/ArmLib: delete AArch64 version of ArmReadIdPfr1
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
` (5 preceding siblings ...)
2020-12-18 14:16 ` [PATCH 6/9] ArmPkg: use helper to check for Security extensions in ArmArchTimerLib Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 8/9] ArmPkg/ArmLib: rename AArch64 variant of ArmReadIdPfr0 Leif Lindholm
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
The AArch64 version of ArmReadIdPfr1 is not used by any code in tree,
or in edk2-platforms. Delete it.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 8 --------
1 file changed, 8 deletions(-)
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index 199374ff59e3..7f942c29ea66 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -453,14 +453,6 @@ ASM_FUNC(ArmReadIdPfr0)
ret
-// Q: id_aa64pfr1_el1 not defined yet. What does this function want to access?
-// A: used to setup arch timer. Check if we have security extensions, permissions to set stuff.
-// See: ArmPkg/Library/ArmArchTimerLib/AArch64/ArmArchTimerLib.c
-// Not defined yet, but stick in here for now, should read all zeros.
-ASM_FUNC(ArmReadIdPfr1)
- mrs x0, id_aa64pfr1_el1 // Read ID_PFR1 Register
- ret
-
// VOID ArmWriteHcr(UINTN Hcr)
ASM_FUNC(ArmWriteHcr)
msr hcr_el2, x0 // Write the passed HCR value
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 8/9] ArmPkg/ArmLib: rename AArch64 variant of ArmReadIdPfr0
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
` (6 preceding siblings ...)
2020-12-18 14:16 ` [PATCH 7/9] ArmPkg/ArmLib: delete AArch64 version of ArmReadIdPfr1 Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 14:16 ` [PATCH 9/9] ArmPkg/ArmLib: move ArmReadIdPfr0/1 into private header ArmV7Lib.h Leif Lindholm
2020-12-18 15:02 ` [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Ard Biesheuvel
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
ArmReadIdPfr0 is now used only inside ArmLib. Rename the AArch64
variant ArmReadIdAA64Pfr0 and add a declaration of that only into
local header AArch64/AArch64Lib.h.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h | 6 ++++++
ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 2 +-
ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 5 +++--
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h
index b2c8a8ea0b84..85bcecda730f 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h
@@ -2,6 +2,7 @@
Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2020, NUVIA Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -35,5 +36,10 @@ ArmCleanInvalidateDataCacheEntryBySetWay (
IN UINTN SetWayFormat
);
+UINTN
+EFIAPI
+ArmReadIdAA64Pfr0 (
+ VOID
+ );
#endif // __AARCH64_LIB_H__
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
index 5b10eb33c97d..53e593bc994b 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
@@ -85,5 +85,5 @@ ArmHasGicSystemRegisters (
VOID
)
{
- return ((ArmReadIdPfr0 () & AARCH64_PFR0_GIC) != 0);
+ return ((ArmReadIdAA64Pfr0 () & AARCH64_PFR0_GIC) != 0);
}
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index 7f942c29ea66..129205d2ac27 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -3,6 +3,7 @@
# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
# Copyright (c) 2011 - 2017, ARM Limited. All rights reserved.
# Copyright (c) 2016, Linaro Limited. All rights reserved.
+# Copyright (c) 2020, NUVIA Inc. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -395,7 +396,7 @@ ASM_FUNC(ArmReadVBar)
ASM_FUNC(ArmEnableVFP)
// Check whether floating-point is implemented in the processor.
mov x1, x30 // Save LR
- bl ArmReadIdPfr0 // Read EL1 Processor Feature Register (PFR0)
+ bl ArmReadIdAA64Pfr0 // Read EL1 Processor Feature Register (PFR0)
mov x30, x1 // Restore LR
ubfx x0, x0, #16, #4 // Extract the FP bits 16:19
cmp x0, #0xF // Check if FP bits are '1111b',
@@ -448,7 +449,7 @@ ASM_FUNC(ArmIsArchTimerImplemented)
ret
-ASM_FUNC(ArmReadIdPfr0)
+ASM_FUNC(ArmReadIdAA64Pfr0)
mrs x0, id_aa64pfr0_el1 // Read ID_AA64PFR0 Register
ret
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 9/9] ArmPkg/ArmLib: move ArmReadIdPfr0/1 into private header ArmV7Lib.h
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
` (7 preceding siblings ...)
2020-12-18 14:16 ` [PATCH 8/9] ArmPkg/ArmLib: rename AArch64 variant of ArmReadIdPfr0 Leif Lindholm
@ 2020-12-18 14:16 ` Leif Lindholm
2020-12-18 15:02 ` [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Ard Biesheuvel
9 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 14:16 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel
ArmReadIdPfr0 () and ArmReadIdPfr1 () are now used only inside ArmLib.
Remove the prototypes from the public header to discourage new id
register accessor additions, and direct id register access in general.
Move them into local header Arm/ArmV7Lib.h.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
ArmPkg/Include/Library/ArmLib.h | 12 ------------
ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h | 12 ++++++++++++
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index c2ed72112b73..26cb05def0a2 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -133,18 +133,6 @@ ArmIsArchTimerImplemented (
VOID
);
-UINTN
-EFIAPI
-ArmReadIdPfr0 (
- VOID
- );
-
-UINTN
-EFIAPI
-ArmReadIdPfr1 (
- VOID
- );
-
UINTN
EFIAPI
ArmCacheInfo (
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h
index 93183e67230e..bb7bda0a3aeb 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h
+++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h
@@ -48,5 +48,17 @@ ArmCleanInvalidateDataCacheEntryBySetWay (
IN UINTN SetWayFormat
);
+UINTN
+EFIAPI
+ArmReadIdPfr0 (
+ VOID
+ );
+
+UINTN
+EFIAPI
+ArmReadIdPfr1 (
+ VOID
+ );
+
#endif // __ARM_V7_LIB_H__
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/9] ArmPkg: Start cleaning up ID register handling
2020-12-18 14:16 [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Leif Lindholm
` (8 preceding siblings ...)
2020-12-18 14:16 ` [PATCH 9/9] ArmPkg/ArmLib: move ArmReadIdPfr0/1 into private header ArmV7Lib.h Leif Lindholm
@ 2020-12-18 15:02 ` Ard Biesheuvel
2020-12-18 15:36 ` Leif Lindholm
9 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2020-12-18 15:02 UTC (permalink / raw)
To: Leif Lindholm, devel
On 12/18/20 3:16 PM, Leif Lindholm wrote:
> As discussed in https://edk2.groups.io/g/devel/topic/78784065,
> the ARM and AArch64 ports were using an unfortunate overloading
> of accessor functions for the ID registers which describe the
> presence and versions of features implemented in a CPU.
>
> This makes no sense, since the layout, and indeed the names of
> the registers for the different architectural states differ.
>
> This set replaces the use of the current accessor functions
> with some helper functions in ArmLib. It also moves the
> accessor function prototypes into private headers so they
> can no longer be called from outside ArmLib
>
> As a pure side effect, we're able to throw away half of
> ArmGicArchLib/ArmGicArchSecLib.
>
> Note that some additional cleanup can also be done in the ARM
> portion of ArmMmuLib (which implements its own direct accessor
> for ID_MMFR0), and internally in ArmLib where some assembly
> code does its own feature checks, which could be moved to C
> helpers.
>
> Furthermore, it might be useful to move the ID register field
> definitions to a private header.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>
> Leif Lindholm (9):
> ArmPkg/ArmLib: add ArmHasGicSystemRegisters () helper function
> ArmPkg: use ID register helper for ArmGicArch(Sec)Lib
> ArmPkg: remove duplicated ARM/AArch64 ArmGicArchLib sources
> ArmPkg: remove duplicated ARM/AArch64 ArmGicArchSecLib sources
> ArmPkg: add ArmHasSecurityExtensions () helper function
> ArmPkg: use helper to check for Security extensions in ArmArchTimerLib
> ArmPkg/ArmLib: delete AArch64 version of ArmReadIdPfr1
> ArmPkg/ArmLib: rename AArch64 variant of ArmReadIdPfr0
> ArmPkg/ArmLib: move ArmReadIdPfr0/1 into private header ArmV7Lib.h
>
Thanks for getting started on this.
For the series,
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf | 7 +--
> ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf | 7 +--
> ArmPkg/Include/Library/ArmLib.h | 47 +++++++++++----
> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h | 6 ++
> ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h | 12 ++++
> ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c | 2 +-
> ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 60 --------------------
> ArmPkg/Library/ArmGicArchLib/{AArch64 => }/ArmGicArchLib.c | 2 +-
> ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c | 45 ---------------
> ArmPkg/Library/ArmGicArchSecLib/{AArch64 => }/ArmGicArchLib.c | 2 +-
> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 16 ++++++
> ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 31 ++++++++++
> ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 13 +----
> 13 files changed, 110 insertions(+), 140 deletions(-)
> delete mode 100644 ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> rename ArmPkg/Library/ArmGicArchLib/{AArch64 => }/ArmGicArchLib.c (94%)
> delete mode 100644 ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
> rename ArmPkg/Library/ArmGicArchSecLib/{AArch64 => }/ArmGicArchLib.c (94%)
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/9] ArmPkg: Start cleaning up ID register handling
2020-12-18 15:02 ` [PATCH 0/9] ArmPkg: Start cleaning up ID register handling Ard Biesheuvel
@ 2020-12-18 15:36 ` Leif Lindholm
0 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2020-12-18 15:36 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel
On Fri, Dec 18, 2020 at 16:02:07 +0100, Ard Biesheuvel wrote:
> On 12/18/20 3:16 PM, Leif Lindholm wrote:
> > As discussed in https://edk2.groups.io/g/devel/topic/78784065,
> > the ARM and AArch64 ports were using an unfortunate overloading
> > of accessor functions for the ID registers which describe the
> > presence and versions of features implemented in a CPU.
> >
> > This makes no sense, since the layout, and indeed the names of
> > the registers for the different architectural states differ.
> >
> > This set replaces the use of the current accessor functions
> > with some helper functions in ArmLib. It also moves the
> > accessor function prototypes into private headers so they
> > can no longer be called from outside ArmLib
> >
> > As a pure side effect, we're able to throw away half of
> > ArmGicArchLib/ArmGicArchSecLib.
> >
> > Note that some additional cleanup can also be done in the ARM
> > portion of ArmMmuLib (which implements its own direct accessor
> > for ID_MMFR0), and internally in ArmLib where some assembly
> > code does its own feature checks, which could be moved to C
> > helpers.
> >
> > Furthermore, it might be useful to move the ID register field
> > definitions to a private header.
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> >
> > Leif Lindholm (9):
> > ArmPkg/ArmLib: add ArmHasGicSystemRegisters () helper function
> > ArmPkg: use ID register helper for ArmGicArch(Sec)Lib
> > ArmPkg: remove duplicated ARM/AArch64 ArmGicArchLib sources
> > ArmPkg: remove duplicated ARM/AArch64 ArmGicArchSecLib sources
> > ArmPkg: add ArmHasSecurityExtensions () helper function
> > ArmPkg: use helper to check for Security extensions in ArmArchTimerLib
> > ArmPkg/ArmLib: delete AArch64 version of ArmReadIdPfr1
> > ArmPkg/ArmLib: rename AArch64 variant of ArmReadIdPfr0
> > ArmPkg/ArmLib: move ArmReadIdPfr0/1 into private header ArmV7Lib.h
> >
>
> Thanks for getting started on this.
>
> For the series,
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Thanks!
Pushed as 6573ae8c8575..e2bfd172e4b9 (#1244).
> > ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf | 7 +--
> > ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf | 7 +--
> > ArmPkg/Include/Library/ArmLib.h | 47 +++++++++++----
> > ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h | 6 ++
> > ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h | 12 ++++
> > ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c | 2 +-
> > ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 60 --------------------
> > ArmPkg/Library/ArmGicArchLib/{AArch64 => }/ArmGicArchLib.c | 2 +-
> > ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c | 45 ---------------
> > ArmPkg/Library/ArmGicArchSecLib/{AArch64 => }/ArmGicArchLib.c | 2 +-
> > ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 16 ++++++
> > ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 31 ++++++++++
> > ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 13 +----
> > 13 files changed, 110 insertions(+), 140 deletions(-)
> > delete mode 100644 ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> > rename ArmPkg/Library/ArmGicArchLib/{AArch64 => }/ArmGicArchLib.c (94%)
> > delete mode 100644 ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
> > rename ArmPkg/Library/ArmGicArchSecLib/{AArch64 => }/ArmGicArchLib.c (94%)
> >
>
^ permalink raw reply [flat|nested] 12+ messages in thread