Even if the CPU id registers indicate hardware support for the System Register interface to the GIC, higher exception levels may disable that interface and only allow access through MMIO.
So move the enabling of the SRE bit to the GIC version detection routine: if we trigger an exception, we would have anyway at a later stage, so the net effect is the same. However, if setting the bit doesn't stick, it means we can switch to MMIO and proceed normally otherwise. (This is analogous to how the Linux kernel behaves when executed as a guest under KVM.) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> --- ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c | 14 +++++++++++++- ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c | 14 +++++++++++++- ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 8 -------- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c b/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c index 5a7837f43d94..e2955ae861d5 100644 --- a/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c +++ b/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c @@ -15,6 +15,8 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h> +#include "GicV3/ArmGicV3Lib.h" + ARM_GIC_ARCH_REVISION EFIAPI ArmGicGetSupportedArchRevision ( @@ -28,7 +30,17 @@ ArmGicGetSupportedArchRevision ( // driver requires SRE. If only Memory mapped access is available we try to // drive the GIC as a v2. if (ArmReadIdPfr0 () & AARCH64_PFR0_GIC) { - return ARM_GIC_ARCH_REVISION_3; + // Make sure System Register access is enabled (SRE). This depends on the + // lower levels 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. + ArmGicV3SetControlSystemRegisterEnable (ArmGicV3GetControlSystemRegisterEnable () | ICC_SRE_EL2_SRE); + if (ArmGicV3GetControlSystemRegisterEnable () & ICC_SRE_EL2_SRE) { + return ARM_GIC_ARCH_REVISION_3; + } } return ARM_GIC_ARCH_REVISION_2; diff --git a/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c b/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c index 668858f79a3d..02e5638f2fcf 100644 --- a/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c +++ b/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c @@ -15,6 +15,8 @@ #include <Library/ArmLib.h> #include <Library/ArmGicLib.h> +#include "GicV3/ArmGicV3Lib.h" + ARM_GIC_ARCH_REVISION EFIAPI ArmGicGetSupportedArchRevision ( @@ -28,7 +30,17 @@ ArmGicGetSupportedArchRevision ( // driver requires SRE. If only Memory mapped access is available we try to // drive the GIC as a v2. if (ArmReadIdPfr1 () & ARM_PFR1_GIC) { - return ARM_GIC_ARCH_REVISION_3; + // Make sure System Register access is enabled (SRE). This depends on the + // lower levels 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. + ArmGicV3SetControlSystemRegisterEnable (ArmGicV3GetControlSystemRegisterEnable () | ICC_SRE_EL2_SRE); + if (ArmGicV3GetControlSystemRegisterEnable () & ICC_SRE_EL2_SRE) { + return ARM_GIC_ARCH_REVISION_3; + } } return ARM_GIC_ARCH_REVISION_2; diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c index 8042f718f5b0..f756d3080386 100644 --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c @@ -281,14 +281,6 @@ GicV3DxeInitialize ( } } - // Make sure System Register access is enabled (SRE). This depends on the - // lower levels giving us permission, otherwise we will cause an exception - // here. - // 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. - ArmGicV3SetControlSystemRegisterEnable (ArmGicV3GetControlSystemRegisterEnable () | ICC_SRE_EL2_SRE); - // Set binary point reg to 0x7 (no preemption) ArmGicV3SetBinaryPointer (0x7); -- 1.8.3.2 ------------------------------------------------------------------------------ Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel