| Message ID | CAO6biSZWjTVDPhw89Kkd+FZjEPuinA3+OqrgZmdjt8uJ+x+r7g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi Steve,
I would say there is nothing wrong in your patch. Except the approach your
are using to boot UEFI on your platform might not be appropriate. Making
these two PCDs pointing into System Memory is not the way there are expected
to be used.
PcdCPUCoreSecStackBase and PcdCPUCoresStackBase (and SEC & PeiCore) should
only be used when EDK2 is started in Secure World and before the (permanent)
system memory (DRAM) is initialized.
PcdCPUCoreSecStackBase should point somewhere into your Secure RAM. And
PcdCPUCoresStackBase should point somewhere into your temporary memory (eg:
SRAM).
If your platform has a Secure/Trusted Firmware then you should not use
ArmPlatformPkg/Sec into your UEFI firmware. Your firmware should start after
the SEC phase.
ArmPlatformPkg/Sec has been implemented to initialize the Secure World.
Running this module in Non-Secure world is not correct.
To get a proper implementation of a Trusted/Secure Firmware on ARMv8
platform, you should have a look at the ARM Trusted Firmware Open SOurce
project: https://github.com/ARM-software/arm-trusted-firmware
If you know what you are doing and still want to use ArmPlatformPkg/Sec,
then there is a chance your Secure RAM is into your 32bit address space
(even if your memory map is bigger than 4GB).
PEI Core is designed to start from XIP memory and used temporary memory. A
PEIM module is expected to initialize your permanent system memory. PEI Core
would then pass control to the DXE phase.
PEI Core relocates itself from temporary memory to permanent memory during
its execution. So starting PEI Core from permanent memory means the module
will copy itself into permanent memory again.
On most ARM Platforms, UEFI starts as a secondary stage boot loader (the
Trusted Firmware being the first stage). To prevent this non-required copy,
I introduced ArmPlatformPkg/PrePi that does the required PI initialization
and pass control to the DXE core.
The drawback of ArmPlatformPkg/PrePi is it does not support PEIMs. But it is
acceptable in most cases.
Again, generally your temporary memory (eg: SRAM) is in your first 4GB of
your address space.
What I want to say is - yes, there is nothing wrong to make
PcdCPUCoreSecStackBase and PcdCPUCoresStackBase 64-bit PCDs. But the reason
to use these PCDs might not be valid in your case.
Have a look at this page for additional information:
http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=ArmPlatformP
kg
Let me know if my email is clear enough or if you need further explanation.
I would like to reuse the content in the wikipages.
Thanks,
Olivier
From: Steven Kinney [mailto:steven.kinney@linaro.org]
Sent: 21 February 2014 17:32
To: edk2-devel@lists.sourceforge.net;
edk2-devel-request@lists.sourceforge.net
Subject: [edk2] [RFC] ArmPlatformPkg: PcdCPUCoreSecStackBase and
PcdCPUCoresStackBase: UINT64 (patch)
Hi Olivier,
I modified the Stacks so that the typedef is UINT64;
for pointing to System Memory outside of the addressable region accessable
via UINT32. This is very similar to the System Memory changes you made
regarding System Memory base address earlier. I can across this issue when
migrating to our silicon memory map, which has the stacks in DRAM outside of
the addressable range afforded by a UINT32 typedef. Please suggest changes
or considerations I might have overlooked. I tested this on AARCH64 across
multiple memory regions; including the RTSM SRAM locations.
Thanks,
Steve
------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
From fe3fe86ec70945cc8612b5c63b62764652a7a415 Mon Sep 17 00:00:00 2001 From: Steven Kinney <steven.kinney@amd.com> Date: Fri, 21 Feb 2014 08:26:21 -0600 Subject: [PATCH] ArmPlatformPkg: PcdCPUCoreSecStackBase and PcdCPUCoresStackBase: UINT64 Modify the type defintion for PcdCPUCoreSecStackBase and PcdCPUCoresStackBase to allow for stacks that reside in upper System Memory. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Steven Kinney <steven.kinney@amd.com> --- ArmPlatformPkg/ArmPlatformPkg.dec | 4 ++-- .../Pei/PeiArmPlatformGlobalVariableLib.c | 6 +++--- ArmPlatformPkg/PrePeiCore/MainMPCore.c | 2 +- ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec index 114c591..71301f4 100644 --- a/ArmPlatformPkg/ArmPlatformPkg.dec +++ b/ArmPlatformPkg/ArmPlatformPkg.dec @@ -63,12 +63,12 @@ gArmPlatformTokenSpaceGuid.PcdClusterCount|1|UINT32|0x00000038 # Stack for CPU Cores in Secure Mode - gArmPlatformTokenSpaceGuid.PcdCPUCoresSecStackBase|0|UINT32|0x00000005 + gArmPlatformTokenSpaceGuid.PcdCPUCoresSecStackBase|0|UINT64|0x00000005 gArmPlatformTokenSpaceGuid.PcdCPUCoreSecPrimaryStackSize|0x10000|UINT32|0x00000036 gArmPlatformTokenSpaceGuid.PcdCPUCoreSecSecondaryStackSize|0x1000|UINT32|0x00000006 # Stack for CPU Cores in Non Secure Mode - gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0|UINT32|0x00000009 + gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0|UINT64|0x00000009 gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x10000|UINT32|0x00000037 gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize|0x1000|UINT32|0x0000000A diff --git a/ArmPlatformPkg/Library/ArmPlatformGlobalVariableLib/Pei/PeiArmPlatformGlobalVariableLib.c b/ArmPlatformPkg/Library/ArmPlatformGlobalVariableLib/Pei/PeiArmPlatformGlobalVariableLib.c index df3e129..2ffbdbe 100644 --- a/ArmPlatformPkg/Library/ArmPlatformGlobalVariableLib/Pei/PeiArmPlatformGlobalVariableLib.c +++ b/ArmPlatformPkg/Library/ArmPlatformGlobalVariableLib/Pei/PeiArmPlatformGlobalVariableLib.c @@ -34,7 +34,7 @@ ArmPlatformGetGlobalVariable ( // Ensure the Global Variable Size have been initialized ASSERT (VariableOffset < PcdGet32 (PcdPeiGlobalVariableSize)); - GlobalVariableBase = PcdGet32 (PcdCPUCoresStackBase) + PcdGet32 (PcdCPUCorePrimaryStackSize) - PcdGet32 (PcdPeiGlobalVariableSize); + GlobalVariableBase = PcdGet64 (PcdCPUCoresStackBase) + PcdGet32 (PcdCPUCorePrimaryStackSize) - PcdGet32 (PcdPeiGlobalVariableSize); if (VariableSize == 4) { *(UINT32*)Variable = ReadUnaligned32 ((CONST UINT32*)(GlobalVariableBase + VariableOffset)); @@ -57,7 +57,7 @@ ArmPlatformSetGlobalVariable ( // Ensure the Global Variable Size have been initialized ASSERT (VariableOffset < PcdGet32 (PcdPeiGlobalVariableSize)); - GlobalVariableBase = PcdGet32 (PcdCPUCoresStackBase) + PcdGet32 (PcdCPUCorePrimaryStackSize) - PcdGet32 (PcdPeiGlobalVariableSize); + GlobalVariableBase = PcdGet64 (PcdCPUCoresStackBase) + PcdGet32 (PcdCPUCorePrimaryStackSize) - PcdGet32 (PcdPeiGlobalVariableSize); if (VariableSize == 4) { WriteUnaligned32 ((UINT32*)(GlobalVariableBase + VariableOffset), *(UINT32*)Variable); @@ -78,7 +78,7 @@ ArmPlatformGetGlobalVariableAddress ( // Ensure the Global Variable Size have been initialized ASSERT (VariableOffset < PcdGet32 (PcdPeiGlobalVariableSize)); - GlobalVariableBase = PcdGet32 (PcdCPUCoresStackBase) + PcdGet32 (PcdCPUCorePrimaryStackSize) - PcdGet32 (PcdPeiGlobalVariableSize); + GlobalVariableBase = PcdGet64 (PcdCPUCoresStackBase) + PcdGet32 (PcdCPUCorePrimaryStackSize) - PcdGet32 (PcdPeiGlobalVariableSize); return (VOID*)(GlobalVariableBase + VariableOffset); } diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c index 9bfc990..f849d85 100644 --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c @@ -124,7 +124,7 @@ PrimaryMain ( // Adjust the Temporary Ram as the new Ppi List (Common + Platform Ppi Lists) is created at // the base of the primary core stack PpiListSize = ALIGN_VALUE(PpiListSize, 0x4); - TemporaryRamBase = (UINTN)PcdGet32 (PcdCPUCoresStackBase) + PpiListSize; + TemporaryRamBase = (UINTN)PcdGet64 (PcdCPUCoresStackBase) + PpiListSize; TemporaryRamSize = (UINTN)PcdGet32 (PcdCPUCorePrimaryStackSize) - PpiListSize; // Make sure the size is 8-byte aligned. Once divided by 2, the size should be 4-byte aligned diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c index 1abefae..9cae823 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c @@ -53,7 +53,7 @@ CreatePpiList ( ArmPlatformGetPlatformPpiList (&PlatformPpiListSize, &PlatformPpiList); // Copy the Common and Platform PPis in Temporrary Memory - ListBase = PcdGet32 (PcdCPUCoresStackBase); + ListBase = PcdGet64 (PcdCPUCoresStackBase); CopyMem ((VOID*)ListBase, gCommonPpiTable, sizeof(gCommonPpiTable)); CopyMem ((VOID*)(ListBase + sizeof(gCommonPpiTable)), PlatformPpiList, PlatformPpiListSize); @@ -153,7 +153,7 @@ PrePeiCoreGetGlobalVariableMemory ( { ASSERT (GlobalVariableBase != NULL); - *GlobalVariableBase = (UINTN)PcdGet32 (PcdCPUCoresStackBase) + + *GlobalVariableBase = (UINTN)PcdGet64 (PcdCPUCoresStackBase) + (UINTN)PcdGet32 (PcdCPUCorePrimaryStackSize) - (UINTN)PcdGet32 (PcdPeiGlobalVariableSize); -- 1.7.9.5
Hi Olivier, I modified the Stacks so that the typedef is UINT64; for pointing to System Memory outside of the addressable region accessable via UINT32. This is very similar to the System Memory changes you made regarding System Memory base address earlier. I can across this issue when migrating to our silicon memory map, which has the stacks in DRAM outside of the addressable range afforded by a UINT32 typedef. Please suggest changes or considerations I might have overlooked. I tested this on AARCH64 across multiple memory regions; including the RTSM SRAM locations. Thanks, Steve ------------------------------------------------------------------------------ Managing the Performance of Cloud-Based Applications Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. Read the Whitepaper. http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk