From 7e410f094417edcbbcb25b9148ae44cb21f70a63 Mon Sep 17 00:00:00 2001 From: Andy Bui Date: Thu, 16 Nov 2023 13:05:29 +1100 Subject: [PATCH 01/12] elfloader: arm: stabilize secondary core booting EFI may boot the elfloader with caches disabled on the secondary cores, we want the value of non_boot_lock to be visible. Some barriers are added to stabilize SMP booting in the elfloader. Co-authored-by: Yanyan Shen Co-authored-by: Matthias Rosenfelder Signed-off-by: Andy Bui --- .../src/arch-arm/armv/armv8-a/64/smp.c | 6 +++++ .../src/arch-arm/drivers/smp-psci.c | 9 ++++++- elfloader-tool/src/arch-arm/smp_boot.c | 25 +++++++++++++++++-- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/elfloader-tool/src/arch-arm/armv/armv8-a/64/smp.c b/elfloader-tool/src/arch-arm/armv/armv8-a/64/smp.c index a86b02b4..edbf07e5 100644 --- a/elfloader-tool/src/arch-arm/armv/armv8-a/64/smp.c +++ b/elfloader-tool/src/arch-arm/armv/armv8-a/64/smp.c @@ -35,6 +35,12 @@ void core_entry(uint64_t sp) int is_core_up(int i) { + /* Secondary core may be booted with caches disabled, + * this value might be written in memory, invalidate our + * copy and get a new one. */ + asm volatile("dc ivac, %0\n\t" + "dmb nsh\n\t" + :: "r"(&core_up[i])); return core_up[i] == i; } diff --git a/elfloader-tool/src/arch-arm/drivers/smp-psci.c b/elfloader-tool/src/arch-arm/drivers/smp-psci.c index ef3ea012..ae5fe951 100644 --- a/elfloader-tool/src/arch-arm/drivers/smp-psci.c +++ b/elfloader-tool/src/arch-arm/drivers/smp-psci.c @@ -3,6 +3,7 @@ * * SPDX-License-Identifier: GPL-2.0-only */ +#include #include #include #include @@ -24,7 +25,13 @@ static int smp_psci_cpu_on(UNUSED struct elfloader_device *dev, } secondary_data.entry = entry; secondary_data.stack = stack; - dmb(); +#if defined(CONFIG_ARCH_AARCH64) + /* If the secondary core caches are off, need to make sure that the info + * is clean to the physical memory so that the sedcondary cores can read it. + */ + asm volatile("dc cvac, %0" :: "r"(&secondary_data)); + dsb(); +#endif int ret = psci_cpu_on(cpu->cpu_id, (unsigned long)&secondary_startup, 0); if (ret != PSCI_SUCCESS) { printf("Failed to bring up core 0x%x with error %d\n", cpu->cpu_id, ret); diff --git a/elfloader-tool/src/arch-arm/smp_boot.c b/elfloader-tool/src/arch-arm/smp_boot.c index d429d113..80c51bc6 100644 --- a/elfloader-tool/src/arch-arm/smp_boot.c +++ b/elfloader-tool/src/arch-arm/smp_boot.c @@ -34,7 +34,11 @@ void non_boot_main(void) #endif /* Spin until the first CPU has finished initialisation. */ while (!non_boot_lock) { -#ifndef CONFIG_ARCH_AARCH64 +#ifdef CONFIG_ARCH_AARCH64 + /* The compiler may optimize this loop away, add a dsb() + * to force a reload. */ + dsb(); +#else cpu_idle(); #endif } @@ -117,7 +121,13 @@ WEAK void init_cpus(void) abort(); } - while (!is_core_up(num_cpus)); + while (!is_core_up(num_cpus)) { +#if defined(CONFIG_ARCH_AARCH64) + /* The compiler may optimize this loop away, add a dsb() + * to force a reload. */ + dsb(); +#endif + } printf("Core %d is up with logic id %d\n", elfloader_cpus[i].cpu_id, num_cpus); num_cpus++; } @@ -134,6 +144,17 @@ void smp_boot(void) arm_disable_dcaches(); #endif init_cpus(); + +#if defined(CONFIG_ARCH_AARCH64) + dsb(); +#endif + non_boot_lock = 1; + +#if defined(CONFIG_ARCH_AARCH64) + /* Secondary CPUs may still run with MMU & caches off. Force the update to be visible. */ + asm volatile("dc civac, %0\n\t" :: "r"(&non_boot_lock) : "memory");; +#endif + } #endif /* CONFIG_MAX_NUM_NODES */ From 78644d1005b18e53d29787e0c48f984151ef2404 Mon Sep 17 00:00:00 2001 From: Tw Date: Tue, 11 Jul 2023 11:14:10 +0800 Subject: [PATCH 02/12] elfloader: arm: fix incorrect sp when restoring bootloader parameters. Signed-off-by: Tw --- elfloader-tool/src/arch-arm/64/crt0.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elfloader-tool/src/arch-arm/64/crt0.S b/elfloader-tool/src/arch-arm/64/crt0.S index acd4de92..7d1249be 100644 --- a/elfloader-tool/src/arch-arm/64/crt0.S +++ b/elfloader-tool/src/arch-arm/64/crt0.S @@ -29,7 +29,7 @@ BEGIN_FUNC(_start) bl fixup_image_base mov x2, x0 /* restore original arguments for next step */ - ldp x0, x1, [sp, #-16]! + ldp x0, x1, [sp], #16 /* fixup_image_base returns 1 if no need to move */ cmp x2, #1 beq 1f From 864c7c708285a0fb8b3f2d42701fe7a1e322e443 Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Tue, 11 Jul 2023 15:54:12 +0200 Subject: [PATCH 03/12] elfloader: arm: fix function declaration (type mismatch). This chops off the aff3 level on Aarch64. Why does the compiler not warn??? Because the own header was not included. If you just include the header (without the change of the return value in the header), we get: seL4_tools/elfloader-tool/src/arch-arm/64/cpuid.c:14:10: error: conflicting types for 'read_cpuid_mpidr' 14 | uint32_t read_cpuid_mpidr(void) | ^~~~~~~~~~~~~~~~ In file included from /home/mro/nvos_neu2/tools/seL4_tools/ elfloader-tool/src/arch-arm/64/cpuid.c:9: elfloader-tool/include/arch-arm/cpuid.h:15:8: note: previous declaration of 'read_cpuid_mpidr' was here 15 | word_t read_cpuid_mpidr(void); | ^~~~~~~~~~~~~~~~ [190/200] Building C object elfloader-tool/CMakeFiles/elfloader.dir/ src/arch-arm/smp_boot.c.obj ninja: build stopped: subcommand failed. Signed-off-by: Matthias Rosenfelder --- elfloader-tool/include/arch-arm/cpuid.h | 2 +- elfloader-tool/src/arch-arm/64/cpuid.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/elfloader-tool/include/arch-arm/cpuid.h b/elfloader-tool/include/arch-arm/cpuid.h index f84612be..c0e1a6ce 100644 --- a/elfloader-tool/include/arch-arm/cpuid.h +++ b/elfloader-tool/include/arch-arm/cpuid.h @@ -12,7 +12,7 @@ uint32_t read_cpuid_id(void); /* read MP ID register from CPUID */ -uint32_t read_cpuid_mpidr(void); +word_t read_cpuid_mpidr(void); /* check if CPU is in HYP/EL2 mode */ word_t is_hyp_mode(void); diff --git a/elfloader-tool/src/arch-arm/64/cpuid.c b/elfloader-tool/src/arch-arm/64/cpuid.c index 66d9d09b..6c979a0f 100644 --- a/elfloader-tool/src/arch-arm/64/cpuid.c +++ b/elfloader-tool/src/arch-arm/64/cpuid.c @@ -6,6 +6,7 @@ #include #include +#include /* we only care about the affinity bits */ #define MPIDR_MASK (0xff00ffffff) From 7417e3ce0c5950d0c2143f82a3388db5f036094b Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Tue, 18 Jul 2023 17:32:57 +0200 Subject: [PATCH 04/12] elfloader: arm: fix alignment of AArch32 pagetables. The 64 kiB alignment is a maximum requirement for a stage2 concatenated pagetable. See Table G5-4 in ARM DDI 0487I.a, page G5-9186. Note: Both comments at the top of the file as well as in line 85 say "64 kiB". 2^14 is unfortunately only 16 kiB. Note2: This code is not executed on AArch64, because the finish_relocation() function panics on AArch64. The latter always takes the "shortcut" via continue_boot(). Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/arch-arm/sys_boot.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/elfloader-tool/src/arch-arm/sys_boot.c b/elfloader-tool/src/arch-arm/sys_boot.c index bf98aaf2..0bcf3c03 100644 --- a/elfloader-tool/src/arch-arm/sys_boot.c +++ b/elfloader-tool/src/arch-arm/sys_boot.c @@ -22,8 +22,13 @@ /* 0xd00dfeed in big endian */ #define DTB_MAGIC (0xedfe0dd0) -/* Maximum alignment we need to preserve when relocating (64K) */ -#define MAX_ALIGN_BITS (14) +/* Maximum alignment we need to preserve when relocating (64K) + * + * The 64 kiB alignment is a maximum requirement for a stage2 + * concatenated pagetable. See Table G5-4 in ARM DDI 0487I.a, page + * G5-9186. + */ +#define MAX_ALIGN_BITS (16) #ifdef CONFIG_IMAGE_EFI ALIGN(BIT(PAGE_BITS)) VISIBLE From 1b4cddf603422002350acb25845558bce50279a5 Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Tue, 22 Aug 2023 20:38:03 +0200 Subject: [PATCH 05/12] elfloader: fix variable prototype and remove var shadowing. The variable "dtb_size" is of type "size_t" and is defined in src/arch-arm/sys_boot.c, line 36. "size_t" is most certainly NOT the same size as "uint32_t", even on 32-bit architectures. Thus, the declaration in smp_boot.c is incorrect, since it does not match the definition in sys_boot.c. Why even create a local declaration - put this in a common header file and you will see those problems right away. Single point of maintenance! This may lead to an incorrectly sized memory access, that only happens to be correct by chance in Little-Endian mode. For ARM in Big-Endian mode this is a bug and will most likely result in an incorrect DTB size of zero. This fixes c5735119 ("elfloader: pass DTB from bootloader to seL4 on ARM"). Moreover, remove the shadowing of global variables by defining local ones with the same name => Rename the local one in src/common.c. This could have been detected with "-Wshadow". Practically speaking our DTBs are always (a lot) smaller than 32-bit. Thus, continue to pass a 32-bit size to the kernel in order to not change the API here. Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/arch-arm/smp_boot.c | 6 +++--- elfloader-tool/src/arch-arm/sys_boot.c | 5 +++-- elfloader-tool/src/common.c | 12 ++++++------ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/elfloader-tool/src/arch-arm/smp_boot.c b/elfloader-tool/src/arch-arm/smp_boot.c index 80c51bc6..da6743c6 100644 --- a/elfloader-tool/src/arch-arm/smp_boot.c +++ b/elfloader-tool/src/arch-arm/smp_boot.c @@ -24,7 +24,7 @@ static volatile int non_boot_lock = 0; void arm_disable_dcaches(void); extern void const *dtb; -extern uint32_t dtb_size; +extern size_t dtb_size; /* Entry point for all CPUs other than the initial. */ void non_boot_main(void) @@ -62,10 +62,10 @@ void non_boot_main(void) arm_enable_mmu(); } - /* Jump to the kernel. */ + /* Jump to the kernel. Note: Our DTB is smaller than 4 GiB. */ ((init_arm_kernel_t)kernel_info.virt_entry)(user_info.phys_region_start, user_info.phys_region_end, user_info.phys_virt_offset, - user_info.virt_entry, (paddr_t)dtb, dtb_size); + user_info.virt_entry, (paddr_t)dtb, (uint32_t)dtb_size); printf("AP Kernel returned back to the elf-loader.\n"); abort(); diff --git a/elfloader-tool/src/arch-arm/sys_boot.c b/elfloader-tool/src/arch-arm/sys_boot.c index 0bcf3c03..bb2e019d 100644 --- a/elfloader-tool/src/arch-arm/sys_boot.c +++ b/elfloader-tool/src/arch-arm/sys_boot.c @@ -221,17 +221,18 @@ void continue_boot(int was_relocated) arm_enable_mmu(); } - /* Enter kernel. The UART may no longer be accessible here. */ + /* The UART may no longer be accessible here. */ if ((uintptr_t)uart_get_mmio() < kernel_info.virt_region_start) { printf("Jumping to kernel-image entry point...\n\n"); } + /* Jump to the kernel. Note: Our DTB is smaller than 4 GiB. */ ((init_arm_kernel_t)kernel_info.virt_entry)(user_info.phys_region_start, user_info.phys_region_end, user_info.phys_virt_offset, user_info.virt_entry, (word_t)dtb, - dtb_size); + (uint32_t)dtb_size); /* We should never get here. */ printf("ERROR: Kernel returned back to the ELF Loader\n"); diff --git a/elfloader-tool/src/common.c b/elfloader-tool/src/common.c index 9846422f..d351df06 100644 --- a/elfloader-tool/src/common.c +++ b/elfloader-tool/src/common.c @@ -468,29 +468,29 @@ int load_images( /* keep it page aligned */ next_phys_addr = dtb_phys_start = ROUND_UP(kernel_phys_end, PAGE_BITS); - size_t dtb_size = fdt_size(dtb); - if (0 == dtb_size) { + size_t dtb_sz = fdt_size(dtb); + if (0 == dtb_sz) { printf("ERROR: Invalid device tree blob supplied\n"); return -1; } /* Make sure this is a sane thing to do */ ret = ensure_phys_range_valid(next_phys_addr, - next_phys_addr + dtb_size); + next_phys_addr + dtb_sz); if (0 != ret) { printf("ERROR: Physical address of DTB invalid\n"); return -1; } - memmove((void *)next_phys_addr, dtb, dtb_size); - next_phys_addr += dtb_size; + memmove((void *)next_phys_addr, dtb, dtb_sz); + next_phys_addr += dtb_sz; next_phys_addr = ROUND_UP(next_phys_addr, PAGE_BITS); dtb_phys_end = next_phys_addr; printf("Loaded DTB from %p.\n", dtb); printf(" paddr=[%p..%p]\n", dtb_phys_start, dtb_phys_end - 1); *chosen_dtb = (void *)dtb_phys_start; - *chosen_dtb_size = dtb_size; + *chosen_dtb_size = dtb_sz; } else { next_phys_addr = ROUND_UP(kernel_phys_end, PAGE_BITS); } From 326dc872d951206d5b450e09841dd8e023c766fb Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Mon, 14 Aug 2023 12:04:25 +0200 Subject: [PATCH 06/12] elfloader: fix EFI image size The size calculation was incorrect, unfortunately. That lead to an incorrect memory map provided by UEFI. When switching on EFI_DEBUG one can see that the memory range occupied by the ELF-loader ("paddr=" line) is not fully marked as used by UEFI and the last few pages of the ELF-loader are actually marked as being free. Output before: ELF-loader started on Image ranges: paddr=[7f9bc0000..7fcde1fff] text[7f9bc0000..7f9bd1e8f] data[7f9bd2000..7fcddd9ff] bss[7f9bd2850..7f9c0a4cf] edata[7fcddda00..7fcde1fff] [...] [paddr=0x180023000-0x7f9bbffff] [type = Conventional, attr: Normal <- ok [paddr=0x7f9bc0000-0x7fcdddfff] [type = Loader Code, attr: Normal <- Not ok: end address too low Should be 0x4000 higher [paddr=0x7fcdde000-0x7ffffffff] [type = Conventional, attr: Normal <- Not ok: start address too low Should be 0x4000 higher After: ELF-loader started on Image ranges: paddr=[7f9bbc000..7fcdddfff] text[7f9bbc000..7f9bcde8f] data[7f9bce000..7fcdd99ff] bss[7f9bce850..7f9c064cf] edata[7fcdd9a00..7fcdddfff] [...] [paddr=0x180023000-0x7f9bbbfff] [type = Conventional, attr: Normal <- ok [paddr=0x7f9bbc000-0x7fcdddfff] [type = Loader Code, attr: Normal <- ok (same as above) [paddr=0x7fcdde000-0x7ffffffff] [type = Conventional, attr: Normal <- ok (starts one after paddr end) Note: You don't have that debug output (EFI_DEBUG) in your code, that prints the UEFI memory map. So you have to believe me that this is the actual output. This fixes 030d83bf ("elfloader: improve EFI support"). Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/binaries/efi/gnuefi/crt0-efi-aarch64.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elfloader-tool/src/binaries/efi/gnuefi/crt0-efi-aarch64.S b/elfloader-tool/src/binaries/efi/gnuefi/crt0-efi-aarch64.S index 13792b87..c735616b 100644 --- a/elfloader-tool/src/binaries/efi/gnuefi/crt0-efi-aarch64.S +++ b/elfloader-tool/src/binaries/efi/gnuefi/crt0-efi-aarch64.S @@ -64,7 +64,7 @@ extra_header_fields: .short 0 // MinorSubsystemVersion .long 0 // Win32VersionValue - .long _edata - ImageBase // SizeOfImage + .long _end - ImageBase // SizeOfImage // Everything before the kernel image is considered part of the header .long _gnuefi_start - ImageBase // SizeOfHeaders From 6e44ac600ba3c23d01e5cf82e146ca4e1afac12c Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Wed, 23 Aug 2023 22:53:00 +0200 Subject: [PATCH 07/12] elfloader: move the data of ELFloader together The driver list section was part of the "*(COMMON)" section that was placed *after* the image payload (kernel, rootserver etc.). The driver list is data and should be placed adjacent to other data belonging to the ELFloader. This is clearly visible in the mapfile (which you don't generate). The driver list entry is present in the aarch32 linker script for EFI, why was it missing for 64 bit? No functional change. Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/binaries/efi/gnuefi/elf_aarch64_efi.lds | 3 +++ 1 file changed, 3 insertions(+) diff --git a/elfloader-tool/src/binaries/efi/gnuefi/elf_aarch64_efi.lds b/elfloader-tool/src/binaries/efi/gnuefi/elf_aarch64_efi.lds index bbbc502f..268c1b3c 100644 --- a/elfloader-tool/src/binaries/efi/gnuefi/elf_aarch64_efi.lds +++ b/elfloader-tool/src/binaries/efi/gnuefi/elf_aarch64_efi.lds @@ -31,6 +31,9 @@ SECTIONS *(.data) *(.data1) *(.data.*) + __start__driver_list = .; + *(_driver_list) + __stop__driver_list = .; *(.got.plt) *(.got) From 33a9819dad747966c1d270438327f33eb7c11970 Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Fri, 14 Jul 2023 12:36:13 +0200 Subject: [PATCH 08/12] elfloader: arm: do not hard-code values Use existing defines to make the code more descriptive. For this move some defines out of the assembler-only file. This is a preparation patch for upcoming patches. No functional change. Signed-off-by: Matthias Rosenfelder --- .../include/arch-arm/64/mode/aarch64.h | 65 +++++++++++++++++++ .../include/arch-arm/64/mode/assembler.h | 58 +---------------- elfloader-tool/src/arch-arm/64/mmu.c | 10 +-- 3 files changed, 72 insertions(+), 61 deletions(-) create mode 100644 elfloader-tool/include/arch-arm/64/mode/aarch64.h diff --git a/elfloader-tool/include/arch-arm/64/mode/aarch64.h b/elfloader-tool/include/arch-arm/64/mode/aarch64.h new file mode 100644 index 00000000..e46611c4 --- /dev/null +++ b/elfloader-tool/include/arch-arm/64/mode/aarch64.h @@ -0,0 +1,65 @@ +/* + * Copyright 2023, NIO GmbH + * + * SPDX-License-Identifier: GPL-2.0-only + */ +#pragma once + +/* This file contains useful defines for assembly and C code. */ + +#define PSR_F_BIT 0x00000040 +#define PSR_I_BIT 0x00000080 +#define PSR_A_BIT 0x00000100 +#define PSR_D_BIT 0x00000200 + +#define PSR_MODE_EL0t 0x00000000 +#define PSR_MODE_EL1t 0x00000004 +#define PSR_MODE_EL1h 0x00000005 +#define PSR_MODE_EL2t 0x00000008 +#define PSR_MODE_EL2h 0x00000009 +#define PSR_MODE_SVC_32 0x00000013 + +#define TCR_T0SZ(x) ((64 - (x))) +#define TCR_T1SZ(x) ((64 - (x)) << 16) +#define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x)) + +#define TCR_IRGN0_WBWC (1 << 8) +#define TCR_IRGN_NC ((0 << 8) | (0 << 24)) +#define TCR_IRGN_WBWA ((1 << 8) | (1 << 24)) +#define TCR_IRGN_WT ((2 << 8) | (2 << 24)) +#define TCR_IRGN_WBnWA ((3 << 8) | (3 << 24)) +#define TCR_IRGN_MASK ((3 << 8) | (3 << 24)) + +#define TCR_ORGN0_WBWC (1 << 10) +#define TCR_ORGN_NC ((0 << 10) | (0 << 26)) +#define TCR_ORGN_WBWA ((1 << 10) | (1 << 26)) +#define TCR_ORGN_WT ((2 << 10) | (2 << 26)) +#define TCR_ORGN_WBnWA ((3 << 10) | (3 << 26)) +#define TCR_ORGN_MASK ((3 << 10) | (3 << 26)) + +#define TCR_SH0_ISH (3 << 12) +#define TCR_SHARED ((3 << 12) | (3 << 28)) + +#define TCR_TG0_4K (0 << 14) +#define TCR_TG0_64K (1 << 14) +#define TCR_TG1_4K (2 << 30) +#define TCR_TG1_64K (3 << 30) + +#define TCR_PS_4G (0 << 16) +#define TCR_PS_64G (1 << 16) +#define TCR_PS_1T (2 << 16) +#define TCR_PS_4T (3 << 16) +#define TCR_PS_16T (4 << 16) +#define TCR_PS_256T (5 << 16) + +/* bits are reserved as 1 */ +#define TCR_EL2_RES1 ((1 << 23) | (1 << 31)) +#define TCR_ASID16 (1 << 36) + +#define MT_DEVICE_nGnRnE 0 +#define MT_DEVICE_nGnRE 1 +#define MT_DEVICE_GRE 2 +#define MT_NORMAL_NC 3 +#define MT_NORMAL 4 +#define MT_NORMAL_WT 5 +#define MAIR(_attr, _mt) ((_attr) << ((_mt) * 8)) diff --git a/elfloader-tool/include/arch-arm/64/mode/assembler.h b/elfloader-tool/include/arch-arm/64/mode/assembler.h index 4f9972c0..75b5c555 100644 --- a/elfloader-tool/include/arch-arm/64/mode/assembler.h +++ b/elfloader-tool/include/arch-arm/64/mode/assembler.h @@ -9,63 +9,7 @@ /* This file contains useful macros for assembly code. */ #ifdef __ASSEMBLER__ - -#define PSR_F_BIT 0x00000040 -#define PSR_I_BIT 0x00000080 -#define PSR_A_BIT 0x00000100 -#define PSR_D_BIT 0x00000200 - -#define PSR_MODE_EL0t 0x00000000 -#define PSR_MODE_EL1t 0x00000004 -#define PSR_MODE_EL1h 0x00000005 -#define PSR_MODE_EL2t 0x00000008 -#define PSR_MODE_EL2h 0x00000009 -#define PSR_MODE_SVC_32 0x00000013 - -#define TCR_T0SZ(x) ((64 - (x))) -#define TCR_T1SZ(x) ((64 - (x)) << 16) -#define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x)) - -#define TCR_IRGN0_WBWC (1 << 8) -#define TCR_IRGN_NC ((0 << 8) | (0 << 24)) -#define TCR_IRGN_WBWA ((1 << 8) | (1 << 24)) -#define TCR_IRGN_WT ((2 << 8) | (2 << 24)) -#define TCR_IRGN_WBnWA ((3 << 8) | (3 << 24)) -#define TCR_IRGN_MASK ((3 << 8) | (3 << 24)) - -#define TCR_ORGN0_WBWC (1 << 10) -#define TCR_ORGN_NC ((0 << 10) | (0 << 26)) -#define TCR_ORGN_WBWA ((1 << 10) | (1 << 26)) -#define TCR_ORGN_WT ((2 << 10) | (2 << 26)) -#define TCR_ORGN_WBnWA ((3 << 10) | (3 << 26)) -#define TCR_ORGN_MASK ((3 << 10) | (3 << 26)) - -#define TCR_SH0_ISH (3 << 12) -#define TCR_SHARED ((3 << 12) | (3 << 28)) - -#define TCR_TG0_4K (0 << 14) -#define TCR_TG0_64K (1 << 14) -#define TCR_TG1_4K (2 << 30) -#define TCR_TG1_64K (3 << 30) - -#define TCR_PS_4G (0 << 16) -#define TCR_PS_64G (1 << 16) -#define TCR_PS_1T (2 << 16) -#define TCR_PS_4T (3 << 16) -#define TCR_PS_16T (4 << 16) -#define TCR_PS_256T (5 << 16) - -/* bits are reserved as 1 */ -#define TCR_EL2_RES1 ((1 << 23) | (1 << 31)) -#define TCR_ASID16 (1 << 36) - -#define MT_DEVICE_nGnRnE 0 -#define MT_DEVICE_nGnRE 1 -#define MT_DEVICE_GRE 2 -#define MT_NORMAL_NC 3 -#define MT_NORMAL 4 -#define MT_NORMAL_WT 5 -#define MAIR(_attr, _mt) ((_attr) << ((_mt) * 8)) +#include .macro enable_mmu sctlr tmp mrs \tmp, \sctlr diff --git a/elfloader-tool/src/arch-arm/64/mmu.c b/elfloader-tool/src/arch-arm/64/mmu.c index 75d3b0a5..d5169e61 100644 --- a/elfloader-tool/src/arch-arm/64/mmu.c +++ b/elfloader-tool/src/arch-arm/64/mmu.c @@ -11,6 +11,8 @@ #include #include #include +#include +#include /* * Create a "boot" page table, which contains a 1:1 mapping below @@ -30,7 +32,7 @@ void init_boot_vspace(struct image_info *kernel_info) for (i = 0; i < BIT(PUD_BITS); i++) { _boot_pud_down[i] = (i << ARM_1GB_BLOCK_BITS) | BIT(10) /* access flag */ - | (0 << 2) /* strongly ordered memory */ + | (MT_DEVICE_nGnRnE << 2) /* strongly ordered memory */ | BIT(0); /* 1G block */ } @@ -51,7 +53,7 @@ void init_boot_vspace(struct image_info *kernel_info) #if CONFIG_MAX_NUM_NODES > 1 | (3 << 8) /* make sure the shareability is the same as the kernel's */ #endif - | (4 << 2) /* MT_NORMAL memory */ + | (MT_NORMAL << 2) /* MT_NORMAL memory */ | BIT(0); /* 2M block */ first_paddr += BIT(ARM_2MB_BLOCK_BITS); } @@ -68,7 +70,7 @@ void init_hyp_boot_vspace(struct image_info *kernel_info) for (i = 0; i < BIT(PUD_BITS); i++) { _boot_pud_down[i] = (i << ARM_1GB_BLOCK_BITS) | BIT(10) /* access flag */ - | (0 << 2) /* strongly ordered memory */ + | (MT_DEVICE_nGnRnE << 2) /* strongly ordered memory */ | BIT(0); /* 1G block */ } @@ -85,7 +87,7 @@ void init_hyp_boot_vspace(struct image_info *kernel_info) #if CONFIG_MAX_NUM_NODES > 1 | (3 << 8) #endif - | (4 << 2) /* MT_NORMAL memory */ + | (MT_NORMAL << 2) /* MT_NORMAL memory */ | BIT(0); /* 2M block */ } } From f31767932d047f8239d3aa7d19c3f5e25551738a Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Mon, 21 Aug 2023 21:09:39 +0200 Subject: [PATCH 09/12] elfloader: arm: fix potential UB in right shift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regarding right shifts the standard says: "The type of the result is that of the promoted left operand. The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand." Corresponding GCC warning (if used on a "small" type like uint8_t): main.c:25:39: warning: right shift count >= width of type [-Wshift-count-overflow] \#define GET_PGD_INDEX(x) (((x) >> (ARM_2MB_BLOCK_BITS + PMD_BITS + PUD_BITS)) & MASK(PGD_BITS)) main.c:46:39: note: in expansion of macro ‘GET_PGD_INDEX’ 46 | printf("GET_PGD_INDEX(x): %lu\n", GET_PGD_INDEX(x)); | ^~~~~~~~~~~~~ Thus, make sure that we never exceed/reach it by explicitly casting to a 64-bit type. It also allows using a pointer as macro parameter. This is a preparation patch for upcoming patches. Signed-off-by: Matthias Rosenfelder --- elfloader-tool/include/arch-arm/64/mode/structures.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/elfloader-tool/include/arch-arm/64/mode/structures.h b/elfloader-tool/include/arch-arm/64/mode/structures.h index f77ef93d..aaa8bced 100644 --- a/elfloader-tool/include/arch-arm/64/mode/structures.h +++ b/elfloader-tool/include/arch-arm/64/mode/structures.h @@ -21,9 +21,9 @@ #define PMD_BITS 9 #define PMD_SIZE_BITS (PMD_BITS + PMDE_SIZE_BITS) -#define GET_PGD_INDEX(x) (((x) >> (ARM_2MB_BLOCK_BITS + PMD_BITS + PUD_BITS)) & MASK(PGD_BITS)) -#define GET_PUD_INDEX(x) (((x) >> (ARM_2MB_BLOCK_BITS + PMD_BITS)) & MASK(PUD_BITS)) -#define GET_PMD_INDEX(x) (((x) >> (ARM_2MB_BLOCK_BITS)) & MASK(PMD_BITS)) +#define GET_PGD_INDEX(x) (((word_t)(x) >> (ARM_2MB_BLOCK_BITS + PMD_BITS + PUD_BITS)) & MASK(PGD_BITS)) +#define GET_PUD_INDEX(x) (((word_t)(x) >> (ARM_2MB_BLOCK_BITS + PMD_BITS)) & MASK(PUD_BITS)) +#define GET_PMD_INDEX(x) (((word_t)(x) >> (ARM_2MB_BLOCK_BITS)) & MASK(PMD_BITS)) extern uint64_t _boot_pgd_up[BIT(PGD_BITS)]; extern uint64_t _boot_pud_up[BIT(PUD_BITS)]; From 970da420ce729726da4866a5dcc53225e1257edd Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Thu, 24 Aug 2023 02:32:24 +0200 Subject: [PATCH 10/12] elfloader: Exit UEFI boot services very early UEFI is an operating system that hides as a bootloader. UEFI is in control of the machine as long as we didn't call exit_boot_services. For instance, UEFI may set up timers to interrupt us while we're fiddling with hardware and UEFI is fiddling with hardware itself and UEFI may be fiddling with the exact same hardware that we are fiddling with, while we're being preempted. That is not good. The previous state of ELFloader is that before exiting UEFI boot services, we already called platform_init() in main(), which may fiddle around with all kinds of hardware. Thus, we should have already exited UEFI boot services when main() is called. Note that exit_boot_services now still executes on the UEFI stack (since we switch the stack in _start()). But so did e.g. the clear_bss() function. I don't see a problem here. It's more a question the other way around: Previously, we called into UEFI with exit_boot_services on our own, potentially too small, stack. Do we have enough space for UEFI to execute? How are we supposed to know that? The UEFI implementation can change, so we can never be sure. But it would be unreasonable for UEFI to start us with a stack that is too small to call any UEFI API, including exit_boot_services. So we can safely assume that there is enough space when using the UEFI stack (since our use of stack to this point is minimal). Also, mask all exceptions until we are about to enter the kernel. We do not want to run with whatever state the bootloader set us up before, do we? We only re-enable the asyncs and debugs; interrupts and FIQs are still masked when entering the kernel. What would we gain from that? We don't expect any. Asyncs (SErrors), however, can indicate that we e.g. touched memory that we shouldn't have touched (secure memory). Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/arch-arm/sys_boot.c | 5 +++++ elfloader-tool/src/binaries/efi/efi_init.c | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/elfloader-tool/src/arch-arm/sys_boot.c b/elfloader-tool/src/arch-arm/sys_boot.c index bb2e019d..d3d6b9e0 100644 --- a/elfloader-tool/src/arch-arm/sys_boot.c +++ b/elfloader-tool/src/arch-arm/sys_boot.c @@ -226,6 +226,11 @@ void continue_boot(int was_relocated) printf("Jumping to kernel-image entry point...\n\n"); } +#if defined(CONFIG_ARCH_AARCH64) + /* Clear D&A in DAIF */ + asm volatile("msr daifclr, #0xC\n\t"); +#endif + /* Jump to the kernel. Note: Our DTB is smaller than 4 GiB. */ ((init_arm_kernel_t)kernel_info.virt_entry)(user_info.phys_region_start, user_info.phys_region_end, diff --git a/elfloader-tool/src/binaries/efi/efi_init.c b/elfloader-tool/src/binaries/efi/efi_init.c index a177c083..c55fb090 100644 --- a/elfloader-tool/src/binaries/efi/efi_init.c +++ b/elfloader-tool/src/binaries/efi/efi_init.c @@ -4,18 +4,28 @@ * SPDX-License-Identifier: GPL-2.0-only */ +#include #include #include void *__application_handle = NULL; // current efi application handler efi_system_table_t *__efi_system_table = NULL; // current efi system table +static unsigned long efi_exit_bs_result = EFI_SUCCESS; +static unsigned long exit_boot_services(void); + +unsigned long efi_exit_boot_services(void) +{ + return efi_exit_bs_result; +} + extern void _start(void); unsigned int efi_main(uintptr_t application_handle, uintptr_t efi_system_table) { clear_bss(); __application_handle = (void *)application_handle; __efi_system_table = (efi_system_table_t *)efi_system_table; + efi_exit_bs_result = exit_boot_services(); _start(); return 0; } @@ -41,7 +51,7 @@ void *efi_get_fdt(void) * This means boot time services are not available anymore. We should store * system information e.g. current memory map and pass them to kernel. */ -unsigned long efi_exit_boot_services(void) +static unsigned long exit_boot_services(void) { unsigned long status; efi_memory_desc_t *memory_map; @@ -78,5 +88,11 @@ unsigned long efi_exit_boot_services(void) } status = bts->exit_boot_services(__application_handle, key); + +#if defined(CONFIG_ARCH_AARCH64) + /* Now that we're free, mask all exceptions until we enter the kernel */ + asm volatile("msr daifset, #0xF\n\t"); +#endif + return status; } From 657d6caa414daccf4ac127bb41ea42f998984dc3 Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Thu, 14 Dec 2023 20:06:31 +0100 Subject: [PATCH 11/12] elfloader: Rewrite loop: Do not use goto There are better ways to loop in C. No functional change. Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/binaries/efi/efi_init.c | 40 ++++++++++++---------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/elfloader-tool/src/binaries/efi/efi_init.c b/elfloader-tool/src/binaries/efi/efi_init.c index c55fb090..e938d037 100644 --- a/elfloader-tool/src/binaries/efi/efi_init.c +++ b/elfloader-tool/src/binaries/efi/efi_init.c @@ -62,30 +62,32 @@ static unsigned long exit_boot_services(void) efi_boot_services_t *bts = get_efi_boot_services(); /* - * As the number of existing memeory segments are unknown, + * As the number of existing memory segments are unknown, * we need to resort to a trial and error to guess that. * We start from 32 and increase it by one until get a valid value. */ map_size = sizeof(*memory_map) * 32; -again: - status = bts->allocate_pool(EFI_LOADER_DATA, map_size, (void **)&memory_map); - - if (status != EFI_SUCCESS) - return status; - - status = bts->get_memory_map(&map_size, memory_map, &key, &desc_size, &desc_version); - if (status == EFI_BUFFER_TOO_SMALL) { - bts->free_pool(memory_map); - - map_size += sizeof(*memory_map); - goto again; - } - - if (status != EFI_SUCCESS){ - bts->free_pool(memory_map); - return status; - } + do { + status = bts->allocate_pool(EFI_LOADER_DATA, map_size, (void **)&memory_map); + /* If the allocation fails, there is something wrong and we cannot continue */ + if (status != EFI_SUCCESS) { + return status; + } + + status = bts->get_memory_map(&map_size, memory_map, &key, &desc_size, &desc_version); + if (status != EFI_SUCCESS) { + bts->free_pool(memory_map); + memory_map = NULL; + + if (status == EFI_BUFFER_TOO_SMALL) { + map_size += sizeof(*memory_map); + } else { + /* some other error; bail out! */ + return status; + } + } + } while (status == EFI_BUFFER_TOO_SMALL); status = bts->exit_boot_services(__application_handle, key); From c555152407c1001e49689dacc8fe2c308f2326c0 Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Thu, 14 Dec 2023 20:12:26 +0100 Subject: [PATCH 12/12] elfloader: fix UEFI integration bug: descriptor size mismatch. The UEFI specification 2.10 says in section 7 for EFI_BOOT_SERVICES.GetMemoryMap(): "The GetMemoryMap() function also returns the size and revision number of the EFI_MEMORY_DESCRIPTOR. The DescriptorSize represents the size in bytes of an EFI_MEMORY_DESCRIPTOR array element returned in MemoryMap. The size is returned to allow for future expansion of the EFI_MEMORY_DESCRIPTOR in response to hardware innovation. The structure of the EFI_MEMORY_DESCRIPTOR may be extended in the future but it will remain backwards compatible with the current definition. Thus OS software must use the DescriptorSize to find the start of each EFI_MEMORY_DESCRIPTOR in the MemoryMap array." This mismatch is the case on (our) Orin UEFI. The compiled size of a memory descriptor is 40 Bytes, but the Orin UEFI implementation uses 48 Bytes per descriptor. Thus, due to the requirement to use a larger size than the returned total size (due to the fact that the buffer allocation itself may lead to one more entry in the memory map), we must increase by the size (in terms of number of descriptors), but use the number of bytes that UEFI uses for one memory map entry, not what we think it might be. Some other people already stumbled over this: https://forum.osdev.org/viewtopic.php?f=1&t=32953 Based on the comment in the existing code, the author seems to not have understood how the size of the memory map can be determined. Just read the spec! So we better update that misleading comment. Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/binaries/efi/efi_init.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/elfloader-tool/src/binaries/efi/efi_init.c b/elfloader-tool/src/binaries/efi/efi_init.c index e938d037..47dc2651 100644 --- a/elfloader-tool/src/binaries/efi/efi_init.c +++ b/elfloader-tool/src/binaries/efi/efi_init.c @@ -63,8 +63,8 @@ static unsigned long exit_boot_services(void) /* * As the number of existing memory segments are unknown, - * we need to resort to a trial and error to guess that. - * We start from 32 and increase it by one until get a valid value. + * we need to start somewhere. The API then tells us how much space we need + * if it is not enough. */ map_size = sizeof(*memory_map) * 32; @@ -81,7 +81,12 @@ static unsigned long exit_boot_services(void) memory_map = NULL; if (status == EFI_BUFFER_TOO_SMALL) { - map_size += sizeof(*memory_map); + /* Note: "map_size" is an IN/OUT-parameter and has been updated to the + * required size. We still add one more entry ("desc_size" is in bytes) + * due to the hint from the spec ("since allocation of the new buffer + * may potentially increase memory map size."). + */ + map_size += desc_size; } else { /* some other error; bail out! */ return status;