[edk2-devel] [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore
Rebecca Cran
quic_rcran at quicinc.com
Fri Nov 18 14:09:32 UTC 2022
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 at 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 at 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 (#96498): https://edk2.groups.io/g/devel/message/96498
Mute This Topic: https://groups.io/mt/94609550/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list