From d9313efc2e61e8f827ed30c5a911a20c1f43df7c Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Mon, 1 Apr 2019 12:45:33 +0200 Subject: [PATCH 01/23] fdt: boot_get_fdt: remove redundant zeroing out Paranoid programming [1] lies at the foundation of proper software development, but the repetitive zeroing-out of output arguments in the context of the same function rather clutters the code and inhibits further refactoring/optimization than is doing any good. In boot_get_fdt(), we already perform zero/NULL-initialization of *of_flat_tree and *of_size at the beginning of the function, so doing the same at function error-out is redundant/superfluous. Moreover, keeping the code unchanged might encourage the developers to update *of_flat_tree and *of_size during some interim computations, which is against the current design of boot_get_fdt(). Currently, writing useful data into these arguments happens just before successfully returning from boot_get_fdt() and it should better stay so. [1] https://blog.regehr.org/archives/1106 Signed-off-by: Eugeniu Rosca Reviewed-by: Simon Glass --- common/image-fdt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/image-fdt.c b/common/image-fdt.c index 01186aeac7..1817ce6bce 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -489,8 +489,6 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, no_fdt: ok_no_fdt = 1; error: - *of_flat_tree = NULL; - *of_size = 0; if (!select && ok_no_fdt) { debug("Continuing to boot without FDT\n"); return 0; From 9ee9cf31aec84332d8f4283cd7e0f3daef36138a Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Mon, 1 Apr 2019 12:45:34 +0200 Subject: [PATCH 02/23] fdt: boot_get_fdt: really boot w/o FDT when "goto no_fdt" The 'no_fdt' goto label was introduced by v2015.01 commit [0] and it had two review stages [1-2]. The *documented* purpose behind commit [0] is (excerpt from commit description): > allows both FDT and non-FDT kernels to boot by making the > third parameter to the bootm/bootz optional While [1] and [2] share the same goal, they have very different implementations: - [1] was based on a very simple 'argc' check at function error out with returning success to the caller if the third parameter was NOT passed to bootm/bootz command. This approach had the downside of returning success to the caller even in case of legitimate internal errors, which should halt booting. - [2] added the "no_fdt" label and several "goto no_fdt" statements. This allowed to report the legitimate internal errors to the caller. IOW the major difference between [1] and [2] is: - [1] boot w/o FDT if FDT address is not passed to boot{m,z,*} - [2] give *freedom* to the developer to boot w/o FDT from any (more or less) arbitrary point in the function flow (and here comes the peculiar aspect, which looks to be a leftover from [1]) with the precondition that the 3rd argument (FDT address) is NOT provided to boot{m,z,*}. In practice, this means that only a subset of "goto no_fdt" end up booting w/o FDT while the other subset is returning an error to the caller. This patch removes the peculiar behavior described above, such that "goto no_fdt" performs really what it tells to the developer. The motivation of this patch is to decrease the unneeded complexity and increase the readability of boot_get_fdt(). [0] 48aead71c1ad ("fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined") [1] https://patchwork.ozlabs.org/patch/412923/ ("[U-Boot,v1] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined") [2] https://patchwork.ozlabs.org/patch/415635/ ("[U-Boot,v2] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined") Signed-off-by: Eugeniu Rosca Reviewed-by: Simon Glass --- common/image-fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/image-fdt.c b/common/image-fdt.c index 1817ce6bce..c335e7e2f2 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -489,7 +489,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, no_fdt: ok_no_fdt = 1; error: - if (!select && ok_no_fdt) { + if (ok_no_fdt) { debug("Continuing to boot without FDT\n"); return 0; } From 80281829a6ac9d5040e710f42ab48604af07d53b Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Mon, 1 Apr 2019 12:45:35 +0200 Subject: [PATCH 03/23] fdt: boot_get_fdt: simplify no_fdt handling (non-functional) Increase the readability of boot_get_fdt(). No change in behavior is expected. Signed-off-by: Eugeniu Rosca Reviewed-by: Simon Glass --- common/image-fdt.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/common/image-fdt.c b/common/image-fdt.c index c335e7e2f2..68bcab85ba 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -279,7 +279,6 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, int fdt_noffset; #endif const char *select = NULL; - int ok_no_fdt = 0; *of_flat_tree = NULL; *of_size = 0; @@ -487,12 +486,9 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, return 0; no_fdt: - ok_no_fdt = 1; + debug("Continuing to boot without FDT\n"); + return 0; error: - if (ok_no_fdt) { - debug("Continuing to boot without FDT\n"); - return 0; - } return 1; } From 18b8f2c49e9b0620caf5b661986c01c575e29b9b Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Mon, 1 Apr 2019 12:45:36 +0200 Subject: [PATCH 04/23] fdt: boot_get_fdt: android: compress handling (non-functional) Prepare for booting Android images which lack any DTB in the second area by using 'fdtaddr' environment variable as source/address of FDT. No functional/behavioral change expected in this patch. Signed-off-by: Eugeniu Rosca Reviewed-by: Simon Glass --- common/image-fdt.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/common/image-fdt.c b/common/image-fdt.c index 68bcab85ba..a5d8b41d02 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -461,17 +461,16 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, struct andr_img_hdr *hdr = buf; ulong fdt_data, fdt_len; - if (android_image_get_second(hdr, &fdt_data, &fdt_len) != 0) + if (!android_image_get_second(hdr, &fdt_data, &fdt_len) && + !fdt_check_header((char *)fdt_data)) { + fdt_blob = (char *)fdt_data; + if (fdt_totalsize(fdt_blob) != fdt_len) + goto error; + + debug("## Using FDT in Android image second area\n"); + } else { goto no_fdt; - - fdt_blob = (char *)fdt_data; - if (fdt_check_header(fdt_blob) != 0) - goto no_fdt; - - if (fdt_totalsize(fdt_blob) != fdt_len) - goto error; - - debug("## Using FDT found in Android image second area\n"); + } #endif } else { debug("## No Flattened Device Tree\n"); From 62392675cdea46ca38a4d2b9be2c82e29b3b6da3 Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Mon, 1 Apr 2019 12:52:52 +0200 Subject: [PATCH 05/23] fdt: boot_get_fdt: android: use ENV 'fdtaddr' as fallback Our platform doesn't store the DTB into the Android image second area, but rather copies the DTB to RAM from a dedicated dtb.img partition [0], prior to booting the Android image by calling bootm. Similar to [1], we find it useful to just call 'bootm' and have the right DTB being passed to OS (assuming its address has been previously stored in 'fdtaddr' by calling `fdt addr `). Booting Android with DTB from 'fdtaddr' will only occur if: - No DTB is embedded in the second area of Android image - 'fdtaddr' points to a valid DTB in RAM [0] https://source.android.com/devices/architecture/dto/partitions [1] https://patchwork.ozlabs.org/patch/1046652/ ("Support boot Android image without address on bootm command") Signed-off-by: Eugeniu Rosca Reviewed-by: Simon Glass --- common/image-fdt.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/common/image-fdt.c b/common/image-fdt.c index a5d8b41d02..3aa5ffff0f 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -469,7 +469,15 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, debug("## Using FDT in Android image second area\n"); } else { - goto no_fdt; + fdt_addr = env_get_hex("fdtaddr", 0); + if (!fdt_addr) + goto no_fdt; + + fdt_blob = map_sysmem(fdt_addr, 0); + if (fdt_check_header(fdt_blob)) + goto no_fdt; + + debug("## Using FDT at ${fdtaddr}=Ox%lx\n", fdt_addr); } #endif } else { From 7ae31fccec16d4885f1704b089eb018be836f619 Mon Sep 17 00:00:00 2001 From: Christoph Muellner Date: Fri, 5 Apr 2019 13:03:46 +0200 Subject: [PATCH 06/23] common: command: Add command execution tracer. When using boot scripts it can become quite hard to understand which commands are actually executed during bootup (e.g. where is a kernel image loaded from or which DTB is in use). Shell scripts suffer from a similar problem and many shells address this problem with a command execution tracer (e.g. BASH has xtrace, which can be enabled by "set -x"). This patch introduces a command tracer for U-Boot, which prints every command with its arguments before it is executed. Signed-off-by: Christoph Muellner Reviewed-by: Philipp Tomsich --- cmd/Kconfig | 11 +++++++++++ common/command.c | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/cmd/Kconfig b/cmd/Kconfig index 2bdbfcb3d0..f6e7cd4303 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -53,6 +53,17 @@ config SYS_PROMPT This string is displayed in the command line to the left of the cursor. +config SYS_XTRACE + string "Command execution tracer" + depends on CMDLINE + default y if CMDLINE + help + This option enables the possiblity to print all commands before + executing them and after all variables are evaluated (similar + to Bash's xtrace/'set -x' feature). + To enable the tracer a variable "xtrace" needs to be defined in + the environment. + menu "Autoboot options" config AUTOBOOT diff --git a/common/command.c b/common/command.c index e14d1fa1d6..e192bb2a61 100644 --- a/common/command.c +++ b/common/command.c @@ -574,6 +574,20 @@ enum command_ret_t cmd_process(int flag, int argc, char * const argv[], enum command_ret_t rc = CMD_RET_SUCCESS; cmd_tbl_t *cmdtp; +#if defined(CONFIG_SYS_XTRACE) + char *xtrace; + + xtrace = env_get("xtrace"); + if (xtrace) { + puts("+"); + for (int i = 0; i < argc; i++) { + puts(" "); + puts(argv[i]); + } + puts("\n"); + } +#endif + /* Look up command in command table */ cmdtp = find_cmd(argv[0]); if (cmdtp == NULL) { From 001d1885f0012dbedef44bd828f91ba048029261 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:41 -0600 Subject: [PATCH 07/23] sandbox: Improve debugging in initcall_run_list() At present if one of the initcalls fails on sandbox the address printing is not help, e.g.: initcall sequence 0000557678967c80 failed at call 00005576709dfe1f (err=-96) This is because U-Boot gets relocated high into memory and the relocation offset (gd->reloc_off) does not work correctly for sandbox. Add support for finding the base address of the text region (at least on Linux) and use that to set the relocation offset. This makes the output better: initcall sequence 0000560775957c80 failed at call 0000000000048134 (err=-96) Then you use can use grep to see which init call failed, e.g.: $ grep 0000000000048134 u-boot.map stdio_add_devices Of course another option is to run it with a debugger such as gdb: $ gdb u-boot ... (gdb) br initcall.h:41 Breakpoint 1 at 0x4db9d: initcall.h:41. (2 locations) Note that two locations are reported, since this function is used in both board_init_f() and board_init_r(). (gdb) r Starting program: /tmp/b/sandbox/u-boot [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". U-Boot 2018.09-00264-ge0c2ba9814-dirty (Sep 22 2018 - 12:21:46 -0600) DRAM: 128 MiB MMC: Breakpoint 1, initcall_run_list (init_sequence=0x5555559619e0 ) at /scratch/sglass/cosarm/src/third_party/u-boot/files/include/initcall.h:41 41 printf("initcall sequence %p failed at call %p (err=%d)\n", (gdb) print *init_fnc_ptr $1 = (const init_fnc_t) 0x55555559c114 (gdb) Signed-off-by: Simon Glass --- arch/sandbox/cpu/os.c | 37 ++++++++++++++++++++++ arch/sandbox/cpu/start.c | 12 +++++-- arch/sandbox/include/asm/global_data.h | 1 + board/sandbox/README.sandbox | 43 ++++++++++++++++++++++++++ common/board_f.c | 2 +- include/initcall.h | 8 +++-- include/os.h | 11 +++++++ 7 files changed, 109 insertions(+), 5 deletions(-) diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index a8d01e4001..c20491493f 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -786,3 +786,40 @@ int os_mprotect_allow(void *start, size_t len) return mprotect(start, len, PROT_READ | PROT_WRITE); } + +void *os_find_text_base(void) +{ + char line[500]; + void *base = NULL; + int len; + int fd; + + /* + * This code assumes that the first line of /proc/self/maps holds + * information about the text, for example: + * + * 5622d9907000-5622d9a55000 r-xp 00000000 08:01 15067168 u-boot + * + * The first hex value is assumed to be the address. + * + * This is tested in Linux 4.15. + */ + fd = open("/proc/self/maps", O_RDONLY); + if (fd == -1) + return NULL; + len = read(fd, line, sizeof(line)); + if (len > 0) { + char *end = memchr(line, '-', len); + + if (end) { + unsigned long long addr; + + *end = '\0'; + if (sscanf(line, "%llx", &addr) == 1) + base = (void *)addr; + } + } + close(fd); + + return base; +} diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index 2f5e6e9518..e22d65f6d9 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -328,6 +328,10 @@ int main(int argc, char *argv[]) gd_t data; int ret; + memset(&data, '\0', sizeof(data)); + gd = &data; + gd->arch.text_base = os_find_text_base(); + ret = state_init(); if (ret) goto err; @@ -340,8 +344,6 @@ int main(int argc, char *argv[]) if (ret) goto err; - memset(&data, '\0', sizeof(data)); - gd = &data; #if CONFIG_VAL(SYS_MALLOC_F_LEN) gd->malloc_base = CONFIG_MALLOC_F_ADDR; #endif @@ -350,6 +352,12 @@ int main(int argc, char *argv[]) #endif setup_ram_buf(state); + /* + * Set up the relocation offset here, since sandbox symbols are always + * relocated by the OS before sandbox is entered. + */ + gd->reloc_off = (ulong)gd->arch.text_base; + /* Do pre- and post-relocation init */ board_init_f(0); diff --git a/arch/sandbox/include/asm/global_data.h b/arch/sandbox/include/asm/global_data.h index f6a6a343d2..f4ce72d566 100644 --- a/arch/sandbox/include/asm/global_data.h +++ b/arch/sandbox/include/asm/global_data.h @@ -12,6 +12,7 @@ /* Architecture-specific global data */ struct arch_global_data { uint8_t *ram_buf; /* emulated RAM buffer */ + void *text_base; /* pointer to base of text region */ }; #include diff --git a/board/sandbox/README.sandbox b/board/sandbox/README.sandbox index 9b09404294..ed8fac6f78 100644 --- a/board/sandbox/README.sandbox +++ b/board/sandbox/README.sandbox @@ -392,6 +392,49 @@ state_setprop() which does this automatically and avoids running out of space. See existing code for examples. +Debugging the init sequence +--------------------------- + +If you get a failure in the initcall sequence, like this: + + initcall sequence 0000560775957c80 failed at call 0000000000048134 (err=-96) + +Then you use can use grep to see which init call failed, e.g.: + + $ grep 0000000000048134 u-boot.map + stdio_add_devices + +Of course another option is to run it with a debugger such as gdb: + + $ gdb u-boot + ... + (gdb) br initcall.h:41 + Breakpoint 1 at 0x4db9d: initcall.h:41. (2 locations) + +Note that two locations are reported, since this function is used in both +board_init_f() and board_init_r(). + + (gdb) r + Starting program: /tmp/b/sandbox/u-boot + [Thread debugging using libthread_db enabled] + Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". + + U-Boot 2018.09-00264-ge0c2ba9814-dirty (Sep 22 2018 - 12:21:46 -0600) + + DRAM: 128 MiB + MMC: + + Breakpoint 1, initcall_run_list (init_sequence=0x5555559619e0 ) + at /scratch/sglass/cosarm/src/third_party/u-boot/files/include/initcall.h:41 + 41 printf("initcall sequence %p failed at call %p (err=%d)\n", + (gdb) print *init_fnc_ptr + $1 = (const init_fnc_t) 0x55555559c114 + (gdb) + + +This approach can be used on normal boards as well as sandbox. + + Testing ------- diff --git a/common/board_f.c b/common/board_f.c index 149a7229e8..7ef20f2042 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -714,7 +714,7 @@ static int setup_reloc(void) * just after the default vector table location, so at 0x400 */ gd->reloc_off = gd->relocaddr - (CONFIG_SYS_TEXT_BASE + 0x400); -#else +#elif !defined(CONFIG_SANDBOX) gd->reloc_off = gd->relocaddr - CONFIG_SYS_TEXT_BASE; #endif #endif diff --git a/include/initcall.h b/include/initcall.h index 3ac01aa2cd..a38c83efa4 100644 --- a/include/initcall.h +++ b/include/initcall.h @@ -22,13 +22,17 @@ static inline int initcall_run_list(const init_fnc_t init_sequence[]) unsigned long reloc_ofs = 0; int ret; - if (gd->flags & GD_FLG_RELOC) + /* + * Sandbox is relocated by the OS, so symbols always appear at + * the relocated address. + */ + if (IS_ENABLED(CONFIG_SANDBOX) || (gd->flags & GD_FLG_RELOC)) reloc_ofs = gd->reloc_off; #ifdef CONFIG_EFI_APP reloc_ofs = (unsigned long)image_base; #endif debug("initcall: %p", (char *)*init_fnc_ptr - reloc_ofs); - if (gd->flags & GD_FLG_RELOC) + if (reloc_ofs) debug(" (relocated to %p)\n", (char *)*init_fnc_ptr); else debug("\n"); diff --git a/include/os.h b/include/os.h index 6f33b08cf0..7a4f78b9b1 100644 --- a/include/os.h +++ b/include/os.h @@ -364,4 +364,15 @@ int os_write_file(const char *name, const void *buf, int size); */ int os_read_file(const char *name, void **bufp, int *sizep); +/* + * os_find_text_base() - Find the text section in this running process + * + * This tries to find the address of the text section in this running process. + * It can be useful to map the address of functions to the address listed in + * the u-boot.map file. + * + * @return address if found, else NULL + */ +void *os_find_text_base(void); + #endif From 4a6409b74cc09b150f234ba3ab7d75ba6358270a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:42 -0600 Subject: [PATCH 08/23] sandbox: Correct maths in allocation routines Allocation routines were adjusted to ensure that the returned addresses are a multiple of the page size, but the header code was not updated to take account of this. These routines assume that the header size is the same as the page size which is unlikely. At present os_realloc() does not work correctly due to this bug. The only user is the hostfs 'ls' command, and only if the directory contains a unusually long filename, which likely explains why this bug was not caught earlier. Fix this by doing the calculations using the obtained page size. Signed-off-by: Simon Glass --- arch/sandbox/cpu/os.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index c20491493f..47dfb476d3 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -209,8 +209,8 @@ void os_tty_raw(int fd, bool allow_sigs) void *os_malloc(size_t length) { - struct os_mem_hdr *hdr; int page_size = getpagesize(); + struct os_mem_hdr *hdr; /* * Use an address that is hopefully available to us so that pointers @@ -229,30 +229,34 @@ void *os_malloc(size_t length) void os_free(void *ptr) { - struct os_mem_hdr *hdr = ptr; + int page_size = getpagesize(); + struct os_mem_hdr *hdr; - hdr--; - if (ptr) - munmap(hdr, hdr->length + sizeof(*hdr)); + if (ptr) { + hdr = ptr - page_size; + munmap(hdr, hdr->length + page_size); + } } void *os_realloc(void *ptr, size_t length) { - struct os_mem_hdr *hdr = ptr; + int page_size = getpagesize(); + struct os_mem_hdr *hdr; void *buf = NULL; - hdr--; - if (length != 0) { + if (length) { buf = os_malloc(length); if (!buf) return buf; if (ptr) { + hdr = ptr - page_size; if (length > hdr->length) length = hdr->length; memcpy(buf, ptr, length); } } - os_free(ptr); + if (ptr) + os_free(ptr); return buf; } From 5dbe794dc0b2cceb76c2042b6806a342e7c4ca7f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:43 -0600 Subject: [PATCH 09/23] sandbox: Drop the printf() in setup_ram_buf() This was really intended for debugging. Drop it. Signed-off-by: Simon Glass --- arch/sandbox/cpu/start.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index e22d65f6d9..82828f0c1d 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -303,10 +303,8 @@ int board_run_command(const char *cmdline) static void setup_ram_buf(struct sandbox_state *state) { /* Zero the RAM buffer if we didn't read it, to keep valgrind happy */ - if (!state->ram_buf_read) { + if (!state->ram_buf_read) memset(state->ram_buf, '\0', state->ram_size); - printf("clear %p %x\n", state->ram_buf, state->ram_size); - } gd->arch.ram_buf = state->ram_buf; gd->ram_size = state->ram_size; From a1396cdc4b557ac678b7af93938e58814612759a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:44 -0600 Subject: [PATCH 10/23] sandbox: Move pre-console buffer out of the way of tracing These two buffers currently conflict if tracing is enabled. Move the pre-console buffer and update the documentation. Signed-off-by: Simon Glass --- board/sandbox/README.sandbox | 3 +++ configs/sandbox_defconfig | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/board/sandbox/README.sandbox b/board/sandbox/README.sandbox index ed8fac6f78..48c1e2b9e7 100644 --- a/board/sandbox/README.sandbox +++ b/board/sandbox/README.sandbox @@ -477,6 +477,9 @@ that are mapped into that memory: 0 CONFIG_SYS_FDT_LOAD_ADDR Device tree e000 CONFIG_BLOBLIST_ADDR Blob list 10000 CONFIG_MALLOC_F_ADDR Early memory allocation + f0000 CONFIG_PRE_CON_BUF_ADDR Pre-console buffer + 100000 CONFIG_TRACE_EARLY_ADDR Early trace buffer (if enabled) += -- diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index bb508a8d02..ba28db6d26 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -16,7 +16,7 @@ CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 CONFIG_SILENT_CONSOLE=y CONFIG_PRE_CONSOLE_BUFFER=y -CONFIG_PRE_CON_BUF_ADDR=0x100000 +CONFIG_PRE_CON_BUF_ADDR=0xf0000 CONFIG_LOG_MAX_LEVEL=6 CONFIG_LOG_ERROR_RETURN=y CONFIG_DISPLAY_BOARDINFO_LATE=y From 173577252f2bebaf17851ce8128ba522f27fb6dd Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:45 -0600 Subject: [PATCH 11/23] bootstage: Allow calling bootstage_mark() before bootstage_init() It is possible for this to happen if something goes wrong very early in the init sequence. Add a check for this. Signed-off-by: Simon Glass --- common/bootstage.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/common/bootstage.c b/common/bootstage.c index 9793b85d4e..56ef91ad85 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -99,6 +99,13 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name, struct bootstage_data *data = gd->bootstage; struct bootstage_record *rec; + /* + * initf_bootstage() is called very early during boot but since hang() + * calls bootstage_error() we can be called before bootstage is set up. + * Add a check to avoid this. + */ + if (!data) + return mark; if (flags & BOOTSTAGEF_ALLOC) id = data->next_id++; From b82de17eb2e89d511df293e02bdea517399d02c3 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:46 -0600 Subject: [PATCH 12/23] sandbox: Increase the early-trace-buffer size This buffer is too small now that sandbox has grown in size. Increase it. Signed-off-by: Simon Glass --- include/configs/sandbox.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index e36a5fec0e..6dd03e31cf 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -9,7 +9,7 @@ #ifdef FTRACE #define CONFIG_TRACE #define CONFIG_TRACE_BUFFER_SIZE (16 << 20) -#define CONFIG_TRACE_EARLY_SIZE (8 << 20) +#define CONFIG_TRACE_EARLY_SIZE (16 << 20) #define CONFIG_TRACE_EARLY #define CONFIG_TRACE_EARLY_ADDR 0x00100000 From 315f60d741a7c16bc44cee87668a02054d8f9f08 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:47 -0600 Subject: [PATCH 13/23] initcall: Drop use of header files This file should not include header files. They have already been included by the time initcall.h is included. Also, document how to enable debugging in this file. Signed-off-by: Simon Glass --- include/initcall.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/include/initcall.h b/include/initcall.h index a38c83efa4..78d15afe69 100644 --- a/include/initcall.h +++ b/include/initcall.h @@ -8,12 +8,11 @@ typedef int (*init_fnc_t)(void); -#include -#include -#include - -DECLARE_GLOBAL_DATA_PTR; - +/* + * To enable debugging. add #define DEBUG at the top of the including file. + * + * To find a symbol, use grep on u-boot.map + */ static inline int initcall_run_list(const init_fnc_t init_sequence[]) { const init_fnc_t *init_fnc_ptr; From ca49b2c6e2cc66d7b84e7559cadfc8bf792a2170 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:48 -0600 Subject: [PATCH 14/23] div64: Use kernel types These functions still use uint32_t and uint64_t but checkpatch now requests that the kernel types be used instead. Update them as well as a few resulting checkpatch errors. Signed-off-by: Simon Glass --- include/div64.h | 70 ++++++++++++++++++++++++------------------------- lib/div64.c | 14 +++++----- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/include/div64.h b/include/div64.h index 76563ef978..8b92d2b183 100644 --- a/include/div64.h +++ b/include/div64.h @@ -9,11 +9,11 @@ * * The semantics of do_div() are: * - * uint32_t do_div(uint64_t *n, uint32_t base) + * u32 do_div(u64 *n, u32 base) * { - * uint32_t remainder = *n % base; - * *n = *n / base; - * return remainder; + * u32 remainder = *n % base; + * *n = *n / base; + * return remainder; * } * * NOTE: macro parameter n is evaluated multiple times, @@ -26,10 +26,10 @@ #if BITS_PER_LONG == 64 # define do_div(n,base) ({ \ - uint32_t __base = (base); \ - uint32_t __rem; \ - __rem = ((uint64_t)(n)) % __base; \ - (n) = ((uint64_t)(n)) / __base; \ + u32 __base = (base); \ + u32 __rem; \ + __rem = ((u64)(n)) % __base; \ + (n) = ((u64)(n)) / __base; \ __rem; \ }) @@ -62,8 +62,8 @@ * Hence this monstrous macro (static inline doesn't always \ * do the trick here). \ */ \ - uint64_t ___res, ___x, ___t, ___m, ___n = (n); \ - uint32_t ___p, ___bias; \ + u64 ___res, ___x, ___t, ___m, ___n = (n); \ + u32 ___p, ___bias; \ \ /* determine MSB of b */ \ ___p = 1 << ilog2(___b); \ @@ -110,7 +110,7 @@ * possible, otherwise that'll need extra overflow \ * handling later. \ */ \ - uint32_t ___bits = -(___m & -___m); \ + u32 ___bits = -(___m & -___m); \ ___bits |= ___m >> 32; \ ___bits = (~___bits) << 1; \ /* \ @@ -150,61 +150,61 @@ /* * Default C implementation for __arch_xprod_64() * - * Prototype: uint64_t __arch_xprod_64(const uint64_t m, uint64_t n, bool bias) + * Prototype: u64 __arch_xprod_64(const u64 m, u64 n, bool bias) * Semantic: retval = ((bias ? m : 0) + m * n) >> 64 * * The product is a 128-bit value, scaled down to 64 bits. * Assuming constant propagation to optimize away unused conditional code. * Architectures may provide their own optimized assembly implementation. */ -static inline uint64_t __arch_xprod_64(const uint64_t m, uint64_t n, bool bias) +static inline u64 __arch_xprod_64(const u64 m, u64 n, bool bias) { - uint32_t m_lo = m; - uint32_t m_hi = m >> 32; - uint32_t n_lo = n; - uint32_t n_hi = n >> 32; - uint64_t res, tmp; + u32 m_lo = m; + u32 m_hi = m >> 32; + u32 n_lo = n; + u32 n_hi = n >> 32; + u64 res, tmp; if (!bias) { - res = ((uint64_t)m_lo * n_lo) >> 32; + res = ((u64)m_lo * n_lo) >> 32; } else if (!(m & ((1ULL << 63) | (1ULL << 31)))) { /* there can't be any overflow here */ - res = (m + (uint64_t)m_lo * n_lo) >> 32; + res = (m + (u64)m_lo * n_lo) >> 32; } else { - res = m + (uint64_t)m_lo * n_lo; + res = m + (u64)m_lo * n_lo; tmp = (res < m) ? (1ULL << 32) : 0; res = (res >> 32) + tmp; } if (!(m & ((1ULL << 63) | (1ULL << 31)))) { /* there can't be any overflow here */ - res += (uint64_t)m_lo * n_hi; - res += (uint64_t)m_hi * n_lo; + res += (u64)m_lo * n_hi; + res += (u64)m_hi * n_lo; res >>= 32; } else { - tmp = res += (uint64_t)m_lo * n_hi; - res += (uint64_t)m_hi * n_lo; + tmp = res += (u64)m_lo * n_hi; + res += (u64)m_hi * n_lo; tmp = (res < tmp) ? (1ULL << 32) : 0; res = (res >> 32) + tmp; } - res += (uint64_t)m_hi * n_hi; + res += (u64)m_hi * n_hi; return res; } #endif #ifndef __div64_32 -extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); +extern u32 __div64_32(u64 *dividend, u32 divisor); #endif /* The unnecessary pointer compare is there * to check for type safety (n must be 64bit) */ # define do_div(n,base) ({ \ - uint32_t __base = (base); \ - uint32_t __rem; \ - (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + u32 __base = (base); \ + u32 __rem; \ + (void)(((typeof((n)) *)0) == ((u64 *)0)); \ if (__builtin_constant_p(__base) && \ is_power_of_2(__base)) { \ __rem = (n) & (__base - 1); \ @@ -212,14 +212,14 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); } else if (__div64_const32_is_OK && \ __builtin_constant_p(__base) && \ __base != 0) { \ - uint32_t __res_lo, __n_lo = (n); \ + u32 __res_lo, __n_lo = (n); \ (n) = __div64_const32(n, __base); \ /* the remainder can be computed with 32-bit regs */ \ __res_lo = (n); \ __rem = __n_lo - __res_lo * __base; \ } else if (likely(((n) >> 32) == 0)) { \ - __rem = (uint32_t)(n) % __base; \ - (n) = (uint32_t)(n) / __base; \ + __rem = (u32)(n) % __base; \ + (n) = (u32)(n) / __base; \ } else \ __rem = __div64_32(&(n), __base); \ __rem; \ @@ -234,9 +234,9 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); /* Wrapper for do_div(). Doesn't modify dividend and returns * the result, not remainder. */ -static inline uint64_t lldiv(uint64_t dividend, uint32_t divisor) +static inline u64 lldiv(u64 dividend, u32 divisor) { - uint64_t __res = dividend; + u64 __res = dividend; do_div(__res, divisor); return(__res); } diff --git a/lib/div64.c b/lib/div64.c index 206f582ca9..7abc68c333 100644 --- a/lib/div64.c +++ b/lib/div64.c @@ -25,19 +25,19 @@ #if BITS_PER_LONG == 32 #ifndef __div64_32 -uint32_t __attribute__((weak)) __div64_32(uint64_t *n, uint32_t base) +u32 __attribute__((weak)) __div64_32(u64 *n, u32 base) { - uint64_t rem = *n; - uint64_t b = base; - uint64_t res, d = 1; - uint32_t high = rem >> 32; + u64 rem = *n; + u64 b = base; + u64 res, d = 1; + u32 high = rem >> 32; /* Reduce the thing a bit first */ res = 0; if (high >= base) { high /= base; - res = (uint64_t) high << 32; - rem -= (uint64_t) (high*base) << 32; + res = (u64)high << 32; + rem -= (u64)(high * base) << 32; } while ((int64_t)b > 0 && b < rem) { From f611a46ef1dc70bbbf013d0c33978d1204aeabc5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:49 -0600 Subject: [PATCH 15/23] div64: Don't instrument the division function This function may be called from tracing code, since that code needs to read the timer and this often requires calling do_div(), which calls __div64_32(). If this function is instrumented it causes an infinite loop, since emitting a trace record requests the time, which in turn emits a trace record, etc. Update the prototype to prevent instrumentation code being added. Signed-off-by: Simon Glass --- lib/div64.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/div64.c b/lib/div64.c index 7abc68c333..62933c92c4 100644 --- a/lib/div64.c +++ b/lib/div64.c @@ -25,7 +25,13 @@ #if BITS_PER_LONG == 32 #ifndef __div64_32 -u32 __attribute__((weak)) __div64_32(u64 *n, u32 base) +/* + * Don't instrument this function as it may be called from tracing code, since + * it needs to read the timer and this often requires calling do_div(), which + * calls this function. + */ +uint32_t __attribute__((weak, no_instrument_function)) __div64_32(u64 *n, + u32 base) { u64 rem = *n; u64 b = base; From f564d09608f422d4584ae7416d9da040a89ebee3 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:50 -0600 Subject: [PATCH 16/23] trace: Tidy up error returns At present many functions in this file return -1. Update them to return a valid error code. Also tidy up the 'return' statements at the same time, since these should have a blank line before them. Signed-off-by: Simon Glass --- lib/trace.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/trace.c b/lib/trace.c index bb089c2eca..fb7658b112 100644 --- a/lib/trace.c +++ b/lib/trace.c @@ -183,7 +183,8 @@ int trace_list_functions(void *buff, int buff_size, unsigned int *needed) /* Work out how must of the buffer we used */ *needed = ptr - buff; if (ptr > end) - return -1; + return -ENOSPC; + return 0; } @@ -227,7 +228,8 @@ int trace_list_calls(void *buff, int buff_size, unsigned *needed) /* Work out how must of the buffer we used */ *needed = ptr - buff; if (ptr > end) - return -1; + return -ENOSPC; + return 0; } @@ -302,7 +304,7 @@ int __attribute__((no_instrument_function)) trace_init(void *buff, memcpy(buff, hdr, used); #else puts("trace: already enabled\n"); - return -1; + return -EALREADY; #endif } hdr = (struct trace_hdr *)buff; @@ -310,7 +312,7 @@ int __attribute__((no_instrument_function)) trace_init(void *buff, if (needed > buff_size) { printf("trace: buffer size %zd bytes: at least %zd needed\n", buff_size, needed); - return -1; + return -ENOSPC; } if (was_disabled) @@ -327,6 +329,7 @@ int __attribute__((no_instrument_function)) trace_init(void *buff, hdr->depth_limit = 15; trace_enabled = 1; trace_inited = 1; + return 0; } @@ -346,7 +349,7 @@ int __attribute__((no_instrument_function)) trace_early_init(void) if (needed > buff_size) { printf("trace: buffer size is %zd bytes, at least %zd needed\n", buff_size, needed); - return -1; + return -ENOSPC; } memset(hdr, '\0', needed); @@ -361,6 +364,7 @@ int __attribute__((no_instrument_function)) trace_early_init(void) printf("trace: early enable at %08x\n", CONFIG_TRACE_EARLY_ADDR); trace_enabled = 1; + return 0; } #endif From a24a78d7e3e8b6008423d1a6aa49a6c9eb904752 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:51 -0600 Subject: [PATCH 17/23] Convert CONFIG_TRACE to Kconfig This converts the following to Kconfig: CONFIG_TRACE Signed-off-by: Simon Glass --- cmd/Kconfig | 2 +- lib/Kconfig | 9 +++++++++ scripts/config_whitelist.txt | 1 - 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index f6e7cd4303..8744cec5c0 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1905,7 +1905,7 @@ config CMD_TRACE Enables a command to control using of function tracing within U-Boot. This allows recording of call traces including timing information. The command can write data to memory for exporting - for analsys (e.g. using bootchart). See doc/README.trace for full + for analysis (e.g. using bootchart). See doc/README.trace for full details. config CMD_AVB diff --git a/lib/Kconfig b/lib/Kconfig index 2120216593..a3352a4fa1 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -165,6 +165,15 @@ config RBTREE config BITREVERSE bool "Bit reverse library from Linux" +config TRACE + bool "Support for tracing of function calls and timing" + imply CMD_TRACE + help + Enables function tracing within U-Boot. This allows recording of call + traces including timing information. The command can write data to + memory for exporting for analysis (e.g. using bootchart). + See doc/README.trace for full details. + source lib/dhry/Kconfig menu "Security support" diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index 421362d953..2942c6945b 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -4401,7 +4401,6 @@ CONFIG_TMU_TIMER CONFIG_TPL_PAD_TO CONFIG_TPM_TIS_BASE_ADDRESS CONFIG_TPS6586X_POWER -CONFIG_TRACE CONFIG_TRACE_BUFFER_SIZE CONFIG_TRACE_EARLY CONFIG_TRACE_EARLY_ADDR From 1c6eb075a3fd31f3a22d72f11abc0b15c5776f1d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:52 -0600 Subject: [PATCH 18/23] Convert CONFIG_TRACE_BUFFER_SIZE et al to Kconfig This converts the following to Kconfig: CONFIG_TRACE_BUFFER_SIZE CONFIG_TRACE_EARLY_SIZE CONFIG_TRACE_EARLY CONFIG_TRACE_EARLY_ADDR Signed-off-by: Simon Glass --- lib/Kconfig | 48 ++++++++++++++++++++++++++++++++++++ lib/trace.c | 3 ++- scripts/config_whitelist.txt | 4 --- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/lib/Kconfig b/lib/Kconfig index a3352a4fa1..05f82d4a50 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -174,6 +174,54 @@ config TRACE memory for exporting for analysis (e.g. using bootchart). See doc/README.trace for full details. +config TRACE_BUFFER_SIZE + hex "Size of trace buffer in U-Boot" + depends on TRACE + default 0x01000000 + help + Sets the size of the trace buffer in U-Boot. This is allocated from + memory during relocation. If this buffer is too small, the trace + history will be truncated, with later records omitted. + + If early trace is enabled (i.e. before relocation), this buffer must + be large enough to include all the data from the early trace buffer as + well, since this is copied over to the main buffer during relocation. + + A trace record is emitted for each function call and each record is + 12 bytes (see struct trace_call). A suggested minimum size is 1MB. If + the size is too small then 'trace stats' will show a message saying + how many records were dropped due to buffer overflow. + +config TRACE_EARLY + bool "Enable tracing before relocation" + depends on TRACE + help + Sometimes it is helpful to trace execution of U-Boot before + relocation. This is possible by using a arch-specific, fixed buffer + position in memory. Enable this option to start tracing as early as + possible after U-Boot starts. + +config TRACE_EARLY_SIZE + hex "Size of early trace buffer in U-Boot" + depends on TRACE_EARLY + default 0x00100000 + help + Sets the size of the early trace buffer in bytes. This is used to hold + tracing information before relocation. + +config TRACE_EARLY_ADDR + hex "Address of early trace buffer in U-Boot" + depends on TRACE_EARLY + default 0x00100000 + help + Sets the address of the early trace buffer in U-Boot. This memory + must be accessible before relocation. + + A trace record is emitted for each function call and each record is + 12 bytes (see struct trace_call). A suggested minimum size is 1MB. If + the size is too small then the message which says the amount of early + data being coped will the the same as the + source lib/dhry/Kconfig menu "Security support" diff --git a/lib/trace.c b/lib/trace.c index fb7658b112..9956442fef 100644 --- a/lib/trace.c +++ b/lib/trace.c @@ -296,7 +296,8 @@ int __attribute__((no_instrument_function)) trace_init(void *buff, trace_enabled = 0; hdr = map_sysmem(CONFIG_TRACE_EARLY_ADDR, CONFIG_TRACE_EARLY_SIZE); - end = (char *)&hdr->ftrace[hdr->ftrace_count]; + end = (char *)&hdr->ftrace[min(hdr->ftrace_count, + hdr->ftrace_size)]; used = end - (char *)hdr; printf("trace: copying %08lx bytes of early data from %x to %08lx\n", used, CONFIG_TRACE_EARLY_ADDR, diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index 2942c6945b..3e6bdf8161 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -4401,10 +4401,6 @@ CONFIG_TMU_TIMER CONFIG_TPL_PAD_TO CONFIG_TPM_TIS_BASE_ADDRESS CONFIG_TPS6586X_POWER -CONFIG_TRACE_BUFFER_SIZE -CONFIG_TRACE_EARLY -CONFIG_TRACE_EARLY_ADDR -CONFIG_TRACE_EARLY_SIZE CONFIG_TRAILBLAZER CONFIG_TRATS CONFIG_TSEC From 569cec5e840601a1491d899e5c434ee59c70cb57 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 8 Apr 2019 13:20:53 -0600 Subject: [PATCH 19/23] sandbox: Enable the 'trace' command when tracing is used Enable this by default so that tracing can be inspected if enabled. This cannot rely on the 'imply' in lib/Kconfig since this method of enabling tracing relates on an environment variable (FTRACE) and does not use Kconfig. Signed-off-by: Simon Glass --- include/configs/sandbox.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 6dd03e31cf..bf03baefe8 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -8,11 +8,11 @@ #ifdef FTRACE #define CONFIG_TRACE +#define CONFIG_CMD_TRACE #define CONFIG_TRACE_BUFFER_SIZE (16 << 20) #define CONFIG_TRACE_EARLY_SIZE (16 << 20) #define CONFIG_TRACE_EARLY #define CONFIG_TRACE_EARLY_ADDR 0x00100000 - #endif #ifndef CONFIG_SPL_BUILD From 3a7c45f6a7725808e2e82908be4bc90d4d78e737 Mon Sep 17 00:00:00 2001 From: Lukas Auer Date: Wed, 10 Apr 2019 14:46:07 +0200 Subject: [PATCH 20/23] simple-bus: add DM_FLAG_PRE_RELOC flag to simple-bus driver Boards such as qemu-riscv, which receive their device tree at runtime, for example from QEMU or firmware, are unable to add the appropriate device tree properties to make devices available pre relocation. Instead, they must rely on the DM_FLAG_PRE_RELOC flag to be set for the required drivers. Add the DM_FLAG_PRE_RELOC flag to the simple-bus driver to make devices under it with drivers that have set the flag as well available pre relocation for these boards. Signed-off-by: Lukas Auer Reviewed-by: Bin Meng Tested-by: Bin Meng --- drivers/core/simple-bus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/core/simple-bus.c b/drivers/core/simple-bus.c index e16d8a9ff4..7fc23ef82d 100644 --- a/drivers/core/simple-bus.c +++ b/drivers/core/simple-bus.c @@ -60,4 +60,5 @@ U_BOOT_DRIVER(simple_bus_drv) = { .name = "generic_simple_bus", .id = UCLASS_SIMPLE_BUS, .of_match = generic_simple_bus_ids, + .flags = DM_FLAG_PRE_RELOC, }; From f2100f6f77212e2a5b3db165e84eed911be5c3f6 Mon Sep 17 00:00:00 2001 From: Stefan Roese Date: Fri, 12 Apr 2019 16:42:28 +0200 Subject: [PATCH 21/23] dm: core: Change platform specific translation-offset handling Testing has shown that the current DM implementation of a platform / board specific translation offset, as its needed for the SPL on MVEBU platforms is buggy. The translation offset is confingured too late, after the driver bind functions are run. This may result in incorrect address translations. With the current implementation its not possible to configure the offset earlier, as the DM code has not run at all. This patch now removed the set_/get_translation_offset() calls and moves the translation offset into the GD variable translation_offset. This variable will get used when CONFIG_TRANSLATION_OFFSET is enabled. This option is enabled only for MVEBU on ARM32 platforms, where its currenty needed and configured in the SPL. Signed-off-by: Stefan Roese Cc: Pierre Bourdon Cc: Baruch Siach Cc: Simon Glass Cc: Heiko Schocher Cc: Tom Rini Tested-by: Pierre Bourdon Tested-by: Baruch Siach --- arch/arm/mach-mvebu/Kconfig | 1 + arch/arm/mach-mvebu/spl.c | 12 +++++++++--- drivers/core/Kconfig | 9 +++++++++ drivers/core/fdtaddr.c | 9 ++++++--- drivers/core/root.c | 21 --------------------- include/asm-generic/global_data.h | 4 ++++ include/dm/fdtaddr.h | 21 --------------------- 7 files changed, 29 insertions(+), 48 deletions(-) diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index f5fd60d784..f99bd3bf65 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -14,6 +14,7 @@ config ARMADA_32BIT select SPL_OF_CONTROL if SPL select SPL_SIMPLE_BUS if SPL select SUPPORT_SPL + select TRANSLATION_OFFSET config ARMADA_64BIT bool diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index 9dd7c84b68..530b98c1aa 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -93,15 +93,21 @@ void board_init_f(ulong dummy) */ #endif + /* + * Use special translation offset for SPL. This needs to be + * configured *before* spl_init() is called as this function + * calls dm_init() which calls the bind functions of the + * device drivers. Here the base address needs to be configured + * (translated) correctly. + */ + gd->translation_offset = 0xd0000000 - 0xf1000000; + ret = spl_init(); if (ret) { debug("spl_init() failed: %d\n", ret); hang(); } - /* Use special translation offset for SPL */ - dm_set_translation_offset(0xd0000000 - 0xf1000000); - preloader_console_init(); timer_init(); diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index ddf2fb3fb8..2d195ae35e 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -225,6 +225,15 @@ config SPL_OF_TRANSLATE used for the address translation. This function is faster and smaller in size than fdt_translate_address(). +config TRANSLATION_OFFSET + bool "Platforms specific translation offset" + depends on DM && OF_CONTROL + help + Some platforms need a special address translation. Those + platforms (e.g. mvebu in SPL) can configure a translation + offset by enabling this option and setting the translation_offset + variable in the GD in their platform- / board-specific code. + config OF_ISA_BUS bool depends on OF_TRANSLATE diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index e113f1dd39..c2873861da 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -74,13 +74,16 @@ fdt_addr_t devfdt_get_addr_index(struct udevice *dev, int index) } } +#if defined(CONFIG_TRANSLATION_OFFSET) /* * Some platforms need a special address translation. Those * platforms (e.g. mvebu in SPL) can configure a translation - * offset in the DM by calling dm_set_translation_offset() that - * will get added to all addresses returned by devfdt_get_addr(). + * offset by setting this value in the GD and enaling this + * feature via CONFIG_TRANSLATION_OFFSET. This value will + * get added to all addresses returned by devfdt_get_addr(). */ - addr += dm_get_translation_offset(); + addr += gd->translation_offset; +#endif return addr; #else diff --git a/drivers/core/root.c b/drivers/core/root.c index e6ec7faf37..8fa096648e 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -25,10 +25,6 @@ DECLARE_GLOBAL_DATA_PTR; -struct root_priv { - fdt_addr_t translation_offset; /* optional translation offset */ -}; - static const struct driver_info root_info = { .name = "root_driver", }; @@ -52,22 +48,6 @@ void dm_fixup_for_gd_move(struct global_data *new_gd) } } -fdt_addr_t dm_get_translation_offset(void) -{ - struct udevice *root = dm_root(); - struct root_priv *priv = dev_get_priv(root); - - return priv->translation_offset; -} - -void dm_set_translation_offset(fdt_addr_t offs) -{ - struct udevice *root = dm_root(); - struct root_priv *priv = dev_get_priv(root); - - priv->translation_offset = offs; -} - #if defined(CONFIG_NEEDS_MANUAL_RELOC) void fix_drivers(void) { @@ -420,7 +400,6 @@ int dm_init_and_scan(bool pre_reloc_only) U_BOOT_DRIVER(root_driver) = { .name = "root_driver", .id = UCLASS_ROOT, - .priv_auto_alloc_size = sizeof(struct root_priv), }; /* This is the root uclass */ diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 78dcf40bff..65ee3e5d5a 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -20,6 +20,7 @@ */ #ifndef __ASSEMBLY__ +#include #include #include @@ -133,6 +134,9 @@ typedef struct global_data { struct spl_handoff *spl_handoff; # endif #endif +#if defined(CONFIG_TRANSLATION_OFFSET) + fdt_addr_t translation_offset; /* optional translation offset */ +#endif } gd_t; #endif diff --git a/include/dm/fdtaddr.h b/include/dm/fdtaddr.h index c171d9bc2f..3bc2599b6c 100644 --- a/include/dm/fdtaddr.h +++ b/include/dm/fdtaddr.h @@ -120,25 +120,4 @@ fdt_addr_t devfdt_get_addr_size_index(struct udevice *dev, int index, */ fdt_addr_t devfdt_get_addr_name(struct udevice *dev, const char *name); -/** - * dm_set_translation_offset() - Set translation offset - * @offs: Translation offset - * - * Some platforms need a special address translation. Those - * platforms (e.g. mvebu in SPL) can configure a translation - * offset in the DM by calling this function. It will be - * added to all addresses returned in devfdt_get_addr(). - */ -void dm_set_translation_offset(fdt_addr_t offs); - -/** - * dm_get_translation_offset() - Get translation offset - * - * This function returns the translation offset that can - * be configured by calling dm_set_translation_offset(). - * - * @return translation offset for the device address (0 as default). - */ -fdt_addr_t dm_get_translation_offset(void); - #endif From d81d96901eacfc9ffbe7c26a8d9b55d02b3fcb0c Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 15 Apr 2019 10:08:20 +0200 Subject: [PATCH 22/23] fdtdec: Use fdt_setprop_u32() for fdtdec_set_phandle() The fdt_setprop_u32() function does everything that we need, so we really only use the function as a convenience wrapper, in which case it can simply be a static inline function. Signed-off-by: Thierry Reding --- include/fdtdec.h | 5 ++++- lib/fdtdec.c | 7 ------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/include/fdtdec.h b/include/fdtdec.h index 266c58271f..110aa6ab6d 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -1029,7 +1029,10 @@ int fdtdec_setup_memory_banksize(void); * @param phandle phandle to set for the given node * @return 0 on success or a negative error code on failure */ -int fdtdec_set_phandle(void *blob, int node, uint32_t phandle); +static inline int fdtdec_set_phandle(void *blob, int node, uint32_t phandle) +{ + return fdt_setprop_u32(blob, node, "phandle", phandle); +} /** * fdtdec_add_reserved_memory() - add or find a reserved-memory node diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 9c9c302347..fea44a9a8c 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1261,13 +1261,6 @@ __weak void *board_fdt_blob_setup(void) } #endif -int fdtdec_set_phandle(void *blob, int node, uint32_t phandle) -{ - fdt32_t value = cpu_to_fdt32(phandle); - - return fdt_setprop(blob, node, "phandle", &value, sizeof(value)); -} - static int fdtdec_init_reserved_memory(void *blob) { int na, ns, node, err; From 8781d04f422e110fef864dd849085054fe5b0e65 Mon Sep 17 00:00:00 2001 From: Ramon Fried Date: Sat, 6 Apr 2019 05:12:01 +0300 Subject: [PATCH 23/23] pci: pci.h: add missing maskbit PCI_MSI_FLAGS_MASKBIT was missing from include file, add it. Signed-off-by: Ramon Fried Reviewed-by: Simon Glass --- include/pci.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/pci.h b/include/pci.h index 5fb212cab1..508f7bca81 100644 --- a/include/pci.h +++ b/include/pci.h @@ -405,6 +405,7 @@ #define PCI_MSI_FLAGS_QSIZE 0x70 /* Message queue size configured */ #define PCI_MSI_FLAGS_QMASK 0x0e /* Maximum queue size available */ #define PCI_MSI_FLAGS_ENABLE 0x01 /* MSI feature enabled */ +#define PCI_MSI_FLAGS_MASKBIT 0x0100 /* Per-vector masking capable */ #define PCI_MSI_RFU 3 /* Rest of capability flags */ #define PCI_MSI_ADDRESS_LO 4 /* Lower 32 bits */ #define PCI_MSI_ADDRESS_HI 8 /* Upper 32 bits (if PCI_MSI_FLAGS_64BIT set) */