On Thu, 27 Oct 2022 at 19:31, Rebecca Cran <quic_rc...@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 <rebe...@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
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95657): https://edk2.groups.io/g/devel/message/95657
Mute This Topic: https://groups.io/mt/94609550/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
