This reworks the Overdrive code so that it no longer relies in the ArmMpCore code in SEC and PEI, for two reasons: - ArmMpCore is unsafe on non-XIP platforms, since the initial pen code (the SecondaryMain() function) resides in normal memory rather than NOR flash, which is not protected from being overwritten by DXE if it feels like it - on Styx, the secondaries are not released by the ROM at the same time as the boot CPU, and instead, we explicitly invoke the SCP or the EL3 firmware to boot each secondary in turn, allowing us to boot them straight into the pen we set up for handing over to the OS
Ard Biesheuvel (7): Platforms/AMD/Styx: fix incorrect usage of ASSERT_EFI_ERROR Platforms/AMD/Styx/Library/AmdStyxLib: use [Ppis] section as intended Platforms/AMD: AmdStyxHelperLib: add helper to retrieve core info array Platforms/Styx: stop using the ArmMpCoreInfo configuration table Platforms/Styx/PlatInitPei: don't bring up secondaries in PEI Platforms/Styx/FdtDxe: boot secondaries straight into OS pen Platforms/ARM/Overdrive: move to PrePeiCoreUnicore even on non-PSCI builds
Platforms/AMD/Styx/AcpiTables/Madt.c | 15 +- Platforms/AMD/Styx/Common/AmdStyxHelperLib.h | 5 + Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c | 124 +++++++++-- Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf | 8 +- Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c | 219 +------------------- Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.h | 4 - Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.inf | 3 - Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.c | 21 ++ Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.inf | 5 +- Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf | 3 + Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf | 3 + Platforms/AMD/Styx/Library/AmdStyxLib/Styx.c | 6 +- Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 4 - Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.fdf | 4 - 14 files changed, 162 insertions(+), 262 deletions(-)
The ASSERT_EFI_ERROR () macro takes a single EFI_STATUS argument, which is converted into its textual equivalent before being printed if its value does not equal EFI_SUCCESS. This macro is not appropriate for doing asserts on arbitrary truth value expressions, we have ASSERT () for that.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/AcpiTables/Madt.c | 4 ++-- Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c | 4 ++-- Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Madt.c b/Platforms/AMD/Styx/AcpiTables/Madt.c index 0138aa370779..c9ee626d7472 100644 --- a/Platforms/AMD/Styx/AcpiTables/Madt.c +++ b/Platforms/AMD/Styx/AcpiTables/Madt.c @@ -283,8 +283,8 @@ MadtHeader (
// Make sure SoC's core count does not exceed what we want to build CoreCount = ArmProcessorTable->NumberOfEntries; - ASSERT_EFI_ERROR (CoreCount > NUM_CORES); - ASSERT_EFI_ERROR (CoreCount > PcdGet32(PcdSocCoreCount)); + ASSERT (CoreCount <= NUM_CORES); + ASSERT (CoreCount <= PcdGet32(PcdSocCoreCount));
GicC = (EFI_ACPI_5_1_GIC_STRUCTURE *)&AcpiMadt.GicC[0]; AcpiMadt.Header.Header.Length = sizeof (EFI_ACPI_5_1_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER); diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c index 8e6fbfc3975e..9ec9e1d5061a 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c @@ -61,9 +61,9 @@ AmdStyxMoveParkedCores(
// Get Parking area (4KB-aligned, 4KB per core) MpParkingBase = FixedPcdGet64 (PcdParkingProtocolBase); - ASSERT_EFI_ERROR (MpParkingBase & (SIZE_4KB - 1)); + ASSERT ((MpParkingBase & (SIZE_4KB - 1)) == 0); MpParkingSize = ArmCoreCount * SIZE_4KB; - ASSERT_EFI_ERROR (MpParkingSize > FixedPcdGet64 (PcdParkingProtocolSize)); + ASSERT (MpParkingSize <= FixedPcdGet64 (PcdParkingProtocolSize));
// // Set Pen at the 2K-offset of the Parking area, skipping an 8-byte slot for the Core#. diff --git a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c index 0ceb873f7b48..a1f6cabee901 100644 --- a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c +++ b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c @@ -228,9 +228,9 @@ PlatInitPeiEntryPoint ( CpuCoreCount = IscpFuseInfo.SocConfiguration.CpuCoreCount; CpuMapSize = sizeof (IscpFuseInfo.SocConfiguration.CpuMap) * 8;
- ASSERT_EFI_ERROR (CpuMap == 0); - ASSERT_EFI_ERROR (CpuCoreCount == 0); - ASSERT_EFI_ERROR (CpuCoreCount > CpuMapSize); + ASSERT (CpuMap != 0); + ASSERT (CpuCoreCount != 0); + ASSERT (CpuCoreCount <= CpuMapSize);
// Update core count based on fusing if (mAmdCoreCount > CpuCoreCount) { @@ -259,8 +259,8 @@ PlatInitPeiEntryPoint ( Status = PeiIscpPpi->ExecuteCpuRetrieveIdTransaction ( PeiServices, &CpuResetInfo ); ASSERT_EFI_ERROR (Status); - ASSERT_EFI_ERROR (CpuResetInfo.CoreStatus.Status == CPU_CORE_DISABLED); - ASSERT_EFI_ERROR (CpuResetInfo.CoreStatus.Status == CPU_CORE_UNDEFINED); + ASSERT (CpuResetInfo.CoreStatus.Status != CPU_CORE_DISABLED); + ASSERT (CpuResetInfo.CoreStatus.Status != CPU_CORE_UNDEFINED);
mAmdMpCoreInfoTable[Index].ClusterId = CpuResetInfo.CoreStatus.ClusterId; mAmdMpCoreInfoTable[Index].CoreId = CpuResetInfo.CoreStatus.CoreId;
Declare the used PPI in the [Ppis] .inf section so that we don't have to define it explicitly in the code.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf | 3 +++ Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf | 3 +++ Platforms/AMD/Styx/Library/AmdStyxLib/Styx.c | 6 +----- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf index 13a58310145b..94ab49047d1e 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf @@ -56,6 +56,9 @@ [Guids] gAmdStyxMpCoreInfoGuid ## CONSUMER
+[Ppis] + gArmMpCoreInfoPpiGuid + [FeaturePcd] gEmbeddedTokenSpaceGuid.PcdCacheEnable gArmPlatformTokenSpaceGuid.PcdNorFlashRemapping diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf index bcdb103d1e09..3375d79f294c 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf @@ -56,6 +56,9 @@ gEmbeddedTokenSpaceGuid.PcdCacheEnable gArmPlatformTokenSpaceGuid.PcdNorFlashRemapping
+[Ppis] + gArmMpCoreInfoPpiGuid + [Pcd] gArmTokenSpaceGuid.PcdSystemMemoryBase gArmTokenSpaceGuid.PcdSystemMemorySize diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/Styx.c b/Platforms/AMD/Styx/Library/AmdStyxLib/Styx.c index 8534af0347d6..79131d965e7f 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxLib/Styx.c +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/Styx.c @@ -142,16 +142,12 @@ PrePeiCoreGetMpCoreInfo ( }
-// -// Needs to be declared in the file. Otherwise gArmMpCoreInfoPpiGuid is undefined in the contect of PrePeiCore -// -EFI_GUID mArmMpCoreInfoPpiGuid = ARM_MP_CORE_INFO_PPI_GUID; ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = { PrePeiCoreGetMpCoreInfo };
EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = { { EFI_PEI_PPI_DESCRIPTOR_PPI, - &mArmMpCoreInfoPpiGuid, + &gArmMpCoreInfoPpiGuid, &mMpCoreInfoPpi } };
Add a helper function which retrieves the ARM_CORE_INFO array directly from the HOB rather than going via the ARM_PROCESSOR_TABLE structure in the configuration table array.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/Common/AmdStyxHelperLib.h | 5 +++++ Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.c | 21 ++++++++++++++++++++ Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.inf | 5 ++++- 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/Platforms/AMD/Styx/Common/AmdStyxHelperLib.h b/Platforms/AMD/Styx/Common/AmdStyxHelperLib.h index 8fb6bc4b6c0d..c4598fe7d108 100644 --- a/Platforms/AMD/Styx/Common/AmdStyxHelperLib.h +++ b/Platforms/AMD/Styx/Common/AmdStyxHelperLib.h @@ -39,4 +39,9 @@ AmdStyxGetArmProcessorTable( VOID );
+ARM_CORE_INFO * +AmdStyxGetArmCoreInfoTable ( + OUT UINTN *NumEntries + ); + #endif // _AMDSTYX_HELPER_LIB_H_ diff --git a/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.c b/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.c index 8189df3cddc6..6e0ed35278e4 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.c +++ b/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.c @@ -20,6 +20,9 @@
#include <AmdStyxHelperLib.h>
+#include <PiDxe.h> +#include <Library/HobLib.h> + extern EFI_SYSTEM_TABLE *gST;
#pragma pack(push, 1) @@ -78,3 +81,21 @@ AmdStyxGetArmProcessorTable( return NULL; }
+ARM_CORE_INFO * +AmdStyxGetArmCoreInfoTable ( + OUT UINTN *NumEntries + ) +{ + EFI_HOB_GUID_TYPE *Hob; + + ASSERT (NumEntries != NULL); + + Hob = GetFirstGuidHob (&gAmdStyxMpCoreInfoGuid); + if (Hob == NULL) { + return NULL; + } + + *NumEntries = GET_GUID_HOB_DATA_SIZE (Hob) / sizeof (ARM_CORE_INFO); + + return GET_GUID_HOB_DATA (Hob); +} diff --git a/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.inf b/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.inf index 57a6ebe74724..540ecfcc6399 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.inf +++ b/Platforms/AMD/Styx/Library/AmdStyxHelperLib/AmdStyxHelperLib.inf @@ -29,6 +29,9 @@ [Sources.common] AmdStyxHelperLib.c
+[LibraryClasses] + HobLib + [Packages] ArmPkg/ArmPkg.dec MdePkg/MdePkg.dec @@ -38,4 +41,4 @@
[Guids] gArmMpCoreInfoGuid - + gAmdStyxMpCoreInfoGuid
The ArmMpCoreInfo configuration table describes the secondary cores that are penned up in PrePeiCore/PrePi and waiting for an SGI to proceed with booting into the next pen. Since this approach is unsafe under a PrePeiCore or PrePi that does not execute in place from flash, move to the platform specific gAmdStyxMpCoreInfoGuid HOB that describes the cores, but does not boot them until we are ready to move them straight into the new pen that we will hand to the OS.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/AcpiTables/Madt.c | 11 ++++------- Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c | 7 ++----- 2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/Platforms/AMD/Styx/AcpiTables/Madt.c b/Platforms/AMD/Styx/AcpiTables/Madt.c index c9ee626d7472..ac98693d5f0f 100644 --- a/Platforms/AMD/Styx/AcpiTables/Madt.c +++ b/Platforms/AMD/Styx/AcpiTables/Madt.c @@ -271,18 +271,15 @@ MadtHeader ( EFI_ACPI_5_1_GIC_STRUCTURE *GicC; EFI_ACPI_5_1_GIC_DISTRIBUTOR_STRUCTURE *GicD; EFI_ACPI_5_1_GIC_MSI_FRAME_STRUCTURE *GicM; - ARM_PROCESSOR_TABLE *ArmProcessorTable; ARM_CORE_INFO *ArmCoreInfoTable; - UINT32 CoreCount, CpuNum; + UINTN CoreCount, CpuNum; EFI_STATUS Status;
- // Get pointer to ARM processor table - ArmProcessorTable = AmdStyxGetArmProcessorTable(); - ASSERT_EFI_ERROR (ArmProcessorTable == NULL); - ArmCoreInfoTable = ArmProcessorTable->ArmCpus; + // Get pointer to ARM core info table + ArmCoreInfoTable = AmdStyxGetArmCoreInfoTable (&CoreCount); + ASSERT (ArmCoreInfoTable != NULL);
// Make sure SoC's core count does not exceed what we want to build - CoreCount = ArmProcessorTable->NumberOfEntries; ASSERT (CoreCount <= NUM_CORES); ASSERT (CoreCount <= PcdGet32(PcdSocCoreCount));
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c index 9ec9e1d5061a..0fb2f4e47dd2 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c @@ -46,7 +46,6 @@ AmdStyxMoveParkedCores( EFI_PHYSICAL_ADDRESS PenBase; UINTN PenSize; UINTN MailboxBase; - ARM_PROCESSOR_TABLE *ArmProcessorTable; ARM_CORE_INFO *ArmCoreInfoTable; UINTN ArmCoreCount; UINTN CoreNum; @@ -54,10 +53,8 @@ AmdStyxMoveParkedCores( UINTN CoreParking;
// Get core information - ArmProcessorTable = AmdStyxGetArmProcessorTable(); - ASSERT_EFI_ERROR (ArmProcessorTable == NULL); - ArmCoreInfoTable = ArmProcessorTable->ArmCpus; - ArmCoreCount = ArmProcessorTable->NumberOfEntries; + ArmCoreInfoTable = AmdStyxGetArmCoreInfoTable (&ArmCoreCount); + ASSERT (ArmCoreInfoTable != NULL);
// Get Parking area (4KB-aligned, 4KB per core) MpParkingBase = FixedPcdGet64 (PcdParkingProtocolBase);
The ARM MpCore implementation is intended for environments where all cores enter UEFI straight out of reset. In this case, we need an early secondary code path that pens up the secondaries until we are ready to release them.
For Styx, we either invoke the SCP or the EL3 firmware explicitly to release the cores into PEI, only to park them until the time we are ready to put them in the pen where the OS can pick them up.
Since the ARM MpCore code is fragile and unmaintained (due to the fact that ARM prefers PSCI as the MP startup implementation), let's not use it if we don't have to.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c | 209 +------------------- Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.h | 4 - Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.inf | 3 - 3 files changed, 3 insertions(+), 213 deletions(-)
diff --git a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c index a1f6cabee901..ed6c67fed38f 100644 --- a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c +++ b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.c @@ -14,8 +14,6 @@
#include "PlatInitPei.h"
-#define PLATINIT_CONTEXT_ID ( 0x0 ) // used on SMC calls - /*---------------------------------------------------------------------------------------- * G L O B A L S *---------------------------------------------------------------------------------------- @@ -23,107 +21,50 @@ // // CoreInfo table // -ARM_CORE_INFO mAmdMpCoreInfoTable[] = { +STATIC ARM_CORE_INFO mAmdMpCoreInfoTable[] = { { // Cluster 0, Core 0 0x0, 0x0, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 }, { // Cluster 0, Core 1 0x0, 0x1, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 }, { // Cluster 1, Core 0 0x1, 0x0, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 }, { // Cluster 1, Core 1 0x1, 0x1, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 }, { // Cluster 2, Core 0 0x2, 0x0, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 }, { // Cluster 2, Core 1 0x2, 0x1, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 }, { // Cluster 3, Core 0 0x3, 0x0, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 }, { // Cluster 3, Core 1 0x3, 0x1, - - // MP Core MailBox Set/Get/Clear Addresses and Clear Value - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (EFI_PHYSICAL_ADDRESS)0, - (UINT64)0x0 } };
// // Core count // -UINTN mAmdCoreCount = sizeof (mAmdMpCoreInfoTable) / sizeof (ARM_CORE_INFO); - -// -// Adding an extra entry to make sure Mailbox is UINTN-aligned -// -UINTN mMailboxTable[NUM_CORES + 1] = { 0 }; - +STATIC UINTN mAmdCoreCount = sizeof (mAmdMpCoreInfoTable) / sizeof (ARM_CORE_INFO);
/*---------------------------------------------------------------------------------------- * E X T E R N S *---------------------------------------------------------------------------------------- */ -extern EFI_GUID gPeiIscpPpiGuid; -extern EFI_GUID gAmdStyxPlatInitPpiGuid; - UINTN ArmGetCpuCountPerCluster ( VOID @@ -131,23 +72,10 @@ ArmGetCpuCountPerCluster (
/*---------------------------------------------------------------------------------------- - * P R O T O T Y P E S O F L O C A L F U N C T I O N S - *---------------------------------------------------------------------------------------- - */ -EFI_STATUS -EFIAPI -PlatInitEndOfPeiPpiCallback ( - IN CONST EFI_PEI_SERVICES **PeiServices, - IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, - IN VOID *Ppi - ); - - -/*---------------------------------------------------------------------------------------- * P P I L I S T *---------------------------------------------------------------------------------------- */ -EFI_PEI_ISCP_PPI *PeiIscpPpi; +STATIC EFI_PEI_ISCP_PPI *PeiIscpPpi;
/*---------------------------------------------------------------------------------------- @@ -161,13 +89,6 @@ STATIC EFI_PEI_PPI_DESCRIPTOR mPlatInitPpiDescriptor = NULL };
-STATIC EFI_PEI_NOTIFY_DESCRIPTOR mPlatInitNotifyDescriptor = -{ - (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), - &gEfiEndOfPeiSignalPpiGuid, - (EFI_PEIM_NOTIFY_ENTRY_POINT)PlatInitEndOfPeiPpiCallback -}; -
/** *--------------------------------------------------------------------------------------- @@ -200,7 +121,6 @@ PlatInitPeiEntryPoint ( ISCP_CPU_RESET_INFO CpuResetInfo = {0}; ISCP_MAC_INFO MacAddrInfo = {0}; UINT64 MacAddr0, MacAddr1; - UINTN MailboxBase; UINTN CpuCoreCount, CpuMap, CpuMapSize; UINTN Index, CoreNum; UINT32 *CpuIdReg = (UINT32 *)FixedPcdGet32 (PcdCpuIdRegister); @@ -244,12 +164,6 @@ PlatInitPeiEntryPoint ( if (!FixedPcdGetBool (PcdIscpSupport)) { DEBUG ((EFI_D_ERROR, "Warning: Could not get CPU info via ISCP, using default values.\n")); } else { - // Make sure Mailbox is UINTN aligned - MailboxBase = (UINTN)&mMailboxTable[0]; - if (MailboxBase % sizeof(MailboxBase) != 0) { - MailboxBase += sizeof(MailboxBase) - MailboxBase % sizeof(MailboxBase); - } - // // Walk CPU map to enumerate active cores // @@ -265,12 +179,6 @@ PlatInitPeiEntryPoint ( mAmdMpCoreInfoTable[Index].ClusterId = CpuResetInfo.CoreStatus.ClusterId; mAmdMpCoreInfoTable[Index].CoreId = CpuResetInfo.CoreStatus.CoreId;
- mAmdMpCoreInfoTable[Index].MailboxSetAddress = MailboxBase; - mAmdMpCoreInfoTable[Index].MailboxGetAddress = MailboxBase; - mAmdMpCoreInfoTable[Index].MailboxClearAddress = MailboxBase; - mAmdMpCoreInfoTable[Index].MailboxClearValue = 0; - MailboxBase += sizeof(MailboxBase); - DEBUG ((EFI_D_ERROR, "Core[%d]: ClusterId = %d CoreId = %d\n", Index, mAmdMpCoreInfoTable[Index].ClusterId, mAmdMpCoreInfoTable[Index].CoreId)); @@ -332,12 +240,6 @@ PlatInitPeiEntryPoint ( DEBUG ((EFI_D_ERROR, "EthMacA = 0x%lX\n", PcdGet64 (PcdEthMacA))); DEBUG ((EFI_D_ERROR, "EthMacB = 0x%lX\n", PcdGet64 (PcdEthMacB)));
- // If the OS can't launch secondary cores, hook callback for end of PEI phase - if (!FixedPcdGetBool (PcdPsciOsSupport)) { - Status = (*PeiServices)->NotifyPpi(PeiServices, &mPlatInitNotifyDescriptor); - ASSERT_EFI_ERROR (Status); - } - // Let other PEI modules know we're done! Status = (*PeiServices)->InstallPpi (PeiServices, &mPlatInitPpiDescriptor); ASSERT_EFI_ERROR (Status); @@ -345,108 +247,3 @@ PlatInitPeiEntryPoint ( return Status; }
- -/** - PEI termination callback. - - @param PeiServices General purpose services available to every PEIM. - @param NotifyDescriptor Not used. - @param Ppi Not used. - - @retval EFI_SUCCESS If the interface could be successfully - installed. - -**/ -EFI_STATUS -EFIAPI -PlatInitEndOfPeiPpiCallback ( - IN CONST EFI_PEI_SERVICES **PeiServices, - IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, - IN VOID *Ppi - ) -{ - EFI_STATUS Status = EFI_SUCCESS; - ISCP_CPU_RESET_INFO CpuResetInfo = {0}; - BOOLEAN fCoreTransitionDone; - ARM_SMC_ARGS SmcRegs; - UINTN Index; - - DEBUG ((EFI_D_ERROR, "PlatInit: END_OF_PEI\n")); - - // - // Launch secondary cores to the UEFI pen. - // - // The cores will land at the SecondaryMain() entry point in either of: - // 1) ArmPlatfromPkg/PrePiCore/MainMPCore.c - // 2) ArmPlatfromPkg/PrePi/MainMPCore.c - // From there, the cores will jump to the OS (via FDT's spin-table) - // or to another UEFI pen that supports the MP-Parking protocol. - // - if (FixedPcdGetBool (PcdTrustedFWSupport)) { - for (Index = 0; Index < mAmdCoreCount; ++Index) { - SmcRegs.Arg0 = ARM_SMC_ID_PSCI_CPU_ON_AARCH64; - SmcRegs.Arg1 = GET_MPID (mAmdMpCoreInfoTable[Index].ClusterId, - mAmdMpCoreInfoTable[Index].CoreId); - SmcRegs.Arg2 = FixedPcdGet64 (PcdUefiEntryAddress); - SmcRegs.Arg3 = PLATINIT_CONTEXT_ID; - ArmCallSmc (&SmcRegs); - - if (SmcRegs.Arg0 == ARM_SMC_PSCI_RET_SUCCESS || - SmcRegs.Arg0 == ARM_SMC_PSCI_RET_ALREADY_ON) { - DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index)); - } else { - DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] to RUN state.\n", Index)); - } - } - } else if (FixedPcdGetBool (PcdIscpSupport)) { - for (Index = 0; Index < mAmdCoreCount; ++Index) { - CpuResetInfo.CoreNum = Index; - Status = PeiIscpPpi->ExecuteCpuRetrieveIdTransaction ( - PeiServices, &CpuResetInfo ); - ASSERT_EFI_ERROR (Status); - - // Transition core to the RUN state - fCoreTransitionDone = FALSE; - do { - switch (CpuResetInfo.CoreStatus.Status) { - case CPU_CORE_POWERDOWN: - DEBUG ((EFI_D_ERROR, "Core[%d]: POWERDOWN -> POWERUP\n", Index)); - CpuResetInfo.CoreStatus.Status = CPU_CORE_POWERUP; - break; - - case CPU_CORE_POWERUP: - case CPU_CORE_SLEEP: - DEBUG ((EFI_D_ERROR, "Core[%d]: POWERUP -> RESET\n", Index)); - CpuResetInfo.CoreStatus.Status = CPU_CORE_RESET; - CpuResetInfo.CoreStatus.ResetVector = FixedPcdGet64 (PcdUefiEntryAddress); - break; - - case CPU_CORE_RESET: - DEBUG ((EFI_D_ERROR, "Core[%d]: RESET -> RUN\n", Index)); - CpuResetInfo.CoreStatus.Status = CPU_CORE_RUN; - break; - - default: - if (CpuResetInfo.CoreStatus.Status == CPU_CORE_RUN) { - DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index)); - } else { - DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] to RUN state.\n", Index)); - } - fCoreTransitionDone = TRUE; - break; - } - - // Transition core to next state - if (!fCoreTransitionDone) { - Status = PeiIscpPpi->ExecuteCpuResetTransaction ( - PeiServices, &CpuResetInfo ); - ASSERT_EFI_ERROR (Status); - } - } while (!fCoreTransitionDone); - } // for - } // else - - return Status; -} - - diff --git a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.h b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.h index 16ec3b837acd..a4893bcf4b50 100644 --- a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.h +++ b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.h @@ -20,14 +20,10 @@ #include <Library/PcdLib.h> #include <Library/PeimEntryPoint.h> #include <Library/PeiServicesLib.h> -#include <Library/PeiServicesTablePointerLib.h> #include <Library/BaseMemoryLib.h> #include <Library/HobLib.h> #include <Library/ArmLib.h> #include <Guid/ArmMpCoreInfo.h> -#include <Ppi/EndOfPeiPhase.h> -#include <Library/ArmSmcLib.h> -#include <IndustryStandard/ArmStdSmc.h>
#include <Ppi/IscpPpi.h> #include <Iscp.h> diff --git a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.inf b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.inf index 06a633c4d82e..52dc47de7e97 100644 --- a/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.inf +++ b/Platforms/AMD/Styx/Drivers/PlatInitPei/PlatInitPei.inf @@ -33,7 +33,6 @@
[Packages] MdePkg/MdePkg.dec - MdeModulePkg/MdeModulePkg.dec ArmPkg/ArmPkg.dec ArmPlatformPkg/ArmPlatformPkg.dec AmdModulePkg/AmdModulePkg.dec @@ -69,9 +68,7 @@
[FixedPcd] gAmdStyxTokenSpaceGuid.PcdIscpSupport - gAmdStyxTokenSpaceGuid.PcdPsciOsSupport gAmdStyxTokenSpaceGuid.PcdTrustedFWSupport - gAmdStyxTokenSpaceGuid.PcdUefiEntryAddress gAmdStyxTokenSpaceGuid.PcdCpuIdRegister
[Depex]
Instead of using the UEFI specific ArmMpCore protocol to boot the cores into UEFI first before moving the into the OS pen, defer the secondary boot until we can move them there straight away. Since Styx does not execute in place, and has always boots the secondaries explicitly, either via the ISCP interface or via PSCI, this is the safest approach.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c | 113 ++++++++++++++++++-- Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf | 8 +- 2 files changed, 107 insertions(+), 14 deletions(-)
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c index 0fb2f4e47dd2..3b88a11bb771 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c @@ -18,14 +18,19 @@
**/
+#include <Library/ArmSmcLib.h> +#include <Library/CacheMaintenanceLib.h> #include <Library/PcdLib.h> #include <Base.h> #include <BdsLib/BdsInternal.h> -#include <Library/ArmGicLib.h> -#include <Library/IoLib.h>
+#include <Common/CoreState.h> +#include <IndustryStandard/ArmStdSmc.h> +#include <Protocol/AmdIscpDxeProtocol.h> #include <AmdStyxHelperLib.h>
+#define PLATINIT_CONTEXT_ID ( 0x0 ) // used on SMC calls + /* These externs are used to relocate some ASM code into Linux memory. */ extern VOID *SecondariesPenStart; extern VOID *SecondariesPenEnd; @@ -34,6 +39,85 @@ extern UINTN *AsmMailboxBase;
extern EFI_BOOT_SERVICES *gBS;
+AMD_ISCP_DXE_PROTOCOL *mIscpDxeProtocol; + +STATIC +VOID +EFIAPI +AmdStyxBringupSecondary ( + ARM_CORE_INFO *ArmCoreInfoTable, + UINTN Index, + EFI_PHYSICAL_ADDRESS SecondaryEntry + ) +{ + EFI_STATUS Status; + ISCP_CPU_RESET_INFO CpuResetInfo; + BOOLEAN fCoreTransitionDone; + ARM_SMC_ARGS SmcRegs; + + if (FixedPcdGetBool (PcdTrustedFWSupport)) { + SmcRegs.Arg0 = ARM_SMC_ID_PSCI_CPU_ON_AARCH64; + SmcRegs.Arg1 = GET_MPID (ArmCoreInfoTable[Index].ClusterId, + ArmCoreInfoTable[Index].CoreId); + SmcRegs.Arg2 = SecondaryEntry; + SmcRegs.Arg3 = PLATINIT_CONTEXT_ID; + ArmCallSmc (&SmcRegs); + + if (SmcRegs.Arg0 == ARM_SMC_PSCI_RET_SUCCESS || + SmcRegs.Arg0 == ARM_SMC_PSCI_RET_ALREADY_ON) { + DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index)); + } else { + DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] to RUN state.\n", Index)); + } + } else if (FixedPcdGetBool (PcdIscpSupport)) { + CpuResetInfo.CoreNum = Index; + Status = mIscpDxeProtocol->AmdExecuteCpuRetrieveId (mIscpDxeProtocol, + &CpuResetInfo); + ASSERT_EFI_ERROR (Status); + + // Transition core to the RUN state + fCoreTransitionDone = FALSE; + do { + switch (CpuResetInfo.CoreStatus.Status) { + case CPU_CORE_POWERDOWN: + DEBUG ((EFI_D_ERROR, "Core[%d]: POWERDOWN -> POWERUP\n", Index)); + CpuResetInfo.CoreStatus.Status = CPU_CORE_POWERUP; + break; + + case CPU_CORE_POWERUP: + case CPU_CORE_SLEEP: + DEBUG ((EFI_D_ERROR, "Core[%d]: POWERUP -> RESET\n", Index)); + CpuResetInfo.CoreStatus.Status = CPU_CORE_RESET; + CpuResetInfo.CoreStatus.ResetVector = SecondaryEntry; + break; + + case CPU_CORE_RESET: + DEBUG ((EFI_D_ERROR, "Core[%d]: RESET -> RUN\n", Index)); + CpuResetInfo.CoreStatus.Status = CPU_CORE_RUN; + break; + + default: + if (CpuResetInfo.CoreStatus.Status == CPU_CORE_RUN) { + DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index)); + } else { + DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] to RUN state.\n", Index)); + } + fCoreTransitionDone = TRUE; + break; + } + + // Transition core to next state + if (!fCoreTransitionDone) { + Status = mIscpDxeProtocol->AmdExecuteCpuCoreReset (mIscpDxeProtocol, + &CpuResetInfo); + ASSERT_EFI_ERROR (Status); + } + } while (!fCoreTransitionDone); + } else { + ASSERT (FALSE); + } +} + VOID EFIAPI AmdStyxMoveParkedCores( @@ -52,6 +136,19 @@ AmdStyxMoveParkedCores( UINTN CoreMailbox; UINTN CoreParking;
+ if (FixedPcdGetBool (PcdIscpSupport)) { + Status = gBS->LocateProtocol ( + &gAmdIscpDxeProtocolGuid, + NULL, + (VOID **)&mIscpDxeProtocol + ); + if (EFI_ERROR (Status)) { + mIscpDxeProtocol = NULL; + DEBUG ((EFI_D_ERROR, "Failed to Locate ISCP DXE Protocol")); + return; + } + } + // Get core information ArmCoreInfoTable = AmdStyxGetArmCoreInfoTable (&ArmCoreCount); ASSERT (ArmCoreInfoTable != NULL); @@ -103,17 +200,15 @@ AmdStyxMoveParkedCores( *((UINTN*)(CoreParking + sizeof (UINT64))) = 0x0; *((UINTN*)(CoreParking + SIZE_2KB)) = CoreNum;
- // Move secondary core to our new Pen - MmioWrite64(ArmCoreInfoTable[CoreNum].MailboxSetAddress, (UINTN)PenBase); - // Update table entry to be consumed by FDT parser. ArmCoreInfoTable[CoreNum].MailboxSetAddress = CoreMailbox; }
- // Flush caches to make sure our pen gets to memory before we release secondary cores. - ArmCleanDataCache(); + WriteBackDataCacheRange ((VOID *)MpParkingBase, MpParkingSize);
- // Send msg to secondary cores to jump to our new Pen. - ArmGicSendSgiTo (PcdGet32(PcdGicDistributorBase), ARM_GIC_ICDSGIR_FILTER_EVERYONEELSE, 0x0E, PcdGet32 (PcdGicSgiIntId)); + for (CoreNum = 0; CoreNum < ArmCoreCount; CoreNum++) { + // Move secondary core to our new Pen + AmdStyxBringupSecondary (ArmCoreInfoTable, CoreNum, PenBase); + } }
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf index 5ac210ba04e4..c660ccd04810 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf @@ -50,6 +50,7 @@ FdtLib DevicePathLib AmdStyxHelperLib + ArmSmcLib
[LibraryClasses.AARCH64] ArmGicLib @@ -63,6 +64,7 @@
[Protocols] gEfiFirmwareVolume2ProtocolGuid ##CONSUMED + gAmdIscpDxeProtocolGuid ## SOMETIMES_CONSUMES
[Pcd] gAmdStyxTokenSpaceGuid.PcdStyxFdt @@ -80,11 +82,7 @@ gAmdStyxTokenSpaceGuid.PcdTrustedFWSupport gAmdStyxTokenSpaceGuid.PcdParkingProtocolBase gAmdStyxTokenSpaceGuid.PcdParkingProtocolSize + gAmdStyxTokenSpaceGuid.PcdIscpSupport
-[Pcd.AARCH64] - gArmTokenSpaceGuid.PcdGicDistributorBase - gArmTokenSpaceGuid.PcdGicSgiIntId - [Depex] TRUE -
Ard, Wow... I do like the way this is going! Q: Have you booted all core using DO_PSCI=0?
All, BTW, I'd like to propose that we require PcdTrustedFWSupport = TRUE (for EL3 handling) This way, we could then can drop the ISCP code that brings cores out of reset into UEFI.
Leo
-----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Monday, May 02, 2016 8:36 AM To: linaro-uefi@lists.linaro.org Cc: leif.lindholm@linaro.org; ricardo.salveti@linaro.org; Duran, Leo leo.duran@amd.com; Ard Biesheuvel ard.biesheuvel@linaro.org Subject: [RFC PATCH 6/7] Platforms/Styx/FdtDxe: boot secondaries straight into OS pen
Instead of using the UEFI specific ArmMpCore protocol to boot the cores into UEFI first before moving the into the OS pen, defer the secondary boot until we can move them there straight away. Since Styx does not execute in place, and has always boots the secondaries explicitly, either via the ISCP interface or via PSCI, this is the safest approach.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c | 113 ++++++++++++++++++-- Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf | 8 +- 2 files changed, 107 insertions(+), 14 deletions(-)
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c index 0fb2f4e47dd2..3b88a11bb771 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c @@ -18,14 +18,19 @@
**/
+#include <Library/ArmSmcLib.h> +#include <Library/CacheMaintenanceLib.h> #include <Library/PcdLib.h> #include <Base.h> #include <BdsLib/BdsInternal.h> -#include <Library/ArmGicLib.h> -#include <Library/IoLib.h>
+#include <Common/CoreState.h> +#include <IndustryStandard/ArmStdSmc.h> #include +<Protocol/AmdIscpDxeProtocol.h> #include <AmdStyxHelperLib.h>
+#define PLATINIT_CONTEXT_ID ( 0x0 ) // used on SMC calls
/* These externs are used to relocate some ASM code into Linux memory. */ extern VOID *SecondariesPenStart; extern VOID *SecondariesPenEnd; @@ -34,6 +39,85 @@ extern UINTN *AsmMailboxBase;
extern EFI_BOOT_SERVICES *gBS;
+AMD_ISCP_DXE_PROTOCOL *mIscpDxeProtocol;
+STATIC +VOID +EFIAPI +AmdStyxBringupSecondary (
- ARM_CORE_INFO *ArmCoreInfoTable,
- UINTN Index,
- EFI_PHYSICAL_ADDRESS SecondaryEntry
- )
+{
- EFI_STATUS Status;
- ISCP_CPU_RESET_INFO CpuResetInfo;
- BOOLEAN fCoreTransitionDone;
- ARM_SMC_ARGS SmcRegs;
- if (FixedPcdGetBool (PcdTrustedFWSupport)) {
- SmcRegs.Arg0 = ARM_SMC_ID_PSCI_CPU_ON_AARCH64;
- SmcRegs.Arg1 = GET_MPID (ArmCoreInfoTable[Index].ClusterId,
ArmCoreInfoTable[Index].CoreId);- SmcRegs.Arg2 = SecondaryEntry;
- SmcRegs.Arg3 = PLATINIT_CONTEXT_ID;
- ArmCallSmc (&SmcRegs);
- if (SmcRegs.Arg0 == ARM_SMC_PSCI_RET_SUCCESS ||
SmcRegs.Arg0 == ARM_SMC_PSCI_RET_ALREADY_ON) {DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index));- } else {
DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] to RUNstate.\n", Index));
- }
- } else if (FixedPcdGetBool (PcdIscpSupport)) {
- CpuResetInfo.CoreNum = Index;
- Status = mIscpDxeProtocol->AmdExecuteCpuRetrieveId
(mIscpDxeProtocol,
&CpuResetInfo);- ASSERT_EFI_ERROR (Status);
- // Transition core to the RUN state
- fCoreTransitionDone = FALSE;
- do {
switch (CpuResetInfo.CoreStatus.Status) {case CPU_CORE_POWERDOWN:DEBUG ((EFI_D_ERROR, "Core[%d]: POWERDOWN -> POWERUP\n",Index));
CpuResetInfo.CoreStatus.Status = CPU_CORE_POWERUP;break;case CPU_CORE_POWERUP:case CPU_CORE_SLEEP:DEBUG ((EFI_D_ERROR, "Core[%d]: POWERUP -> RESET\n", Index));CpuResetInfo.CoreStatus.Status = CPU_CORE_RESET;CpuResetInfo.CoreStatus.ResetVector = SecondaryEntry;break;case CPU_CORE_RESET:DEBUG ((EFI_D_ERROR, "Core[%d]: RESET -> RUN\n", Index));CpuResetInfo.CoreStatus.Status = CPU_CORE_RUN;break;default:if (CpuResetInfo.CoreStatus.Status == CPU_CORE_RUN) {DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index));} else {DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] toRUN state.\n", Index));
}fCoreTransitionDone = TRUE;break;}// Transition core to next stateif (!fCoreTransitionDone) {Status = mIscpDxeProtocol->AmdExecuteCpuCoreReset(mIscpDxeProtocol,
&CpuResetInfo);ASSERT_EFI_ERROR (Status);}- } while (!fCoreTransitionDone);
- } else {
- ASSERT (FALSE);
- }
+}
VOID EFIAPI AmdStyxMoveParkedCores( @@ -52,6 +136,19 @@ AmdStyxMoveParkedCores( UINTN CoreMailbox; UINTN CoreParking;
- if (FixedPcdGetBool (PcdIscpSupport)) {
- Status = gBS->LocateProtocol (
&gAmdIscpDxeProtocolGuid,NULL,(VOID **)&mIscpDxeProtocol);- if (EFI_ERROR (Status)) {
mIscpDxeProtocol = NULL;DEBUG ((EFI_D_ERROR, "Failed to Locate ISCP DXE Protocol"));return;- }
- }
- // Get core information ArmCoreInfoTable = AmdStyxGetArmCoreInfoTable (&ArmCoreCount); ASSERT (ArmCoreInfoTable != NULL);
@@ -103,17 +200,15 @@ AmdStyxMoveParkedCores( *((UINTN*)(CoreParking + sizeof (UINT64))) = 0x0; *((UINTN*)(CoreParking + SIZE_2KB)) = CoreNum;
- // Move secondary core to our new Pen
- MmioWrite64(ArmCoreInfoTable[CoreNum].MailboxSetAddress,
(UINTN)PenBase);
// Update table entry to be consumed by FDT parser. ArmCoreInfoTable[CoreNum].MailboxSetAddress = CoreMailbox; }
// Flush caches to make sure our pen gets to memory before we release
secondary cores.
- ArmCleanDataCache();
- WriteBackDataCacheRange ((VOID *)MpParkingBase, MpParkingSize);
- // Send msg to secondary cores to jump to our new Pen.
- ArmGicSendSgiTo (PcdGet32(PcdGicDistributorBase),
ARM_GIC_ICDSGIR_FILTER_EVERYONEELSE, 0x0E, PcdGet32 (PcdGicSgiIntId));
- for (CoreNum = 0; CoreNum < ArmCoreCount; CoreNum++) {
- // Move secondary core to our new Pen
- AmdStyxBringupSecondary (ArmCoreInfoTable, CoreNum, PenBase); }
}
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf index 5ac210ba04e4..c660ccd04810 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf @@ -50,6 +50,7 @@ FdtLib DevicePathLib AmdStyxHelperLib
- ArmSmcLib
[LibraryClasses.AARCH64] ArmGicLib @@ -63,6 +64,7 @@
[Protocols] gEfiFirmwareVolume2ProtocolGuid ##CONSUMED
- gAmdIscpDxeProtocolGuid ## SOMETIMES_CONSUMES
[Pcd] gAmdStyxTokenSpaceGuid.PcdStyxFdt @@ -80,11 +82,7 @@ gAmdStyxTokenSpaceGuid.PcdTrustedFWSupport gAmdStyxTokenSpaceGuid.PcdParkingProtocolBase gAmdStyxTokenSpaceGuid.PcdParkingProtocolSize
- gAmdStyxTokenSpaceGuid.PcdIscpSupport
-[Pcd.AARCH64]
- gArmTokenSpaceGuid.PcdGicDistributorBase
- gArmTokenSpaceGuid.PcdGicSgiIntId
[Depex] TRUE
-- 2.7.4
On 2 May 2016 at 16:41, Duran, Leo leo.duran@amd.com wrote:
Ard, Wow... I do like the way this is going! Q: Have you booted all core using DO_PSCI=0?
Into the pen, yes, but not all the way into the OS. I was getting an SError which turned out to be unrelated to these changes.
All, BTW, I'd like to propose that we require PcdTrustedFWSupport = TRUE (for EL3 handling) This way, we could then can drop the ISCP code that brings cores out of reset into UEFI.
Yes, I think that is reasonable. That would also allow us to move this to generic code, i.e., a generic DXE driver that implements ACPI parking protocol/spin-table on top of PSCI. For now, I would like to see it working first, especially since the MpCore stuff is somewhat broken if you run your XIP PEI code from DRAM
-----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Monday, May 02, 2016 8:36 AM To: linaro-uefi@lists.linaro.org Cc: leif.lindholm@linaro.org; ricardo.salveti@linaro.org; Duran, Leo leo.duran@amd.com; Ard Biesheuvel ard.biesheuvel@linaro.org Subject: [RFC PATCH 6/7] Platforms/Styx/FdtDxe: boot secondaries straight into OS pen
Instead of using the UEFI specific ArmMpCore protocol to boot the cores into UEFI first before moving the into the OS pen, defer the secondary boot until we can move them there straight away. Since Styx does not execute in place, and has always boots the secondaries explicitly, either via the ISCP interface or via PSCI, this is the safest approach.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c | 113 ++++++++++++++++++-- Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf | 8 +- 2 files changed, 107 insertions(+), 14 deletions(-)
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c index 0fb2f4e47dd2..3b88a11bb771 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c @@ -18,14 +18,19 @@
**/
+#include <Library/ArmSmcLib.h> +#include <Library/CacheMaintenanceLib.h> #include <Library/PcdLib.h> #include <Base.h> #include <BdsLib/BdsInternal.h> -#include <Library/ArmGicLib.h> -#include <Library/IoLib.h>
+#include <Common/CoreState.h> +#include <IndustryStandard/ArmStdSmc.h> #include +<Protocol/AmdIscpDxeProtocol.h> #include <AmdStyxHelperLib.h>
+#define PLATINIT_CONTEXT_ID ( 0x0 ) // used on SMC calls
/* These externs are used to relocate some ASM code into Linux memory. */ extern VOID *SecondariesPenStart; extern VOID *SecondariesPenEnd; @@ -34,6 +39,85 @@ extern UINTN *AsmMailboxBase;
extern EFI_BOOT_SERVICES *gBS;
+AMD_ISCP_DXE_PROTOCOL *mIscpDxeProtocol;
+STATIC +VOID +EFIAPI +AmdStyxBringupSecondary (
- ARM_CORE_INFO *ArmCoreInfoTable,
- UINTN Index,
- EFI_PHYSICAL_ADDRESS SecondaryEntry
- )
+{
- EFI_STATUS Status;
- ISCP_CPU_RESET_INFO CpuResetInfo;
- BOOLEAN fCoreTransitionDone;
- ARM_SMC_ARGS SmcRegs;
- if (FixedPcdGetBool (PcdTrustedFWSupport)) {
- SmcRegs.Arg0 = ARM_SMC_ID_PSCI_CPU_ON_AARCH64;
- SmcRegs.Arg1 = GET_MPID (ArmCoreInfoTable[Index].ClusterId,
ArmCoreInfoTable[Index].CoreId);- SmcRegs.Arg2 = SecondaryEntry;
- SmcRegs.Arg3 = PLATINIT_CONTEXT_ID;
- ArmCallSmc (&SmcRegs);
- if (SmcRegs.Arg0 == ARM_SMC_PSCI_RET_SUCCESS ||
SmcRegs.Arg0 == ARM_SMC_PSCI_RET_ALREADY_ON) {DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index));- } else {
DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] to RUNstate.\n", Index));
- }
- } else if (FixedPcdGetBool (PcdIscpSupport)) {
- CpuResetInfo.CoreNum = Index;
- Status = mIscpDxeProtocol->AmdExecuteCpuRetrieveId
(mIscpDxeProtocol,
&CpuResetInfo);- ASSERT_EFI_ERROR (Status);
- // Transition core to the RUN state
- fCoreTransitionDone = FALSE;
- do {
switch (CpuResetInfo.CoreStatus.Status) {case CPU_CORE_POWERDOWN:DEBUG ((EFI_D_ERROR, "Core[%d]: POWERDOWN -> POWERUP\n",Index));
CpuResetInfo.CoreStatus.Status = CPU_CORE_POWERUP;break;case CPU_CORE_POWERUP:case CPU_CORE_SLEEP:DEBUG ((EFI_D_ERROR, "Core[%d]: POWERUP -> RESET\n", Index));CpuResetInfo.CoreStatus.Status = CPU_CORE_RESET;CpuResetInfo.CoreStatus.ResetVector = SecondaryEntry;break;case CPU_CORE_RESET:DEBUG ((EFI_D_ERROR, "Core[%d]: RESET -> RUN\n", Index));CpuResetInfo.CoreStatus.Status = CPU_CORE_RUN;break;default:if (CpuResetInfo.CoreStatus.Status == CPU_CORE_RUN) {DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index));} else {DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d] toRUN state.\n", Index));
}fCoreTransitionDone = TRUE;break;}// Transition core to next stateif (!fCoreTransitionDone) {Status = mIscpDxeProtocol->AmdExecuteCpuCoreReset(mIscpDxeProtocol,
&CpuResetInfo);ASSERT_EFI_ERROR (Status);}- } while (!fCoreTransitionDone);
- } else {
- ASSERT (FALSE);
- }
+}
VOID EFIAPI AmdStyxMoveParkedCores( @@ -52,6 +136,19 @@ AmdStyxMoveParkedCores( UINTN CoreMailbox; UINTN CoreParking;
- if (FixedPcdGetBool (PcdIscpSupport)) {
- Status = gBS->LocateProtocol (
&gAmdIscpDxeProtocolGuid,NULL,(VOID **)&mIscpDxeProtocol);- if (EFI_ERROR (Status)) {
mIscpDxeProtocol = NULL;DEBUG ((EFI_D_ERROR, "Failed to Locate ISCP DXE Protocol"));return;- }
- }
- // Get core information ArmCoreInfoTable = AmdStyxGetArmCoreInfoTable (&ArmCoreCount); ASSERT (ArmCoreInfoTable != NULL);
@@ -103,17 +200,15 @@ AmdStyxMoveParkedCores( *((UINTN*)(CoreParking + sizeof (UINT64))) = 0x0; *((UINTN*)(CoreParking + SIZE_2KB)) = CoreNum;
- // Move secondary core to our new Pen
- MmioWrite64(ArmCoreInfoTable[CoreNum].MailboxSetAddress,
(UINTN)PenBase);
// Update table entry to be consumed by FDT parser. ArmCoreInfoTable[CoreNum].MailboxSetAddress = CoreMailbox; }
// Flush caches to make sure our pen gets to memory before we release
secondary cores.
- ArmCleanDataCache();
- WriteBackDataCacheRange ((VOID *)MpParkingBase, MpParkingSize);
- // Send msg to secondary cores to jump to our new Pen.
- ArmGicSendSgiTo (PcdGet32(PcdGicDistributorBase),
ARM_GIC_ICDSGIR_FILTER_EVERYONEELSE, 0x0E, PcdGet32 (PcdGicSgiIntId));
- for (CoreNum = 0; CoreNum < ArmCoreCount; CoreNum++) {
- // Move secondary core to our new Pen
- AmdStyxBringupSecondary (ArmCoreInfoTable, CoreNum, PenBase); }
}
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf index 5ac210ba04e4..c660ccd04810 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf @@ -50,6 +50,7 @@ FdtLib DevicePathLib AmdStyxHelperLib
- ArmSmcLib
[LibraryClasses.AARCH64] ArmGicLib @@ -63,6 +64,7 @@
[Protocols] gEfiFirmwareVolume2ProtocolGuid ##CONSUMED
- gAmdIscpDxeProtocolGuid ## SOMETIMES_CONSUMES
[Pcd] gAmdStyxTokenSpaceGuid.PcdStyxFdt @@ -80,11 +82,7 @@ gAmdStyxTokenSpaceGuid.PcdTrustedFWSupport gAmdStyxTokenSpaceGuid.PcdParkingProtocolBase gAmdStyxTokenSpaceGuid.PcdParkingProtocolSize
- gAmdStyxTokenSpaceGuid.PcdIscpSupport
-[Pcd.AARCH64]
- gArmTokenSpaceGuid.PcdGicDistributorBase
- gArmTokenSpaceGuid.PcdGicSgiIntId
[Depex] TRUE
-- 2.7.4
-----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Monday, May 02, 2016 9:45 AM To: Duran, Leo leo.duran@amd.com Cc: linaro-uefi@lists.linaro.org; leif.lindholm@linaro.org; ricardo.salveti@linaro.org Subject: Re: [RFC PATCH 6/7] Platforms/Styx/FdtDxe: boot secondaries straight into OS pen
On 2 May 2016 at 16:41, Duran, Leo leo.duran@amd.com wrote:
Ard, Wow... I do like the way this is going! Q: Have you booted all core using DO_PSCI=0?
Into the pen, yes, but not all the way into the OS. I was getting an SError which turned out to be unrelated to these changes.
All, BTW, I'd like to propose that we require PcdTrustedFWSupport = TRUE (for EL3 handling) This way, we could then can drop the ISCP code that
brings cores out of reset into UEFI.
Yes, I think that is reasonable. That would also allow us to move this to generic code, i.e., a generic DXE driver that implements ACPI parking protocol/spin-table on top of PSCI. For now, I would like to see it working first, especially since the MpCore stuff is somewhat broken if you run your XIP PEI code from DRAM
Ummh... the baseline code (prior this patch series) used to be able to boot secondary cores, using DO_PSCI=0 and the patched MainMPCore.c. However, the secondary boot path now gets a "Synchronous Exception" when it invokes GetMpCoreInfo().
I have not tried exercising the (DO_PSCI=0) code path in quite a while, so I don't know when it got broken. (I suspect due to change in EDK2, because the Styx code has not changed)
Leo.
-----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Monday, May 02, 2016 8:36 AM To: linaro-uefi@lists.linaro.org Cc: leif.lindholm@linaro.org; ricardo.salveti@linaro.org; Duran, Leo leo.duran@amd.com; Ard Biesheuvel ard.biesheuvel@linaro.org Subject: [RFC PATCH 6/7] Platforms/Styx/FdtDxe: boot secondaries straight into OS pen
Instead of using the UEFI specific ArmMpCore protocol to boot the cores into UEFI first before moving the into the OS pen, defer the secondary boot until we can move them there straight away. Since Styx does not execute in place, and has always boots the secondaries explicitly, either via the ISCP interface or via PSCI, this is the safest
approach.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c | 113 ++++++++++++++++++-- Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf | 8 +- 2 files changed, 107 insertions(+), 14 deletions(-)
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c index 0fb2f4e47dd2..3b88a11bb771 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/AArch64/BdsLinuxLoader.c @@ -18,14 +18,19 @@
**/
+#include <Library/ArmSmcLib.h> +#include <Library/CacheMaintenanceLib.h> #include <Library/PcdLib.h> #include <Base.h> #include <BdsLib/BdsInternal.h> -#include <Library/ArmGicLib.h> -#include <Library/IoLib.h>
+#include <Common/CoreState.h> +#include <IndustryStandard/ArmStdSmc.h> #include +<Protocol/AmdIscpDxeProtocol.h> #include <AmdStyxHelperLib.h>
+#define PLATINIT_CONTEXT_ID ( 0x0 ) // used on SMC calls
/* These externs are used to relocate some ASM code into Linux
memory.
*/ extern VOID *SecondariesPenStart; extern VOID *SecondariesPenEnd; @@ -34,6 +39,85 @@ extern UINTN
*AsmMailboxBase;
extern EFI_BOOT_SERVICES *gBS;
+AMD_ISCP_DXE_PROTOCOL *mIscpDxeProtocol;
+STATIC +VOID +EFIAPI +AmdStyxBringupSecondary (
- ARM_CORE_INFO *ArmCoreInfoTable,
- UINTN Index,
- EFI_PHYSICAL_ADDRESS SecondaryEntry
- )
+{
- EFI_STATUS Status;
- ISCP_CPU_RESET_INFO CpuResetInfo;
- BOOLEAN fCoreTransitionDone;
- ARM_SMC_ARGS SmcRegs;
- if (FixedPcdGetBool (PcdTrustedFWSupport)) {
- SmcRegs.Arg0 = ARM_SMC_ID_PSCI_CPU_ON_AARCH64;
- SmcRegs.Arg1 = GET_MPID (ArmCoreInfoTable[Index].ClusterId,
ArmCoreInfoTable[Index].CoreId);- SmcRegs.Arg2 = SecondaryEntry;
- SmcRegs.Arg3 = PLATINIT_CONTEXT_ID;
- ArmCallSmc (&SmcRegs);
- if (SmcRegs.Arg0 == ARM_SMC_PSCI_RET_SUCCESS ||
SmcRegs.Arg0 == ARM_SMC_PSCI_RET_ALREADY_ON) {DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index));- } else {
DEBUG ((EFI_D_ERROR, "Warning: Could not transition Core[%d]- to RUN
state.\n", Index));
- }
- } else if (FixedPcdGetBool (PcdIscpSupport)) {
- CpuResetInfo.CoreNum = Index;
- Status = mIscpDxeProtocol->AmdExecuteCpuRetrieveId
(mIscpDxeProtocol,
&CpuResetInfo);- ASSERT_EFI_ERROR (Status);
- // Transition core to the RUN state
- fCoreTransitionDone = FALSE;
- do {
switch (CpuResetInfo.CoreStatus.Status) {case CPU_CORE_POWERDOWN:DEBUG ((EFI_D_ERROR, "Core[%d]: POWERDOWN -> POWERUP\n",Index));
CpuResetInfo.CoreStatus.Status = CPU_CORE_POWERUP;break;case CPU_CORE_POWERUP:case CPU_CORE_SLEEP:DEBUG ((EFI_D_ERROR, "Core[%d]: POWERUP -> RESET\n", Index));CpuResetInfo.CoreStatus.Status = CPU_CORE_RESET;CpuResetInfo.CoreStatus.ResetVector = SecondaryEntry;break;case CPU_CORE_RESET:DEBUG ((EFI_D_ERROR, "Core[%d]: RESET -> RUN\n", Index));CpuResetInfo.CoreStatus.Status = CPU_CORE_RUN;break;default:if (CpuResetInfo.CoreStatus.Status == CPU_CORE_RUN) {DEBUG ((EFI_D_ERROR, "Core[%d] at RUN state.\n", Index));} else {DEBUG ((EFI_D_ERROR, "Warning: Could not transition- Core[%d] to
RUN state.\n", Index));
}fCoreTransitionDone = TRUE;break;}// Transition core to next stateif (!fCoreTransitionDone) {Status = mIscpDxeProtocol->AmdExecuteCpuCoreReset(mIscpDxeProtocol,
&CpuResetInfo);ASSERT_EFI_ERROR (Status);}- } while (!fCoreTransitionDone);
- } else {
- ASSERT (FALSE);
- }
+}
VOID EFIAPI AmdStyxMoveParkedCores( @@ -52,6 +136,19 @@ AmdStyxMoveParkedCores( UINTN CoreMailbox; UINTN CoreParking;
- if (FixedPcdGetBool (PcdIscpSupport)) {
- Status = gBS->LocateProtocol (
&gAmdIscpDxeProtocolGuid,NULL,(VOID **)&mIscpDxeProtocol);- if (EFI_ERROR (Status)) {
mIscpDxeProtocol = NULL;DEBUG ((EFI_D_ERROR, "Failed to Locate ISCP DXE Protocol"));return;- }
- }
- // Get core information ArmCoreInfoTable = AmdStyxGetArmCoreInfoTable (&ArmCoreCount); ASSERT (ArmCoreInfoTable != NULL); @@ -103,17 +200,15 @@
AmdStyxMoveParkedCores( *((UINTN*)(CoreParking + sizeof (UINT64))) = 0x0; *((UINTN*)(CoreParking + SIZE_2KB)) = CoreNum;
- // Move secondary core to our new Pen
- MmioWrite64(ArmCoreInfoTable[CoreNum].MailboxSetAddress,
(UINTN)PenBase);
// Update table entry to be consumed by FDT parser. ArmCoreInfoTable[CoreNum].MailboxSetAddress = CoreMailbox; }
// Flush caches to make sure our pen gets to memory before we
release secondary cores.
- ArmCleanDataCache();
- WriteBackDataCacheRange ((VOID *)MpParkingBase, MpParkingSize);
- // Send msg to secondary cores to jump to our new Pen.
- ArmGicSendSgiTo (PcdGet32(PcdGicDistributorBase),
ARM_GIC_ICDSGIR_FILTER_EVERYONEELSE, 0x0E, PcdGet32 (PcdGicSgiIntId));
- for (CoreNum = 0; CoreNum < ArmCoreCount; CoreNum++) {
- // Move secondary core to our new Pen
- AmdStyxBringupSecondary (ArmCoreInfoTable, CoreNum, PenBase);
}
}
diff --git a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf index 5ac210ba04e4..c660ccd04810 100644 --- a/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf +++ b/Platforms/AMD/Styx/Drivers/FdtDxe/FdtDxe.inf @@ -50,6 +50,7 @@ FdtLib DevicePathLib AmdStyxHelperLib
- ArmSmcLib
[LibraryClasses.AARCH64] ArmGicLib @@ -63,6 +64,7 @@
[Protocols] gEfiFirmwareVolume2ProtocolGuid ##CONSUMED
- gAmdIscpDxeProtocolGuid ## SOMETIMES_CONSUMES
[Pcd] gAmdStyxTokenSpaceGuid.PcdStyxFdt @@ -80,11 +82,7 @@ gAmdStyxTokenSpaceGuid.PcdTrustedFWSupport gAmdStyxTokenSpaceGuid.PcdParkingProtocolBase gAmdStyxTokenSpaceGuid.PcdParkingProtocolSize
- gAmdStyxTokenSpaceGuid.PcdIscpSupport
-[Pcd.AARCH64]
- gArmTokenSpaceGuid.PcdGicDistributorBase
- gArmTokenSpaceGuid.PcdGicSgiIntId
[Depex] TRUE
-- 2.7.4
On 2 May 2016 at 16:58, Duran, Leo leo.duran@amd.com wrote:
-----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Monday, May 02, 2016 9:45 AM To: Duran, Leo leo.duran@amd.com Cc: linaro-uefi@lists.linaro.org; leif.lindholm@linaro.org; ricardo.salveti@linaro.org Subject: Re: [RFC PATCH 6/7] Platforms/Styx/FdtDxe: boot secondaries straight into OS pen
On 2 May 2016 at 16:41, Duran, Leo leo.duran@amd.com wrote:
Ard, Wow... I do like the way this is going! Q: Have you booted all core using DO_PSCI=0?
Into the pen, yes, but not all the way into the OS. I was getting an SError which turned out to be unrelated to these changes.
All, BTW, I'd like to propose that we require PcdTrustedFWSupport = TRUE (for EL3 handling) This way, we could then can drop the ISCP code that
brings cores out of reset into UEFI.
Yes, I think that is reasonable. That would also allow us to move this to generic code, i.e., a generic DXE driver that implements ACPI parking protocol/spin-table on top of PSCI. For now, I would like to see it working first, especially since the MpCore stuff is somewhat broken if you run your XIP PEI code from DRAM
Ummh... the baseline code (prior this patch series) used to be able to boot secondary cores, using DO_PSCI=0 and the patched MainMPCore.c. However, the secondary boot path now gets a "Synchronous Exception" when it invokes GetMpCoreInfo().
I have not tried exercising the (DO_PSCI=0) code path in quite a while, so I don't know when it got broken. (I suspect due to change in EDK2, because the Styx code has not changed)
That's a shame. I don't have the bandwidth currently to dig deeper into this than I already have. In any case, using MpCore with SEC and PEI executing from RAM is a configuration we should stop using asap, so I think you should consider DO_PSCI=0 broken for now, even without the synchronous exception you reported.
Since we are no longer booting the secondaries into SEC and PEI, switch to the unicore PrePeiCore unconditionally.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 4 ---- Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.fdf | 4 ---- 2 files changed, 8 deletions(-)
diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc index e4ece5e23218..7a88c95bc5d1 100644 --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc @@ -558,11 +558,7 @@ DEFINE TRANS_CODE = $(EL3_TO_EL2) # # PEI Phase modules # -!if $(DO_PSCI) ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf -!else - ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf -!endif MdeModulePkg/Core/Pei/PeiMain.inf MdeModulePkg/Universal/PCD/Pei/Pcd.inf { <LibraryClasses> diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.fdf b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.fdf index 263a267fbae7..8d70e9d7c462 100644 --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.fdf +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.fdf @@ -247,11 +247,7 @@ READ_STATUS = TRUE READ_LOCK_CAP = TRUE READ_LOCK_STATUS = TRUE
-!if $(DO_PSCI) INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf -!else - INF ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf -!endif INF MdeModulePkg/Core/Pei/PeiMain.inf INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf INF AmdModulePkg/Iscp/IscpPei.inf
-
Ard Biesheuvel -
Duran, Leo