* [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore
@ 2022-10-27 17:31 Rebecca Cran
2022-10-27 18:10 ` [edk2-devel] " Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Rebecca Cran @ 2022-10-27 17:31 UTC (permalink / raw)
To: devel, Leif Lindholm, Ard Biesheuvel; +Cc: Rebecca Cran
Modern platforms use TF-A, so there's no need for support of
secondary cores in EDK2 since TF-A will keep them in a holding
pen until the PSCI_CPU_ON SMC call is received.
Therefore, remove the code that handles secondary CPUs from
PrePeiCore and PrePi and add ASSERTs if a secondary core
reaches the functions.
Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
---
ArmPlatformPkg/PrePeiCore/MainMPCore.c | 92 --------------------
ArmPlatformPkg/PrePeiCore/MainUniCore.c | 9 --
ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 37 ++++----
ArmPlatformPkg/PrePi/MainMPCore.c | 69 ---------------
ArmPlatformPkg/PrePi/MainUniCore.c | 9 --
ArmPlatformPkg/PrePi/PrePi.c | 36 ++++----
6 files changed, 34 insertions(+), 218 deletions(-)
diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
index b5d0d3a6442f..44850a4f3946 100644
--- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
+++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
@@ -12,98 +12,6 @@
#include "PrePeiCore.h"
-/*
- * This is the main function for secondary cores. They loop around until a non Null value is written to
- * SYS_FLAGS register.The SYS_FLAGS register is platform specific.
- * Note:The secondary cores, while executing secondary_main, assumes that:
- * : SGI 0 is configured as Non-secure interrupt
- * : Priority Mask is configured to allow SGI 0
- * : Interrupt Distributor and CPU interfaces are enabled
- *
- */
-VOID
-EFIAPI
-SecondaryMain (
- IN UINTN MpId
- )
-{
- EFI_STATUS Status;
- UINTN PpiListSize;
- UINTN PpiListCount;
- EFI_PEI_PPI_DESCRIPTOR *PpiList;
- ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
- UINTN Index;
- UINTN ArmCoreCount;
- ARM_CORE_INFO *ArmCoreInfoTable;
- UINT32 ClusterId;
- UINT32 CoreId;
-
- VOID (*SecondaryStart)(
- VOID
- );
- UINTN SecondaryEntryAddr;
- UINTN AcknowledgeInterrupt;
- UINTN InterruptId;
-
- ClusterId = GET_CLUSTER_ID (MpId);
- CoreId = GET_CORE_ID (MpId);
-
- // Get the gArmMpCoreInfoPpiGuid
- PpiListSize = 0;
- ArmPlatformGetPlatformPpiList (&PpiListSize, &PpiList);
- PpiListCount = PpiListSize / sizeof (EFI_PEI_PPI_DESCRIPTOR);
- for (Index = 0; Index < PpiListCount; Index++, PpiList++) {
- if (CompareGuid (PpiList->Guid, &gArmMpCoreInfoPpiGuid) == TRUE) {
- break;
- }
- }
-
- // On MP Core Platform we must implement the ARM MP Core Info PPI
- ASSERT (Index != PpiListCount);
-
- ArmMpCoreInfoPpi = PpiList->Ppi;
- ArmCoreCount = 0;
- Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
- ASSERT_EFI_ERROR (Status);
-
- // Find the core in the ArmCoreTable
- for (Index = 0; Index < ArmCoreCount; Index++) {
- if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
- (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
- {
- break;
- }
- }
-
- // The ARM Core Info Table must define every core
- ASSERT (Index != ArmCoreCount);
-
- // Clear Secondary cores MailBox
- MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
-
- do {
- ArmCallWFI ();
-
- // Read the Mailbox
- SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
-
- // Acknowledge the interrupt and send End of Interrupt signal.
- AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
- // Check if it is a valid interrupt ID
- if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
- // Got a valid SGI number hence signal End of Interrupt
- ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
- }
- } while (SecondaryEntryAddr == 0);
-
- // Jump to secondary core entry point.
- SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
- SecondaryStart ();
-
- // The secondaries shouldn't reach here
- ASSERT (FALSE);
-}
-
VOID
EFIAPI
PrimaryMain (
diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
index 1c2580eb923b..3d3c6caaa32a 100644
--- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
+++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
@@ -8,15 +8,6 @@
#include "PrePeiCore.h"
-VOID
-EFIAPI
-SecondaryMain (
- IN UINTN MpId
- )
-{
- ASSERT (FALSE);
-}
-
VOID
EFIAPI
PrimaryMain (
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
index 42a7ccc9c6a0..64d1ef601ea3 100644
--- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
+++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
@@ -117,27 +117,26 @@ CEntryPoint (
// Note: The MMU will be enabled by MemoryPeim. Only the primary core will have the MMU on.
- // If not primary Jump to Secondary Main
- if (ArmPlatformIsPrimaryCore (MpId)) {
- // Invoke "ProcessLibraryConstructorList" to have all library constructors
- // called.
- ProcessLibraryConstructorList ();
-
- PrintFirmwareVersion ();
-
- // Initialize the Debug Agent for Source Level Debugging
- InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
- SaveAndSetDebugTimerInterrupt (TRUE);
-
- // Initialize the platform specific controllers
- ArmPlatformInitialize (MpId);
-
- // Goto primary Main.
- PrimaryMain (PeiCoreEntryPoint);
- } else {
- SecondaryMain (MpId);
+ if (!ArmPlatformIsPrimaryCore (MpId)) {
+ ASSERT (FALSE);
}
+ // Invoke "ProcessLibraryConstructorList" to have all library constructors
+ // called.
+ ProcessLibraryConstructorList ();
+
+ PrintFirmwareVersion ();
+
+ // Initialize the Debug Agent for Source Level Debugging
+ InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
+ SaveAndSetDebugTimerInterrupt (TRUE);
+
+ // Initialize the platform specific controllers
+ ArmPlatformInitialize (MpId);
+
+ // Goto primary Main.
+ PrimaryMain (PeiCoreEntryPoint);
+
// PEI Core should always load and never return
ASSERT (FALSE);
}
diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c b/ArmPlatformPkg/PrePi/MainMPCore.c
index 68a7c13298d0..ce7058a2846f 100644
--- a/ArmPlatformPkg/PrePi/MainMPCore.c
+++ b/ArmPlatformPkg/PrePi/MainMPCore.c
@@ -33,72 +33,3 @@ PrimaryMain (
// We must never return
ASSERT (FALSE);
}
-
-VOID
-SecondaryMain (
- IN UINTN MpId
- )
-{
- EFI_STATUS Status;
- ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
- UINTN Index;
- UINTN ArmCoreCount;
- ARM_CORE_INFO *ArmCoreInfoTable;
- UINT32 ClusterId;
- UINT32 CoreId;
-
- VOID (*SecondaryStart)(
- VOID
- );
- UINTN SecondaryEntryAddr;
- UINTN AcknowledgeInterrupt;
- UINTN InterruptId;
-
- ClusterId = GET_CLUSTER_ID (MpId);
- CoreId = GET_CORE_ID (MpId);
-
- // On MP Core Platform we must implement the ARM MP Core Info PPI (gArmMpCoreInfoPpiGuid)
- Status = GetPlatformPpi (&gArmMpCoreInfoPpiGuid, (VOID **)&ArmMpCoreInfoPpi);
- ASSERT_EFI_ERROR (Status);
-
- ArmCoreCount = 0;
- Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
- ASSERT_EFI_ERROR (Status);
-
- // Find the core in the ArmCoreTable
- for (Index = 0; Index < ArmCoreCount; Index++) {
- if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
- (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
- {
- break;
- }
- }
-
- // The ARM Core Info Table must define every core
- ASSERT (Index != ArmCoreCount);
-
- // Clear Secondary cores MailBox
- MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
-
- do {
- ArmCallWFI ();
-
- // Read the Mailbox
- SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
-
- // Acknowledge the interrupt and send End of Interrupt signal.
- AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
- // Check if it is a valid interrupt ID
- if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
- // Got a valid SGI number hence signal End of Interrupt
- ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
- }
- } while (SecondaryEntryAddr == 0);
-
- // Jump to secondary core entry point.
- SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
- SecondaryStart ();
-
- // The secondaries shouldn't reach here
- ASSERT (FALSE);
-}
diff --git a/ArmPlatformPkg/PrePi/MainUniCore.c b/ArmPlatformPkg/PrePi/MainUniCore.c
index 6162d1241f84..7449facacd51 100644
--- a/ArmPlatformPkg/PrePi/MainUniCore.c
+++ b/ArmPlatformPkg/PrePi/MainUniCore.c
@@ -20,12 +20,3 @@ PrimaryMain (
// We must never return
ASSERT (FALSE);
}
-
-VOID
-SecondaryMain (
- IN UINTN MpId
- )
-{
- // We must never get into this function on UniCore system
- ASSERT (FALSE);
-}
diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
index 9b127b94a67c..60061b8b6963 100644
--- a/ArmPlatformPkg/PrePi/PrePi.c
+++ b/ArmPlatformPkg/PrePi/PrePi.c
@@ -177,7 +177,11 @@ CEntryPoint (
// Initialize the platform specific controllers
ArmPlatformInitialize (MpId);
- if (ArmPlatformIsPrimaryCore (MpId) && PerformanceMeasurementEnabled ()) {
+ if (!ArmPlatformIsPrimaryCore (MpId)) {
+ ASSERT (FALSE);
+ }
+
+ if (PerformanceMeasurementEnabled ()) {
// Initialize the Timer Library to setup the Timer HW controller
TimerConstructor ();
// We cannot call yet the PerformanceLib because the HOB List has not been initialized
@@ -195,29 +199,21 @@ CEntryPoint (
// Define the Global Variable region when we are not running in XIP
if (!IS_XIP ()) {
- if (ArmPlatformIsPrimaryCore (MpId)) {
- if (ArmIsMpCore ()) {
- // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
- ArmCallSEV ();
- }
- } else {
- // Wait the Primary core has defined the address of the Global Variable region (event: ARM_CPU_EVENT_DEFAULT)
- ArmCallWFE ();
+ if (ArmIsMpCore ()) {
+ // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
+ ArmCallSEV ();
}
}
- // If not primary Jump to Secondary Main
- if (ArmPlatformIsPrimaryCore (MpId)) {
- InvalidateDataCacheRange (
- (VOID *)UefiMemoryBase,
- FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
- );
+ InvalidateDataCacheRange (
+ (VOID *)UefiMemoryBase,
+ FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
+ );
- // Goto primary Main.
- PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
- } else {
- SecondaryMain (MpId);
- }
+ PrePiMain (UefiMemoryBase, StacksBase, StartTimeStamp);
+
+ // We must never return
+ ASSERT (FALSE);
// DXE Core should always load and never return
ASSERT (FALSE);
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore
2022-10-27 17:31 [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore Rebecca Cran
@ 2022-10-27 18:10 ` Ard Biesheuvel
2022-10-28 9:17 ` Leif Lindholm
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2022-10-27 18:10 UTC (permalink / raw)
To: devel, quic_rcran; +Cc: Leif Lindholm, Ard Biesheuvel, Rebecca Cran
On Thu, 27 Oct 2022 at 19:31, Rebecca Cran <quic_rcran@quicinc.com> wrote:
>
> Modern platforms use TF-A, so there's no need for support of
> secondary cores in EDK2 since TF-A will keep them in a holding
> pen until the PSCI_CPU_ON SMC call is received.
>
> Therefore, remove the code that handles secondary CPUs from
> PrePeiCore and PrePi and add ASSERTs if a secondary core
> reaches the functions.
>
> Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
No objections to this patch, but this change will break the old SMP
32-bit ARM platforms in edk2-platforms so you will need to propose a
solution for those as well.
> ---
> ArmPlatformPkg/PrePeiCore/MainMPCore.c | 92 --------------------
> ArmPlatformPkg/PrePeiCore/MainUniCore.c | 9 --
> ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 37 ++++----
> ArmPlatformPkg/PrePi/MainMPCore.c | 69 ---------------
> ArmPlatformPkg/PrePi/MainUniCore.c | 9 --
> ArmPlatformPkg/PrePi/PrePi.c | 36 ++++----
> 6 files changed, 34 insertions(+), 218 deletions(-)
>
> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> index b5d0d3a6442f..44850a4f3946 100644
> --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> @@ -12,98 +12,6 @@
>
> #include "PrePeiCore.h"
>
> -/*
> - * This is the main function for secondary cores. They loop around until a non Null value is written to
> - * SYS_FLAGS register.The SYS_FLAGS register is platform specific.
> - * Note:The secondary cores, while executing secondary_main, assumes that:
> - * : SGI 0 is configured as Non-secure interrupt
> - * : Priority Mask is configured to allow SGI 0
> - * : Interrupt Distributor and CPU interfaces are enabled
> - *
> - */
> -VOID
> -EFIAPI
> -SecondaryMain (
> - IN UINTN MpId
> - )
> -{
> - EFI_STATUS Status;
> - UINTN PpiListSize;
> - UINTN PpiListCount;
> - EFI_PEI_PPI_DESCRIPTOR *PpiList;
> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
> - UINTN Index;
> - UINTN ArmCoreCount;
> - ARM_CORE_INFO *ArmCoreInfoTable;
> - UINT32 ClusterId;
> - UINT32 CoreId;
> -
> - VOID (*SecondaryStart)(
> - VOID
> - );
> - UINTN SecondaryEntryAddr;
> - UINTN AcknowledgeInterrupt;
> - UINTN InterruptId;
> -
> - ClusterId = GET_CLUSTER_ID (MpId);
> - CoreId = GET_CORE_ID (MpId);
> -
> - // Get the gArmMpCoreInfoPpiGuid
> - PpiListSize = 0;
> - ArmPlatformGetPlatformPpiList (&PpiListSize, &PpiList);
> - PpiListCount = PpiListSize / sizeof (EFI_PEI_PPI_DESCRIPTOR);
> - for (Index = 0; Index < PpiListCount; Index++, PpiList++) {
> - if (CompareGuid (PpiList->Guid, &gArmMpCoreInfoPpiGuid) == TRUE) {
> - break;
> - }
> - }
> -
> - // On MP Core Platform we must implement the ARM MP Core Info PPI
> - ASSERT (Index != PpiListCount);
> -
> - ArmMpCoreInfoPpi = PpiList->Ppi;
> - ArmCoreCount = 0;
> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
> - ASSERT_EFI_ERROR (Status);
> -
> - // Find the core in the ArmCoreTable
> - for (Index = 0; Index < ArmCoreCount; Index++) {
> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
> - {
> - break;
> - }
> - }
> -
> - // The ARM Core Info Table must define every core
> - ASSERT (Index != ArmCoreCount);
> -
> - // Clear Secondary cores MailBox
> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
> -
> - do {
> - ArmCallWFI ();
> -
> - // Read the Mailbox
> - SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
> -
> - // Acknowledge the interrupt and send End of Interrupt signal.
> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
> - // Check if it is a valid interrupt ID
> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
> - // Got a valid SGI number hence signal End of Interrupt
> - ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
> - }
> - } while (SecondaryEntryAddr == 0);
> -
> - // Jump to secondary core entry point.
> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
> - SecondaryStart ();
> -
> - // The secondaries shouldn't reach here
> - ASSERT (FALSE);
> -}
> -
> VOID
> EFIAPI
> PrimaryMain (
> diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
> index 1c2580eb923b..3d3c6caaa32a 100644
> --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
> +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
> @@ -8,15 +8,6 @@
>
> #include "PrePeiCore.h"
>
> -VOID
> -EFIAPI
> -SecondaryMain (
> - IN UINTN MpId
> - )
> -{
> - ASSERT (FALSE);
> -}
> -
> VOID
> EFIAPI
> PrimaryMain (
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> index 42a7ccc9c6a0..64d1ef601ea3 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> @@ -117,27 +117,26 @@ CEntryPoint (
>
> // Note: The MMU will be enabled by MemoryPeim. Only the primary core will have the MMU on.
>
> - // If not primary Jump to Secondary Main
> - if (ArmPlatformIsPrimaryCore (MpId)) {
> - // Invoke "ProcessLibraryConstructorList" to have all library constructors
> - // called.
> - ProcessLibraryConstructorList ();
> -
> - PrintFirmwareVersion ();
> -
> - // Initialize the Debug Agent for Source Level Debugging
> - InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
> - SaveAndSetDebugTimerInterrupt (TRUE);
> -
> - // Initialize the platform specific controllers
> - ArmPlatformInitialize (MpId);
> -
> - // Goto primary Main.
> - PrimaryMain (PeiCoreEntryPoint);
> - } else {
> - SecondaryMain (MpId);
> + if (!ArmPlatformIsPrimaryCore (MpId)) {
> + ASSERT (FALSE);
> }
>
> + // Invoke "ProcessLibraryConstructorList" to have all library constructors
> + // called.
> + ProcessLibraryConstructorList ();
> +
> + PrintFirmwareVersion ();
> +
> + // Initialize the Debug Agent for Source Level Debugging
> + InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
> + SaveAndSetDebugTimerInterrupt (TRUE);
> +
> + // Initialize the platform specific controllers
> + ArmPlatformInitialize (MpId);
> +
> + // Goto primary Main.
> + PrimaryMain (PeiCoreEntryPoint);
> +
> // PEI Core should always load and never return
> ASSERT (FALSE);
> }
> diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c b/ArmPlatformPkg/PrePi/MainMPCore.c
> index 68a7c13298d0..ce7058a2846f 100644
> --- a/ArmPlatformPkg/PrePi/MainMPCore.c
> +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
> @@ -33,72 +33,3 @@ PrimaryMain (
> // We must never return
> ASSERT (FALSE);
> }
> -
> -VOID
> -SecondaryMain (
> - IN UINTN MpId
> - )
> -{
> - EFI_STATUS Status;
> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
> - UINTN Index;
> - UINTN ArmCoreCount;
> - ARM_CORE_INFO *ArmCoreInfoTable;
> - UINT32 ClusterId;
> - UINT32 CoreId;
> -
> - VOID (*SecondaryStart)(
> - VOID
> - );
> - UINTN SecondaryEntryAddr;
> - UINTN AcknowledgeInterrupt;
> - UINTN InterruptId;
> -
> - ClusterId = GET_CLUSTER_ID (MpId);
> - CoreId = GET_CORE_ID (MpId);
> -
> - // On MP Core Platform we must implement the ARM MP Core Info PPI (gArmMpCoreInfoPpiGuid)
> - Status = GetPlatformPpi (&gArmMpCoreInfoPpiGuid, (VOID **)&ArmMpCoreInfoPpi);
> - ASSERT_EFI_ERROR (Status);
> -
> - ArmCoreCount = 0;
> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
> - ASSERT_EFI_ERROR (Status);
> -
> - // Find the core in the ArmCoreTable
> - for (Index = 0; Index < ArmCoreCount; Index++) {
> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
> - {
> - break;
> - }
> - }
> -
> - // The ARM Core Info Table must define every core
> - ASSERT (Index != ArmCoreCount);
> -
> - // Clear Secondary cores MailBox
> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
> -
> - do {
> - ArmCallWFI ();
> -
> - // Read the Mailbox
> - SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
> -
> - // Acknowledge the interrupt and send End of Interrupt signal.
> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
> - // Check if it is a valid interrupt ID
> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
> - // Got a valid SGI number hence signal End of Interrupt
> - ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
> - }
> - } while (SecondaryEntryAddr == 0);
> -
> - // Jump to secondary core entry point.
> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
> - SecondaryStart ();
> -
> - // The secondaries shouldn't reach here
> - ASSERT (FALSE);
> -}
> diff --git a/ArmPlatformPkg/PrePi/MainUniCore.c b/ArmPlatformPkg/PrePi/MainUniCore.c
> index 6162d1241f84..7449facacd51 100644
> --- a/ArmPlatformPkg/PrePi/MainUniCore.c
> +++ b/ArmPlatformPkg/PrePi/MainUniCore.c
> @@ -20,12 +20,3 @@ PrimaryMain (
> // We must never return
> ASSERT (FALSE);
> }
> -
> -VOID
> -SecondaryMain (
> - IN UINTN MpId
> - )
> -{
> - // We must never get into this function on UniCore system
> - ASSERT (FALSE);
> -}
> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> index 9b127b94a67c..60061b8b6963 100644
> --- a/ArmPlatformPkg/PrePi/PrePi.c
> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> @@ -177,7 +177,11 @@ CEntryPoint (
> // Initialize the platform specific controllers
> ArmPlatformInitialize (MpId);
>
> - if (ArmPlatformIsPrimaryCore (MpId) && PerformanceMeasurementEnabled ()) {
> + if (!ArmPlatformIsPrimaryCore (MpId)) {
> + ASSERT (FALSE);
> + }
> +
> + if (PerformanceMeasurementEnabled ()) {
> // Initialize the Timer Library to setup the Timer HW controller
> TimerConstructor ();
> // We cannot call yet the PerformanceLib because the HOB List has not been initialized
> @@ -195,29 +199,21 @@ CEntryPoint (
>
> // Define the Global Variable region when we are not running in XIP
> if (!IS_XIP ()) {
> - if (ArmPlatformIsPrimaryCore (MpId)) {
> - if (ArmIsMpCore ()) {
> - // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
> - ArmCallSEV ();
> - }
> - } else {
> - // Wait the Primary core has defined the address of the Global Variable region (event: ARM_CPU_EVENT_DEFAULT)
> - ArmCallWFE ();
> + if (ArmIsMpCore ()) {
> + // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
> + ArmCallSEV ();
> }
> }
>
> - // If not primary Jump to Secondary Main
> - if (ArmPlatformIsPrimaryCore (MpId)) {
> - InvalidateDataCacheRange (
> - (VOID *)UefiMemoryBase,
> - FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
> - );
> + InvalidateDataCacheRange (
> + (VOID *)UefiMemoryBase,
> + FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
> + );
>
> - // Goto primary Main.
> - PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
> - } else {
> - SecondaryMain (MpId);
> - }
> + PrePiMain (UefiMemoryBase, StacksBase, StartTimeStamp);
> +
> + // We must never return
> + ASSERT (FALSE);
>
> // DXE Core should always load and never return
> ASSERT (FALSE);
> --
> 2.30.2
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore
2022-10-27 18:10 ` [edk2-devel] " Ard Biesheuvel
@ 2022-10-28 9:17 ` Leif Lindholm
2022-11-18 14:09 ` Rebecca Cran
0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2022-10-28 9:17 UTC (permalink / raw)
To: Ard Biesheuvel, Sami Mujawar, Thomas Abraham
Cc: devel, quic_rcran, Ard Biesheuvel, Rebecca Cran
On Thu, Oct 27, 2022 at 20:10:33 +0200, Ard Biesheuvel wrote:
> On Thu, 27 Oct 2022 at 19:31, Rebecca Cran <quic_rcran@quicinc.com> wrote:
> >
> > Modern platforms use TF-A, so there's no need for support of
> > secondary cores in EDK2 since TF-A will keep them in a holding
> > pen until the PSCI_CPU_ON SMC call is received.
> >
> > Therefore, remove the code that handles secondary CPUs from
> > PrePeiCore and PrePi and add ASSERTs if a secondary core
> > reaches the functions.
> >
> > Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
>
> No objections to this patch, but this change will break the old SMP
> 32-bit ARM platforms in edk2-platforms so you will need to propose a
> solution for those as well.
I think TC2 is the last one of those standing. And I don't see much
value in keeping all of this around for such a niche (and old)
platform. If someone ports TF-A to it, we could always add it back in
later.
Single-core non-PSCI platforms (i.e. beagleboard) aren't affected.
Sami: would you be OK with deleting the TC2 support in edk2-platforms?
/
Leif
> > ---
> > ArmPlatformPkg/PrePeiCore/MainMPCore.c | 92 --------------------
> > ArmPlatformPkg/PrePeiCore/MainUniCore.c | 9 --
> > ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 37 ++++----
> > ArmPlatformPkg/PrePi/MainMPCore.c | 69 ---------------
> > ArmPlatformPkg/PrePi/MainUniCore.c | 9 --
> > ArmPlatformPkg/PrePi/PrePi.c | 36 ++++----
> > 6 files changed, 34 insertions(+), 218 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> > index b5d0d3a6442f..44850a4f3946 100644
> > --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> > +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> > @@ -12,98 +12,6 @@
> >
> > #include "PrePeiCore.h"
> >
> > -/*
> > - * This is the main function for secondary cores. They loop around until a non Null value is written to
> > - * SYS_FLAGS register.The SYS_FLAGS register is platform specific.
> > - * Note:The secondary cores, while executing secondary_main, assumes that:
> > - * : SGI 0 is configured as Non-secure interrupt
> > - * : Priority Mask is configured to allow SGI 0
> > - * : Interrupt Distributor and CPU interfaces are enabled
> > - *
> > - */
> > -VOID
> > -EFIAPI
> > -SecondaryMain (
> > - IN UINTN MpId
> > - )
> > -{
> > - EFI_STATUS Status;
> > - UINTN PpiListSize;
> > - UINTN PpiListCount;
> > - EFI_PEI_PPI_DESCRIPTOR *PpiList;
> > - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
> > - UINTN Index;
> > - UINTN ArmCoreCount;
> > - ARM_CORE_INFO *ArmCoreInfoTable;
> > - UINT32 ClusterId;
> > - UINT32 CoreId;
> > -
> > - VOID (*SecondaryStart)(
> > - VOID
> > - );
> > - UINTN SecondaryEntryAddr;
> > - UINTN AcknowledgeInterrupt;
> > - UINTN InterruptId;
> > -
> > - ClusterId = GET_CLUSTER_ID (MpId);
> > - CoreId = GET_CORE_ID (MpId);
> > -
> > - // Get the gArmMpCoreInfoPpiGuid
> > - PpiListSize = 0;
> > - ArmPlatformGetPlatformPpiList (&PpiListSize, &PpiList);
> > - PpiListCount = PpiListSize / sizeof (EFI_PEI_PPI_DESCRIPTOR);
> > - for (Index = 0; Index < PpiListCount; Index++, PpiList++) {
> > - if (CompareGuid (PpiList->Guid, &gArmMpCoreInfoPpiGuid) == TRUE) {
> > - break;
> > - }
> > - }
> > -
> > - // On MP Core Platform we must implement the ARM MP Core Info PPI
> > - ASSERT (Index != PpiListCount);
> > -
> > - ArmMpCoreInfoPpi = PpiList->Ppi;
> > - ArmCoreCount = 0;
> > - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
> > - ASSERT_EFI_ERROR (Status);
> > -
> > - // Find the core in the ArmCoreTable
> > - for (Index = 0; Index < ArmCoreCount; Index++) {
> > - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
> > - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
> > - {
> > - break;
> > - }
> > - }
> > -
> > - // The ARM Core Info Table must define every core
> > - ASSERT (Index != ArmCoreCount);
> > -
> > - // Clear Secondary cores MailBox
> > - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
> > -
> > - do {
> > - ArmCallWFI ();
> > -
> > - // Read the Mailbox
> > - SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
> > -
> > - // Acknowledge the interrupt and send End of Interrupt signal.
> > - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
> > - // Check if it is a valid interrupt ID
> > - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
> > - // Got a valid SGI number hence signal End of Interrupt
> > - ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
> > - }
> > - } while (SecondaryEntryAddr == 0);
> > -
> > - // Jump to secondary core entry point.
> > - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
> > - SecondaryStart ();
> > -
> > - // The secondaries shouldn't reach here
> > - ASSERT (FALSE);
> > -}
> > -
> > VOID
> > EFIAPI
> > PrimaryMain (
> > diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
> > index 1c2580eb923b..3d3c6caaa32a 100644
> > --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
> > +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
> > @@ -8,15 +8,6 @@
> >
> > #include "PrePeiCore.h"
> >
> > -VOID
> > -EFIAPI
> > -SecondaryMain (
> > - IN UINTN MpId
> > - )
> > -{
> > - ASSERT (FALSE);
> > -}
> > -
> > VOID
> > EFIAPI
> > PrimaryMain (
> > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> > index 42a7ccc9c6a0..64d1ef601ea3 100644
> > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> > @@ -117,27 +117,26 @@ CEntryPoint (
> >
> > // Note: The MMU will be enabled by MemoryPeim. Only the primary core will have the MMU on.
> >
> > - // If not primary Jump to Secondary Main
> > - if (ArmPlatformIsPrimaryCore (MpId)) {
> > - // Invoke "ProcessLibraryConstructorList" to have all library constructors
> > - // called.
> > - ProcessLibraryConstructorList ();
> > -
> > - PrintFirmwareVersion ();
> > -
> > - // Initialize the Debug Agent for Source Level Debugging
> > - InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
> > - SaveAndSetDebugTimerInterrupt (TRUE);
> > -
> > - // Initialize the platform specific controllers
> > - ArmPlatformInitialize (MpId);
> > -
> > - // Goto primary Main.
> > - PrimaryMain (PeiCoreEntryPoint);
> > - } else {
> > - SecondaryMain (MpId);
> > + if (!ArmPlatformIsPrimaryCore (MpId)) {
> > + ASSERT (FALSE);
> > }
> >
> > + // Invoke "ProcessLibraryConstructorList" to have all library constructors
> > + // called.
> > + ProcessLibraryConstructorList ();
> > +
> > + PrintFirmwareVersion ();
> > +
> > + // Initialize the Debug Agent for Source Level Debugging
> > + InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
> > + SaveAndSetDebugTimerInterrupt (TRUE);
> > +
> > + // Initialize the platform specific controllers
> > + ArmPlatformInitialize (MpId);
> > +
> > + // Goto primary Main.
> > + PrimaryMain (PeiCoreEntryPoint);
> > +
> > // PEI Core should always load and never return
> > ASSERT (FALSE);
> > }
> > diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c b/ArmPlatformPkg/PrePi/MainMPCore.c
> > index 68a7c13298d0..ce7058a2846f 100644
> > --- a/ArmPlatformPkg/PrePi/MainMPCore.c
> > +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
> > @@ -33,72 +33,3 @@ PrimaryMain (
> > // We must never return
> > ASSERT (FALSE);
> > }
> > -
> > -VOID
> > -SecondaryMain (
> > - IN UINTN MpId
> > - )
> > -{
> > - EFI_STATUS Status;
> > - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
> > - UINTN Index;
> > - UINTN ArmCoreCount;
> > - ARM_CORE_INFO *ArmCoreInfoTable;
> > - UINT32 ClusterId;
> > - UINT32 CoreId;
> > -
> > - VOID (*SecondaryStart)(
> > - VOID
> > - );
> > - UINTN SecondaryEntryAddr;
> > - UINTN AcknowledgeInterrupt;
> > - UINTN InterruptId;
> > -
> > - ClusterId = GET_CLUSTER_ID (MpId);
> > - CoreId = GET_CORE_ID (MpId);
> > -
> > - // On MP Core Platform we must implement the ARM MP Core Info PPI (gArmMpCoreInfoPpiGuid)
> > - Status = GetPlatformPpi (&gArmMpCoreInfoPpiGuid, (VOID **)&ArmMpCoreInfoPpi);
> > - ASSERT_EFI_ERROR (Status);
> > -
> > - ArmCoreCount = 0;
> > - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
> > - ASSERT_EFI_ERROR (Status);
> > -
> > - // Find the core in the ArmCoreTable
> > - for (Index = 0; Index < ArmCoreCount; Index++) {
> > - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
> > - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
> > - {
> > - break;
> > - }
> > - }
> > -
> > - // The ARM Core Info Table must define every core
> > - ASSERT (Index != ArmCoreCount);
> > -
> > - // Clear Secondary cores MailBox
> > - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
> > -
> > - do {
> > - ArmCallWFI ();
> > -
> > - // Read the Mailbox
> > - SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
> > -
> > - // Acknowledge the interrupt and send End of Interrupt signal.
> > - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
> > - // Check if it is a valid interrupt ID
> > - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
> > - // Got a valid SGI number hence signal End of Interrupt
> > - ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
> > - }
> > - } while (SecondaryEntryAddr == 0);
> > -
> > - // Jump to secondary core entry point.
> > - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
> > - SecondaryStart ();
> > -
> > - // The secondaries shouldn't reach here
> > - ASSERT (FALSE);
> > -}
> > diff --git a/ArmPlatformPkg/PrePi/MainUniCore.c b/ArmPlatformPkg/PrePi/MainUniCore.c
> > index 6162d1241f84..7449facacd51 100644
> > --- a/ArmPlatformPkg/PrePi/MainUniCore.c
> > +++ b/ArmPlatformPkg/PrePi/MainUniCore.c
> > @@ -20,12 +20,3 @@ PrimaryMain (
> > // We must never return
> > ASSERT (FALSE);
> > }
> > -
> > -VOID
> > -SecondaryMain (
> > - IN UINTN MpId
> > - )
> > -{
> > - // We must never get into this function on UniCore system
> > - ASSERT (FALSE);
> > -}
> > diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> > index 9b127b94a67c..60061b8b6963 100644
> > --- a/ArmPlatformPkg/PrePi/PrePi.c
> > +++ b/ArmPlatformPkg/PrePi/PrePi.c
> > @@ -177,7 +177,11 @@ CEntryPoint (
> > // Initialize the platform specific controllers
> > ArmPlatformInitialize (MpId);
> >
> > - if (ArmPlatformIsPrimaryCore (MpId) && PerformanceMeasurementEnabled ()) {
> > + if (!ArmPlatformIsPrimaryCore (MpId)) {
> > + ASSERT (FALSE);
> > + }
> > +
> > + if (PerformanceMeasurementEnabled ()) {
> > // Initialize the Timer Library to setup the Timer HW controller
> > TimerConstructor ();
> > // We cannot call yet the PerformanceLib because the HOB List has not been initialized
> > @@ -195,29 +199,21 @@ CEntryPoint (
> >
> > // Define the Global Variable region when we are not running in XIP
> > if (!IS_XIP ()) {
> > - if (ArmPlatformIsPrimaryCore (MpId)) {
> > - if (ArmIsMpCore ()) {
> > - // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
> > - ArmCallSEV ();
> > - }
> > - } else {
> > - // Wait the Primary core has defined the address of the Global Variable region (event: ARM_CPU_EVENT_DEFAULT)
> > - ArmCallWFE ();
> > + if (ArmIsMpCore ()) {
> > + // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
> > + ArmCallSEV ();
> > }
> > }
> >
> > - // If not primary Jump to Secondary Main
> > - if (ArmPlatformIsPrimaryCore (MpId)) {
> > - InvalidateDataCacheRange (
> > - (VOID *)UefiMemoryBase,
> > - FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
> > - );
> > + InvalidateDataCacheRange (
> > + (VOID *)UefiMemoryBase,
> > + FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
> > + );
> >
> > - // Goto primary Main.
> > - PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
> > - } else {
> > - SecondaryMain (MpId);
> > - }
> > + PrePiMain (UefiMemoryBase, StacksBase, StartTimeStamp);
> > +
> > + // We must never return
> > + ASSERT (FALSE);
> >
> > // DXE Core should always load and never return
> > ASSERT (FALSE);
> > --
> > 2.30.2
> >
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore
2022-10-28 9:17 ` Leif Lindholm
@ 2022-11-18 14:09 ` Rebecca Cran
2022-11-24 14:51 ` Sami Mujawar
2022-11-24 14:52 ` Sami Mujawar
0 siblings, 2 replies; 6+ messages in thread
From: Rebecca Cran @ 2022-11-18 14:09 UTC (permalink / raw)
To: Leif Lindholm, Ard Biesheuvel, Sami Mujawar, Thomas Abraham
Cc: devel, quic_rcran, Ard Biesheuvel
Hi Sami,
I was wondering if you could answer Leif's question?
Thanks.
Rebecca Cran
On 10/28/22 03:17, Leif Lindholm wrote:
> On Thu, Oct 27, 2022 at 20:10:33 +0200, Ard Biesheuvel wrote:
>> On Thu, 27 Oct 2022 at 19:31, Rebecca Cran <quic_rcran@quicinc.com> wrote:
>>>
>>> Modern platforms use TF-A, so there's no need for support of
>>> secondary cores in EDK2 since TF-A will keep them in a holding
>>> pen until the PSCI_CPU_ON SMC call is received.
>>>
>>> Therefore, remove the code that handles secondary CPUs from
>>> PrePeiCore and PrePi and add ASSERTs if a secondary core
>>> reaches the functions.
>>>
>>> Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
>>
>> No objections to this patch, but this change will break the old SMP
>> 32-bit ARM platforms in edk2-platforms so you will need to propose a
>> solution for those as well.
>
> I think TC2 is the last one of those standing. And I don't see much
> value in keeping all of this around for such a niche (and old)
> platform. If someone ports TF-A to it, we could always add it back in
> later.
>
> Single-core non-PSCI platforms (i.e. beagleboard) aren't affected.
>
> Sami: would you be OK with deleting the TC2 support in edk2-platforms?
>
> /
> Leif
>
>>> ---
>>> ArmPlatformPkg/PrePeiCore/MainMPCore.c | 92 --------------------
>>> ArmPlatformPkg/PrePeiCore/MainUniCore.c | 9 --
>>> ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 37 ++++----
>>> ArmPlatformPkg/PrePi/MainMPCore.c | 69 ---------------
>>> ArmPlatformPkg/PrePi/MainUniCore.c | 9 --
>>> ArmPlatformPkg/PrePi/PrePi.c | 36 ++++----
>>> 6 files changed, 34 insertions(+), 218 deletions(-)
>>>
>>> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>> index b5d0d3a6442f..44850a4f3946 100644
>>> --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>> +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>> @@ -12,98 +12,6 @@
>>>
>>> #include "PrePeiCore.h"
>>>
>>> -/*
>>> - * This is the main function for secondary cores. They loop around until a non Null value is written to
>>> - * SYS_FLAGS register.The SYS_FLAGS register is platform specific.
>>> - * Note:The secondary cores, while executing secondary_main, assumes that:
>>> - * : SGI 0 is configured as Non-secure interrupt
>>> - * : Priority Mask is configured to allow SGI 0
>>> - * : Interrupt Distributor and CPU interfaces are enabled
>>> - *
>>> - */
>>> -VOID
>>> -EFIAPI
>>> -SecondaryMain (
>>> - IN UINTN MpId
>>> - )
>>> -{
>>> - EFI_STATUS Status;
>>> - UINTN PpiListSize;
>>> - UINTN PpiListCount;
>>> - EFI_PEI_PPI_DESCRIPTOR *PpiList;
>>> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
>>> - UINTN Index;
>>> - UINTN ArmCoreCount;
>>> - ARM_CORE_INFO *ArmCoreInfoTable;
>>> - UINT32 ClusterId;
>>> - UINT32 CoreId;
>>> -
>>> - VOID (*SecondaryStart)(
>>> - VOID
>>> - );
>>> - UINTN SecondaryEntryAddr;
>>> - UINTN AcknowledgeInterrupt;
>>> - UINTN InterruptId;
>>> -
>>> - ClusterId = GET_CLUSTER_ID (MpId);
>>> - CoreId = GET_CORE_ID (MpId);
>>> -
>>> - // Get the gArmMpCoreInfoPpiGuid
>>> - PpiListSize = 0;
>>> - ArmPlatformGetPlatformPpiList (&PpiListSize, &PpiList);
>>> - PpiListCount = PpiListSize / sizeof (EFI_PEI_PPI_DESCRIPTOR);
>>> - for (Index = 0; Index < PpiListCount; Index++, PpiList++) {
>>> - if (CompareGuid (PpiList->Guid, &gArmMpCoreInfoPpiGuid) == TRUE) {
>>> - break;
>>> - }
>>> - }
>>> -
>>> - // On MP Core Platform we must implement the ARM MP Core Info PPI
>>> - ASSERT (Index != PpiListCount);
>>> -
>>> - ArmMpCoreInfoPpi = PpiList->Ppi;
>>> - ArmCoreCount = 0;
>>> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
>>> - ASSERT_EFI_ERROR (Status);
>>> -
>>> - // Find the core in the ArmCoreTable
>>> - for (Index = 0; Index < ArmCoreCount; Index++) {
>>> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
>>> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
>>> - {
>>> - break;
>>> - }
>>> - }
>>> -
>>> - // The ARM Core Info Table must define every core
>>> - ASSERT (Index != ArmCoreCount);
>>> -
>>> - // Clear Secondary cores MailBox
>>> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
>>> -
>>> - do {
>>> - ArmCallWFI ();
>>> -
>>> - // Read the Mailbox
>>> - SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
>>> -
>>> - // Acknowledge the interrupt and send End of Interrupt signal.
>>> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
>>> - // Check if it is a valid interrupt ID
>>> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
>>> - // Got a valid SGI number hence signal End of Interrupt
>>> - ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
>>> - }
>>> - } while (SecondaryEntryAddr == 0);
>>> -
>>> - // Jump to secondary core entry point.
>>> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
>>> - SecondaryStart ();
>>> -
>>> - // The secondaries shouldn't reach here
>>> - ASSERT (FALSE);
>>> -}
>>> -
>>> VOID
>>> EFIAPI
>>> PrimaryMain (
>>> diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>> index 1c2580eb923b..3d3c6caaa32a 100644
>>> --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>> +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>> @@ -8,15 +8,6 @@
>>>
>>> #include "PrePeiCore.h"
>>>
>>> -VOID
>>> -EFIAPI
>>> -SecondaryMain (
>>> - IN UINTN MpId
>>> - )
>>> -{
>>> - ASSERT (FALSE);
>>> -}
>>> -
>>> VOID
>>> EFIAPI
>>> PrimaryMain (
>>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>> index 42a7ccc9c6a0..64d1ef601ea3 100644
>>> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>> @@ -117,27 +117,26 @@ CEntryPoint (
>>>
>>> // Note: The MMU will be enabled by MemoryPeim. Only the primary core will have the MMU on.
>>>
>>> - // If not primary Jump to Secondary Main
>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>> - // Invoke "ProcessLibraryConstructorList" to have all library constructors
>>> - // called.
>>> - ProcessLibraryConstructorList ();
>>> -
>>> - PrintFirmwareVersion ();
>>> -
>>> - // Initialize the Debug Agent for Source Level Debugging
>>> - InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>>> - SaveAndSetDebugTimerInterrupt (TRUE);
>>> -
>>> - // Initialize the platform specific controllers
>>> - ArmPlatformInitialize (MpId);
>>> -
>>> - // Goto primary Main.
>>> - PrimaryMain (PeiCoreEntryPoint);
>>> - } else {
>>> - SecondaryMain (MpId);
>>> + if (!ArmPlatformIsPrimaryCore (MpId)) {
>>> + ASSERT (FALSE);
>>> }
>>>
>>> + // Invoke "ProcessLibraryConstructorList" to have all library constructors
>>> + // called.
>>> + ProcessLibraryConstructorList ();
>>> +
>>> + PrintFirmwareVersion ();
>>> +
>>> + // Initialize the Debug Agent for Source Level Debugging
>>> + InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>>> + SaveAndSetDebugTimerInterrupt (TRUE);
>>> +
>>> + // Initialize the platform specific controllers
>>> + ArmPlatformInitialize (MpId);
>>> +
>>> + // Goto primary Main.
>>> + PrimaryMain (PeiCoreEntryPoint);
>>> +
>>> // PEI Core should always load and never return
>>> ASSERT (FALSE);
>>> }
>>> diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c b/ArmPlatformPkg/PrePi/MainMPCore.c
>>> index 68a7c13298d0..ce7058a2846f 100644
>>> --- a/ArmPlatformPkg/PrePi/MainMPCore.c
>>> +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
>>> @@ -33,72 +33,3 @@ PrimaryMain (
>>> // We must never return
>>> ASSERT (FALSE);
>>> }
>>> -
>>> -VOID
>>> -SecondaryMain (
>>> - IN UINTN MpId
>>> - )
>>> -{
>>> - EFI_STATUS Status;
>>> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
>>> - UINTN Index;
>>> - UINTN ArmCoreCount;
>>> - ARM_CORE_INFO *ArmCoreInfoTable;
>>> - UINT32 ClusterId;
>>> - UINT32 CoreId;
>>> -
>>> - VOID (*SecondaryStart)(
>>> - VOID
>>> - );
>>> - UINTN SecondaryEntryAddr;
>>> - UINTN AcknowledgeInterrupt;
>>> - UINTN InterruptId;
>>> -
>>> - ClusterId = GET_CLUSTER_ID (MpId);
>>> - CoreId = GET_CORE_ID (MpId);
>>> -
>>> - // On MP Core Platform we must implement the ARM MP Core Info PPI (gArmMpCoreInfoPpiGuid)
>>> - Status = GetPlatformPpi (&gArmMpCoreInfoPpiGuid, (VOID **)&ArmMpCoreInfoPpi);
>>> - ASSERT_EFI_ERROR (Status);
>>> -
>>> - ArmCoreCount = 0;
>>> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount, &ArmCoreInfoTable);
>>> - ASSERT_EFI_ERROR (Status);
>>> -
>>> - // Find the core in the ArmCoreTable
>>> - for (Index = 0; Index < ArmCoreCount; Index++) {
>>> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
>>> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
>>> - {
>>> - break;
>>> - }
>>> - }
>>> -
>>> - // The ARM Core Info Table must define every core
>>> - ASSERT (Index != ArmCoreCount);
>>> -
>>> - // Clear Secondary cores MailBox
>>> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, ArmCoreInfoTable[Index].MailboxClearValue);
>>> -
>>> - do {
>>> - ArmCallWFI ();
>>> -
>>> - // Read the Mailbox
>>> - SecondaryEntryAddr = MmioRead32 (ArmCoreInfoTable[Index].MailboxGetAddress);
>>> -
>>> - // Acknowledge the interrupt and send End of Interrupt signal.
>>> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), &InterruptId);
>>> - // Check if it is a valid interrupt ID
>>> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64 (PcdGicDistributorBase))) {
>>> - // Got a valid SGI number hence signal End of Interrupt
>>> - ArmGicEndOfInterrupt (PcdGet64 (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
>>> - }
>>> - } while (SecondaryEntryAddr == 0);
>>> -
>>> - // Jump to secondary core entry point.
>>> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
>>> - SecondaryStart ();
>>> -
>>> - // The secondaries shouldn't reach here
>>> - ASSERT (FALSE);
>>> -}
>>> diff --git a/ArmPlatformPkg/PrePi/MainUniCore.c b/ArmPlatformPkg/PrePi/MainUniCore.c
>>> index 6162d1241f84..7449facacd51 100644
>>> --- a/ArmPlatformPkg/PrePi/MainUniCore.c
>>> +++ b/ArmPlatformPkg/PrePi/MainUniCore.c
>>> @@ -20,12 +20,3 @@ PrimaryMain (
>>> // We must never return
>>> ASSERT (FALSE);
>>> }
>>> -
>>> -VOID
>>> -SecondaryMain (
>>> - IN UINTN MpId
>>> - )
>>> -{
>>> - // We must never get into this function on UniCore system
>>> - ASSERT (FALSE);
>>> -}
>>> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
>>> index 9b127b94a67c..60061b8b6963 100644
>>> --- a/ArmPlatformPkg/PrePi/PrePi.c
>>> +++ b/ArmPlatformPkg/PrePi/PrePi.c
>>> @@ -177,7 +177,11 @@ CEntryPoint (
>>> // Initialize the platform specific controllers
>>> ArmPlatformInitialize (MpId);
>>>
>>> - if (ArmPlatformIsPrimaryCore (MpId) && PerformanceMeasurementEnabled ()) {
>>> + if (!ArmPlatformIsPrimaryCore (MpId)) {
>>> + ASSERT (FALSE);
>>> + }
>>> +
>>> + if (PerformanceMeasurementEnabled ()) {
>>> // Initialize the Timer Library to setup the Timer HW controller
>>> TimerConstructor ();
>>> // We cannot call yet the PerformanceLib because the HOB List has not been initialized
>>> @@ -195,29 +199,21 @@ CEntryPoint (
>>>
>>> // Define the Global Variable region when we are not running in XIP
>>> if (!IS_XIP ()) {
>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>> - if (ArmIsMpCore ()) {
>>> - // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
>>> - ArmCallSEV ();
>>> - }
>>> - } else {
>>> - // Wait the Primary core has defined the address of the Global Variable region (event: ARM_CPU_EVENT_DEFAULT)
>>> - ArmCallWFE ();
>>> + if (ArmIsMpCore ()) {
>>> + // Signal the Global Variable Region is defined (event: ARM_CPU_EVENT_DEFAULT)
>>> + ArmCallSEV ();
>>> }
>>> }
>>>
>>> - // If not primary Jump to Secondary Main
>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>> - InvalidateDataCacheRange (
>>> - (VOID *)UefiMemoryBase,
>>> - FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
>>> - );
>>> + InvalidateDataCacheRange (
>>> + (VOID *)UefiMemoryBase,
>>> + FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
>>> + );
>>>
>>> - // Goto primary Main.
>>> - PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
>>> - } else {
>>> - SecondaryMain (MpId);
>>> - }
>>> + PrePiMain (UefiMemoryBase, StacksBase, StartTimeStamp);
>>> +
>>> + // We must never return
>>> + ASSERT (FALSE);
>>>
>>> // DXE Core should always load and never return
>>> ASSERT (FALSE);
>>> --
>>> 2.30.2
>>>
>>>
>>>
>>>
>>>
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore
2022-11-18 14:09 ` Rebecca Cran
@ 2022-11-24 14:51 ` Sami Mujawar
2022-11-24 14:52 ` Sami Mujawar
1 sibling, 0 replies; 6+ messages in thread
From: Sami Mujawar @ 2022-11-24 14:51 UTC (permalink / raw)
To: Rebecca Cran, Leif Lindholm, Ard Biesheuvel, Thomas Abraham
Cc: devel, quic_rcran, Ard Biesheuvel, nd@arm.com
Hi Leif, Rebecca,
Apologies for the delay in getting back.
We can drop the TC2 support in edk2-platforms.
Regards,
Sami Mujawar
On 18/11/2022 02:09 pm, Rebecca Cran wrote:
> Hi Sami,
>
> I was wondering if you could answer Leif's question?
>
> Thanks.
> Rebecca Cran
>
> On 10/28/22 03:17, Leif Lindholm wrote:
>> On Thu, Oct 27, 2022 at 20:10:33 +0200, Ard Biesheuvel wrote:
>>> On Thu, 27 Oct 2022 at 19:31, Rebecca Cran <quic_rcran@quicinc.com>
>>> wrote:
>>>>
>>>> Modern platforms use TF-A, so there's no need for support of
>>>> secondary cores in EDK2 since TF-A will keep them in a holding
>>>> pen until the PSCI_CPU_ON SMC call is received.
>>>>
>>>> Therefore, remove the code that handles secondary CPUs from
>>>> PrePeiCore and PrePi and add ASSERTs if a secondary core
>>>> reaches the functions.
>>>>
>>>> Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
>>>
>>> No objections to this patch, but this change will break the old SMP
>>> 32-bit ARM platforms in edk2-platforms so you will need to propose a
>>> solution for those as well.
>>
>> I think TC2 is the last one of those standing. And I don't see much
>> value in keeping all of this around for such a niche (and old)
>> platform. If someone ports TF-A to it, we could always add it back in
>> later.
>>
>> Single-core non-PSCI platforms (i.e. beagleboard) aren't affected.
>>
>> Sami: would you be OK with deleting the TC2 support in edk2-platforms?
>>
>> /
>> Leif
>>
>>>> ---
>>>> ArmPlatformPkg/PrePeiCore/MainMPCore.c | 92 --------------------
>>>> ArmPlatformPkg/PrePeiCore/MainUniCore.c | 9 --
>>>> ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 37 ++++----
>>>> ArmPlatformPkg/PrePi/MainMPCore.c | 69 ---------------
>>>> ArmPlatformPkg/PrePi/MainUniCore.c | 9 --
>>>> ArmPlatformPkg/PrePi/PrePi.c | 36 ++++----
>>>> 6 files changed, 34 insertions(+), 218 deletions(-)
>>>>
>>>> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> index b5d0d3a6442f..44850a4f3946 100644
>>>> --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> @@ -12,98 +12,6 @@
>>>>
>>>> #include "PrePeiCore.h"
>>>>
>>>> -/*
>>>> - * This is the main function for secondary cores. They loop around
>>>> until a non Null value is written to
>>>> - * SYS_FLAGS register.The SYS_FLAGS register is platform specific.
>>>> - * Note:The secondary cores, while executing secondary_main,
>>>> assumes that:
>>>> - * : SGI 0 is configured as Non-secure interrupt
>>>> - * : Priority Mask is configured to allow SGI 0
>>>> - * : Interrupt Distributor and CPU interfaces are enabled
>>>> - *
>>>> - */
>>>> -VOID
>>>> -EFIAPI
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - EFI_STATUS Status;
>>>> - UINTN PpiListSize;
>>>> - UINTN PpiListCount;
>>>> - EFI_PEI_PPI_DESCRIPTOR *PpiList;
>>>> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
>>>> - UINTN Index;
>>>> - UINTN ArmCoreCount;
>>>> - ARM_CORE_INFO *ArmCoreInfoTable;
>>>> - UINT32 ClusterId;
>>>> - UINT32 CoreId;
>>>> -
>>>> - VOID (*SecondaryStart)(
>>>> - VOID
>>>> - );
>>>> - UINTN SecondaryEntryAddr;
>>>> - UINTN AcknowledgeInterrupt;
>>>> - UINTN InterruptId;
>>>> -
>>>> - ClusterId = GET_CLUSTER_ID (MpId);
>>>> - CoreId = GET_CORE_ID (MpId);
>>>> -
>>>> - // Get the gArmMpCoreInfoPpiGuid
>>>> - PpiListSize = 0;
>>>> - ArmPlatformGetPlatformPpiList (&PpiListSize, &PpiList);
>>>> - PpiListCount = PpiListSize / sizeof (EFI_PEI_PPI_DESCRIPTOR);
>>>> - for (Index = 0; Index < PpiListCount; Index++, PpiList++) {
>>>> - if (CompareGuid (PpiList->Guid, &gArmMpCoreInfoPpiGuid) ==
>>>> TRUE) {
>>>> - break;
>>>> - }
>>>> - }
>>>> -
>>>> - // On MP Core Platform we must implement the ARM MP Core Info PPI
>>>> - ASSERT (Index != PpiListCount);
>>>> -
>>>> - ArmMpCoreInfoPpi = PpiList->Ppi;
>>>> - ArmCoreCount = 0;
>>>> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo
>>>> (&ArmCoreCount, &ArmCoreInfoTable);
>>>> - ASSERT_EFI_ERROR (Status);
>>>> -
>>>> - // Find the core in the ArmCoreTable
>>>> - for (Index = 0; Index < ArmCoreCount; Index++) {
>>>> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) ==
>>>> ClusterId) &&
>>>> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
>>>> - {
>>>> - break;
>>>> - }
>>>> - }
>>>> -
>>>> - // The ARM Core Info Table must define every core
>>>> - ASSERT (Index != ArmCoreCount);
>>>> -
>>>> - // Clear Secondary cores MailBox
>>>> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress,
>>>> ArmCoreInfoTable[Index].MailboxClearValue);
>>>> -
>>>> - do {
>>>> - ArmCallWFI ();
>>>> -
>>>> - // Read the Mailbox
>>>> - SecondaryEntryAddr = MmioRead32
>>>> (ArmCoreInfoTable[Index].MailboxGetAddress);
>>>> -
>>>> - // Acknowledge the interrupt and send End of Interrupt signal.
>>>> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), &InterruptId);
>>>> - // Check if it is a valid interrupt ID
>>>> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64
>>>> (PcdGicDistributorBase))) {
>>>> - // Got a valid SGI number hence signal End of Interrupt
>>>> - ArmGicEndOfInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
>>>> - }
>>>> - } while (SecondaryEntryAddr == 0);
>>>> -
>>>> - // Jump to secondary core entry point.
>>>> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
>>>> - SecondaryStart ();
>>>> -
>>>> - // The secondaries shouldn't reach here
>>>> - ASSERT (FALSE);
>>>> -}
>>>> -
>>>> VOID
>>>> EFIAPI
>>>> PrimaryMain (
>>>> diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> index 1c2580eb923b..3d3c6caaa32a 100644
>>>> --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> @@ -8,15 +8,6 @@
>>>>
>>>> #include "PrePeiCore.h"
>>>>
>>>> -VOID
>>>> -EFIAPI
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - ASSERT (FALSE);
>>>> -}
>>>> -
>>>> VOID
>>>> EFIAPI
>>>> PrimaryMain (
>>>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> index 42a7ccc9c6a0..64d1ef601ea3 100644
>>>> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> @@ -117,27 +117,26 @@ CEntryPoint (
>>>>
>>>> // Note: The MMU will be enabled by MemoryPeim. Only the
>>>> primary core will have the MMU on.
>>>>
>>>> - // If not primary Jump to Secondary Main
>>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>>> - // Invoke "ProcessLibraryConstructorList" to have all library
>>>> constructors
>>>> - // called.
>>>> - ProcessLibraryConstructorList ();
>>>> -
>>>> - PrintFirmwareVersion ();
>>>> -
>>>> - // Initialize the Debug Agent for Source Level Debugging
>>>> - InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>>>> - SaveAndSetDebugTimerInterrupt (TRUE);
>>>> -
>>>> - // Initialize the platform specific controllers
>>>> - ArmPlatformInitialize (MpId);
>>>> -
>>>> - // Goto primary Main.
>>>> - PrimaryMain (PeiCoreEntryPoint);
>>>> - } else {
>>>> - SecondaryMain (MpId);
>>>> + if (!ArmPlatformIsPrimaryCore (MpId)) {
>>>> + ASSERT (FALSE);
>>>> }
>>>>
>>>> + // Invoke "ProcessLibraryConstructorList" to have all library
>>>> constructors
>>>> + // called.
>>>> + ProcessLibraryConstructorList ();
>>>> +
>>>> + PrintFirmwareVersion ();
>>>> +
>>>> + // Initialize the Debug Agent for Source Level Debugging
>>>> + InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>>>> + SaveAndSetDebugTimerInterrupt (TRUE);
>>>> +
>>>> + // Initialize the platform specific controllers
>>>> + ArmPlatformInitialize (MpId);
>>>> +
>>>> + // Goto primary Main.
>>>> + PrimaryMain (PeiCoreEntryPoint);
>>>> +
>>>> // PEI Core should always load and never return
>>>> ASSERT (FALSE);
>>>> }
>>>> diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> b/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> index 68a7c13298d0..ce7058a2846f 100644
>>>> --- a/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> @@ -33,72 +33,3 @@ PrimaryMain (
>>>> // We must never return
>>>> ASSERT (FALSE);
>>>> }
>>>> -
>>>> -VOID
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - EFI_STATUS Status;
>>>> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
>>>> - UINTN Index;
>>>> - UINTN ArmCoreCount;
>>>> - ARM_CORE_INFO *ArmCoreInfoTable;
>>>> - UINT32 ClusterId;
>>>> - UINT32 CoreId;
>>>> -
>>>> - VOID (*SecondaryStart)(
>>>> - VOID
>>>> - );
>>>> - UINTN SecondaryEntryAddr;
>>>> - UINTN AcknowledgeInterrupt;
>>>> - UINTN InterruptId;
>>>> -
>>>> - ClusterId = GET_CLUSTER_ID (MpId);
>>>> - CoreId = GET_CORE_ID (MpId);
>>>> -
>>>> - // On MP Core Platform we must implement the ARM MP Core Info
>>>> PPI (gArmMpCoreInfoPpiGuid)
>>>> - Status = GetPlatformPpi (&gArmMpCoreInfoPpiGuid, (VOID
>>>> **)&ArmMpCoreInfoPpi);
>>>> - ASSERT_EFI_ERROR (Status);
>>>> -
>>>> - ArmCoreCount = 0;
>>>> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount,
>>>> &ArmCoreInfoTable);
>>>> - ASSERT_EFI_ERROR (Status);
>>>> -
>>>> - // Find the core in the ArmCoreTable
>>>> - for (Index = 0; Index < ArmCoreCount; Index++) {
>>>> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) ==
>>>> ClusterId) &&
>>>> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
>>>> - {
>>>> - break;
>>>> - }
>>>> - }
>>>> -
>>>> - // The ARM Core Info Table must define every core
>>>> - ASSERT (Index != ArmCoreCount);
>>>> -
>>>> - // Clear Secondary cores MailBox
>>>> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress,
>>>> ArmCoreInfoTable[Index].MailboxClearValue);
>>>> -
>>>> - do {
>>>> - ArmCallWFI ();
>>>> -
>>>> - // Read the Mailbox
>>>> - SecondaryEntryAddr = MmioRead32
>>>> (ArmCoreInfoTable[Index].MailboxGetAddress);
>>>> -
>>>> - // Acknowledge the interrupt and send End of Interrupt signal.
>>>> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), &InterruptId);
>>>> - // Check if it is a valid interrupt ID
>>>> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64
>>>> (PcdGicDistributorBase))) {
>>>> - // Got a valid SGI number hence signal End of Interrupt
>>>> - ArmGicEndOfInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
>>>> - }
>>>> - } while (SecondaryEntryAddr == 0);
>>>> -
>>>> - // Jump to secondary core entry point.
>>>> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
>>>> - SecondaryStart ();
>>>> -
>>>> - // The secondaries shouldn't reach here
>>>> - ASSERT (FALSE);
>>>> -}
>>>> diff --git a/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> b/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> index 6162d1241f84..7449facacd51 100644
>>>> --- a/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> +++ b/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> @@ -20,12 +20,3 @@ PrimaryMain (
>>>> // We must never return
>>>> ASSERT (FALSE);
>>>> }
>>>> -
>>>> -VOID
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - // We must never get into this function on UniCore system
>>>> - ASSERT (FALSE);
>>>> -}
>>>> diff --git a/ArmPlatformPkg/PrePi/PrePi.c
>>>> b/ArmPlatformPkg/PrePi/PrePi.c
>>>> index 9b127b94a67c..60061b8b6963 100644
>>>> --- a/ArmPlatformPkg/PrePi/PrePi.c
>>>> +++ b/ArmPlatformPkg/PrePi/PrePi.c
>>>> @@ -177,7 +177,11 @@ CEntryPoint (
>>>> // Initialize the platform specific controllers
>>>> ArmPlatformInitialize (MpId);
>>>>
>>>> - if (ArmPlatformIsPrimaryCore (MpId) &&
>>>> PerformanceMeasurementEnabled ()) {
>>>> + if (!ArmPlatformIsPrimaryCore (MpId)) {
>>>> + ASSERT (FALSE);
>>>> + }
>>>> +
>>>> + if (PerformanceMeasurementEnabled ()) {
>>>> // Initialize the Timer Library to setup the Timer HW controller
>>>> TimerConstructor ();
>>>> // We cannot call yet the PerformanceLib because the HOB List
>>>> has not been initialized
>>>> @@ -195,29 +199,21 @@ CEntryPoint (
>>>>
>>>> // Define the Global Variable region when we are not running in
>>>> XIP
>>>> if (!IS_XIP ()) {
>>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>>> - if (ArmIsMpCore ()) {
>>>> - // Signal the Global Variable Region is defined (event:
>>>> ARM_CPU_EVENT_DEFAULT)
>>>> - ArmCallSEV ();
>>>> - }
>>>> - } else {
>>>> - // Wait the Primary core has defined the address of the
>>>> Global Variable region (event: ARM_CPU_EVENT_DEFAULT)
>>>> - ArmCallWFE ();
>>>> + if (ArmIsMpCore ()) {
>>>> + // Signal the Global Variable Region is defined (event:
>>>> ARM_CPU_EVENT_DEFAULT)
>>>> + ArmCallSEV ();
>>>> }
>>>> }
>>>>
>>>> - // If not primary Jump to Secondary Main
>>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>>> - InvalidateDataCacheRange (
>>>> - (VOID *)UefiMemoryBase,
>>>> - FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
>>>> - );
>>>> + InvalidateDataCacheRange (
>>>> + (VOID *)UefiMemoryBase,
>>>> + FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
>>>> + );
>>>>
>>>> - // Goto primary Main.
>>>> - PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
>>>> - } else {
>>>> - SecondaryMain (MpId);
>>>> - }
>>>> + PrePiMain (UefiMemoryBase, StacksBase, StartTimeStamp);
>>>> +
>>>> + // We must never return
>>>> + ASSERT (FALSE);
>>>>
>>>> // DXE Core should always load and never return
>>>> ASSERT (FALSE);
>>>> --
>>>> 2.30.2
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore
2022-11-18 14:09 ` Rebecca Cran
2022-11-24 14:51 ` Sami Mujawar
@ 2022-11-24 14:52 ` Sami Mujawar
1 sibling, 0 replies; 6+ messages in thread
From: Sami Mujawar @ 2022-11-24 14:52 UTC (permalink / raw)
To: Rebecca Cran, Leif Lindholm, Ard Biesheuvel, Thomas Abraham
Cc: devel, quic_rcran, Ard Biesheuvel, nd@arm.com
Hi Leif, Rebecca,
Apologies for the delay in getting back.
We can drop the TC2 support in edk2-platforms.
Regards,
Sami Mujawar
On 18/11/2022 02:09 pm, Rebecca Cran wrote:
> Hi Sami,
>
> I was wondering if you could answer Leif's question?
>
> Thanks.
> Rebecca Cran
>
> On 10/28/22 03:17, Leif Lindholm wrote:
>> On Thu, Oct 27, 2022 at 20:10:33 +0200, Ard Biesheuvel wrote:
>>> On Thu, 27 Oct 2022 at 19:31, Rebecca Cran <quic_rcran@quicinc.com>
>>> wrote:
>>>>
>>>> Modern platforms use TF-A, so there's no need for support of
>>>> secondary cores in EDK2 since TF-A will keep them in a holding
>>>> pen until the PSCI_CPU_ON SMC call is received.
>>>>
>>>> Therefore, remove the code that handles secondary CPUs from
>>>> PrePeiCore and PrePi and add ASSERTs if a secondary core
>>>> reaches the functions.
>>>>
>>>> Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
>>>
>>> No objections to this patch, but this change will break the old SMP
>>> 32-bit ARM platforms in edk2-platforms so you will need to propose a
>>> solution for those as well.
>>
>> I think TC2 is the last one of those standing. And I don't see much
>> value in keeping all of this around for such a niche (and old)
>> platform. If someone ports TF-A to it, we could always add it back in
>> later.
>>
>> Single-core non-PSCI platforms (i.e. beagleboard) aren't affected.
>>
>> Sami: would you be OK with deleting the TC2 support in edk2-platforms?
>>
>> /
>> Leif
>>
>>>> ---
>>>> ArmPlatformPkg/PrePeiCore/MainMPCore.c | 92 --------------------
>>>> ArmPlatformPkg/PrePeiCore/MainUniCore.c | 9 --
>>>> ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 37 ++++----
>>>> ArmPlatformPkg/PrePi/MainMPCore.c | 69 ---------------
>>>> ArmPlatformPkg/PrePi/MainUniCore.c | 9 --
>>>> ArmPlatformPkg/PrePi/PrePi.c | 36 ++++----
>>>> 6 files changed, 34 insertions(+), 218 deletions(-)
>>>>
>>>> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> index b5d0d3a6442f..44850a4f3946 100644
>>>> --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
>>>> @@ -12,98 +12,6 @@
>>>>
>>>> #include "PrePeiCore.h"
>>>>
>>>> -/*
>>>> - * This is the main function for secondary cores. They loop around
>>>> until a non Null value is written to
>>>> - * SYS_FLAGS register.The SYS_FLAGS register is platform specific.
>>>> - * Note:The secondary cores, while executing secondary_main,
>>>> assumes that:
>>>> - * : SGI 0 is configured as Non-secure interrupt
>>>> - * : Priority Mask is configured to allow SGI 0
>>>> - * : Interrupt Distributor and CPU interfaces are enabled
>>>> - *
>>>> - */
>>>> -VOID
>>>> -EFIAPI
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - EFI_STATUS Status;
>>>> - UINTN PpiListSize;
>>>> - UINTN PpiListCount;
>>>> - EFI_PEI_PPI_DESCRIPTOR *PpiList;
>>>> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
>>>> - UINTN Index;
>>>> - UINTN ArmCoreCount;
>>>> - ARM_CORE_INFO *ArmCoreInfoTable;
>>>> - UINT32 ClusterId;
>>>> - UINT32 CoreId;
>>>> -
>>>> - VOID (*SecondaryStart)(
>>>> - VOID
>>>> - );
>>>> - UINTN SecondaryEntryAddr;
>>>> - UINTN AcknowledgeInterrupt;
>>>> - UINTN InterruptId;
>>>> -
>>>> - ClusterId = GET_CLUSTER_ID (MpId);
>>>> - CoreId = GET_CORE_ID (MpId);
>>>> -
>>>> - // Get the gArmMpCoreInfoPpiGuid
>>>> - PpiListSize = 0;
>>>> - ArmPlatformGetPlatformPpiList (&PpiListSize, &PpiList);
>>>> - PpiListCount = PpiListSize / sizeof (EFI_PEI_PPI_DESCRIPTOR);
>>>> - for (Index = 0; Index < PpiListCount; Index++, PpiList++) {
>>>> - if (CompareGuid (PpiList->Guid, &gArmMpCoreInfoPpiGuid) ==
>>>> TRUE) {
>>>> - break;
>>>> - }
>>>> - }
>>>> -
>>>> - // On MP Core Platform we must implement the ARM MP Core Info PPI
>>>> - ASSERT (Index != PpiListCount);
>>>> -
>>>> - ArmMpCoreInfoPpi = PpiList->Ppi;
>>>> - ArmCoreCount = 0;
>>>> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo
>>>> (&ArmCoreCount, &ArmCoreInfoTable);
>>>> - ASSERT_EFI_ERROR (Status);
>>>> -
>>>> - // Find the core in the ArmCoreTable
>>>> - for (Index = 0; Index < ArmCoreCount; Index++) {
>>>> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) ==
>>>> ClusterId) &&
>>>> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
>>>> - {
>>>> - break;
>>>> - }
>>>> - }
>>>> -
>>>> - // The ARM Core Info Table must define every core
>>>> - ASSERT (Index != ArmCoreCount);
>>>> -
>>>> - // Clear Secondary cores MailBox
>>>> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress,
>>>> ArmCoreInfoTable[Index].MailboxClearValue);
>>>> -
>>>> - do {
>>>> - ArmCallWFI ();
>>>> -
>>>> - // Read the Mailbox
>>>> - SecondaryEntryAddr = MmioRead32
>>>> (ArmCoreInfoTable[Index].MailboxGetAddress);
>>>> -
>>>> - // Acknowledge the interrupt and send End of Interrupt signal.
>>>> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), &InterruptId);
>>>> - // Check if it is a valid interrupt ID
>>>> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64
>>>> (PcdGicDistributorBase))) {
>>>> - // Got a valid SGI number hence signal End of Interrupt
>>>> - ArmGicEndOfInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
>>>> - }
>>>> - } while (SecondaryEntryAddr == 0);
>>>> -
>>>> - // Jump to secondary core entry point.
>>>> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
>>>> - SecondaryStart ();
>>>> -
>>>> - // The secondaries shouldn't reach here
>>>> - ASSERT (FALSE);
>>>> -}
>>>> -
>>>> VOID
>>>> EFIAPI
>>>> PrimaryMain (
>>>> diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> index 1c2580eb923b..3d3c6caaa32a 100644
>>>> --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c
>>>> @@ -8,15 +8,6 @@
>>>>
>>>> #include "PrePeiCore.h"
>>>>
>>>> -VOID
>>>> -EFIAPI
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - ASSERT (FALSE);
>>>> -}
>>>> -
>>>> VOID
>>>> EFIAPI
>>>> PrimaryMain (
>>>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> index 42a7ccc9c6a0..64d1ef601ea3 100644
>>>> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>>> @@ -117,27 +117,26 @@ CEntryPoint (
>>>>
>>>> // Note: The MMU will be enabled by MemoryPeim. Only the
>>>> primary core will have the MMU on.
>>>>
>>>> - // If not primary Jump to Secondary Main
>>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>>> - // Invoke "ProcessLibraryConstructorList" to have all library
>>>> constructors
>>>> - // called.
>>>> - ProcessLibraryConstructorList ();
>>>> -
>>>> - PrintFirmwareVersion ();
>>>> -
>>>> - // Initialize the Debug Agent for Source Level Debugging
>>>> - InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>>>> - SaveAndSetDebugTimerInterrupt (TRUE);
>>>> -
>>>> - // Initialize the platform specific controllers
>>>> - ArmPlatformInitialize (MpId);
>>>> -
>>>> - // Goto primary Main.
>>>> - PrimaryMain (PeiCoreEntryPoint);
>>>> - } else {
>>>> - SecondaryMain (MpId);
>>>> + if (!ArmPlatformIsPrimaryCore (MpId)) {
>>>> + ASSERT (FALSE);
>>>> }
>>>>
>>>> + // Invoke "ProcessLibraryConstructorList" to have all library
>>>> constructors
>>>> + // called.
>>>> + ProcessLibraryConstructorList ();
>>>> +
>>>> + PrintFirmwareVersion ();
>>>> +
>>>> + // Initialize the Debug Agent for Source Level Debugging
>>>> + InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>>>> + SaveAndSetDebugTimerInterrupt (TRUE);
>>>> +
>>>> + // Initialize the platform specific controllers
>>>> + ArmPlatformInitialize (MpId);
>>>> +
>>>> + // Goto primary Main.
>>>> + PrimaryMain (PeiCoreEntryPoint);
>>>> +
>>>> // PEI Core should always load and never return
>>>> ASSERT (FALSE);
>>>> }
>>>> diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> b/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> index 68a7c13298d0..ce7058a2846f 100644
>>>> --- a/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
>>>> @@ -33,72 +33,3 @@ PrimaryMain (
>>>> // We must never return
>>>> ASSERT (FALSE);
>>>> }
>>>> -
>>>> -VOID
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - EFI_STATUS Status;
>>>> - ARM_MP_CORE_INFO_PPI *ArmMpCoreInfoPpi;
>>>> - UINTN Index;
>>>> - UINTN ArmCoreCount;
>>>> - ARM_CORE_INFO *ArmCoreInfoTable;
>>>> - UINT32 ClusterId;
>>>> - UINT32 CoreId;
>>>> -
>>>> - VOID (*SecondaryStart)(
>>>> - VOID
>>>> - );
>>>> - UINTN SecondaryEntryAddr;
>>>> - UINTN AcknowledgeInterrupt;
>>>> - UINTN InterruptId;
>>>> -
>>>> - ClusterId = GET_CLUSTER_ID (MpId);
>>>> - CoreId = GET_CORE_ID (MpId);
>>>> -
>>>> - // On MP Core Platform we must implement the ARM MP Core Info
>>>> PPI (gArmMpCoreInfoPpiGuid)
>>>> - Status = GetPlatformPpi (&gArmMpCoreInfoPpiGuid, (VOID
>>>> **)&ArmMpCoreInfoPpi);
>>>> - ASSERT_EFI_ERROR (Status);
>>>> -
>>>> - ArmCoreCount = 0;
>>>> - Status = ArmMpCoreInfoPpi->GetMpCoreInfo (&ArmCoreCount,
>>>> &ArmCoreInfoTable);
>>>> - ASSERT_EFI_ERROR (Status);
>>>> -
>>>> - // Find the core in the ArmCoreTable
>>>> - for (Index = 0; Index < ArmCoreCount; Index++) {
>>>> - if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) ==
>>>> ClusterId) &&
>>>> - (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId))
>>>> - {
>>>> - break;
>>>> - }
>>>> - }
>>>> -
>>>> - // The ARM Core Info Table must define every core
>>>> - ASSERT (Index != ArmCoreCount);
>>>> -
>>>> - // Clear Secondary cores MailBox
>>>> - MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress,
>>>> ArmCoreInfoTable[Index].MailboxClearValue);
>>>> -
>>>> - do {
>>>> - ArmCallWFI ();
>>>> -
>>>> - // Read the Mailbox
>>>> - SecondaryEntryAddr = MmioRead32
>>>> (ArmCoreInfoTable[Index].MailboxGetAddress);
>>>> -
>>>> - // Acknowledge the interrupt and send End of Interrupt signal.
>>>> - AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), &InterruptId);
>>>> - // Check if it is a valid interrupt ID
>>>> - if (InterruptId < ArmGicGetMaxNumInterrupts (PcdGet64
>>>> (PcdGicDistributorBase))) {
>>>> - // Got a valid SGI number hence signal End of Interrupt
>>>> - ArmGicEndOfInterrupt (PcdGet64
>>>> (PcdGicInterruptInterfaceBase), AcknowledgeInterrupt);
>>>> - }
>>>> - } while (SecondaryEntryAddr == 0);
>>>> -
>>>> - // Jump to secondary core entry point.
>>>> - SecondaryStart = (VOID (*)()) SecondaryEntryAddr;
>>>> - SecondaryStart ();
>>>> -
>>>> - // The secondaries shouldn't reach here
>>>> - ASSERT (FALSE);
>>>> -}
>>>> diff --git a/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> b/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> index 6162d1241f84..7449facacd51 100644
>>>> --- a/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> +++ b/ArmPlatformPkg/PrePi/MainUniCore.c
>>>> @@ -20,12 +20,3 @@ PrimaryMain (
>>>> // We must never return
>>>> ASSERT (FALSE);
>>>> }
>>>> -
>>>> -VOID
>>>> -SecondaryMain (
>>>> - IN UINTN MpId
>>>> - )
>>>> -{
>>>> - // We must never get into this function on UniCore system
>>>> - ASSERT (FALSE);
>>>> -}
>>>> diff --git a/ArmPlatformPkg/PrePi/PrePi.c
>>>> b/ArmPlatformPkg/PrePi/PrePi.c
>>>> index 9b127b94a67c..60061b8b6963 100644
>>>> --- a/ArmPlatformPkg/PrePi/PrePi.c
>>>> +++ b/ArmPlatformPkg/PrePi/PrePi.c
>>>> @@ -177,7 +177,11 @@ CEntryPoint (
>>>> // Initialize the platform specific controllers
>>>> ArmPlatformInitialize (MpId);
>>>>
>>>> - if (ArmPlatformIsPrimaryCore (MpId) &&
>>>> PerformanceMeasurementEnabled ()) {
>>>> + if (!ArmPlatformIsPrimaryCore (MpId)) {
>>>> + ASSERT (FALSE);
>>>> + }
>>>> +
>>>> + if (PerformanceMeasurementEnabled ()) {
>>>> // Initialize the Timer Library to setup the Timer HW controller
>>>> TimerConstructor ();
>>>> // We cannot call yet the PerformanceLib because the HOB List
>>>> has not been initialized
>>>> @@ -195,29 +199,21 @@ CEntryPoint (
>>>>
>>>> // Define the Global Variable region when we are not running in
>>>> XIP
>>>> if (!IS_XIP ()) {
>>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>>> - if (ArmIsMpCore ()) {
>>>> - // Signal the Global Variable Region is defined (event:
>>>> ARM_CPU_EVENT_DEFAULT)
>>>> - ArmCallSEV ();
>>>> - }
>>>> - } else {
>>>> - // Wait the Primary core has defined the address of the
>>>> Global Variable region (event: ARM_CPU_EVENT_DEFAULT)
>>>> - ArmCallWFE ();
>>>> + if (ArmIsMpCore ()) {
>>>> + // Signal the Global Variable Region is defined (event:
>>>> ARM_CPU_EVENT_DEFAULT)
>>>> + ArmCallSEV ();
>>>> }
>>>> }
>>>>
>>>> - // If not primary Jump to Secondary Main
>>>> - if (ArmPlatformIsPrimaryCore (MpId)) {
>>>> - InvalidateDataCacheRange (
>>>> - (VOID *)UefiMemoryBase,
>>>> - FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
>>>> - );
>>>> + InvalidateDataCacheRange (
>>>> + (VOID *)UefiMemoryBase,
>>>> + FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)
>>>> + );
>>>>
>>>> - // Goto primary Main.
>>>> - PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
>>>> - } else {
>>>> - SecondaryMain (MpId);
>>>> - }
>>>> + PrePiMain (UefiMemoryBase, StacksBase, StartTimeStamp);
>>>> +
>>>> + // We must never return
>>>> + ASSERT (FALSE);
>>>>
>>>> // DXE Core should always load and never return
>>>> ASSERT (FALSE);
>>>> --
>>>> 2.30.2
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-24 14:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-27 17:31 [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore Rebecca Cran
2022-10-27 18:10 ` [edk2-devel] " Ard Biesheuvel
2022-10-28 9:17 ` Leif Lindholm
2022-11-18 14:09 ` Rebecca Cran
2022-11-24 14:51 ` Sami Mujawar
2022-11-24 14:52 ` Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox