Re: [Linaro-uefi] [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

...

-----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 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

-- 2.7.4