From 555bbc6643eaef7192f948391e5d16fae439a8c2 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 16 Jan 2025 15:21:15 +0100 Subject: [PATCH] ArmPkg/ArmGic: Drop GICv2 legacy support Support for GICv2 legacy mode was introduced to accommodate secure world firmware that was lagging behind in terms of development, and was running the GIC in v2 mode on the secure side for lack of a GICv3 driver. As per the GIC architecture, the non-secure world can only run the GIC in v3 mode if the secure world does so too. At this point, there is no longer a need to for this fallback: GICv2 support it still needed for platforms such as Raspberry Pi 4 that do not implement GICv3 at all. But on platforms that do implement it, falling back to GICv2 is unnecessary. Signed-off-by: Ard Biesheuvel --- ArmPkg/ArmPkg.dec | 3 - ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 1 - ArmPkg/Drivers/ArmGic/ArmGicLib.c | 4 -- ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 3 - ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 71 +++++++---------------- 5 files changed, 21 insertions(+), 61 deletions(-) diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index c65660658d..cdeabd7856 100644 --- a/ArmPkg/ArmPkg.dec +++ b/ArmPkg/ArmPkg.dec @@ -148,9 +148,6 @@ # it has been configured by the CPU DXE gArmTokenSpaceGuid.PcdDebuggerExceptionSupport|FALSE|BOOLEAN|0x00000032 - # Define if the GICv3 controller should use the GICv2 legacy - gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042 - # Whether to remap all unused memory NX before installing the CPU arch # protocol driver. This is needed on platforms that map all DRAM with RWX # attributes initially, and can be disabled otherwise. diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf index cd2cec248b..082a24a2aa 100644 --- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf +++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf @@ -51,7 +51,6 @@ gArmTokenSpaceGuid.PcdGicDistributorBase gArmTokenSpaceGuid.PcdGicRedistributorsBase gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase - gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy [Depex] TRUE diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c index ddfd2e5203..5daca3fde7 100644 --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c @@ -193,7 +193,6 @@ ArmGicSetInterruptPriority ( Revision = ArmGicGetSupportedArchRevision (); if ((Revision == ARM_GIC_ARCH_REVISION_2) || - FeaturePcdGet (PcdArmGicV3WithV2Legacy) || SourceIsSpi (Source)) { MmioAndThenOr32 ( @@ -237,7 +236,6 @@ ArmGicEnableInterrupt ( Revision = ArmGicGetSupportedArchRevision (); if ((Revision == ARM_GIC_ARCH_REVISION_2) || - FeaturePcdGet (PcdArmGicV3WithV2Legacy) || SourceIsSpi (Source)) { // Write set-enable register @@ -282,7 +280,6 @@ ArmGicDisableInterrupt ( Revision = ArmGicGetSupportedArchRevision (); if ((Revision == ARM_GIC_ARCH_REVISION_2) || - FeaturePcdGet (PcdArmGicV3WithV2Legacy) || SourceIsSpi (Source)) { // Write clear-enable register @@ -327,7 +324,6 @@ ArmGicIsInterruptEnabled ( Revision = ArmGicGetSupportedArchRevision (); if ((Revision == ARM_GIC_ARCH_REVISION_2) || - FeaturePcdGet (PcdArmGicV3WithV2Legacy) || SourceIsSpi (Source)) { Interrupts = MmioRead32 ( diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf index addb8d3153..7c79de8cc2 100644 --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf @@ -36,6 +36,3 @@ ArmPkg/ArmPkg.dec ArmPlatformPkg/ArmPlatformPkg.dec MdePkg/MdePkg.dec - -[FeaturePcd] - gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c index 71f8a4211e..2bcd3f2154 100644 --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c @@ -375,6 +375,7 @@ GicV3DxeInitialize ( EFI_STATUS Status; UINTN Index; UINT64 MpId; + UINT64 CpuTarget; // Make sure the Interrupt Controller Protocol is not already installed in // the system. @@ -386,9 +387,7 @@ GicV3DxeInitialize ( // We will be driving this GIC in native v3 mode, i.e., with Affinity // Routing enabled. So ensure that the ARE bit is set. - if (!FeaturePcdGet (PcdArmGicV3WithV2Legacy)) { - MmioOr32 (mGicDistributorBase + ARM_GIC_ICDDCR, ARM_GIC_ICDDCR_ARE); - } + MmioOr32 (mGicDistributorBase + ARM_GIC_ICDDCR, ARM_GIC_ICDDCR_ARE); for (Index = 0; Index < mGicNumInterrupts; Index++) { GicV3DisableInterruptSource (&gHardwareInterruptV3Protocol, Index); @@ -404,59 +403,31 @@ GicV3DxeInitialize ( // Targets the interrupts to the Primary Cpu - if (FeaturePcdGet (PcdArmGicV3WithV2Legacy)) { - UINT32 CpuTarget; + MpId = ArmReadMpidr (); + CpuTarget = MpId & + (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3); - // Only Primary CPU will run this code. We can identify our GIC CPU ID by - // reading the GIC Distributor Target register. The 8 first - // GICD_ITARGETSRn are banked to each connected CPU. These 8 registers - // hold the CPU targets fields for interrupts 0-31. More Info in the GIC - // Specification about "Interrupt Processor Targets Registers" + if ((MmioRead32 ( + mGicDistributorBase + ARM_GIC_ICDDCR + ) & ARM_GIC_ICDDCR_DS) != 0) + { + // If the Disable Security (DS) control bit is set, we are dealing with a + // GIC that has only one security state. In this case, let's assume we are + // executing in non-secure state (which is appropriate for DXE modules) + // and that no other firmware has performed any configuration on the GIC. + // This means we need to reconfigure all interrupts to non-secure Group 1 + // first. - // Read the first Interrupt Processor Targets Register (that corresponds - // to the 4 first SGIs) - CpuTarget = MmioRead32 (mGicDistributorBase + ARM_GIC_ICDIPTR); - - // The CPU target is a bit field mapping each CPU to a GIC CPU Interface. - // This value is 0 when we run on a uniprocessor platform. - if (CpuTarget != 0) { - // The 8 first Interrupt Processor Targets Registers are read-only - for (Index = 8; Index < (mGicNumInterrupts / 4); Index++) { - MmioWrite32 ( - mGicDistributorBase + ARM_GIC_ICDIPTR + (Index * 4), - CpuTarget - ); - } - } - } else { - UINT64 CpuTarget; - - MpId = ArmReadMpidr (); - CpuTarget = MpId & - (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | ARM_CORE_AFF3); - - if ((MmioRead32 ( - mGicDistributorBase + ARM_GIC_ICDDCR - ) & ARM_GIC_ICDDCR_DS) != 0) - { - // If the Disable Security (DS) control bit is set, we are dealing with a - // GIC that has only one security state. In this case, let's assume we are - // executing in non-secure state (which is appropriate for DXE modules) - // and that no other firmware has performed any configuration on the GIC. - // This means we need to reconfigure all interrupts to non-secure Group 1 - // first. + MmioWrite32 ( + mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, + 0xffffffff + ); + for (Index = 32; Index < mGicNumInterrupts; Index += 32) { MmioWrite32 ( - mGicRedistributorsBase + ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDISR, + mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, 0xffffffff ); - - for (Index = 32; Index < mGicNumInterrupts; Index += 32) { - MmioWrite32 ( - mGicDistributorBase + ARM_GIC_ICDISR + Index / 8, - 0xffffffff - ); - } } // Route the SPIs to the primary CPU. SPIs start at the INTID 32