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 <init_sequence_f>) 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 <stdio_add_devices> (gdb)

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Adjust code to allow it to work before relocation also - Update commit message to take account of initcall.h being in a header - Add documentation to README.sandbox

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 a8d01e40011..c20491493fc 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 2f5e6e95182..e22d65f6d9e 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 f6a6a343d25..f4ce72d5660 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 <asm-generic/global_data.h> diff --git a/board/sandbox/README.sandbox b/board/sandbox/README.sandbox index 9b094042943..ed8fac6f786 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 <init_sequence_f>) + 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 <stdio_add_devices> + (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 149a7229e8f..7ef20f20423 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 3ac01aa2cd2..a38c83efa44 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 6f33b08cf0b..7a4f78b9b1f 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

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 sjg@chromium.org ---

Changes in v2: - Add a new patch to Correct maths in allocation routines

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 c20491493fc..47dfb476d37 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; }

This was really intended for debugging. Drop it.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Add a new patch to drop the printf() in setup_ram_buf()

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 e22d65f6d9e..82828f0c1d4 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;

These two buffers currently conflict if tracing is enabled. Move the pre-console buffer and update the documentation.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Add new patch to move pre-console buffer out of the way of tracing

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 ed8fac6f786..48c1e2b9e7b 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 193e41896cb..e496d9e05c1 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

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 sjg@chromium.org ---

Changes in v2: - Add new patch to allow calling bootstage_mark() before bootstage_init()

common/bootstage.c | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/common/bootstage.c b/common/bootstage.c index 9793b85d4e4..56ef91ad859 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++;

This buffer is too small now that sandbox has grown in size. Increase it.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Add new patch to increase the early-trace-buffer size for sandbox

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 e36a5fec0ef..6dd03e31cfa 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

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 sjg@chromium.org ---

Changes in v2: - Add new patch to drop use of header files in initcall.h

include/initcall.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/initcall.h b/include/initcall.h index a38c83efa44..78d15afe69b 100644 --- a/include/initcall.h +++ b/include/initcall.h @@ -8,12 +8,11 @@

typedef int (*init_fnc_t)(void);

-#include <common.h> -#include <initcall.h> -#include <efi.h> - -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;

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 sjg@chromium.org ---

Changes in v2: - Add new patch to use kernel types in div64

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 76563ef9786..8b92d2b1834 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 206f582ca96..7abc68c3332 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) {

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 sjg@chromium.org ---

Changes in v2: - Add new patch to avoid instrumenting the division function

lib/div64.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/div64.c b/lib/div64.c index 7abc68c3332..62933c92c4f 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;

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 sjg@chromium.org ---

Changes in v2: - Add new patch to tidy up error returns in trace.c

lib/trace.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/trace.c b/lib/trace.c index bb089c2eca9..fb7658b1127 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

This converts the following to Kconfig: CONFIG_TRACE

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

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 0b07b3b9d77..bd1c5afc7e7 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1888,7 +1888,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 366d164cd76..6909756e307 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 8c7c1592a56..3c99b77be49 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -4406,7 +4406,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

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 sjg@chromium.org ---

Changes in v2: None

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 6909756e307..cc38024a29f 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 fb7658b1127..9956442fefe 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 3c99b77be49..980352d8286 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -4406,10 +4406,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

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 sjg@chromium.org ---

Changes in v2: - Add new patch to enable the 'trace' command on sandbox when needed

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 6dd03e31cfa..bf03baefe83 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