The Arm and Aarch64 source files for ArmGicArchLib are copy/paste of each other with one minor difference to how they check for if the platform has GICv3.
I made the change in two patches so that the diff could be identified separately from the move commit.
The first patch makes both the Arm and Aarch64 versions identical and uses conditional compilation to handle Arm vs Aarch64.
[PATCH 1/2] ArmPkg/ArmGicLibArchLib: common check for GICv3
The second patch then removes the duplication in favour of a single source file.
[PATCH 2/2] ArmPkg/ArmGicArchLib: use common source file
Make Arm and Aarch64 both use the same code, conditionally compiled, to check if the platform has GICv3.
Both source files for Arm and Aarch64 are now identical and the next step is to move to a common source file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- .../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 26 +++++++++++++++------- ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 24 ++++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c index 9853c7b..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2; + +RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 ( + VOID + ) +{ +#if defined (MDE_CPU_ARM) + return (ArmReadIdPfr1 () & ARM_PFR1_GIC); +#elif defined(MDE_CPU_AARCH64) + return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC); +#else + #error "Unknown chipset." +#endif +}
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -41,18 +56,13 @@ ArmGicArchLibInitialize ( // It is the OS responsibility to set this bit. IccSre = ArmGicV3GetControlSystemRegisterEnable (); if (!(IccSre & ICC_SRE_EL2_SRE)) { - ArmGicV3SetControlSystemRegisterEnable (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; }
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c index f8822a2..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2; + +RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 ( + VOID + ) +{ +#if defined (MDE_CPU_ARM) + return (ArmReadIdPfr1 () & ARM_PFR1_GIC); +#elif defined(MDE_CPU_AARCH64) + return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC); +#else + #error "Unknown chipset." +#endif +}
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -46,13 +61,8 @@ ArmGicArchLibInitialize ( } if (IccSre & ICC_SRE_EL2_SRE) { mGicArchRevision = ARM_GIC_ARCH_REVISION_3; - goto Done; } } - - mGicArchRevision = ARM_GIC_ARCH_REVISION_2; - -Done: return RETURN_SUCCESS; }
On 31 July 2015 at 14:06, Ryan Harkin ryan.harkin@linaro.org wrote:
Make Arm and Aarch64 both use the same code, conditionally compiled, to check if the platform has GICv3.
Both source files for Arm and Aarch64 are now identical and the next step is to move to a common source file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 26 +++++++++++++++------- ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 24 ++++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c index 9853c7b..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
Officially, the bit indicates whether the GIC system register interface is supported at the current exception level, and from that we infer that the system most likely has a GICv3. So if we don't have the SRE enabled, we try to drive it as a GICv2 which only happens to work if this particular GICv3 has the GICv2 compatibility mode implemented. (This is also mentioned in the comments)
So the function name is a bit misleading here, although I won't insist that you change it.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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
@@ -41,18 +56,13 @@ ArmGicArchLibInitialize ( // It is the OS responsibility to set this bit. IccSre = ArmGicV3GetControlSystemRegisterEnable (); if (!(IccSre & ICC_SRE_EL2_SRE)) {
ArmGicV3SetControlSystemRegisterEnable (IccSre | ICC_SRE_EL2_SRE);
ArmGicV3SetControlSystemRegisterEnable (IccSre| ICC_SRE_EL2_SRE); IccSre = ArmGicV3GetControlSystemRegisterEnable ();
goto Done;- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-Done: return RETURN_SUCCESS; }
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c index f8822a2..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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
@@ -46,13 +61,8 @@ ArmGicArchLibInitialize ( } if (IccSre & ICC_SRE_EL2_SRE) { mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
goto Done;- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-Done: return RETURN_SUCCESS; }
-- 2.1.0
On 31 July 2015 at 13:13, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 31 July 2015 at 14:06, Ryan Harkin ryan.harkin@linaro.org wrote:
Make Arm and Aarch64 both use the same code, conditionally compiled, to check if the platform has GICv3.
Both source files for Arm and Aarch64 are now identical and the next step is to move to a common source file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 26
+++++++++++++++-------
ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 24
++++++++++++++------
2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
index 9853c7b..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision =
ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
Officially, the bit indicates whether the GIC system register interface is supported at the current exception level, and from that we infer that the system most likely has a GICv3. So if we don't have the SRE enabled, we try to drive it as a GICv2 which only happens to work if this particular GICv3 has the GICv2 compatibility mode implemented. (This is also mentioned in the comments)
So the function name is a bit misleading here, although I won't insist that you change it.
I'm happy to change it if you want to recommend a better name, this seems a but unwieldy:
HasGicv3 -> GicSystemRegistersSupported
... but is perhaps more correct.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Thanks for the quick review turnaround :-)
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // Make sure System Register access is enabled (SRE). This depends
on the
// higher privilege level giving us permission, otherwise we willeither
// cause an exception here, or the write doesn't stick in whichcase we need
@@ -41,18 +56,13 @@ ArmGicArchLibInitialize ( // It is the OS responsibility to set this bit. IccSre = ArmGicV3GetControlSystemRegisterEnable (); if (!(IccSre & ICC_SRE_EL2_SRE)) {
ArmGicV3SetControlSystemRegisterEnable (IccSre | ICC_SRE_EL2_SRE);
ArmGicV3SetControlSystemRegisterEnable (IccSre| ICC_SRE_EL2_SRE); IccSre = ArmGicV3GetControlSystemRegisterEnable ();
goto Done;- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-Done: return RETURN_SUCCESS; }
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
index f8822a2..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision =
ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // Make sure System Register access is enabled (SRE). This depends
on the
// higher privilege level giving us permission, otherwise we willeither
// cause an exception here, or the write doesn't stick in whichcase we need
@@ -46,13 +61,8 @@ ArmGicArchLibInitialize ( } if (IccSre & ICC_SRE_EL2_SRE) { mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
goto Done;- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-Done: return RETURN_SUCCESS; }
-- 2.1.0
On 31 July 2015 at 14:21, Ryan Harkin ryan.harkin@linaro.org wrote:
On 31 July 2015 at 13:13, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 31 July 2015 at 14:06, Ryan Harkin ryan.harkin@linaro.org wrote:
Make Arm and Aarch64 both use the same code, conditionally compiled, to check if the platform has GICv3.
Both source files for Arm and Aarch64 are now identical and the next step is to move to a common source file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 26 +++++++++++++++------- ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 24 ++++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c index 9853c7b..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
Officially, the bit indicates whether the GIC system register interface is supported at the current exception level, and from that we infer that the system most likely has a GICv3. So if we don't have the SRE enabled, we try to drive it as a GICv2 which only happens to work if this particular GICv3 has the GICv2 compatibility mode implemented. (This is also mentioned in the comments)
So the function name is a bit misleading here, although I won't insist that you change it.
I'm happy to change it if you want to recommend a better name, this seems a but unwieldy:
HasGicv3 -> GicSystemRegistersSupported
... but is perhaps more correct.
Well, the function and the caller are so close together that you shouldn't be able to miss the comment. I do recommend STATIC for the function though.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Thanks for the quick review turnaround :-)
Too quick, as it seems. Spotted an inadvertent white space change below :-)
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -41,18 +56,13 @@ ArmGicArchLibInitialize ( // It is the OS responsibility to set this bit. IccSre = ArmGicV3GetControlSystemRegisterEnable (); if (!(IccSre & ICC_SRE_EL2_SRE)) {
ArmGicV3SetControlSystemRegisterEnable (IccSre |ICC_SRE_EL2_SRE);
ArmGicV3SetControlSystemRegisterEnable (IccSre| ICC_SRE_EL2_SRE);
here ^^^
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; }
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c index f8822a2..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -46,13 +61,8 @@ ArmGicArchLibInitialize ( } if (IccSre & ICC_SRE_EL2_SRE) { mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
goto Done;- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-Done: return RETURN_SUCCESS; }
-- 2.1.0
On 31 July 2015 at 13:28, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 31 July 2015 at 14:21, Ryan Harkin ryan.harkin@linaro.org wrote:
On 31 July 2015 at 13:13, Ard Biesheuvel ard.biesheuvel@linaro.org
wrote:
On 31 July 2015 at 14:06, Ryan Harkin ryan.harkin@linaro.org wrote:
Make Arm and Aarch64 both use the same code, conditionally compiled,
to
check if the platform has GICv3.
Both source files for Arm and Aarch64 are now identical and the next step is to move to a common source file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 26 +++++++++++++++------- ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 24 ++++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c index 9853c7b..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
Officially, the bit indicates whether the GIC system register interface is supported at the current exception level, and from that we infer that the system most likely has a GICv3. So if we don't have the SRE enabled, we try to drive it as a GICv2 which only happens to work if this particular GICv3 has the GICv2 compatibility mode implemented. (This is also mentioned in the comments)
So the function name is a bit misleading here, although I won't insist that you change it.
I'm happy to change it if you want to recommend a better name, this
seems a
but unwieldy:
HasGicv3 -> GicSystemRegistersSupported
... but is perhaps more correct.
Well, the function and the caller are so close together that you shouldn't be able to miss the comment.
I do recommend STATIC for the function though.
Sure, good point. I'll add it for v2.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Thanks for the quick review turnaround :-)
Too quick, as it seems. Spotted an inadvertent white space change below :-)
Drat, I would have got away with it if it wasn't for you meddling kids :-)
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -41,18 +56,13 @@ ArmGicArchLibInitialize ( // It is the OS responsibility to set this bit. IccSre = ArmGicV3GetControlSystemRegisterEnable (); if (!(IccSre & ICC_SRE_EL2_SRE)) {
ArmGicV3SetControlSystemRegisterEnable (IccSre |ICC_SRE_EL2_SRE);
ArmGicV3SetControlSystemRegisterEnable (IccSre|ICC_SRE_EL2_SRE);
here ^^^
I sort-of did that on purpose to make the two files identical. The space vanished when the original copy/paste/hack operation happened and it crept back in when I made the Arm and Aarch64 files identical.
I know it's not important, but if I omit that whitespace change, then the two files won't actually be identical before moving to a single common source.
Now, do you *really* want me to drop this hunk for v2??
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; }
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c index f8822a2..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -46,13 +61,8 @@ ArmGicArchLibInitialize ( } if (IccSre & ICC_SRE_EL2_SRE) { mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
goto Done;- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-Done: return RETURN_SUCCESS; }
-- 2.1.0
On 31 July 2015 at 14:22, Ryan Harkin ryan.harkin@linaro.org wrote:
On 31 July 2015 at 13:28, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 31 July 2015 at 14:21, Ryan Harkin ryan.harkin@linaro.org wrote:
On 31 July 2015 at 13:13, Ard Biesheuvel ard.biesheuvel@linaro.org
wrote:
On 31 July 2015 at 14:06, Ryan Harkin ryan.harkin@linaro.org wrote:
Make Arm and Aarch64 both use the same code, conditionally compiled,
to
check if the platform has GICv3.
Both source files for Arm and Aarch64 are now identical and the next step is to move to a common source file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 26 +++++++++++++++------- ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 24 ++++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c index 9853c7b..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
Officially, the bit indicates whether the GIC system register interface is supported at the current exception level, and from that we infer that the system most likely has a GICv3. So if we don't have the SRE enabled, we try to drive it as a GICv2 which only happens to work if this particular GICv3 has the GICv2 compatibility mode implemented. (This is also mentioned in the comments)
So the function name is a bit misleading here, although I won't insist that you change it.
I'm happy to change it if you want to recommend a better name, this
seems a
but unwieldy:
HasGicv3 -> GicSystemRegistersSupported
... but is perhaps more correct.
Well, the function and the caller are so close together that you shouldn't be able to miss the comment.
And thinking about this, as it isn't exported, I don't need the ArmGicArchLib prefix, so I can just call it "GicSystemRegistersSupported" and be done with it.
I do recommend STATIC for the function though.
Sure, good point. I'll add it for v2.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Thanks for the quick review turnaround :-)
Too quick, as it seems. Spotted an inadvertent white space change below :-)
Drat, I would have got away with it if it wasn't for you meddling kids :-)
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -41,18 +56,13 @@ ArmGicArchLibInitialize ( // It is the OS responsibility to set this bit. IccSre = ArmGicV3GetControlSystemRegisterEnable (); if (!(IccSre & ICC_SRE_EL2_SRE)) {
ArmGicV3SetControlSystemRegisterEnable (IccSre |ICC_SRE_EL2_SRE);
ArmGicV3SetControlSystemRegisterEnable (IccSre|ICC_SRE_EL2_SRE);
here ^^^
I sort-of did that on purpose to make the two files identical. The space vanished when the original copy/paste/hack operation happened and it crept back in when I made the Arm and Aarch64 files identical.
I know it's not important, but if I omit that whitespace change, then the two files won't actually be identical before moving to a single common source.
Now, do you *really* want me to drop this hunk for v2??
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; }
diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c index f8822a2..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -46,13 +61,8 @@ ArmGicArchLibInitialize ( } if (IccSre & ICC_SRE_EL2_SRE) { mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
goto Done;- mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-Done: return RETURN_SUCCESS; }
-- 2.1.0
On 31 July 2015 at 15:27, Ryan Harkin ryan.harkin@linaro.org wrote:
On 31 July 2015 at 14:22, Ryan Harkin ryan.harkin@linaro.org wrote:
On 31 July 2015 at 13:28, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 31 July 2015 at 14:21, Ryan Harkin ryan.harkin@linaro.org wrote:
On 31 July 2015 at 13:13, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 31 July 2015 at 14:06, Ryan Harkin ryan.harkin@linaro.org wrote:
Make Arm and Aarch64 both use the same code, conditionally compiled, to check if the platform has GICv3.
Both source files for Arm and Aarch64 are now identical and the next step is to move to a common source file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 26 +++++++++++++++------- ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 24 ++++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c index 9853c7b..b092e3a 100644 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c @@ -15,7 +15,22 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision; +STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
+RETURN_STATUS +EFIAPI +ArmGicArchLibHasGicv3 (
- VOID
- )
+{ +#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
+#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
+#else
- #error "Unknown chipset."
+#endif +}
Officially, the bit indicates whether the GIC system register interface is supported at the current exception level, and from that we infer that the system most likely has a GICv3. So if we don't have the SRE enabled, we try to drive it as a GICv2 which only happens to work if this particular GICv3 has the GICv2 compatibility mode implemented. (This is also mentioned in the comments)
So the function name is a bit misleading here, although I won't insist that you change it.
I'm happy to change it if you want to recommend a better name, this seems a but unwieldy:
HasGicv3 -> GicSystemRegistersSupported
... but is perhaps more correct.
Well, the function and the caller are so close together that you shouldn't be able to miss the comment.
And thinking about this, as it isn't exported, I don't need the ArmGicArchLib prefix, so I can just call it "GicSystemRegistersSupported" and be done with it.
Indeed.
I do recommend STATIC for the function though.
Sure, good point. I'll add it for v2.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Thanks for the quick review turnaround :-)
Too quick, as it seems. Spotted an inadvertent white space change below :-)
Drat, I would have got away with it if it wasn't for you meddling kids :-)
RETURN_STATUS EFIAPI @@ -31,7 +46,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 (ArmGicArchLibHasGicv3()) { // 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 @@ -41,18 +56,13 @@ ArmGicArchLibInitialize ( // It is the OS responsibility to set this bit. IccSre = ArmGicV3GetControlSystemRegisterEnable (); if (!(IccSre & ICC_SRE_EL2_SRE)) {
ArmGicV3SetControlSystemRegisterEnable (IccSre |ICC_SRE_EL2_SRE);
ArmGicV3SetControlSystemRegisterEnable (IccSre|ICC_SRE_EL2_SRE);
here ^^^
I sort-of did that on purpose to make the two files identical. The space vanished when the original copy/paste/hack operation happened and it crept back in when I made the Arm and Aarch64 files identical.
I know it's not important, but if I omit that whitespace change, then the two files won't actually be identical before moving to a single common source.
Now, do you *really* want me to drop this hunk for v2??
i am happy as long as the version that ultimately remains has a space at either end of the |
Now that the Arm and Aarch64 source files are identical and rely on conditional compilation to provide arch specific code, remove the duplicated files and use one common file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- .../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 76 ---------------------- .../ArmGicArchLib/{Arm => }/ArmGicArchLib.c | 0 ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf | 7 +- 3 files changed, 2 insertions(+), 81 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c deleted file mode 100644 index b092e3a..0000000 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ /dev/null @@ -1,76 +0,0 @@ -/** @file -* -* Copyright (c) 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 <Library/ArmLib.h> -#include <Library/ArmGicLib.h> - -STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2; - -RETURN_STATUS -EFIAPI -ArmGicArchLibHasGicv3 ( - VOID - ) -{ -#if defined (MDE_CPU_ARM) - return (ArmReadIdPfr1 () & ARM_PFR1_GIC); -#elif defined(MDE_CPU_AARCH64) - return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC); -#else - #error "Unknown chipset." -#endif -} - -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 (ArmGicArchLibHasGicv3()) { - // 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; - } - } - return RETURN_SUCCESS; -} - -ARM_GIC_ARCH_REVISION -EFIAPI -ArmGicGetSupportedArchRevision ( - VOID - ) -{ - return mGicArchRevision; -} diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.c similarity index 100% rename from ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c rename to ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.c diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf index 7dbcb08..5c968e6 100644 --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf +++ b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf @@ -20,11 +20,8 @@ LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION CONSTRUCTOR = ArmGicArchLibInitialize
-[Sources.ARM] - Arm/ArmGicArchLib.c - -[Sources.AARCH64] - AArch64/ArmGicArchLib.c +[Sources.common] + ArmGicArchLib.c
[Packages] MdePkg/MdePkg.dec
On 31 July 2015 at 14:06, Ryan Harkin ryan.harkin@linaro.org wrote:
Now that the Arm and Aarch64 source files are identical and rely on conditional compilation to provide arch specific code, remove the duplicated files and use one common file.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
.../Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 76 ---------------------- .../ArmGicArchLib/{Arm => }/ArmGicArchLib.c | 0 ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf | 7 +- 3 files changed, 2 insertions(+), 81 deletions(-)
diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c deleted file mode 100644 index b092e3a..0000000 --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c +++ /dev/null @@ -1,76 +0,0 @@ -/** @file -* -* Copyright (c) 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 <Library/ArmLib.h> -#include <Library/ArmGicLib.h>
-STATIC ARM_GIC_ARCH_REVISION mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
-RETURN_STATUS -EFIAPI -ArmGicArchLibHasGicv3 (
- VOID
- )
-{ -#if defined (MDE_CPU_ARM)
- return (ArmReadIdPfr1 () & ARM_PFR1_GIC);
-#elif defined(MDE_CPU_AARCH64)
- return (ArmReadIdPfr0 () & AARCH64_PFR0_GIC);
-#else
- #error "Unknown chipset."
-#endif -}
-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 (ArmGicArchLibHasGicv3()) {
- // 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;- }
- }
- return RETURN_SUCCESS;
-}
-ARM_GIC_ARCH_REVISION -EFIAPI -ArmGicGetSupportedArchRevision (
- VOID
- )
-{
- return mGicArchRevision;
-} diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.c similarity index 100% rename from ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c rename to ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.c diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf index 7dbcb08..5c968e6 100644 --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf +++ b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf @@ -20,11 +20,8 @@ LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION CONSTRUCTOR = ArmGicArchLibInitialize
-[Sources.ARM]
- Arm/ArmGicArchLib.c
-[Sources.AARCH64]
- AArch64/ArmGicArchLib.c
+[Sources.common]
- ArmGicArchLib.c
[Packages] MdePkg/MdePkg.dec -- 2.1.0
-
Ard Biesheuvel -
Ryan Harkin