The previous post was a single patch. I've expanded the remit here a little, but I'll count them all as v2.
This patch is only an informational DEBUG build only bit of output to let the developer be confident that things are loaded to sensible location. It is optional: [PATCH v2 1/6] ArmPkg: BdsLib: debug to show load addresses
These two are the ones that actually fix the problem of the FDT and initrd being loaded to the wrong location according to the kernel recommendations: [PATCH v2 2/6] ArmPkg: BdsLib: move FDT above 128MiB boundary [PATCH v2 3/6] ArmPkg: BdsLib: move initrd above 128MiB boundary
This patch removes the fixed PCDs that Ard mentioned. It is also optional, but I think it is a worthy tidyup: [PATCH v2 4/6] ArmPkg: BdsLib: replace fixed pcds with constants
These two patches aren't part of the series, but I've included them because I needed them to get things working again. I actually would like these upstream because they are infini tely better than what is there, but I don't mind too much if I have to carry them in my own branch: [PATCH v2 5/6] ArmPlatformPkg/Bds: remove detection of EFI stub [PATCH v2 6/6] ArmPlatformPkg: tc2: match default config to Linaro
In DEBUG builds, output the addresses of the FDT, initrd and kernel images before booting the kernel.
This will help spot problems with images being loaded at addresses the kernel doesn't like.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c | 11 +++++++++++ ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c | 11 +++++++++++ 2 files changed, 22 insertions(+)
diff --git a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c index 0d540c9..da4a79d 100644 --- a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c @@ -128,6 +128,12 @@ StartLinux ( // // Start the Linux Kernel // + DEBUG_CODE_BEGIN(); + DEBUG ((EFI_D_ERROR, "linux: address 0x%x\n", LinuxKernel)); + DEBUG ((EFI_D_ERROR, "linux: length 0x%x\n", LinuxImageSize)); + DEBUG ((EFI_D_ERROR, "fdt: address 0x%x\n", FdtBlobBase)); + DEBUG ((EFI_D_ERROR, "fdt: length 0x%x\n", FdtBlobSize)); + DEBUG_CODE_END();
// x1-x3 are reserved (set to zero) for future use. LinuxKernel ((UINTN)FdtBlobBase, 0, 0, 0); @@ -349,6 +355,11 @@ BootLinuxFdt ( goto EXIT_FREE_FDT; }
+ DEBUG_CODE_BEGIN(); + DEBUG ((EFI_D_ERROR, "initrd: address 0x%x\n", InitrdImage)); + DEBUG ((EFI_D_ERROR, "initrd: length 0x%x\n", InitrdImageSize)); + DEBUG_CODE_END(); + return StartLinux (LinuxImage, LinuxImageSize, FdtBlobBase, FdtBlobSize);
EXIT_FREE_FDT: diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c index 960fdd6..34185b8 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c @@ -126,6 +126,12 @@ StartLinux ( // // Start the Linux Kernel // + DEBUG_CODE_BEGIN(); + DEBUG ((EFI_D_ERROR, "linux: address 0x%x\n", LinuxKernel)); + DEBUG ((EFI_D_ERROR, "linux: length 0x%x\n", LinuxImageSize)); + DEBUG ((EFI_D_ERROR, "fdt: address 0x%x\n", KernelParamsAddress)); + DEBUG ((EFI_D_ERROR, "fdt: length 0x%x\n", KernelParamsSize)); + DEBUG_CODE_END();
// Outside BootServices, so can't use Print(); DEBUG ((EFI_D_ERROR, "\nStarting the kernel:\n\n")); @@ -329,6 +335,11 @@ BootLinuxFdt ( goto EXIT_FREE_FDT; }
+ DEBUG_CODE_BEGIN(); + DEBUG ((EFI_D_ERROR, "initrd: address 0x%x\n", InitrdImage)); + DEBUG ((EFI_D_ERROR, "initrd: length 0x%x\n", InitrdImageSize)); + DEBUG_CODE_END(); + return StartLinux (SystemMemoryBase, LinuxImage, LinuxImageSize, FdtBlobBase, FdtBlobSize, ARM_FDT_MACHINE_TYPE);
EXIT_FREE_FDT:
The device tree was being located within the first 16KB of memory. When the device tree increases in size to >16KB, it no long fits and the behaviour is erratic.
This patch follws the kernel recommendations that the device tree should live at an address greater thatn the 128MiB boundary.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- ArmPkg/ArmPkg.dec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index da0c8a9..b2122b1 100644 --- a/ArmPkg/ArmPkg.dec +++ b/ArmPkg/ArmPkg.dec @@ -158,7 +158,7 @@ # If the fixed FDT address is not available, then it should be loaded below the kernel. # The recommendation from the Linux kernel is to have the FDT below 16KB. # (see the kernel doc: Documentation/arm/Booting) - gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x4000|UINT32|0x00000023 + gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x08400000|UINT32|0x00000023 # The FDT blob must be loaded at a 64bit aligned address. gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x00000026
Move the initrd to an address greater than the 128MiB boundary as requested in the kernel recommendation.
We do this by reusing LINX_FDT_MAX_OFFSET, which is above the 128MiB boundary.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c | 2 +- ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c index da4a79d..e72537f 100644 --- a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c @@ -248,7 +248,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) { - InitrdImageBase = LINUX_KERNEL_MAX_OFFSET; + InitrdImageBase = LINUX_FDT_MAX_OFFSET; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize); diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c index 34185b8..ca6331a 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c @@ -192,7 +192,7 @@ BootLinuxAtag (
if (InitrdDevicePath) { // Load the initrd near to the Linux kernel - InitrdImageBase = LINUX_KERNEL_MAX_OFFSET; + InitrdImageBase = LINUX_FDT_MAX_OFFSET; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize); @@ -280,7 +280,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) { - InitrdImageBase = LINUX_KERNEL_MAX_OFFSET; + InitrdImageBase = LINUX_FDT_MAX_OFFSET; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize);
BdsLib was using fixed PCDs where constants could easily perform that same function.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- .../Application/LinuxLoader/AArch64/LinuxStarter.c | 8 +++--- ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +- ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c | 22 +++++++-------- ArmPkg/Application/LinuxLoader/LinuxLoader.h | 32 ++++++++++++++++++++-- ArmPkg/Application/LinuxLoader/LinuxLoader.inf | 8 ------ ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c | 14 +++------- ArmPkg/ArmPkg.dec | 19 ------------- 7 files changed, 49 insertions(+), 56 deletions(-)
diff --git a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c index e72537f..317e73e 100644 --- a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c @@ -229,12 +229,12 @@ BootLinuxFdt ( //
// Try to put the kernel at the start of RAM so as to give it access to all memory. - // If that fails fall back to try loading it within LINUX_KERNEL_MAX_OFFSET of memory start. + // If that fails fall back to try loading it within LINUX_KERNEL_MAX_ADDR of memory start. LinuxImage = SystemMemoryBase + 0x80000; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateAddress, &LinuxImage, &LinuxImageSize); if (EFI_ERROR (Status)) { // Try again but give the loader more freedom of where to put the image. - LinuxImage = LINUX_KERNEL_MAX_OFFSET; + LinuxImage = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateMaxAddress, &LinuxImage, &LinuxImageSize); if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Linux kernel (%r).\n", Status); @@ -248,7 +248,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) { - InitrdImageBase = LINUX_FDT_MAX_OFFSET; + InitrdImageBase = LINUX_INITRD_MAX_ADDR; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize); @@ -287,7 +287,7 @@ BootLinuxFdt ( // FDT device path explicitly defined. The FDT is relocated later to a // more appropriate location for the Linux kernel. // - FdtBlobBase = LINUX_KERNEL_MAX_OFFSET; + FdtBlobBase = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (FdtDevicePath, AllocateMaxAddress, &FdtBlobBase, &FdtBlobSize); if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Device Tree blob (%r).\n", Status); diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c index fd7ee9c..a6dbf7a 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c @@ -129,7 +129,7 @@ PrepareAtagList ( EFI_PHYSICAL_ADDRESS AtagStartAddress; SYSTEM_MEMORY_RESOURCE *Resource;
- AtagStartAddress = LINUX_ATAG_MAX_OFFSET; + AtagStartAddress = LINUX_ATAG_MAX_ADDR; Status = gBS->AllocatePages (AllocateMaxAddress, EfiBootServicesData, EFI_SIZE_TO_PAGES (ATAG_MAX_SIZE), &AtagStartAddress); if (EFI_ERROR (Status)) { DEBUG ((EFI_D_WARN, "Warning: Failed to allocate Atag at 0x%lX (%r). The Atag will be allocated somewhere else in System Memory.\n", AtagStartAddress, Status)); diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c index ca6331a..11be291 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c @@ -84,18 +84,18 @@ StartLinux ( // physical memory. //Note: There is no requirement on the alignment if (MachineType != ARM_FDT_MACHINE_TYPE) { - if (((UINTN)KernelParamsAddress > LINUX_ATAG_MAX_OFFSET) && (KernelParamsSize < PcdGet32 (PcdArmLinuxAtagMaxOffset))) { - KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem (ALIGN32_BELOW (LINUX_ATAG_MAX_OFFSET - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize); + if (((UINTN)KernelParamsAddress > LINUX_ATAG_MAX_ADDR) && (KernelParamsSize < LINUX_ATAG_MAX_OFFSET)) { + KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem (ALIGN32_BELOW (LINUX_ATAG_MAX_ADDR - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize); } } else { - if (((UINTN)KernelParamsAddress > LINUX_FDT_MAX_OFFSET) && (KernelParamsSize < PcdGet32 (PcdArmLinuxFdtMaxOffset))) { - KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem (ALIGN32_BELOW (LINUX_FDT_MAX_OFFSET - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize); + if (((UINTN)KernelParamsAddress > LINUX_FDT_MAX_ADDR) && (KernelParamsSize < LINUX_FDT_MAX_OFFSET)) { + KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem (ALIGN32_BELOW (LINUX_FDT_MAX_ADDR - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize); } }
- if ((UINTN)LinuxImage > LINUX_KERNEL_MAX_OFFSET) { + if ((UINTN)LinuxImage > LINUX_KERNEL_MAX_ADDR) { //Note: There is no requirement on the alignment - LinuxKernel = (LINUX_KERNEL)CopyMem (ALIGN32_BELOW (LINUX_KERNEL_MAX_OFFSET - LinuxImageSize), (VOID*)(UINTN)LinuxImage, LinuxImageSize); + LinuxKernel = (LINUX_KERNEL)CopyMem (ALIGN32_BELOW (LINUX_KERNEL_MAX_ADDR - LinuxImageSize), (VOID*)(UINTN)LinuxImage, LinuxImageSize); } else { LinuxKernel = (LINUX_KERNEL)(UINTN)LinuxImage; } @@ -183,7 +183,7 @@ BootLinuxAtag ( PERF_START (NULL, "BDS", NULL, 0);
// Load the Linux kernel from a device path - LinuxImage = LINUX_KERNEL_MAX_OFFSET; + LinuxImage = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateMaxAddress, &LinuxImage, &LinuxImageSize); if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Linux kernel.\n"); @@ -192,7 +192,7 @@ BootLinuxAtag (
if (InitrdDevicePath) { // Load the initrd near to the Linux kernel - InitrdImageBase = LINUX_FDT_MAX_OFFSET; + InitrdImageBase = LINUX_FDT_MAX_ADDR; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize); @@ -272,7 +272,7 @@ BootLinuxFdt ( PERF_START (NULL, "BDS", NULL, 0);
// Load the Linux kernel from a device path - LinuxImage = LINUX_KERNEL_MAX_OFFSET; + LinuxImage = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateMaxAddress, &LinuxImage, &LinuxImageSize); if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Linux kernel.\n"); @@ -280,7 +280,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) { - InitrdImageBase = LINUX_FDT_MAX_OFFSET; + InitrdImageBase = LINUX_FDT_MAX_ADDR; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize); @@ -319,7 +319,7 @@ BootLinuxFdt ( // FDT device path explicitly defined. The FDT is relocated later to a // more appropriate location for the Linux kernel. // - FdtBlobBase = LINUX_KERNEL_MAX_OFFSET; + FdtBlobBase = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (FdtDevicePath, AllocateMaxAddress, &FdtBlobBase, &FdtBlobSize); if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Device Tree blob (%r).\n", Status); diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.h b/ArmPkg/Application/LinuxLoader/LinuxLoader.h index 8a23d7f..2544147 100644 --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.h +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.h @@ -37,9 +37,35 @@ #define MAX_MSG_LEN 80
#define LINUX_UIMAGE_SIGNATURE 0x56190527 -#define LINUX_KERNEL_MAX_OFFSET (SystemMemoryBase + PcdGet32(PcdArmLinuxKernelMaxOffset)) -#define LINUX_ATAG_MAX_OFFSET (SystemMemoryBase + PcdGet32(PcdArmLinuxAtagMaxOffset)) -#define LINUX_FDT_MAX_OFFSET (SystemMemoryBase + PcdGet32(PcdArmLinuxFdtMaxOffset)) + +#if defined (MDE_CPU_ARM) + // On ARM, the kernel recommendations say that the kernel should live before + // the 128MiB (0x08000000) boundary. + #define LINUX_KERNEL_MAX_OFFSET 0x08000000 + + // ATAGS data should be in the first 16KB + #define LINUX_ATAG_MAX_OFFSET 0x00004000 + + // The device tree and initrd should live after the 128MiB boundary. + #define LINUX_FDT_MAX_OFFSET 0x08400000 + #define LINUX_INITRD_MAX_OFFSET 0x08400000 + #define LINUX_FDT_ALIGNMENT 0x00000008 +#elif defined (MDE_CPU_AARCH64) + #define LINUX_KERNEL_MAX_OFFSET 0x08000000 + + // arm64 Kernels prior to v4.2 required the device tree to reside in the + // first 512MB of memory + #define LINUX_FDT_MAX_OFFSET 0x20000000 + #define LINUX_INITRD_MAX_OFFSET 0x20000000 + + // Recommendations are to align the FDT to 2MiB + #define LINUX_FDT_ALIGNMENT 0x00200000 +#endif + +#define LINUX_KERNEL_MAX_ADDR (SystemMemoryBase + LINUX_KERNEL_MAX_OFFSET) +#define LINUX_ATAG_MAX_ADDR (SystemMemoryBase + LINUX_ATAG_MAX_OFFSET) +#define LINUX_FDT_MAX_ADDR (SystemMemoryBase + LINUX_FDT_MAX_OFFSET) +#define LINUX_INITRD_MAX_ADDR (SystemMemoryBase + LINUX_INITRD_MAX_OFFSET)
#define ARM_FDT_MACHINE_TYPE 0xFFFFFFFF
diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.inf b/ArmPkg/Application/LinuxLoader/LinuxLoader.inf index 59ab99c..3b640b8 100644 --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.inf +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.inf @@ -78,14 +78,6 @@ [FeaturePcd] gArmTokenSpaceGuid.PcdArmLinuxSpinTable
-[FixedPcd] - gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset - gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment - gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset - -[FixedPcd.ARM] - gArmTokenSpaceGuid.PcdArmLinuxAtagMaxOffset - [Pcd.AARCH64] gArmTokenSpaceGuid.PcdGicDistributorBase gArmTokenSpaceGuid.PcdGicSgiIntId diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c b/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c index 0f53784..6f7bcdc 100644 --- a/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c +++ b/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c @@ -78,20 +78,16 @@ RelocateFdt ( { EFI_STATUS Status; INTN Error; - UINT64 FdtAlignment;
*RelocatedFdtSize = OriginalFdtSize + FDT_ADDITIONAL_ENTRIES_SIZE;
// If FDT load address needs to be aligned, allocate more space. - FdtAlignment = PcdGet32 (PcdArmLinuxFdtAlignment); - if (FdtAlignment != 0) { - *RelocatedFdtSize += FdtAlignment; - } + *RelocatedFdtSize += LINUX_FDT_ALIGNMENT;
// Try below a watermark address. Status = EFI_NOT_FOUND; - if (PcdGet32 (PcdArmLinuxFdtMaxOffset) != 0) { - *RelocatedFdt = LINUX_FDT_MAX_OFFSET; + if (LINUX_FDT_MAX_OFFSET != 0) { + *RelocatedFdt = LINUX_FDT_MAX_ADDR; Status = gBS->AllocatePages (AllocateMaxAddress, EfiBootServicesData, EFI_SIZE_TO_PAGES (*RelocatedFdtSize), RelocatedFdt); if (EFI_ERROR (Status)) { @@ -112,9 +108,7 @@ RelocateFdt ( }
*RelocatedFdtAlloc = *RelocatedFdt; - if (FdtAlignment != 0) { - *RelocatedFdt = ALIGN (*RelocatedFdt, FdtAlignment); - } + *RelocatedFdt = ALIGN (*RelocatedFdt, LINUX_FDT_ALIGNMENT);
// Load the Original FDT tree into the new region Error = fdt_open_into ((VOID*)(UINTN) OriginalFdt, diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index b2122b1..59916d2 100644 --- a/ArmPkg/ArmPkg.dec +++ b/ArmPkg/ArmPkg.dec @@ -115,8 +115,6 @@ # # BdsLib # - # The compressed Linux kernel is expected to be under 128MB from the beginning of the System Memory - gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x08000000|UINT32|0x0000001F # Maximum file size for TFTP servers that do not support 'tsize' extension gArmTokenSpaceGuid.PcdMaxTftpFileSize|0x01000000|UINT32|0x00000000
@@ -152,16 +150,6 @@ # By default we do not do a transition to non-secure mode gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x0|UINT32|0x0000003E
- # The Linux ATAGs are expected to be under 0x4000 (16KB) from the beginning of the System Memory - gArmTokenSpaceGuid.PcdArmLinuxAtagMaxOffset|0x4000|UINT32|0x00000020 - - # If the fixed FDT address is not available, then it should be loaded below the kernel. - # The recommendation from the Linux kernel is to have the FDT below 16KB. - # (see the kernel doc: Documentation/arm/Booting) - gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x08400000|UINT32|0x00000023 - # The FDT blob must be loaded at a 64bit aligned address. - gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x00000026 - # Non Secure Access Control Register # - BIT15 : NSASEDIS - Disable Non-secure Advanced SIMD functionality # - BIT14 : NSD32DIS - Disable Non-secure use of D16-D31 @@ -200,13 +188,6 @@ # Other modes include using SP0 or switching to Aarch32, but these are # not currently supported. gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x3c9|UINT32|0x0000003E - # If the fixed FDT address is not available, then it should be loaded above the kernel. - # The recommendation from the AArch64 Linux kernel is to have the FDT below 512MB. - # (see the kernel doc: Documentation/arm64/booting.txt) - gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x20000000|UINT32|0x00000023 - # The FDT blob must be loaded at a 2MB aligned address. - gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x00200000|UINT32|0x00000026 -
# # These PCDs are also defined as 'PcdsDynamic' or 'PcdsPatchableInModule' to be
If the ARM BDS detected that there was an EFI stub on the kernel or application that was being booted, it forced the user to boot it as an EFI application and didn't give the user an option to boot it as an FDT or ATAGS kernel.
It was also very slow because it had to load the image from disk/wherever to check the image.
This patch removes that check and reverts to the original behaviour where the user is asked if it is an EFI application or not.
This patch is also needed to be able to boot via TFTP. The orginal code would download the file from the server and if it found the EFI header, it doesn't give the user the option to specify an initrd or FDT to load.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- ArmPlatformPkg/Bds/BootLinux.c | 44 +++++------------------------------------- 1 file changed, 5 insertions(+), 39 deletions(-)
diff --git a/ArmPlatformPkg/Bds/BootLinux.c b/ArmPlatformPkg/Bds/BootLinux.c index 0445e89..82a23d4 100644 --- a/ArmPlatformPkg/Bds/BootLinux.c +++ b/ArmPlatformPkg/Bds/BootLinux.c @@ -47,11 +47,6 @@ IsEfiBinary ( CHAR16* FileName; EFI_DEVICE_PATH* PrevDevicePathNode; EFI_DEVICE_PATH* DevicePathNode; - EFI_PHYSICAL_ADDRESS Image; - UINTN FileSize; - EFI_IMAGE_DOS_HEADER* DosHeader; - UINTN PeCoffHeaderOffset; - EFI_IMAGE_NT_HEADERS32* NtHeader;
ASSERT (EfiBinary != NULL);
@@ -83,40 +78,11 @@ IsEfiBinary ( } else if (HasFilePathEfiExtension (FileName)) { *EfiBinary = TRUE; } else { - // Check if the file exist - Status = BdsLoadImage (DevicePath, AllocateAnyPages, &Image, &FileSize); - if (!EFI_ERROR (Status)) { - - DosHeader = (EFI_IMAGE_DOS_HEADER *)(UINTN) Image; - if (DosHeader->e_magic == EFI_IMAGE_DOS_SIGNATURE) { - // - // DOS image header is present, - // so read the PE header after the DOS image header. - // - PeCoffHeaderOffset = DosHeader->e_lfanew; - } else { - PeCoffHeaderOffset = 0; - } - - // - // Check PE/COFF image. - // - NtHeader = (EFI_IMAGE_NT_HEADERS32 *)(UINTN) (Image + PeCoffHeaderOffset); - if (NtHeader->Signature != EFI_IMAGE_NT_SIGNATURE) { - *EfiBinary = FALSE; - } else { - *EfiBinary = TRUE; - } - - // Free memory - gBS->FreePages (Image, EFI_SIZE_TO_PAGES (FileSize)); - } else { - // If we did not manage to open it then ask for the type - Print (L"Is an EFI Application? "); - Status = GetHIInputBoolean (EfiBinary); - if (EFI_ERROR (Status)) { - return EFI_ABORTED; - } + // If we did not manage to open it then ask for the type + Print (L"Is an EFI Application? "); + Status = GetHIInputBoolean (EfiBinary); + if (EFI_ERROR (Status)) { + return EFI_ABORTED; } }
Update the default boot device config to match the layout supplied by Linaro, on behalf of ARM, in the recommended firmware [1].
[1] https://git.linaro.org/arm/vexpress-firmware.git/shortlog/refs/heads/vexpres...
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org --- ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc index c1e3513..f58de8e 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc @@ -176,14 +176,14 @@ # # Define the device path to the FDT for the platform # - gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/ca15a7" + gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L"VenHw(1F15DA3C-37FF-4070-B471-BB4AF12A724A)/board.dtb"
# # ARM OS Loader # gArmPlatformTokenSpaceGuid.PcdDefaultBootDescription|L"Linux from NorFlash" gArmPlatformTokenSpaceGuid.PcdDefaultBootDevicePath|L"Fv(73DCB643-3862-4904-9076-A94AF1890243)/LinuxLoader.efi" - gArmPlatformTokenSpaceGuid.PcdDefaultBootArgument|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/kernel -c "console=ttyAMA0,38400 earlyprintk debug verbose"" + gArmPlatformTokenSpaceGuid.PcdDefaultBootArgument|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/zImage -c "console=ttyAMA0,38400 rootwait androidboot.hardware=arm-versatileexpress-usb root=/dev/sda2" -f VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/ramdisk.img"
# Use the serial console (ConIn & ConOut) and the Graphic driver (ConOut) # PL111 - CLCD
On 28 July 2015 at 19:10, Ryan Harkin ryan.harkin@linaro.org wrote:
In DEBUG builds, output the addresses of the FDT, initrd and kernel images before booting the kernel.
This will help spot problems with images being loaded at addresses the kernel doesn't like.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c | 11 +++++++++++ ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c | 11 +++++++++++ 2 files changed, 22 insertions(+)
diff --git a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c index 0d540c9..da4a79d 100644 --- a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c @@ -128,6 +128,12 @@ StartLinux ( // // Start the Linux Kernel //
DEBUG_CODE_BEGIN();
DEBUG ((EFI_D_ERROR, "linux: address 0x%x\n", LinuxKernel));
DEBUG ((EFI_D_ERROR, "linux: length 0x%x\n", LinuxImageSize));
DEBUG ((EFI_D_ERROR, "fdt: address 0x%x\n", FdtBlobBase));
DEBUG ((EFI_D_ERROR, "fdt: length 0x%x\n", FdtBlobSize));
DEBUG_CODE_END();
// x1-x3 are reserved (set to zero) for future use. LinuxKernel ((UINTN)FdtBlobBase, 0, 0, 0);
@@ -349,6 +355,11 @@ BootLinuxFdt ( goto EXIT_FREE_FDT; }
- DEBUG_CODE_BEGIN();
- DEBUG ((EFI_D_ERROR, "initrd: address 0x%x\n", InitrdImage));
- DEBUG ((EFI_D_ERROR, "initrd: length 0x%x\n", InitrdImageSize));
- DEBUG_CODE_END();
- return StartLinux (LinuxImage, LinuxImageSize, FdtBlobBase, FdtBlobSize);
EXIT_FREE_FDT: diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c index 960fdd6..34185b8 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c @@ -126,6 +126,12 @@ StartLinux ( // // Start the Linux Kernel //
DEBUG_CODE_BEGIN();
DEBUG ((EFI_D_ERROR, "linux: address 0x%x\n", LinuxKernel));
DEBUG ((EFI_D_ERROR, "linux: length 0x%x\n", LinuxImageSize));
DEBUG ((EFI_D_ERROR, "fdt: address 0x%x\n", KernelParamsAddress));
DEBUG ((EFI_D_ERROR, "fdt: length 0x%x\n", KernelParamsSize));
DEBUG_CODE_END();
// Outside BootServices, so can't use Print(); DEBUG ((EFI_D_ERROR, "\nStarting the kernel:\n\n"));
@@ -329,6 +335,11 @@ BootLinuxFdt ( goto EXIT_FREE_FDT; }
- DEBUG_CODE_BEGIN();
- DEBUG ((EFI_D_ERROR, "initrd: address 0x%x\n", InitrdImage));
- DEBUG ((EFI_D_ERROR, "initrd: length 0x%x\n", InitrdImageSize));
- DEBUG_CODE_END();
- return StartLinux (SystemMemoryBase, LinuxImage, LinuxImageSize, FdtBlobBase, FdtBlobSize, ARM_FDT_MACHINE_TYPE);
EXIT_FREE_FDT:
2.1.0
On 28 July 2015 at 19:10, Ryan Harkin ryan.harkin@linaro.org wrote:
The device tree was being located within the first 16KB of memory. When the device tree increases in size to >16KB, it no long fits and the behaviour is erratic.
This patch follws the kernel recommendations that the device tree should live at an address greater thatn the 128MiB boundary.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
ArmPkg/ArmPkg.dec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index da0c8a9..b2122b1 100644 --- a/ArmPkg/ArmPkg.dec +++ b/ArmPkg/ArmPkg.dec @@ -158,7 +158,7 @@ # If the fixed FDT address is not available, then it should be loaded below the kernel. # The recommendation from the Linux kernel is to have the FDT below 16KB. # (see the kernel doc: Documentation/arm/Booting)
- gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x4000|UINT32|0x00000023
- gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x08400000|UINT32|0x00000023 # The FDT blob must be loaded at a 64bit aligned address. gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x00000026
-- 2.1.0
On 28 July 2015 at 19:10, Ryan Harkin ryan.harkin@linaro.org wrote:
Move the initrd to an address greater than the 128MiB boundary as requested in the kernel recommendation.
We do this by reusing LINX_FDT_MAX_OFFSET, which is above the 128MiB boundary.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c | 2 +- ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c index da4a79d..e72537f 100644 --- a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c @@ -248,7 +248,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) {
- InitrdImageBase = LINUX_KERNEL_MAX_OFFSET;
- InitrdImageBase = LINUX_FDT_MAX_OFFSET; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize);
arm64 has no such requirement, but since it can support the same logic we apply in the 32-bit case, I still think it is the preferred strategy.
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c index 34185b8..ca6331a 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c @@ -192,7 +192,7 @@ BootLinuxAtag (
if (InitrdDevicePath) { // Load the initrd near to the Linux kernel
- InitrdImageBase = LINUX_KERNEL_MAX_OFFSET;
- InitrdImageBase = LINUX_FDT_MAX_OFFSET; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize);
@@ -280,7 +280,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) {
- InitrdImageBase = LINUX_KERNEL_MAX_OFFSET;
- InitrdImageBase = LINUX_FDT_MAX_OFFSET; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize);
-- 2.1.0
On 28 July 2015 at 19:10, Ryan Harkin ryan.harkin@linaro.org wrote:
BdsLib was using fixed PCDs where constants could easily perform that same function.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Application/LinuxLoader/AArch64/LinuxStarter.c | 8 +++--- ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +- ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c | 22 +++++++-------- ArmPkg/Application/LinuxLoader/LinuxLoader.h | 32 ++++++++++++++++++++-- ArmPkg/Application/LinuxLoader/LinuxLoader.inf | 8 ------ ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c | 14 +++------- ArmPkg/ArmPkg.dec | 19 ------------- 7 files changed, 49 insertions(+), 56 deletions(-)
diff --git a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c index e72537f..317e73e 100644 --- a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c @@ -229,12 +229,12 @@ BootLinuxFdt ( //
// Try to put the kernel at the start of RAM so as to give it access to all memory.
- // If that fails fall back to try loading it within LINUX_KERNEL_MAX_OFFSET of memory start.
- // If that fails fall back to try loading it within LINUX_KERNEL_MAX_ADDR of memory start. LinuxImage = SystemMemoryBase + 0x80000; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateAddress, &LinuxImage, &LinuxImageSize); if (EFI_ERROR (Status)) { // Try again but give the loader more freedom of where to put the image.
- LinuxImage = LINUX_KERNEL_MAX_OFFSET;
- LinuxImage = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateMaxAddress, &LinuxImage, &LinuxImageSize); if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Linux kernel (%r).\n", Status);
@@ -248,7 +248,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) {
- InitrdImageBase = LINUX_FDT_MAX_OFFSET;
- InitrdImageBase = LINUX_INITRD_MAX_ADDR; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress, &InitrdImageBase, &InitrdImageBaseSize); if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages, &InitrdImageBase, &InitrdImageBaseSize);
@@ -287,7 +287,7 @@ BootLinuxFdt ( // FDT device path explicitly defined. The FDT is relocated later to a // more appropriate location for the Linux kernel. //
- FdtBlobBase = LINUX_KERNEL_MAX_OFFSET;
- FdtBlobBase = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (FdtDevicePath, AllocateMaxAddress, &FdtBlobBase, &FdtBlobSize); if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Device Tree blob (%r).\n", Status);
I'd recommend dropping all hunks against this file. First of all, the preferred offset is as low as possible as long as it is 2 MB aligned. But the other logic in this file is severely flawed as well: - TEXT_OFFSET is usually 0x80000 but we should really read the actual value from the Image header - the CopyMem () that occurs may move the Image outside of the allocated area - we should allocate some slack above the kernel Image: the header has an image_size field these days, but for older kernels, we should have at least around half a meg for the page tables - the spin table code puts the mailboxes outside of the allocation it performs for the pen code
I am in two minds between proposing a fix for all of this and removing the LinuxLoader altogether from the AARCH64 platforms
On 28 July 2015 at 19:10, Ryan Harkin ryan.harkin@linaro.org wrote:
If the ARM BDS detected that there was an EFI stub on the kernel or application that was being booted, it forced the user to boot it as an EFI application and didn't give the user an option to boot it as an FDT or ATAGS kernel.
It was also very slow because it had to load the image from disk/wherever to check the image.
This patch removes that check and reverts to the original behaviour where the user is asked if it is an EFI application or not.
This patch is also needed to be able to boot via TFTP. The orginal code would download the file from the server and if it found the EFI header, it doesn't give the user the option to specify an initrd or FDT to load.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
ArmPlatformPkg/Bds/BootLinux.c | 44 +++++------------------------------------- 1 file changed, 5 insertions(+), 39 deletions(-)
diff --git a/ArmPlatformPkg/Bds/BootLinux.c b/ArmPlatformPkg/Bds/BootLinux.c index 0445e89..82a23d4 100644 --- a/ArmPlatformPkg/Bds/BootLinux.c +++ b/ArmPlatformPkg/Bds/BootLinux.c @@ -47,11 +47,6 @@ IsEfiBinary ( CHAR16* FileName; EFI_DEVICE_PATH* PrevDevicePathNode; EFI_DEVICE_PATH* DevicePathNode;
EFI_PHYSICAL_ADDRESS Image;
UINTN FileSize;
EFI_IMAGE_DOS_HEADER* DosHeader;
UINTN PeCoffHeaderOffset;
EFI_IMAGE_NT_HEADERS32* NtHeader;
ASSERT (EfiBinary != NULL);
@@ -83,40 +78,11 @@ IsEfiBinary ( } else if (HasFilePathEfiExtension (FileName)) { *EfiBinary = TRUE; } else {
- // Check if the file exist
- Status = BdsLoadImage (DevicePath, AllocateAnyPages, &Image, &FileSize);
- if (!EFI_ERROR (Status)) {
DosHeader = (EFI_IMAGE_DOS_HEADER *)(UINTN) Image;if (DosHeader->e_magic == EFI_IMAGE_DOS_SIGNATURE) {//// DOS image header is present,// so read the PE header after the DOS image header.//PeCoffHeaderOffset = DosHeader->e_lfanew;} else {PeCoffHeaderOffset = 0;}//// Check PE/COFF image.//NtHeader = (EFI_IMAGE_NT_HEADERS32 *)(UINTN) (Image + PeCoffHeaderOffset);if (NtHeader->Signature != EFI_IMAGE_NT_SIGNATURE) {*EfiBinary = FALSE;} else {*EfiBinary = TRUE;}// Free memorygBS->FreePages (Image, EFI_SIZE_TO_PAGES (FileSize));- } else {
// If we did not manage to open it then ask for the typePrint (L"Is an EFI Application? ");Status = GetHIInputBoolean (EfiBinary);if (EFI_ERROR (Status)) {return EFI_ABORTED;}
- // If we did not manage to open it then ask for the type
- Print (L"Is an EFI Application? ");
- Status = GetHIInputBoolean (EfiBinary);
- if (EFI_ERROR (Status)) {
return EFI_ABORTED;-- 2.1.0
On 28 July 2015 at 19:10, Ryan Harkin ryan.harkin@linaro.org wrote:
Update the default boot device config to match the layout supplied by Linaro, on behalf of ARM, in the recommended firmware [1].
[1] https://git.linaro.org/arm/vexpress-firmware.git/shortlog/refs/heads/vexpres...
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc index c1e3513..f58de8e 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-CTA15-A7.dsc @@ -176,14 +176,14 @@ # # Define the device path to the FDT for the platform #
- gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/ca15a7"
gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L"VenHw(1F15DA3C-37FF-4070-B471-BB4AF12A724A)/board.dtb"
# # ARM OS Loader # gArmPlatformTokenSpaceGuid.PcdDefaultBootDescription|L"Linux from NorFlash" gArmPlatformTokenSpaceGuid.PcdDefaultBootDevicePath|L"Fv(73DCB643-3862-4904-9076-A94AF1890243)/LinuxLoader.efi"
- gArmPlatformTokenSpaceGuid.PcdDefaultBootArgument|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/kernel -c "console=ttyAMA0,38400 earlyprintk debug verbose""
gArmPlatformTokenSpaceGuid.PcdDefaultBootArgument|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/zImage -c "console=ttyAMA0,38400 rootwait androidboot.hardware=arm-versatileexpress-usb root=/dev/sda2" -f VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/ramdisk.img"
# Use the serial console (ConIn & ConOut) and the Graphic driver (ConOut) # PL111 - CLCD
-- 2.1.0
On 30 July 2015 at 15:44, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 28 July 2015 at 19:10, Ryan Harkin ryan.harkin@linaro.org wrote:
BdsLib was using fixed PCDs where constants could easily perform that same function.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org
.../Application/LinuxLoader/AArch64/LinuxStarter.c | 8 +++--- ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +- ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c | 22 +++++++-------- ArmPkg/Application/LinuxLoader/LinuxLoader.h | 32
++++++++++++++++++++--
ArmPkg/Application/LinuxLoader/LinuxLoader.inf | 8 ------ ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c | 14 +++------- ArmPkg/ArmPkg.dec | 19 ------------- 7 files changed, 49 insertions(+), 56 deletions(-)
diff --git a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c
b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c
index e72537f..317e73e 100644 --- a/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/AArch64/LinuxStarter.c @@ -229,12 +229,12 @@ BootLinuxFdt ( //
// Try to put the kernel at the start of RAM so as to give it access
to all memory.
- // If that fails fall back to try loading it within
LINUX_KERNEL_MAX_OFFSET of memory start.
- // If that fails fall back to try loading it within
LINUX_KERNEL_MAX_ADDR of memory start.
LinuxImage = SystemMemoryBase + 0x80000; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateAddress,
&LinuxImage, &LinuxImageSize);
if (EFI_ERROR (Status)) { // Try again but give the loader more freedom of where to put the
image.
- LinuxImage = LINUX_KERNEL_MAX_OFFSET;
- LinuxImage = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateMaxAddress,
&LinuxImage, &LinuxImageSize);
if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Linux kernel (%r).\n", Status);@@ -248,7 +248,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) {
- InitrdImageBase = LINUX_FDT_MAX_OFFSET;
- InitrdImageBase = LINUX_INITRD_MAX_ADDR; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress,
&InitrdImageBase, &InitrdImageBaseSize);
if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages,&InitrdImageBase, &InitrdImageBaseSize);
@@ -287,7 +287,7 @@ BootLinuxFdt ( // FDT device path explicitly defined. The FDT is relocated later
to a
// more appropriate location for the Linux kernel. //
- FdtBlobBase = LINUX_KERNEL_MAX_OFFSET;
- FdtBlobBase = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (FdtDevicePath, AllocateMaxAddress,
&FdtBlobBase, &FdtBlobSize);
if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Device Tree blob (%r).\n", Status);I'd recommend dropping all hunks against this file. First of all, the preferred offset is as low as possible as long as it is 2 MB aligned. But the other logic in this file is severely flawed as well:
- TEXT_OFFSET is usually 0x80000 but we should really read the actual
value from the Image header
- the CopyMem () that occurs may move the Image outside of the allocated
area
- we should allocate some slack above the kernel Image: the header has
an image_size field these days, but for older kernels, we should have at least around half a meg for the page tables
- the spin table code puts the mailboxes outside of the allocation it
performs for the pen code
I am in two minds between proposing a fix for all of this and removing the LinuxLoader altogether from the AARCH64 platforms
This change is simply replacing PCDs with constants. It's not attempting to change the logic, which as you point out, is severely flawed. So the problems you point out already exist.
To my mind, if I'm going to replace the PCDs, I have to do it everywhere.
I'd expect fixes to the logic - or removing the LinuxLoader from Aarch64 - to be implemented as a separate patch. But unless that patch goes in before these, I think I need to keep the hunks against this file.
I'd be happy to see the LinuxLoader removed from Aarch64, except some people use it for Aarch64 TFTP booting because the EFI stub is unable to fetch an initrd or device tree via TFTP, but Olivier reckoned I could work around that issue in a better way. But I haven't out worked if I can or not.
I know the device tree can be pre-loaded via TFTP, but I am not sure how to handle the initrd.
-- Ard.
diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
index fd7ee9c..a6dbf7a 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c @@ -129,7 +129,7 @@ PrepareAtagList ( EFI_PHYSICAL_ADDRESS AtagStartAddress; SYSTEM_MEMORY_RESOURCE *Resource;
- AtagStartAddress = LINUX_ATAG_MAX_OFFSET;
- AtagStartAddress = LINUX_ATAG_MAX_ADDR; Status = gBS->AllocatePages (AllocateMaxAddress, EfiBootServicesData,
EFI_SIZE_TO_PAGES (ATAG_MAX_SIZE), &AtagStartAddress);
if (EFI_ERROR (Status)) { DEBUG ((EFI_D_WARN, "Warning: Failed to allocate Atag at 0x%lX
(%r). The Atag will be allocated somewhere else in System Memory.\n", AtagStartAddress, Status));
diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c
b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c
index ca6331a..11be291 100644 --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxStarter.c @@ -84,18 +84,18 @@ StartLinux ( // physical memory. //Note: There is no requirement on the alignment if (MachineType != ARM_FDT_MACHINE_TYPE) {
- if (((UINTN)KernelParamsAddress > LINUX_ATAG_MAX_OFFSET) &&
(KernelParamsSize < PcdGet32 (PcdArmLinuxAtagMaxOffset))) {
KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem(ALIGN32_BELOW (LINUX_ATAG_MAX_OFFSET - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize);
- if (((UINTN)KernelParamsAddress > LINUX_ATAG_MAX_ADDR) &&
(KernelParamsSize < LINUX_ATAG_MAX_OFFSET)) {
KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem(ALIGN32_BELOW (LINUX_ATAG_MAX_ADDR - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize);
}} else {
- if (((UINTN)KernelParamsAddress > LINUX_FDT_MAX_OFFSET) &&
(KernelParamsSize < PcdGet32 (PcdArmLinuxFdtMaxOffset))) {
KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem(ALIGN32_BELOW (LINUX_FDT_MAX_OFFSET - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize);
- if (((UINTN)KernelParamsAddress > LINUX_FDT_MAX_ADDR) &&
(KernelParamsSize < LINUX_FDT_MAX_OFFSET)) {
KernelParamsAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)CopyMem(ALIGN32_BELOW (LINUX_FDT_MAX_ADDR - KernelParamsSize), (VOID*)(UINTN)KernelParamsAddress, KernelParamsSize);
}}
- if ((UINTN)LinuxImage > LINUX_KERNEL_MAX_OFFSET) {
- if ((UINTN)LinuxImage > LINUX_KERNEL_MAX_ADDR) { //Note: There is no requirement on the alignment
- LinuxKernel = (LINUX_KERNEL)CopyMem (ALIGN32_BELOW
(LINUX_KERNEL_MAX_OFFSET - LinuxImageSize), (VOID*)(UINTN)LinuxImage, LinuxImageSize);
- LinuxKernel = (LINUX_KERNEL)CopyMem (ALIGN32_BELOW
(LINUX_KERNEL_MAX_ADDR - LinuxImageSize), (VOID*)(UINTN)LinuxImage, LinuxImageSize);
} else { LinuxKernel = (LINUX_KERNEL)(UINTN)LinuxImage; } @@ -183,7 +183,7 @@ BootLinuxAtag ( PERF_START (NULL, "BDS", NULL, 0);
// Load the Linux kernel from a device path
- LinuxImage = LINUX_KERNEL_MAX_OFFSET;
- LinuxImage = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateMaxAddress,
&LinuxImage, &LinuxImageSize);
if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Linux kernel.\n"); @@ -192,7 +192,7 @@ BootLinuxAtag (
if (InitrdDevicePath) { // Load the initrd near to the Linux kernel
- InitrdImageBase = LINUX_FDT_MAX_OFFSET;
- InitrdImageBase = LINUX_FDT_MAX_ADDR; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress,
&InitrdImageBase, &InitrdImageBaseSize);
if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages,&InitrdImageBase, &InitrdImageBaseSize);
@@ -272,7 +272,7 @@ BootLinuxFdt ( PERF_START (NULL, "BDS", NULL, 0);
// Load the Linux kernel from a device path
- LinuxImage = LINUX_KERNEL_MAX_OFFSET;
- LinuxImage = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (LinuxKernelDevicePath, AllocateMaxAddress,
&LinuxImage, &LinuxImageSize);
if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Linux kernel.\n"); @@ -280,7 +280,7 @@ BootLinuxFdt ( }
if (InitrdDevicePath) {
- InitrdImageBase = LINUX_FDT_MAX_OFFSET;
- InitrdImageBase = LINUX_FDT_MAX_ADDR; Status = BdsLoadImage (InitrdDevicePath, AllocateMaxAddress,
&InitrdImageBase, &InitrdImageBaseSize);
if (Status == EFI_OUT_OF_RESOURCES) { Status = BdsLoadImage (InitrdDevicePath, AllocateAnyPages,&InitrdImageBase, &InitrdImageBaseSize);
@@ -319,7 +319,7 @@ BootLinuxFdt ( // FDT device path explicitly defined. The FDT is relocated later
to a
// more appropriate location for the Linux kernel. //
- FdtBlobBase = LINUX_KERNEL_MAX_OFFSET;
- FdtBlobBase = LINUX_KERNEL_MAX_ADDR; Status = BdsLoadImage (FdtDevicePath, AllocateMaxAddress,
&FdtBlobBase, &FdtBlobSize);
if (EFI_ERROR (Status)) { Print (L"ERROR: Did not find Device Tree blob (%r).\n", Status);diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.h
b/ArmPkg/Application/LinuxLoader/LinuxLoader.h
index 8a23d7f..2544147 100644 --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.h +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.h @@ -37,9 +37,35 @@ #define MAX_MSG_LEN 80
#define LINUX_UIMAGE_SIGNATURE 0x56190527 -#define LINUX_KERNEL_MAX_OFFSET (SystemMemoryBase +
PcdGet32(PcdArmLinuxKernelMaxOffset))
-#define LINUX_ATAG_MAX_OFFSET (SystemMemoryBase +
PcdGet32(PcdArmLinuxAtagMaxOffset))
-#define LINUX_FDT_MAX_OFFSET (SystemMemoryBase +
PcdGet32(PcdArmLinuxFdtMaxOffset))
+#if defined (MDE_CPU_ARM)
- // On ARM, the kernel recommendations say that the kernel should live
before
- // the 128MiB (0x08000000) boundary.
- #define LINUX_KERNEL_MAX_OFFSET 0x08000000
- // ATAGS data should be in the first 16KB
- #define LINUX_ATAG_MAX_OFFSET 0x00004000
- // The device tree and initrd should live after the 128MiB boundary.
- #define LINUX_FDT_MAX_OFFSET 0x08400000
- #define LINUX_INITRD_MAX_OFFSET 0x08400000
- #define LINUX_FDT_ALIGNMENT 0x00000008
+#elif defined (MDE_CPU_AARCH64)
- #define LINUX_KERNEL_MAX_OFFSET 0x08000000
- // arm64 Kernels prior to v4.2 required the device tree to reside in
the
- // first 512MB of memory
- #define LINUX_FDT_MAX_OFFSET 0x20000000
- #define LINUX_INITRD_MAX_OFFSET 0x20000000
- // Recommendations are to align the FDT to 2MiB
- #define LINUX_FDT_ALIGNMENT 0x00200000
+#endif
+#define LINUX_KERNEL_MAX_ADDR (SystemMemoryBase +
LINUX_KERNEL_MAX_OFFSET)
+#define LINUX_ATAG_MAX_ADDR (SystemMemoryBase +
LINUX_ATAG_MAX_OFFSET)
+#define LINUX_FDT_MAX_ADDR (SystemMemoryBase +
LINUX_FDT_MAX_OFFSET)
+#define LINUX_INITRD_MAX_ADDR (SystemMemoryBase +
LINUX_INITRD_MAX_OFFSET)
#define ARM_FDT_MACHINE_TYPE 0xFFFFFFFF
diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.inf
b/ArmPkg/Application/LinuxLoader/LinuxLoader.inf
index 59ab99c..3b640b8 100644 --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.inf +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.inf @@ -78,14 +78,6 @@ [FeaturePcd] gArmTokenSpaceGuid.PcdArmLinuxSpinTable
-[FixedPcd]
- gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset
- gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment
- gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset
-[FixedPcd.ARM]
- gArmTokenSpaceGuid.PcdArmLinuxAtagMaxOffset
[Pcd.AARCH64] gArmTokenSpaceGuid.PcdGicDistributorBase gArmTokenSpaceGuid.PcdGicSgiIntId diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c
b/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c
index 0f53784..6f7bcdc 100644 --- a/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c +++ b/ArmPkg/Application/LinuxLoader/LinuxLoaderFdt.c @@ -78,20 +78,16 @@ RelocateFdt ( { EFI_STATUS Status; INTN Error;
UINT64 FdtAlignment;
*RelocatedFdtSize = OriginalFdtSize + FDT_ADDITIONAL_ENTRIES_SIZE;
// If FDT load address needs to be aligned, allocate more space.
FdtAlignment = PcdGet32 (PcdArmLinuxFdtAlignment);
if (FdtAlignment != 0) {
*RelocatedFdtSize += FdtAlignment;
}
*RelocatedFdtSize += LINUX_FDT_ALIGNMENT;
// Try below a watermark address. Status = EFI_NOT_FOUND;
- if (PcdGet32 (PcdArmLinuxFdtMaxOffset) != 0) {
- *RelocatedFdt = LINUX_FDT_MAX_OFFSET;
- if (LINUX_FDT_MAX_OFFSET != 0) {
- *RelocatedFdt = LINUX_FDT_MAX_ADDR; Status = gBS->AllocatePages (AllocateMaxAddress,
EfiBootServicesData,
EFI_SIZE_TO_PAGES (*RelocatedFdtSize),RelocatedFdt);
if (EFI_ERROR (Status)) {@@ -112,9 +108,7 @@ RelocateFdt ( }
*RelocatedFdtAlloc = *RelocatedFdt;
- if (FdtAlignment != 0) {
- *RelocatedFdt = ALIGN (*RelocatedFdt, FdtAlignment);
- }
*RelocatedFdt = ALIGN (*RelocatedFdt, LINUX_FDT_ALIGNMENT);
// Load the Original FDT tree into the new region Error = fdt_open_into ((VOID*)(UINTN) OriginalFdt,
diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index b2122b1..59916d2 100644 --- a/ArmPkg/ArmPkg.dec +++ b/ArmPkg/ArmPkg.dec @@ -115,8 +115,6 @@ # # BdsLib #
- # The compressed Linux kernel is expected to be under 128MB from the
beginning of the System Memory
gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x08000000|UINT32|0x0000001F
# Maximum file size for TFTP servers that do not support 'tsize'
extension
gArmTokenSpaceGuid.PcdMaxTftpFileSize|0x01000000|UINT32|0x00000000
@@ -152,16 +150,6 @@ # By default we do not do a transition to non-secure mode gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x0|UINT32|0x0000003E
- # The Linux ATAGs are expected to be under 0x4000 (16KB) from the
beginning of the System Memory
- gArmTokenSpaceGuid.PcdArmLinuxAtagMaxOffset|0x4000|UINT32|0x00000020
- # If the fixed FDT address is not available, then it should be loaded
below the kernel.
- # The recommendation from the Linux kernel is to have the FDT below
16KB.
- # (see the kernel doc: Documentation/arm/Booting)
gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x08400000|UINT32|0x00000023
- # The FDT blob must be loaded at a 64bit aligned address.
- gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x00000026
- # Non Secure Access Control Register # - BIT15 : NSASEDIS - Disable Non-secure Advanced SIMD functionality # - BIT14 : NSD32DIS - Disable Non-secure use of D16-D31
@@ -200,13 +188,6 @@ # Other modes include using SP0 or switching to Aarch32, but these are # not currently supported. gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x3c9|UINT32|0x0000003E
- # If the fixed FDT address is not available, then it should be loaded
above the kernel.
- # The recommendation from the AArch64 Linux kernel is to have the FDT
below 512MB.
- # (see the kernel doc: Documentation/arm64/booting.txt)
gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x20000000|UINT32|0x00000023
- # The FDT blob must be loaded at a 2MB aligned address.
gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x00200000|UINT32|0x00000026
# # These PCDs are also defined as 'PcdsDynamic' or
'PcdsPatchableInModule' to be
-- 2.1.0
-
Ard Biesheuvel -
Ryan Harkin