From 4afb9e50d5d3cda0654b81d8b302ce8e3dddeb09 Mon Sep 17 00:00:00 2001 From: Kent McLeod Date: Tue, 1 Feb 2022 22:24:39 +1100 Subject: [PATCH 1/6] elfloader: port fixup_image_base to C Porting from assembly to a higher level language makes this function more portable. Signed-off-by: Kent McLeod --- elfloader-tool/CMakeLists.txt | 2 + elfloader-tool/src/arch-arm/64/crt0.S | 71 +-------------------------- elfloader-tool/src/common.c | 50 +++++++++++++++++++ 3 files changed, 54 insertions(+), 69 deletions(-) diff --git a/elfloader-tool/CMakeLists.txt b/elfloader-tool/CMakeLists.txt index c913452e..ca4f228c 100644 --- a/elfloader-tool/CMakeLists.txt +++ b/elfloader-tool/CMakeLists.txt @@ -344,6 +344,8 @@ if(DEFINED KernelDTBPath) set_property(SOURCE src/drivers/driver.c PROPERTY OBJECT_DEPENDS ${DEVICES_GEN_H}) endif() +set_property(SOURCE src/common.c PROPERTY OBJECT_DEPENDS ${IMAGE_START_ADDR_H}) + # Generate linker script separate_arguments(c_arguments NATIVE_COMMAND "${CMAKE_C_FLAGS}") # Add extra compilation flags required for clang diff --git a/elfloader-tool/src/arch-arm/64/crt0.S b/elfloader-tool/src/arch-arm/64/crt0.S index acd4de92..aa559303 100644 --- a/elfloader-tool/src/arch-arm/64/crt0.S +++ b/elfloader-tool/src/arch-arm/64/crt0.S @@ -6,9 +6,6 @@ #include #include -#ifdef CONFIG_IMAGE_BINARY -#include -#endif #include @@ -30,8 +27,8 @@ BEGIN_FUNC(_start) mov x2, x0 /* restore original arguments for next step */ ldp x0, x1, [sp, #-16]! - /* fixup_image_base returns 1 if no need to move */ - cmp x2, #1 + /* fixup_image_base returns 0 if no need to move */ + cmp x2, #0 beq 1f /* otherwise, jump to the start of the new elfloader */ @@ -41,70 +38,6 @@ BEGIN_FUNC(_start) b main END_FUNC(_start) -#ifdef CONFIG_IMAGE_BINARY -BEGIN_FUNC(fixup_image_base) - stp x29, x30, [sp, #-16]! - mov x29, sp - ldr x0, =IMAGE_START_ADDR - adr x1, _start - cmp x0, x1 - beq image_ok - - adrp x2, _end - add x2, x2, #:lo12:_end - sub x2, x2, x1 - - /* sanity check: don't want to overwrite ourselves! we assume - * everything between _start and _archive_start is important - * (i.e. code that might be run while relocating) - * but allow overlap for things after _archive_start. - */ - adrp x3, _archive_start - add x3, x3, #:lo12:_archive_start - - add x4, x0, x2 /* Dest end */ - - /* check: if (end < archive_start && end >= _start) { abort } */ - cmp x4, x3 - bge 1f - - cmp x4, x1 - blt 1f - - b cant_reloc - -1: - /* check: if (dest < archive_start && dest >= _start) { abort } */ - cmp x0, x3 - bge 2f - - cmp x0, x1 - blt 2f - -cant_reloc: - b abort - -2: - /* x0 = desired image base */ - /* x1 = current image space */ - /* x2 = image size */ - bl memmove - /* x0 = dest, save it to a callee-saved register while we invalidate icache */ - mov x19, x0 - bl flush_dcache - bl invalidate_icache - mov x0, x19 - b 1f - -image_ok: - /* already in the right place, just keep booting */ - mov x0, #1 -1: - ldp x29, x30, [sp], #16 - ret -END_FUNC(fixup_image_base) -#endif - /* Move the elf loader out of the kernel's way */ BEGIN_FUNC(finish_relocation) /* diff --git a/elfloader-tool/src/common.c b/elfloader-tool/src/common.c index c3fa882b..9f085876 100644 --- a/elfloader-tool/src/common.c +++ b/elfloader-tool/src/common.c @@ -29,8 +29,13 @@ #include // this provides memory_region #endif +#include +#include + extern char _bss[]; extern char _bss_end[]; +extern char _start[]; +extern char _end[]; /* * Clear the BSS segment @@ -618,3 +623,48 @@ WEAK void platform_init(void) { /* nothing by default */ } + +#ifdef CONFIG_ARCH_ARM +extern void flush_dcache(void); +extern void invalidate_icache(void); +#endif + +/* + * Moves the elfloader to its expected start address in memory. + * This is for when the elfloader needs to be in a specific location + * to ensure that it doesn't occupy memory that it needs to load the + * kernel and user applications into. + */ +char * fixup_image_base(void) { + char *load_base = _start; + char *target_base = (char *) IMAGE_START_ADDR; + if (load_base == target_base) { + /* already in the right place, just keep booting */ + return (char *)0; + } + + /* Check that the current image location doesn't overlap with the + * destination location. */ + size_t image_size = _end - _start; + size_t code_size = _archive_start - _start; + char *target_end = target_base + image_size; + if (image_size < code_size) { + /* This shouldn't happen as image size must be greater than + * code size. This allows the region check below to only consider + * if the smaller region is inside the larger region. */ + abort(); + } + if ((_start >= target_base && _start < target_end) || + (_archive_start >= target_base && _archive_start < target_end)) { + abort(); + } + + /* Perform the move and clean/invalidate caches if necessary */ + char *ret = memmove(target_base, load_base, image_size); +#ifdef CONFIG_ARCH_ARM + flush_dcache(); + invalidate_icache(); +#endif + return ret; + +} From a26563005268e1229e8814f8ae5cd32414e321dc Mon Sep 17 00:00:00 2001 From: Kent McLeod Date: Tue, 1 Feb 2022 22:30:33 +1100 Subject: [PATCH 2/6] elfloader,RISC-V: Relax kernel alignment checks It's not necessary to require the kernel and ELFloader images are loaded at the start of page boundaries. Signed-off-by: Kent McLeod --- elfloader-tool/src/arch-riscv/boot.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/elfloader-tool/src/arch-riscv/boot.c b/elfloader-tool/src/arch-riscv/boot.c index f485594a..003aab00 100644 --- a/elfloader-tool/src/arch-riscv/boot.c +++ b/elfloader-tool/src/arch-riscv/boot.c @@ -19,7 +19,11 @@ #define PT_LEVEL_2 2 #define PT_LEVEL_1_BITS 30 +#if __riscv_xlen == 32 +#define PT_LEVEL_2_BITS 22 +#else #define PT_LEVEL_2_BITS 21 +#endif #define PTE_TYPE_TABLE 0x00 #define PTE_TYPE_SRWX 0xCE @@ -46,8 +50,6 @@ #define GET_PT_INDEX(addr, n) (((addr) >> (((PT_INDEX_BITS) * ((CONFIG_PT_LEVELS) - (n))) + RISCV_PGSHIFT)) % PTES_PER_PT) -#define VIRT_PHYS_ALIGNED(virt, phys, level_bits) (IS_ALIGNED((virt), (level_bits)) && IS_ALIGNED((phys), (level_bits))) - struct image_info kernel_info; struct image_info user_info; @@ -89,11 +91,6 @@ static int map_kernel_window(struct image_info *kernel_info) /* Map the elfloader into the new address space */ - if (!IS_ALIGNED((uintptr_t)_text, PT_LEVEL_2_BITS)) { - printf("ERROR: ELF Loader not properly aligned\n"); - return -1; - } - index = GET_PT_INDEX((uintptr_t)_text, PT_LEVEL_1); #if __riscv_xlen == 32 @@ -105,18 +102,12 @@ static int map_kernel_window(struct image_info *kernel_info) #endif for (unsigned int page = 0; index < PTES_PER_PT; index++, page++) { - lpt[index] = PTE_CREATE_LEAF((uintptr_t)_text + + lpt[index] = PTE_CREATE_LEAF(ROUND_DOWN((uintptr_t)_text, PT_LEVEL_2_BITS) + (page << PT_LEVEL_2_BITS)); } /* Map the kernel into the new address space */ - if (!VIRT_PHYS_ALIGNED(kernel_info->virt_region_start, - kernel_info->phys_region_start, PT_LEVEL_2_BITS)) { - printf("ERROR: Kernel not properly aligned\n"); - return -1; - } - index = GET_PT_INDEX(kernel_info->virt_region_start, PT_LEVEL_1); #if __riscv_xlen == 64 @@ -126,7 +117,7 @@ static int map_kernel_window(struct image_info *kernel_info) #endif for (unsigned int page = 0; index < PTES_PER_PT; index++, page++) { - lpt[index] = PTE_CREATE_LEAF(kernel_info->phys_region_start + + lpt[index] = PTE_CREATE_LEAF(ROUND_DOWN(kernel_info->phys_region_start, PT_LEVEL_2_BITS) + (page << PT_LEVEL_2_BITS)); } From 6f20da5ad2ff8f3672c8491b5570c89c075e79bb Mon Sep 17 00:00:00 2001 From: Kent McLeod Date: Fri, 4 Feb 2022 22:11:20 +1100 Subject: [PATCH 3/6] elfloader,RISC-V: clear_bss unconditionally When booting via opensbi, the image format is a binary image and so if the .bss section is at the end it must be initialized unconditionally. Signed-off-by: Kent McLeod --- elfloader-tool/src/arch-riscv/crt0.S | 2 -- 1 file changed, 2 deletions(-) diff --git a/elfloader-tool/src/arch-riscv/crt0.S b/elfloader-tool/src/arch-riscv/crt0.S index 3447cc38..343e4643 100644 --- a/elfloader-tool/src/arch-riscv/crt0.S +++ b/elfloader-tool/src/arch-riscv/crt0.S @@ -58,10 +58,8 @@ _start: /* Attach the stack to sp before calling any C functions */ la sp, (elfloader_stack_alloc + BIT(12)) -#ifdef CONFIG_IMAGE_BINARY /* Clear the BSS before we get to do anything more specific */ jal clear_bss -#endif /* Check if the Heart State Management (HSM) extension exists, so it can be * used to switch harts if we are not running on hart CONFIG_FIRST_HART_ID. From 83064a6c17ec59eedb1c977910e9b53b028f4023 Mon Sep 17 00:00:00 2001 From: Kent McLeod Date: Fri, 4 Feb 2022 17:38:06 +1100 Subject: [PATCH 4/6] Revert "riscv: Reposition .rodata to after .bss" This reverts commit 8de8c9f4385ff4436f03efb807da25c99890239f. Signed-off-by: Kent McLeod --- elfloader-tool/src/arch-riscv/linker.lds | 26 ------------------------ 1 file changed, 26 deletions(-) diff --git a/elfloader-tool/src/arch-riscv/linker.lds b/elfloader-tool/src/arch-riscv/linker.lds index 2076b71a..0c5eeba9 100644 --- a/elfloader-tool/src/arch-riscv/linker.lds +++ b/elfloader-tool/src/arch-riscv/linker.lds @@ -4,13 +4,6 @@ * SPDX-License-Identifier: GPL-2.0-only */ -/* - * NOTE: For RISC-V 32-bit, having the .rodata section before the .data section causes the elfloader to - * freeze up. Thus, for 32-bit we move the .rodata section to after the .bss section as previously - * before some refactoring. The 64-bit version does not seem to be affect by this issue and uses thus - * uses the new layout. - */ - #include #include #include "image_start_addr.h" @@ -29,7 +22,6 @@ SECTIONS { *(.text) } -#ifdef CONFIG_ARCH_RISCV64 . = ALIGN(16); .rodata : { @@ -45,7 +37,6 @@ SECTIONS *(._archive_cpio) _archive_end = .; } -#endif . = ALIGN(16); .data : { @@ -63,22 +54,5 @@ SECTIONS *(.bss.*) _bss_end = .; } -#ifdef CONFIG_ARCH_RISCV32 - . = ALIGN(16); - .rodata : - { - *(.srodata*) - . = ALIGN(16); - *(.rodata) - *(.rodata.*) - /* - * ld crashes when we add this here: *(_driver_list) - */ - . = ALIGN(16); - _archive_start = .; - *(._archive_cpio) - _archive_end = .; - } -#endif _end = .; } From 9dd253710b4194b44f24bc29d6543bd1e4b95dd4 Mon Sep 17 00:00:00 2001 From: Kent McLeod Date: Tue, 1 Feb 2022 22:27:12 +1100 Subject: [PATCH 5/6] elfloader, RISC-V: Move the elfloader during boot The SBI implementation usually loads the elfloader where the kernel would prefer to also be loaded. Allow the elfloader to move itself to a higher address before loading the kernel and user images. This requires ensuring that the stacks are not overwritten during the merge, and so must be placed before the _archive_cpio section in the linker script. Signed-off-by: Kent McLeod --- cmake-tool/helpers/application_settings.cmake | 3 --- elfloader-tool/src/arch-riscv/boot.c | 5 ++++- elfloader-tool/src/arch-riscv/crt0.S | 15 +++++++++++++++ elfloader-tool/src/arch-riscv/linker.lds | 10 ++++++---- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/cmake-tool/helpers/application_settings.cmake b/cmake-tool/helpers/application_settings.cmake index eaac50cc..4706537d 100644 --- a/cmake-tool/helpers/application_settings.cmake +++ b/cmake-tool/helpers/application_settings.cmake @@ -49,9 +49,6 @@ function(ApplyData61ElfLoaderSettings kernel_platform kernel_sel4_arch) if(KernelPlatformZynqmp AND KernelSel4ArchAarch32) set(IMAGE_START_ADDR 0x8000000 CACHE INTERNAL "" FORCE) endif() - if(KernelPlatformSpike AND KernelSel4ArchRiscV32) - set(IMAGE_START_ADDR 0x80400000 CACHE INTERNAL "" FORCE) - endif() endfunction() function(ApplyCommonSimulationSettings kernel_sel4_arch) diff --git a/elfloader-tool/src/arch-riscv/boot.c b/elfloader-tool/src/arch-riscv/boot.c index 003aab00..e55a7f04 100644 --- a/elfloader-tool/src/arch-riscv/boot.c +++ b/elfloader-tool/src/arch-riscv/boot.c @@ -59,7 +59,10 @@ unsigned long l2pt[PTES_PER_PT] __attribute__((aligned(4096))); unsigned long l2pt_elf[PTES_PER_PT] __attribute__((aligned(4096))); #endif -char elfloader_stack_alloc[BIT(CONFIG_KERNEL_STACK_BITS)]; +/* This stack cannot go in the .bss section because it's already in use when the + * .bss section is zeroed. + */ +char elfloader_stack_alloc[BIT(CONFIG_KERNEL_STACK_BITS)] __attribute__((section(".data"))) ; /* first HART will initialise these */ void const *dtb = NULL; diff --git a/elfloader-tool/src/arch-riscv/crt0.S b/elfloader-tool/src/arch-riscv/crt0.S index 343e4643..223fb1a1 100644 --- a/elfloader-tool/src/arch-riscv/crt0.S +++ b/elfloader-tool/src/arch-riscv/crt0.S @@ -58,7 +58,22 @@ _start: /* Attach the stack to sp before calling any C functions */ la sp, (elfloader_stack_alloc + BIT(12)) + /* + * Binary images may not be loaded in the correct location. + * Try and move ourselves so we're in the right place. + */ + jal fixup_image_base + /* fixup_image_base returns 0 if no need to move */ + beqz a0, 1f + + /* otherwise, restore args and jump to the start of the new elfloader */ + mv a2, a0 + mv a0, s0 + mv a1, s2 + jr a2 + /* Clear the BSS before we get to do anything more specific */ +1: jal clear_bss /* Check if the Heart State Management (HSM) extension exists, so it can be diff --git a/elfloader-tool/src/arch-riscv/linker.lds b/elfloader-tool/src/arch-riscv/linker.lds index 0c5eeba9..e02758b1 100644 --- a/elfloader-tool/src/arch-riscv/linker.lds +++ b/elfloader-tool/src/arch-riscv/linker.lds @@ -32,10 +32,6 @@ SECTIONS /* * ld crashes when we add this here: *(_driver_list) */ - . = ALIGN(16); - _archive_start = .; - *(._archive_cpio) - _archive_end = .; } . = ALIGN(16); .data : @@ -46,6 +42,12 @@ SECTIONS *(.data.*) } . = ALIGN(16); + ._archive_cpio : { + _archive_start = .; + *(._archive_cpio) + _archive_end = .; + } + . = ALIGN(16); .bss : { _bss = .; From 067589782c6ad232316b034ee7dab2d49e23ed3d Mon Sep 17 00:00:00 2001 From: Kent McLeod Date: Tue, 22 Feb 2022 17:00:16 +1100 Subject: [PATCH 6/6] elfloader: Don't overwrite the platform DTB If a DTB has been passed in from an earlier stage boot loader during an elfloader image relocation, make sure that it doesn't get overwritten. On Arm this isn't relevant as the protocol when relocation happens doesn't also pass in a DTB. Signed-off-by: Kent McLeod --- elfloader-tool/src/arch-arm/64/crt0.S | 1 + elfloader-tool/src/arch-riscv/crt0.S | 1 + elfloader-tool/src/common.c | 21 ++++++++++++++++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/elfloader-tool/src/arch-arm/64/crt0.S b/elfloader-tool/src/arch-arm/64/crt0.S index aa559303..75abafc6 100644 --- a/elfloader-tool/src/arch-arm/64/crt0.S +++ b/elfloader-tool/src/arch-arm/64/crt0.S @@ -23,6 +23,7 @@ BEGIN_FUNC(_start) * Binary images may not be loaded in the correct location. * Try and move ourselves so we're in the right place. */ + mov x0, #0 /* Arg 0 of fixup_image_base is NULL */ bl fixup_image_base mov x2, x0 /* restore original arguments for next step */ diff --git a/elfloader-tool/src/arch-riscv/crt0.S b/elfloader-tool/src/arch-riscv/crt0.S index 223fb1a1..81b030dd 100644 --- a/elfloader-tool/src/arch-riscv/crt0.S +++ b/elfloader-tool/src/arch-riscv/crt0.S @@ -62,6 +62,7 @@ _start: * Binary images may not be loaded in the correct location. * Try and move ourselves so we're in the right place. */ + mv a0, a1 /* Pass dtb as first argument */ jal fixup_image_base /* fixup_image_base returns 0 if no need to move */ beqz a0, 1f diff --git a/elfloader-tool/src/common.c b/elfloader-tool/src/common.c index 9f085876..35f7ac86 100644 --- a/elfloader-tool/src/common.c +++ b/elfloader-tool/src/common.c @@ -635,7 +635,8 @@ extern void invalidate_icache(void); * to ensure that it doesn't occupy memory that it needs to load the * kernel and user applications into. */ -char * fixup_image_base(void) { +char *fixup_image_base(char const *fdt) +{ char *load_base = _start; char *target_base = (char *) IMAGE_START_ADDR; if (load_base == target_base) { @@ -659,6 +660,24 @@ char * fixup_image_base(void) { abort(); } + /* If a fdt was passed in from the previous boot stage, also + * check that it isn't overwritten. + */ + if (fdt) { + size_t dtb_size = fdt_size(fdt); + const char *fdt_end = fdt + dtb_size; + if (image_size < dtb_size) { + /* Assume the dtb is smaller than the images size to + * simplify region check */ + abort(); + } + if ((fdt >= target_base && fdt < target_end) || + (fdt_end >= target_base && fdt_end < target_end)) { + abort(); + } + } + + /* Perform the move and clean/invalidate caches if necessary */ char *ret = memmove(target_base, load_base, image_size); #ifdef CONFIG_ARCH_ARM