On Thu, Oct 27, 2022 at 20:10:33 +0200, Ard Biesheuvel wrote:
> 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.
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
> >
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95674): https://edk2.groups.io/g/devel/message/95674
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]
-=-=-=-=-=-=-=-=-=-=-=-
