From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web08.770.1666894247740828933 for ; Thu, 27 Oct 2022 11:10:48 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=NH8h9tfx; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 16B2A6240E for ; Thu, 27 Oct 2022 18:10:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7D894C433D7 for ; Thu, 27 Oct 2022 18:10:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1666894246; bh=HNkb3oPSkrhencuLtoW0X/95GZBz6/gZal/enjXICtY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=NH8h9tfxajbfuQiIRZijel35V2UvTJXMSQc+EjGfP7c9PS4Oa7HgQ+4EnCZQ3YWIw Jl9MAzhjC4k/vf9rdYVoQvtP7uSrmCaoidKknK4K5C2SLFWByVVRn45B7joNmzTtPf 7BocgY3vEKLfqSD6ZHasQ7EIqr8rpt79sdieuXw4hl854lIdmg1G6KDIhg8c3Cl/rf R9vBpcGENJsNqd4ZuAHa+u0IhPzROkOzxQXeNK8hAv4E/ty2fNhjpVTmjabwZ3d4G1 0U5WBsmlUnkq4Jam28Y7Lnm7Ac/62rf+8RQmJHKLrjj4SG2Q1qZLD/QcHa8fEj4Ufd ioOSTwmDdhIkQ== Received: by mail-lj1-f180.google.com with SMTP id bn35so4663810ljb.5 for ; Thu, 27 Oct 2022 11:10:46 -0700 (PDT) X-Gm-Message-State: ACrzQf2i1mhFU8V8EnA+NyZLENG7Pn4U+kmJlG/Q0cML+qZR6uMtYUP5 qYhY3VjPhwxOOP/GHkkzd6BUiFUQIOmX/CZdBB4= X-Google-Smtp-Source: AMsMyM6t481gVa9McDhoJmUJcW7i5s9pqWjag57Sd4l8mjAfYWwWwIB5ngq79q8qVw2skX7lSk541YfAERhb30TMIeM= X-Received: by 2002:ac2:4c47:0:b0:4a2:c07b:4b62 with SMTP id o7-20020ac24c47000000b004a2c07b4b62mr17437716lfk.426.1666894244375; Thu, 27 Oct 2022 11:10:44 -0700 (PDT) MIME-Version: 1.0 References: <20221027173121.754041-1-rebecca@quicinc.com> In-Reply-To: <20221027173121.754041-1-rebecca@quicinc.com> From: "Ard Biesheuvel" Date: Thu, 27 Oct 2022 20:10:33 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH 1/1] ArmPlatformPkg: Remove AP support from PrePi/PrePeiCore To: devel@edk2.groups.io, quic_rcran@quicinc.com Cc: Leif Lindholm , Ard Biesheuvel , Rebecca Cran Content-Type: text/plain; charset="UTF-8" On Thu, 27 Oct 2022 at 19:31, Rebecca Cran 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 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 > > > > > >