* [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