Hi Ard,
Thanks for the review comments. Please see my replies inline.
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Tuesday, October 18, 2016 3:12 PM
On 17 October 2016 at 21:04, Bhupesh Sharma bhupesh.sharma@freescale.com wrote:
From: Sakar Arora sakar.arora@nxp.com
This is the first UEFI module that executes after power on.
It runs from XIP NOR flash memory and uses on-chip OCRAM as stack.
It also iniitializes DDR controller, copies rest of UEFI firmware to DDR
typo ^^
Sorry, could not get your meaning here. Am I missing the typo (as I am not able to locate it :( )
memory and transfers control to it.
Signed-off-by: Sakar Arora sakar.arora@nxp.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@nxp.com
.../Library/PrePiNor/AArch64/ModuleEntryPoint.S | 34
++++++++++++++
Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.c | 52
++++++++++++++++++++++
Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.inf | 48
++++++++++++++++++++
3 files changed, 134 insertions(+) create mode 100644 Chips/Nxp/QoriqLs/Library/PrePiNor/AArch64/ModuleEntryPoint.S create mode 100644 Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.c create mode 100644 Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.inf
diff --git a/Chips/Nxp/QoriqLs/Library/PrePiNor/AArch64/ModuleEntryPoint.S b/Chips/Nxp/QoriqLs/Library/PrePiNor/AArch64/ModuleEntryPoint.S new file mode 100644 index 0000000..6f3b373 --- /dev/null +++ b/Chips/Nxp/QoriqLs/Library/PrePiNor/AArch64/ModuleEntryPoint.S @@ -0,0 +1,34 @@ +// @ModuleEntryPoint.S +// +// Copyright (c) 2016, Freescale Semiconductor, Inc. All rights
reserved.
+// +// This program and the accompanying materials // are licensed and +made available under the terms and conditions of the BSD License // +which accompanies this distribution. The full text of the license +may be found at // http://opensource.org/licenses/bsd-license.php +// +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" +BASIS, // WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
+//
+#include <AutoGen.h> +#include <AsmMacroIoLibV8.h> +#include <Library/PcdLib.h>
+.text +.align 3
+GCC_ASM_EXPORT(_ModuleEntryPoint)
+StartupAddr: .8byte ASM_PFX(CEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
- LoadConstantToReg (FixedPcdGet32(PcdOcramStackBase), x0)
- mov sp, x0
- LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x0)
- LoadConstantToReg (FixedPcdGet32(PcdFdNorBaseAddress), x1)
- LoadConstantToReg (FixedPcdGet32(PcdPiFdSize), x5)
- add x1, x1, x5
- LoadConstantToReg (FixedPcdGet32(PcdFdSize), x2)
Please drop these LoadConstantToReg() invocations, and use MOV32/64 instead
Ok.
- ldr x4, StartupAddr
- blr x4
diff --git a/Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.c b/Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.c new file mode 100644 index 0000000..4c808db --- /dev/null +++ b/Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.c @@ -0,0 +1,52 @@ +/** @LS1043aPrePiNor.c +# +# Copyright (c) 2016, Freescale Semiconductor, Inc. All rights
reserved.
+# +# This program and the accompanying materials # are licensed and +made available under the terms and conditions of the BSD License # +which accompanies this distribution. The full text of the license +may be found at # http://opensource.org/licenses/bsd-license.php +# +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" +BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
+# +**/
+#include <Library/ArmLib.h> +#include <Library/PcdLib.h>
+extern VOID DramInit();
Please include the appropriate library header
Ok.
+UINTN mGlobalVariableBase = 0;
What is this?
Ok, will add appropriate comments.
+VOID CopyImage(UINT8* Dest, UINT8* Src, UINTN Size) {
- UINTN Count;
- for(Count = 0; Count < Size; Count++) {
- Dest[Count] = Src[Count];
- }
+}
Please use BaseMemoryLib in MdePkg. It works fine with the MMU and caches off
Hmmm. Ok, will try this.
+VOID CEntryPoint(
- UINTN UefiMemoryBase,
- UINTN UefiNorBase,
- UINTN UefiMemorySize
+) +{
- VOID (*PrePiStart)(VOID);
+// Data Cache enabled on Primary core when MMU is enabled.
- ArmDisableDataCache ();
- // Invalidate instruction cache
- ArmInvalidateInstructionCache ();
- // Enable Instruction Caches on all cores.
- ArmEnableInstructionCache ();
- DramInit();
- CopyImage((VOID*)UefiMemoryBase, (VOID*)UefiNorBase,
- UefiMemorySize);
Why is this necessary? The code is already executing, so why do we need to move it?
We are executing currently from NOR (XIP) which is usually slower.
In general, SEC and PEI can execute fine from NOR, and PEI_CORE will relocate itself to RAM as soon as it becomes available.
Right. But our idea is to start relocating to DDR from the PEI phase itself (BTW we don't use the SEC phase for the UEFI implementation on our SoCs), as DDR speeds are much faster than the code execution speed from the NOR flash.
We have tried setting the fastest NOR flash access timings but till these cannot be compared with the DDR execution speeds. With more and more customers demanding a very aggressive Linux boot time (from a cold reset), we put this hack in the PEI phase.
- PrePiStart = (VOID (*)())((UINT64)PcdGet64(PcdFvBaseAddress));
- PrePiStart();
+} diff --git a/Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.inf b/Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.inf new file mode 100644 index 0000000..1b78fa5 --- /dev/null +++ b/Chips/Nxp/QoriqLs/Library/PrePiNor/PrePiNor.inf @@ -0,0 +1,48 @@ +#/** @PrePiNor.inf +# +# Copyright (c) 2016, Freescale Semiconductor, Inc. All rights
reserved.
+# +# This program and the accompanying materials # are licensed and +made available under the terms and conditions of the BSD License # +which accompanies this distribution. The full text of the license +may be found at # http://opensource.org/licenses/bsd-license.php +# +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" +BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
+# +#**/
+[Defines]
- INF_VERSION = 0x00010005
- BASE_NAME = PrePiNor
- FILE_GUID = d959e387-7b91-452c-90e1-
a1dbac10ddb8
Fresh GUID
Ok.
- MODULE_TYPE = SEC
- VERSION_STRING = 1.0
+[Sources]
- PrePiNor.c
+[Sources.AArch64]
- AArch64/ModuleEntryPoint.S
+[Packages]
- MdePkg/MdePkg.dec
- ArmPkg/ArmPkg.dec
- ArmPlatformPkg/ArmPlatformPkg.dec
- OpenPlatformPkg/Chips/Nxp/QoriqLs/NxpQoriqLs.dec
+[LibraryClasses]
- ArmLib
- PcdLib
- DdrLib
+[FixedPcd]
- gNxpQoriqLsTokenSpaceGuid.PcdOcramStackBase
- gNxpQoriqLsTokenSpaceGuid.PcdFdNorBaseAddress
- gNxpQoriqLsTokenSpaceGuid.PcdPiFdSize
- gArmTokenSpaceGuid.PcdFdBaseAddress
- gArmTokenSpaceGuid.PcdFdSize
- gArmTokenSpaceGuid.PcdSystemMemoryBase
- gArmTokenSpaceGuid.PcdSystemMemorySize
- gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
- gArmTokenSpaceGuid.PcdFvBaseAddress
-- 1.9.1
Regards, Bhupesh