[U-Boot] [PATCH 00/15] bootstage: Update to support early malloc() and SPL

The bootstage feature is useful for timing booting of a board, both for debugging and for production. On ARM devices, detailed timing information can be passed to Linux (where for example it could be logged or reported to a connected server).
Bootstage was written before early malloc(). At present bootstage uses the data section and a large block of memory, which is not ideal. Also it does not support SPL.
This series revamps bootstage. It allows a full timing trace to be collected from SPL through to booting Linux, for example:
Mark Elapsed Stage 0 0 reset 2,809,110 2,809,110 spl 2,824,791 15,681 end_spl 2,879,616 54,825 board_init_f 3,180,834 301,218 board_init_r 3,496,057 315,223 id=64 3,497,957 1,900 id=64 3,497,959 2 main_loop
Accumulated time: 723 dm_r 3,763 of_live 8,027 dm_spl 120,731 dm_f 168,207 lcd
Simon Glass (15): bootstage: Provide a default timer function bootstage: Require timer_get_boot_us() to be defined bootstage: Change CONFIG_BOOTSTAGE_USER_COUNT to an int bootstage: Convert to use malloc() bootstage: Fix up code style and comments bootstage: Use rec_count as the array index bootstage: Show records with a zero time bootstage: Use debug() for stashing messages bootstage: Support relocating boostage data bootstage: Init as early as possible bootstage: Record the time taken to set up driver model bootstage: Tidy up error return values bootstage: Adjust to use const * where possible bootstage: Support SPL bootstage: Record time taken to set up the live device tree
arch/sandbox/cpu/cpu.c | 11 ++ common/Kconfig | 18 ++- common/Makefile | 3 +- common/board_f.c | 61 ++++++++- common/board_r.c | 16 ++- common/bootstage.c | 257 ++++++++++++++++++++++--------------- common/spl/spl.c | 18 +++ configs/sandbox_defconfig | 2 +- configs/sandbox_flattree_defconfig | 2 +- configs/sandbox_noblk_defconfig | 2 +- configs/sandbox_spl_defconfig | 2 +- include/asm-generic/global_data.h | 4 + include/bootstage.h | 62 +++++++-- lib/time.c | 17 +++ 14 files changed, 349 insertions(+), 126 deletions(-)

If CONFIG_SYS_TIMER_COUNTER is used we can provide a default microsecond timer implementation.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/time.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/lib/time.c b/lib/time.c index 3c49243e6a..aed1a091f2 100644 --- a/lib/time.c +++ b/lib/time.c @@ -36,6 +36,23 @@ unsigned long notrace timer_read_counter(void) return readl(CONFIG_SYS_TIMER_COUNTER); #endif } + +ulong timer_get_boot_us(void) +{ + ulong count = timer_read_counter(); + +#if CONFIG_SYS_TIMER_RATE == 1000000 + return count; +#elif CONFIG_SYS_TIMER_RATE > 1000000 + return lldiv(count, CONFIG_SYS_TIMER_RATE / 1000000); +#elif defined(CONFIG_SYS_TIMER_RATE) + return (unsigned long long)count * 1000000 / CONFIG_SYS_TIMER_RATE; +#else + /* Assume the counter is in microseconds */ + return count; +#endif +} + #else extern unsigned long __weak timer_read_counter(void); #endif

On Mon, May 22, 2017 at 05:05:22AM -0600, Simon Glass wrote:
If CONFIG_SYS_TIMER_COUNTER is used we can provide a default microsecond timer implementation.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

At present we provide a default version of this function for use by bootstage. However it uses the system timer and therefore likely requires driver model. This makes it impossible to time driver-model init.
Drop the function and require boards to provide their own. Add a sandbox version also. There is a default implememtation in lib/time.c for boards which use CONFIG_SYS_TIMER_COUNTER.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/cpu.c | 11 +++++++++++ common/bootstage.c | 19 +------------------ include/bootstage.h | 3 ++- 3 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 3fe99b853d..eefed2ebd9 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -139,3 +139,14 @@ done:
return 0; } + +ulong timer_get_boot_us(void) +{ + static uint64_t base_count; + uint64_t count = os_get_nsec(); + + if (!base_count) + base_count = count; + + return (count - base_count) / 1000; +} diff --git a/common/bootstage.c b/common/bootstage.c index 35bce3d881..bcfbda99e5 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -8,7 +8,7 @@ /* * This module records the progress of boot and arbitrary commands, and * permits accurate timestamping of each. - * + * * TBD: Pass timings to kernel in the FDT */
@@ -292,23 +292,6 @@ void bootstage_report(void) } }
-ulong __timer_get_boot_us(void) -{ - static ulong base_time; - - /* - * We can't implement this properly. Return 0 on the first call and - * larger values after that. - */ - if (base_time) - return get_timer(base_time) * 1000; - base_time = get_timer(0); - return 0; -} - -ulong timer_get_boot_us(void) - __attribute__((weak, alias("__timer_get_boot_us"))); - /** * Append data to a memory buffer * diff --git a/include/bootstage.h b/include/bootstage.h index a589be6316..6ee923c2eb 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -2,6 +2,7 @@ * This file implements recording of each stage of the boot process. It is * intended to implement timing of each stage, reporting this information * to the user and passing it to the OS for logging / further analysis. + * Note that it requires timer_get_boot_us() to be defined by the board * * Copyright (c) 2011 The Chromium OS Authors. * @@ -209,7 +210,7 @@ enum bootstage_id { /* * Return the time since boot in microseconds, This is needed for bootstage * and should be defined in CPU- or board-specific code. If undefined then - * millisecond resolution will be used (the standard get_timer()). + * you will get a link error. */ ulong timer_get_boot_us(void);

On Mon, May 22, 2017 at 05:05:23AM -0600, Simon Glass wrote:
At present we provide a default version of this function for use by bootstage. However it uses the system timer and therefore likely requires driver model. This makes it impossible to time driver-model init.
Drop the function and require boards to provide their own. Add a sandbox version also. There is a default implememtation in lib/time.c for boards which use CONFIG_SYS_TIMER_COUNTER.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

There is no good read to make this hex, and integer is more natural for this type of setting. Update it.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/Kconfig | 2 +- configs/sandbox_defconfig | 2 +- configs/sandbox_flattree_defconfig | 2 +- configs/sandbox_noblk_defconfig | 2 +- configs/sandbox_spl_defconfig | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index 1879aefaf8..fe9f0d2184 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -38,7 +38,7 @@ config BOOTSTAGE_REPORT 30,361,327 445,160 start_kernel
config BOOTSTAGE_USER_COUNT - hex "Number of boot ID numbers available for user use" + int "Number of boot ID numbers available for user use" default 20 help This is the number of available user bootstage records. diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index a7656a3ff1..4247a001fa 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -7,7 +7,7 @@ CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=0x20 +CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 83efb23dbc..4fe2318bfd 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -7,7 +7,7 @@ CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=0x20 +CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_noblk_defconfig b/configs/sandbox_noblk_defconfig index 6c6e6596b3..859797590a 100644 --- a/configs/sandbox_noblk_defconfig +++ b/configs/sandbox_noblk_defconfig @@ -6,7 +6,7 @@ CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=0x20 +CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 3061e5a9d9..d3b1a6454e 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -12,7 +12,7 @@ CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=0x20 +CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0

On Mon, May 22, 2017 at 05:05:24AM -0600, Simon Glass wrote:
There is no good read to make this hex, and integer is more natural for this type of setting. Update it.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

At present bootstage uses the data section of the image to store its information. There are a few problems with this:
- It does not work on all boards (e.g. those which run from flash before relocation) - Allocated strings still point back to the pre-relocation data after relocation
Now that U-Boot has a pre-relocation malloc() we can use this instead, with a pointer to the data in global_data. Update bootstage to do this and set up an init routine to allocate the memory.
Now that we have a real init function, we can drop the fake 'reset' record and add a normal one instead.
Note that part of the problem with allocated strings remains. They are reallocated but this will only work where pre-relocation memory is accessible after relocation.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 10 ++++- common/bootstage.c | 86 +++++++++++++++++++++++++-------------- include/asm-generic/global_data.h | 3 ++ include/bootstage.h | 13 ++++++ 4 files changed, 79 insertions(+), 33 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index a212f2b539..a4f8627ddf 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -670,8 +670,14 @@ static int jump_to_copy(void) #endif
/* Record the board_init_f() bootstage (after arch_cpu_init()) */ -static int mark_bootstage(void) +static int initf_bootstage(void) { + int ret; + + ret = bootstage_init(true); + if (ret) + return ret; + bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
return 0; @@ -732,7 +738,7 @@ static const init_fnc_t init_sequence_f[] = { mach_cpu_init, /* SoC/machine dependent CPU setup */ initf_dm, arch_cpu_init_dm, - mark_bootstage, /* need timer, go after init dm */ + initf_bootstage, /* need timer, go after init dm */ #if defined(CONFIG_BOARD_EARLY_INIT_F) board_early_init_f, #endif diff --git a/common/bootstage.c b/common/bootstage.c index bcfbda99e5..5026705a86 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -27,8 +27,10 @@ struct bootstage_record { enum bootstage_id id; };
-static struct bootstage_record record[BOOTSTAGE_ID_COUNT] = { {1} }; -static int next_id = BOOTSTAGE_ID_USER; +struct bootstage_data { + uint next_id; + struct bootstage_record record[BOOTSTAGE_ID_COUNT]; +};
enum { BOOTSTAGE_VERSION = 0, @@ -45,6 +47,7 @@ struct bootstage_hdr {
int bootstage_relocate(void) { + struct bootstage_data *data = gd->bootstage; int i;
/* @@ -52,8 +55,8 @@ int bootstage_relocate(void) * program .text section that can eventually get trashed. */ for (i = 0; i < BOOTSTAGE_ID_COUNT; i++) - if (record[i].name) - record[i].name = strdup(record[i].name); + if (data->record[i].name) + data->record[i].name = strdup(data->record[i].name);
return 0; } @@ -61,13 +64,14 @@ int bootstage_relocate(void) ulong bootstage_add_record(enum bootstage_id id, const char *name, int flags, ulong mark) { + struct bootstage_data *data = gd->bootstage; struct bootstage_record *rec;
if (flags & BOOTSTAGEF_ALLOC) - id = next_id++; + id = data->next_id++;
if (id < BOOTSTAGE_ID_COUNT) { - rec = &record[id]; + rec = &data->record[id];
/* Only record the first event for each */ if (!rec->time_us) { @@ -133,7 +137,8 @@ ulong bootstage_mark_code(const char *file, const char *func, int linenum)
uint32_t bootstage_start(enum bootstage_id id, const char *name) { - struct bootstage_record *rec = &record[id]; + struct bootstage_data *data = gd->bootstage; + struct bootstage_record *rec = &data->record[id];
rec->start_us = timer_get_boot_us(); rec->name = name; @@ -142,7 +147,8 @@ uint32_t bootstage_start(enum bootstage_id id, const char *name)
uint32_t bootstage_accum(enum bootstage_id id) { - struct bootstage_record *rec = &record[id]; + struct bootstage_data *data = gd->bootstage; + struct bootstage_record *rec = &data->record[id]; uint32_t duration;
duration = (uint32_t)timer_get_boot_us() - rec->start_us; @@ -171,8 +177,7 @@ static const char *get_record_name(char *buf, int len, return buf; }
-static uint32_t print_time_record(enum bootstage_id id, - struct bootstage_record *rec, uint32_t prev) +static uint32_t print_time_record(struct bootstage_record *rec, uint32_t prev) { char buf[20];
@@ -204,6 +209,7 @@ static int h_compare_record(const void *r1, const void *r2) */ static int add_bootstages_devicetree(struct fdt_header *blob) { + struct bootstage_data *data = gd->bootstage; int bootstage; char buf[20]; int id; @@ -225,7 +231,7 @@ static int add_bootstages_devicetree(struct fdt_header *blob) * that they can be printed in the Linux kernel in the right order. */ for (id = BOOTSTAGE_ID_COUNT - 1, i = 0; id >= 0; id--, i++) { - struct bootstage_record *rec = &record[id]; + struct bootstage_record *rec = &data->record[id]; int node;
if (id != BOOTSTAGE_ID_AWAKE && rec->time_us == 0) @@ -261,34 +267,33 @@ int bootstage_fdt_add_report(void)
void bootstage_report(void) { - struct bootstage_record *rec = record; + struct bootstage_data *data = gd->bootstage; + struct bootstage_record *rec = data->record; int id; uint32_t prev;
puts("Timer summary in microseconds:\n"); printf("%11s%11s %s\n", "Mark", "Elapsed", "Stage");
- /* Fake the first record - we could get it from early boot */ - rec->name = "reset"; - rec->time_us = 0; - prev = print_time_record(BOOTSTAGE_ID_AWAKE, rec, 0); + prev = print_time_record(rec, 0);
/* Sort records by increasing time */ - qsort(record, ARRAY_SIZE(record), sizeof(*rec), h_compare_record); + qsort(data->record, ARRAY_SIZE(data->record), sizeof(*rec), + h_compare_record);
for (id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { if (rec->time_us != 0 && !rec->start_us) - prev = print_time_record(rec->id, rec, prev); + prev = print_time_record(rec, prev); } - if (next_id > BOOTSTAGE_ID_COUNT) + if (data->next_id > BOOTSTAGE_ID_COUNT) printf("(Overflowed internal boot id table by %d entries\n" "- please increase CONFIG_BOOTSTAGE_USER_COUNT\n", - next_id - BOOTSTAGE_ID_COUNT); + data->next_id - BOOTSTAGE_ID_COUNT);
puts("\nAccumulated time:\n"); - for (id = 0, rec = record; id < BOOTSTAGE_ID_COUNT; id++, rec++) { + for (id = 0, rec = data->record; id < BOOTSTAGE_ID_COUNT; id++, rec++) { if (rec->start_us) - prev = print_time_record(id, rec, -1); + prev = print_time_record(rec, -1); } }
@@ -316,6 +321,7 @@ static void append_data(char **ptrp, char *end, const void *data, int size)
int bootstage_stash(void *base, int size) { + struct bootstage_data *data = gd->bootstage; struct bootstage_hdr *hdr = (struct bootstage_hdr *)base; struct bootstage_record *rec; char buf[20]; @@ -332,7 +338,7 @@ int bootstage_stash(void *base, int size) hdr->version = BOOTSTAGE_VERSION;
/* Count the number of records, and write that value first */ - for (rec = record, id = count = 0; id < BOOTSTAGE_ID_COUNT; + for (rec = data->record, id = count = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { if (rec->time_us != 0) count++; @@ -343,13 +349,13 @@ int bootstage_stash(void *base, int size) ptr += sizeof(*hdr);
/* Write the records, silently stopping when we run out of space */ - for (rec = record, id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { + for (rec = data->record, id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { if (rec->time_us != 0) append_data(&ptr, end, rec, sizeof(*rec)); }
/* Write the name strings */ - for (rec = record, id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { + for (rec = data->record, id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { if (rec->time_us != 0) { const char *name;
@@ -373,6 +379,7 @@ int bootstage_stash(void *base, int size)
int bootstage_unstash(void *base, int size) { + struct bootstage_data *data = gd->bootstage; struct bootstage_hdr *hdr = (struct bootstage_hdr *)base; struct bootstage_record *rec; char *ptr = base, *end = ptr + size; @@ -410,22 +417,23 @@ int bootstage_unstash(void *base, int size) return -1; }
- if (next_id + hdr->count > BOOTSTAGE_ID_COUNT) { + if (data->next_id + hdr->count > BOOTSTAGE_ID_COUNT) { debug("%s: Bootstage has %d records, we have space for %d\n" "- please increase CONFIG_BOOTSTAGE_USER_COUNT\n", - __func__, hdr->count, BOOTSTAGE_ID_COUNT - next_id); + __func__, hdr->count, BOOTSTAGE_ID_COUNT - data->next_id); return -1; }
ptr += sizeof(*hdr);
/* Read the records */ - rec_size = hdr->count * sizeof(*record); - memcpy(record + next_id, ptr, rec_size); + rec_size = hdr->count * sizeof(*data->record); + memcpy(data->record + data->next_id, ptr, rec_size);
/* Read the name strings */ ptr += rec_size; - for (rec = record + next_id, id = 0; id < hdr->count; id++, rec++) { + for (rec = data->record + data->next_id, id = 0; id < hdr->count; + id++, rec++) { rec->name = ptr;
/* Assume no data corruption here */ @@ -433,8 +441,24 @@ int bootstage_unstash(void *base, int size) }
/* Mark the records as read */ - next_id += hdr->count; + data->next_id += hdr->count; printf("Unstashed %d records\n", hdr->count);
return 0; } + +int bootstage_init(bool first) +{ + struct bootstage_data *data; + int size = sizeof(struct bootstage_data); + + gd->bootstage = (struct bootstage_data *)malloc(size); + if (!gd->bootstage) + return -ENOMEM; + data = gd->bootstage; + memset(data, '\0', size); + if (first) + bootstage_add_record(BOOTSTAGE_ID_AWAKE, "reset", 0, 0); + + return 0; +} diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index e6f905110e..8b3229e5b8 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -110,6 +110,9 @@ typedef struct global_data { ulong video_top; /* Top of video frame buffer area */ ulong video_bottom; /* Bottom of video frame buffer area */ #endif +#ifdef CONFIG_BOOTSTAGE + struct bootstage_data *bootstage; /* Bootstage information */ +#endif } gd_t; #endif
diff --git a/include/bootstage.h b/include/bootstage.h index 6ee923c2eb..e1aec1b549 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -331,6 +331,14 @@ int bootstage_stash(void *base, int size); */ int bootstage_unstash(void *base, int size);
+/** + * bootstage_init() - Prepare bootstage for use + * + * @first: true if this is the first time bootstage is set up. This causes it + * to add a 'reset' record with a time of 0. + */ +int bootstage_init(bool first); + #else static inline ulong bootstage_add_record(enum bootstage_id id, const char *name, int flags, ulong mark) @@ -391,6 +399,11 @@ static inline int bootstage_unstash(void *base, int size) { return 0; /* Pretend to succeed */ } + +static inline int bootstage_init(bool first) +{ + return 0; +} #endif /* CONFIG_BOOTSTAGE */
/* Helper macro for adding a bootstage to a line of code */

On Mon, May 22, 2017 at 05:05:25AM -0600, Simon Glass wrote:
At present bootstage uses the data section of the image to store its information. There are a few problems with this:
- It does not work on all boards (e.g. those which run from flash before
relocation)
- Allocated strings still point back to the pre-relocation data after
relocation
Now that U-Boot has a pre-relocation malloc() we can use this instead, with a pointer to the data in global_data. Update bootstage to do this and set up an init routine to allocate the memory.
Now that we have a real init function, we can drop the fake 'reset' record and add a normal one instead.
Note that part of the problem with allocated strings remains. They are reallocated but this will only work where pre-relocation memory is accessible after relocation.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

There are several code style and comment nits. Fix them and also remove the comment about passing bootstage to the kernel being TBD. This is already supported.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/bootstage.c | 6 ++++-- include/bootstage.h | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/common/bootstage.c b/common/bootstage.c index 5026705a86..a959026946 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -8,8 +8,6 @@ /* * This module records the progress of boot and arbitrary commands, and * permits accurate timestamping of each. - * - * TBD: Pass timings to kernel in the FDT */
#include <common.h> @@ -84,6 +82,7 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name,
/* Tell the board about this progress */ show_boot_progress(flags & BOOTSTAGEF_ERROR ? -id : id); + return mark; }
@@ -105,6 +104,7 @@ ulong bootstage_mark_name(enum bootstage_id id, const char *name)
if (id == BOOTSTAGE_ID_ALLOC) flags = BOOTSTAGEF_ALLOC; + return bootstage_add_record(id, name, flags, timer_get_boot_us()); }
@@ -142,6 +142,7 @@ uint32_t bootstage_start(enum bootstage_id id, const char *name)
rec->start_us = timer_get_boot_us(); rec->name = name; + return rec->start_us; }
@@ -153,6 +154,7 @@ uint32_t bootstage_accum(enum bootstage_id id)
duration = (uint32_t)timer_get_boot_us() - rec->start_us; rec->time_us += duration; + return duration; }
diff --git a/include/bootstage.h b/include/bootstage.h index e1aec1b549..41bd61785c 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -217,7 +217,7 @@ ulong timer_get_boot_us(void); #if defined(USE_HOSTCC) #define show_boot_progress(val) do {} while (0) #else -/* +/** * Board code can implement show_boot_progress() if needed. * * @param val Progress state (enum bootstage_id), or -id if an error @@ -235,7 +235,7 @@ void show_boot_progress(int val); * * Call this after relocation has happened and after malloc has been initted. * We need to copy any pointers in bootstage records that were added pre- - * relocation, since memory can be overritten later. + * relocation, since memory can be overwritten later. * @return Always returns 0, to indicate success */ int bootstage_relocate(void); @@ -251,7 +251,7 @@ int bootstage_relocate(void); ulong bootstage_add_record(enum bootstage_id id, const char *name, int flags, ulong mark);
-/* +/** * Mark a time stamp for the current boot stage. */ ulong bootstage_mark(enum bootstage_id id); @@ -310,7 +310,7 @@ void bootstage_report(void); */ int bootstage_fdt_add_report(void);
-/* +/** * Stash bootstage data into memory * * @param base Base address of memory buffer

On Mon, May 22, 2017 at 05:05:26AM -0600, Simon Glass wrote:
There are several code style and comment nits. Fix them and also remove the comment about passing bootstage to the kernel being TBD. This is already supported.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

At present bootstage has a large array with all possible bootstage IDs recorded. It adds times to the array element indexed by the ID. This is inefficient because many IDs are not used during boot. We can save space by only recording those IDs which actually have timestamps.
Update the array to use a record count, which increments with each addition of a new timestamp. This takes longer to record a time, since it may involve an array search. Such a search may be particularly expensive before relocation when the CPU is running slowly or the cache is off. But at that stage there should be very few records.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/Kconfig | 7 +++ common/bootstage.c | 138 ++++++++++++++++++++++++++++++++++------------------- 2 files changed, 96 insertions(+), 49 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index fe9f0d2184..83ef11ec35 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -46,6 +46,13 @@ config BOOTSTAGE_USER_COUNT a new ID will be allocated from this stash. If you exceed the limit, recording will stop.
+config BOOTSTAGE_RECORD_COUNT + int "Number of boot stage records to store" + default 30 + help + This is the size of the bootstage record list and is the maximum + number of bootstage records that can be recorded. + config BOOTSTAGE_FDT bool "Store boot timing information in the OS device tree" depends on BOOTSTAGE diff --git a/common/bootstage.c b/common/bootstage.c index a959026946..8cd2fb5e1b 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -17,6 +17,10 @@
DECLARE_GLOBAL_DATA_PTR;
+enum { + RECORD_COUNT = CONFIG_BOOTSTAGE_RECORD_COUNT, +}; + struct bootstage_record { ulong time_us; uint32_t start_us; @@ -26,8 +30,9 @@ struct bootstage_record { };
struct bootstage_data { + uint rec_count; uint next_id; - struct bootstage_record record[BOOTSTAGE_ID_COUNT]; + struct bootstage_record record[RECORD_COUNT]; };
enum { @@ -52,13 +57,43 @@ int bootstage_relocate(void) * Duplicate all strings. They may point to an old location in the * program .text section that can eventually get trashed. */ - for (i = 0; i < BOOTSTAGE_ID_COUNT; i++) - if (data->record[i].name) - data->record[i].name = strdup(data->record[i].name); + debug("Relocating %d records\n", data->rec_count); + for (i = 0; i < data->rec_count; i++) + data->record[i].name = strdup(data->record[i].name);
return 0; }
+struct bootstage_record *find_id(struct bootstage_data *data, + enum bootstage_id id) +{ + struct bootstage_record *rec; + struct bootstage_record *end; + + for (rec = data->record, end = rec + data->rec_count; rec < end; + rec++) { + if (rec->id == id) + return rec; + } + + return NULL; +} + +struct bootstage_record *ensure_id(struct bootstage_data *data, + enum bootstage_id id) +{ + struct bootstage_record *rec; + + rec = find_id(data, id); + if (!rec && data->rec_count < RECORD_COUNT) { + rec = &data->record[data->rec_count++]; + rec->id = id; + return rec; + } + + return rec; +} + ulong bootstage_add_record(enum bootstage_id id, const char *name, int flags, ulong mark) { @@ -68,16 +103,14 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name, if (flags & BOOTSTAGEF_ALLOC) id = data->next_id++;
- if (id < BOOTSTAGE_ID_COUNT) { - rec = &data->record[id]; - - /* Only record the first event for each */ - if (!rec->time_us) { - rec->time_us = mark; - rec->name = name; - rec->flags = flags; - rec->id = id; - } + /* Only record the first event for each */ + rec = find_id(data, id); + if (!rec && data->rec_count < RECORD_COUNT) { + rec = &data->record[data->rec_count++]; + rec->time_us = mark; + rec->name = name; + rec->flags = flags; + rec->id = id; }
/* Tell the board about this progress */ @@ -138,20 +171,25 @@ ulong bootstage_mark_code(const char *file, const char *func, int linenum) uint32_t bootstage_start(enum bootstage_id id, const char *name) { struct bootstage_data *data = gd->bootstage; - struct bootstage_record *rec = &data->record[id]; + struct bootstage_record *rec = ensure_id(data, id); + ulong start_us = timer_get_boot_us();
- rec->start_us = timer_get_boot_us(); - rec->name = name; + if (rec) { + rec->start_us = start_us; + rec->name = name; + }
- return rec->start_us; + return start_us; }
uint32_t bootstage_accum(enum bootstage_id id) { struct bootstage_data *data = gd->bootstage; - struct bootstage_record *rec = &data->record[id]; + struct bootstage_record *rec = ensure_id(data, id); uint32_t duration;
+ if (!rec) + return 0; duration = (uint32_t)timer_get_boot_us() - rec->start_us; rec->time_us += duration;
@@ -214,7 +252,7 @@ static int add_bootstages_devicetree(struct fdt_header *blob) struct bootstage_data *data = gd->bootstage; int bootstage; char buf[20]; - int id; + int recnum; int i;
if (!blob) @@ -232,11 +270,11 @@ static int add_bootstages_devicetree(struct fdt_header *blob) * Insert the timings to the device tree in the reverse order so * that they can be printed in the Linux kernel in the right order. */ - for (id = BOOTSTAGE_ID_COUNT - 1, i = 0; id >= 0; id--, i++) { - struct bootstage_record *rec = &data->record[id]; + for (recnum = data->rec_count - 1, i = 0; recnum >= 0; recnum--, i++) { + struct bootstage_record *rec = &data->record[recnum]; int node;
- if (id != BOOTSTAGE_ID_AWAKE && rec->time_us == 0) + if (rec->id != BOOTSTAGE_ID_AWAKE && rec->time_us == 0) continue;
node = fdt_add_subnode(blob, bootstage, simple_itoa(i)); @@ -245,7 +283,7 @@ static int add_bootstages_devicetree(struct fdt_header *blob)
/* add properties to the node. */ if (fdt_setprop_string(blob, node, "name", - get_record_name(buf, sizeof(buf), rec))) + get_record_name(buf, sizeof(buf), rec))) return -1;
/* Check if this is a 'mark' or 'accum' record */ @@ -271,29 +309,29 @@ void bootstage_report(void) { struct bootstage_data *data = gd->bootstage; struct bootstage_record *rec = data->record; - int id; uint32_t prev; + int i;
- puts("Timer summary in microseconds:\n"); + printf("Timer summary in microseconds (%d records):\n", + data->rec_count); printf("%11s%11s %s\n", "Mark", "Elapsed", "Stage");
prev = print_time_record(rec, 0);
/* Sort records by increasing time */ - qsort(data->record, ARRAY_SIZE(data->record), sizeof(*rec), - h_compare_record); + qsort(data->record, data->rec_count, sizeof(*rec), h_compare_record);
- for (id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { - if (rec->time_us != 0 && !rec->start_us) + for (i = 1, rec++; i < data->rec_count; i++, rec++) { + if (rec->id && !rec->start_us) prev = print_time_record(rec, prev); } - if (data->next_id > BOOTSTAGE_ID_COUNT) - printf("(Overflowed internal boot id table by %d entries\n" - "- please increase CONFIG_BOOTSTAGE_USER_COUNT\n", - data->next_id - BOOTSTAGE_ID_COUNT); + if (data->rec_count > RECORD_COUNT) + printf("Overflowed internal boot id table by %d entries\n" + "- please increase CONFIG_BOOTSTAGE_RECORD_COUNT\n", + data->rec_count - RECORD_COUNT);
puts("\nAccumulated time:\n"); - for (id = 0, rec = data->record; id < BOOTSTAGE_ID_COUNT; id++, rec++) { + for (i = 0, rec = data->record; i < data->rec_count; i++, rec++) { if (rec->start_us) prev = print_time_record(rec, -1); } @@ -329,7 +367,7 @@ int bootstage_stash(void *base, int size) char buf[20]; char *ptr = base, *end = ptr + size; uint32_t count; - int id; + int i;
if (hdr + 1 > (struct bootstage_hdr *)end) { debug("%s: Not enough space for bootstage hdr\n", __func__); @@ -340,9 +378,9 @@ int bootstage_stash(void *base, int size) hdr->version = BOOTSTAGE_VERSION;
/* Count the number of records, and write that value first */ - for (rec = data->record, id = count = 0; id < BOOTSTAGE_ID_COUNT; - id++, rec++) { - if (rec->time_us != 0) + for (rec = data->record, i = count = 0; i < data->rec_count; + i++, rec++) { + if (rec->id != 0) count++; } hdr->count = count; @@ -351,13 +389,13 @@ int bootstage_stash(void *base, int size) ptr += sizeof(*hdr);
/* Write the records, silently stopping when we run out of space */ - for (rec = data->record, id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { + for (rec = data->record, i = 0; i < data->rec_count; i++, rec++) { if (rec->time_us != 0) append_data(&ptr, end, rec, sizeof(*rec)); }
/* Write the name strings */ - for (rec = data->record, id = 0; id < BOOTSTAGE_ID_COUNT; id++, rec++) { + for (rec = data->record, i = 0; i < data->rec_count; i++, rec++) { if (rec->time_us != 0) { const char *name;
@@ -386,7 +424,7 @@ int bootstage_unstash(void *base, int size) struct bootstage_record *rec; char *ptr = base, *end = ptr + size; uint rec_size; - int id; + int i;
if (size == -1) end = (char *)(~(uintptr_t)0); @@ -419,10 +457,10 @@ int bootstage_unstash(void *base, int size) return -1; }
- if (data->next_id + hdr->count > BOOTSTAGE_ID_COUNT) { + if (data->rec_count + hdr->count > RECORD_COUNT) { debug("%s: Bootstage has %d records, we have space for %d\n" "- please increase CONFIG_BOOTSTAGE_USER_COUNT\n", - __func__, hdr->count, BOOTSTAGE_ID_COUNT - data->next_id); + __func__, hdr->count, RECORD_COUNT - data->rec_count); return -1; }
@@ -430,12 +468,12 @@ int bootstage_unstash(void *base, int size)
/* Read the records */ rec_size = hdr->count * sizeof(*data->record); - memcpy(data->record + data->next_id, ptr, rec_size); + memcpy(data->record + data->rec_count, ptr, rec_size);
/* Read the name strings */ ptr += rec_size; - for (rec = data->record + data->next_id, id = 0; id < hdr->count; - id++, rec++) { + for (rec = data->record + data->next_id, i = 0; i < hdr->count; + i++, rec++) { rec->name = ptr;
/* Assume no data corruption here */ @@ -443,7 +481,7 @@ int bootstage_unstash(void *base, int size) }
/* Mark the records as read */ - data->next_id += hdr->count; + data->rec_count += hdr->count; printf("Unstashed %d records\n", hdr->count);
return 0; @@ -459,8 +497,10 @@ int bootstage_init(bool first) return -ENOMEM; data = gd->bootstage; memset(data, '\0', size); - if (first) + if (first) { + data->next_id = BOOTSTAGE_ID_USER; bootstage_add_record(BOOTSTAGE_ID_AWAKE, "reset", 0, 0); + }
return 0; }

On Mon, May 22, 2017 at 05:05:27AM -0600, Simon Glass wrote:
At present bootstage has a large array with all possible bootstage IDs recorded. It adds times to the array element indexed by the ID. This is inefficient because many IDs are not used during boot. We can save space by only recording those IDs which actually have timestamps.
Update the array to use a record count, which increments with each addition of a new timestamp. This takes longer to record a time, since it may involve an array search. Such a search may be particularly expensive before relocation when the CPU is running slowly or the cache is off. But at that stage there should be very few records.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

We can now use the record count to determine whether a record is valid or not. Drop the test for a zero time.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/bootstage.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/common/bootstage.c b/common/bootstage.c index 8cd2fb5e1b..1afdee3018 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -390,18 +390,15 @@ int bootstage_stash(void *base, int size)
/* Write the records, silently stopping when we run out of space */ for (rec = data->record, i = 0; i < data->rec_count; i++, rec++) { - if (rec->time_us != 0) - append_data(&ptr, end, rec, sizeof(*rec)); + append_data(&ptr, end, rec, sizeof(*rec)); }
/* Write the name strings */ for (rec = data->record, i = 0; i < data->rec_count; i++, rec++) { - if (rec->time_us != 0) { - const char *name; + const char *name;
- name = get_record_name(buf, sizeof(buf), rec); - append_data(&ptr, end, name, strlen(name) + 1); - } + name = get_record_name(buf, sizeof(buf), rec); + append_data(&ptr, end, name, strlen(name) + 1); }
/* Check for buffer overflow */

On Mon, May 22, 2017 at 05:05:28AM -0600, Simon Glass wrote:
We can now use the record count to determine whether a record is valid or not. Drop the test for a zero time.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

We don't normally want to see these messages. Change them to debug-only.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/bootstage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/bootstage.c b/common/bootstage.c index 1afdee3018..32f4e8b706 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -409,7 +409,7 @@ int bootstage_stash(void *base, int size)
/* Update total data size */ hdr->size = ptr - (char *)base; - printf("Stashed %d records\n", hdr->count); + debug("Stashed %d records\n", hdr->count);
return 0; } @@ -479,7 +479,7 @@ int bootstage_unstash(void *base, int size)
/* Mark the records as read */ data->rec_count += hdr->count; - printf("Unstashed %d records\n", hdr->count); + debug("Unstashed %d records\n", hdr->count);
return 0; }

On Mon, May 22, 2017 at 05:05:29AM -0600, Simon Glass wrote:
We don't normally want to see these messages. Change them to debug-only.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Some boards cannot access pre-relocation data after relocation. Reserve space for this and copy it during preparation for relocation.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 34 ++++++++++++++++++++++++++++++++++ common/bootstage.c | 5 +++++ include/asm-generic/global_data.h | 1 + include/bootstage.h | 12 ++++++++++++ 4 files changed, 52 insertions(+)
diff --git a/common/board_f.c b/common/board_f.c index a4f8627ddf..96949eba3c 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -488,6 +488,20 @@ static int reserve_fdt(void) return 0; }
+static int reserve_bootstage(void) +{ +#ifdef CONFIG_BOOTSTAGE + int size = bootstage_get_size(); + + gd->start_addr_sp -= size; + gd->new_bootstage = map_sysmem(gd->start_addr_sp, size); + debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, + gd->start_addr_sp); +#endif + + return 0; +} + int arch_reserve_stacks(void) { return 0; @@ -602,6 +616,24 @@ static int reloc_fdt(void) return 0; }
+static int reloc_bootstage(void) +{ +#ifdef CONFIG_BOOTSTAGE + if (gd->flags & GD_FLG_SKIP_RELOC) + return 0; + if (gd->new_bootstage) { + int size = bootstage_get_size(); + + debug("Copying bootstage from %p to %p, size %x\n", + gd->bootstage, gd->new_bootstage, size); + memcpy(gd->new_bootstage, gd->bootstage, size); + gd->bootstage = gd->new_bootstage; + } +#endif + + return 0; +} + static int setup_reloc(void) { if (gd->flags & GD_FLG_SKIP_RELOC) { @@ -825,6 +857,7 @@ static const init_fnc_t init_sequence_f[] = { setup_machine, reserve_global_data, reserve_fdt, + reserve_bootstage, reserve_arch, reserve_stacks, dram_init_banksize, @@ -846,6 +879,7 @@ static const init_fnc_t init_sequence_f[] = { #endif INIT_FUNC_WATCHDOG_RESET reloc_fdt, + reloc_bootstage, setup_reloc, #if defined(CONFIG_X86) || defined(CONFIG_ARC) copy_uboot_to_ram, diff --git a/common/bootstage.c b/common/bootstage.c index 32f4e8b706..a6705af111 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -484,6 +484,11 @@ int bootstage_unstash(void *base, int size) return 0; }
+int bootstage_get_size(void) +{ + return sizeof(struct bootstage_data); +} + int bootstage_init(bool first) { struct bootstage_data *data; diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 8b3229e5b8..fb90be9d3e 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -112,6 +112,7 @@ typedef struct global_data { #endif #ifdef CONFIG_BOOTSTAGE struct bootstage_data *bootstage; /* Bootstage information */ + struct bootstage_data *new_bootstage; /* Relocated bootstage info */ #endif } gd_t; #endif diff --git a/include/bootstage.h b/include/bootstage.h index 41bd61785c..8607e887d8 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -332,6 +332,13 @@ int bootstage_stash(void *base, int size); int bootstage_unstash(void *base, int size);
/** + * bootstage_get_size() - Get the size of the bootstage data + * + * @return size of boostage data in bytes + */ +int bootstage_get_size(void); + +/** * bootstage_init() - Prepare bootstage for use * * @first: true if this is the first time bootstage is set up. This causes it @@ -400,6 +407,11 @@ static inline int bootstage_unstash(void *base, int size) return 0; /* Pretend to succeed */ }
+static inline int bootstage_get_size(void) +{ + return 0; +} + static inline int bootstage_init(bool first) { return 0;

On Mon, May 22, 2017 at 05:05:30AM -0600, Simon Glass wrote:
Some boards cannot access pre-relocation data after relocation. Reserve space for this and copy it during preparation for relocation.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

At present we don't allow use of bootstage before driver model is running. This means we cannot time the init of driver model itself.
Now that bootstage requires its own board-specific timer, we can move its init to earlier in the sequence, both before and after relocation.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 2 +- common/board_r.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 96949eba3c..80388c50bc 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -762,6 +762,7 @@ static const init_fnc_t init_sequence_f[] = { trace_early_init, #endif initf_malloc, + initf_bootstage, /* uses its own timer, so does not need DM */ initf_console_record, #if defined(CONFIG_HAVE_FSP) arch_fsp_init, @@ -770,7 +771,6 @@ static const init_fnc_t init_sequence_f[] = { mach_cpu_init, /* SoC/machine dependent CPU setup */ initf_dm, arch_cpu_init_dm, - initf_bootstage, /* need timer, go after init dm */ #if defined(CONFIG_BOARD_EARLY_INIT_F) board_early_init_f, #endif diff --git a/common/board_r.c b/common/board_r.c index a9c6a84ce4..990d87021e 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -329,7 +329,6 @@ static int initr_dm(void)
static int initr_bootstage(void) { - /* We cannot do this before initr_dm() */ bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_R, "board_init_r");
return 0; @@ -728,6 +727,7 @@ static init_fnc_t init_sequence_r[] = { #endif initr_barrier, initr_malloc, + initr_bootstage, /* Needs malloc() but has its own timer */ initr_console_record, #ifdef CONFIG_SYS_NONCACHED_MEMORY initr_noncached, @@ -739,7 +739,6 @@ static init_fnc_t init_sequence_r[] = { #ifdef CONFIG_DM initr_dm, #endif - initr_bootstage, #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) board_init, /* Setup chipselects */ #endif

On Mon, May 22, 2017 at 05:05:31AM -0600, Simon Glass wrote:
At present we don't allow use of bootstage before driver model is running. This means we cannot time the init of driver model itself.
Now that bootstage requires its own board-specific timer, we can move its init to earlier in the sequence, both before and after relocation.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Driver model is set up ones before relocation and once after. Record the time taken in each case.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 2 ++ common/board_r.c | 2 ++ include/bootstage.h | 2 ++ 3 files changed, 6 insertions(+)
diff --git a/common/board_f.c b/common/board_f.c index 80388c50bc..f18301996a 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -729,7 +729,9 @@ static int initf_dm(void) #if defined(CONFIG_DM) && defined(CONFIG_SYS_MALLOC_F_LEN) int ret;
+ bootstage_start(BOOTSTATE_ID_ACCUM_DM_F, "dm_f"); ret = dm_init_and_scan(true); + bootstage_accum(BOOTSTATE_ID_ACCUM_DM_F); if (ret) return ret; #endif diff --git a/common/board_r.c b/common/board_r.c index 990d87021e..2d717e8423 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -314,7 +314,9 @@ static int initr_dm(void) #ifdef CONFIG_TIMER gd->timer = NULL; #endif + bootstage_start(BOOTSTATE_ID_ACCUM_DM_R, "dm_r"); ret = dm_init_and_scan(false); + bootstage_accum(BOOTSTATE_ID_ACCUM_DM_R); if (ret) return ret; #ifdef CONFIG_TIMER_EARLY diff --git a/include/bootstage.h b/include/bootstage.h index 8607e887d8..9c7b515a74 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -200,6 +200,8 @@ enum bootstage_id { BOOTSTAGE_ID_ACCUM_SPI, BOOTSTAGE_ID_ACCUM_DECOMP, BOOTSTAGE_ID_FPGA_INIT, + BOOTSTATE_ID_ACCUM_DM_F, + BOOTSTATE_ID_ACCUM_DM_R,
/* a few spare for the user, from here */ BOOTSTAGE_ID_USER,

On Mon, May 22, 2017 at 05:05:32AM -0600, Simon Glass wrote:
Driver model is set up ones before relocation and once after. Record the time taken in each case.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

We should return a proper error number instead of just -1. This helps the caller to determine what when wrong. Update a few functions to fix this.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/bootstage.c | 22 +++++++++++----------- include/bootstage.h | 4 +++- 2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/common/bootstage.c b/common/bootstage.c index a6705af111..9ef931fd83 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -264,7 +264,7 @@ static int add_bootstages_devicetree(struct fdt_header *blob) */ bootstage = fdt_add_subnode(blob, 0, "bootstage"); if (bootstage < 0) - return -1; + return -EINVAL;
/* * Insert the timings to the device tree in the reverse order so @@ -284,13 +284,13 @@ static int add_bootstages_devicetree(struct fdt_header *blob) /* add properties to the node. */ if (fdt_setprop_string(blob, node, "name", get_record_name(buf, sizeof(buf), rec))) - return -1; + return -EINVAL;
/* Check if this is a 'mark' or 'accum' record */ if (fdt_setprop_cell(blob, node, rec->start_us ? "accum" : "mark", rec->time_us)) - return -1; + return -EINVAL; }
return 0; @@ -371,7 +371,7 @@ int bootstage_stash(void *base, int size)
if (hdr + 1 > (struct bootstage_hdr *)end) { debug("%s: Not enough space for bootstage hdr\n", __func__); - return -1; + return -ENOSPC; }
/* Write an arbitrary version number */ @@ -404,7 +404,7 @@ int bootstage_stash(void *base, int size) /* Check for buffer overflow */ if (ptr > end) { debug("%s: Not enough space for bootstage stash\n", __func__); - return -1; + return -ENOSPC; }
/* Update total data size */ @@ -428,37 +428,37 @@ int bootstage_unstash(void *base, int size)
if (hdr + 1 > (struct bootstage_hdr *)end) { debug("%s: Not enough space for bootstage hdr\n", __func__); - return -1; + return -EPERM; }
if (hdr->magic != BOOTSTAGE_MAGIC) { debug("%s: Invalid bootstage magic\n", __func__); - return -1; + return -ENOENT; }
if (ptr + hdr->size > end) { debug("%s: Bootstage data runs past buffer end\n", __func__); - return -1; + return -ENOSPC; }
if (hdr->count * sizeof(*rec) > hdr->size) { debug("%s: Bootstage has %d records needing %lu bytes, but " "only %d bytes is available\n", __func__, hdr->count, (ulong)hdr->count * sizeof(*rec), hdr->size); - return -1; + return -ENOSPC; }
if (hdr->version != BOOTSTAGE_VERSION) { debug("%s: Bootstage data version %#0x unrecognised\n", __func__, hdr->version); - return -1; + return -EINVAL; }
if (data->rec_count + hdr->count > RECORD_COUNT) { debug("%s: Bootstage has %d records, we have space for %d\n" "- please increase CONFIG_BOOTSTAGE_USER_COUNT\n", __func__, hdr->count, RECORD_COUNT - data->rec_count); - return -1; + return -ENOSPC; }
ptr += sizeof(*hdr); diff --git a/include/bootstage.h b/include/bootstage.h index 9c7b515a74..848769bd1f 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -329,7 +329,9 @@ int bootstage_stash(void *base, int size); * * @param base Base address of memory buffer * @param size Size of memory buffer (-1 if unknown) - * @return 0 if unstashed ok, -1 if bootstage info not found, or out of space + * @return 0 if unstashed ok, -ENOENT if bootstage info not found, -ENOSPC if + * there is not space for read the stacked data, or other error if + * something else went wrong */ int bootstage_unstash(void *base, int size);

On Mon, May 22, 2017 at 05:05:33AM -0600, Simon Glass wrote:
We should return a proper error number instead of just -1. This helps the caller to determine what when wrong. Update a few functions to fix this.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

There are a few places that should use const *, such as bootstage_unstash(). Update these to make it clearer when parameters are changed.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/bootstage.c | 12 ++++++------ include/bootstage.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/bootstage.c b/common/bootstage.c index 9ef931fd83..61479d7f07 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -205,7 +205,7 @@ uint32_t bootstage_accum(enum bootstage_id id) * @return pointer to name, either from the record or pointing to buf. */ static const char *get_record_name(char *buf, int len, - struct bootstage_record *rec) + const struct bootstage_record *rec) { if (rec->name) return rec->name; @@ -361,9 +361,9 @@ static void append_data(char **ptrp, char *end, const void *data, int size)
int bootstage_stash(void *base, int size) { - struct bootstage_data *data = gd->bootstage; + const struct bootstage_data *data = gd->bootstage; struct bootstage_hdr *hdr = (struct bootstage_hdr *)base; - struct bootstage_record *rec; + const struct bootstage_record *rec; char buf[20]; char *ptr = base, *end = ptr + size; uint32_t count; @@ -414,12 +414,12 @@ int bootstage_stash(void *base, int size) return 0; }
-int bootstage_unstash(void *base, int size) +int bootstage_unstash(const void *base, int size) { + const struct bootstage_hdr *hdr = (struct bootstage_hdr *)base; struct bootstage_data *data = gd->bootstage; - struct bootstage_hdr *hdr = (struct bootstage_hdr *)base; + const char *ptr = base, *end = ptr + size; struct bootstage_record *rec; - char *ptr = base, *end = ptr + size; uint rec_size; int i;
diff --git a/include/bootstage.h b/include/bootstage.h index 848769bd1f..5f4fffaffb 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -333,7 +333,7 @@ int bootstage_stash(void *base, int size); * there is not space for read the stacked data, or other error if * something else went wrong */ -int bootstage_unstash(void *base, int size); +int bootstage_unstash(const void *base, int size);
/** * bootstage_get_size() - Get the size of the bootstage data @@ -406,7 +406,7 @@ static inline int bootstage_stash(void *base, int size) return 0; /* Pretend to succeed */ }
-static inline int bootstage_unstash(void *base, int size) +static inline int bootstage_unstash(const void *base, int size) { return 0; /* Pretend to succeed */ }

On Mon, May 22, 2017 at 05:05:34AM -0600, Simon Glass wrote:
There are a few places that should use const *, such as bootstage_unstash(). Update these to make it clearer when parameters are changed.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

At present bootstage only supports U-Boot proper. But SPL can also consume boot time so it is useful to have the record start there.
Add bootstage support to SPL. Also support stashing the timing information when SPL finishes so that it can be picked up and reported by U-Boot proper. This provides a full boot time record, excluding only the time taken by the boot ROM.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/Kconfig | 9 +++++++++ common/Makefile | 3 ++- common/board_f.c | 17 ++++++++++++++++- common/spl/spl.c | 18 ++++++++++++++++++ include/bootstage.h | 15 ++++++++++++--- 5 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index 83ef11ec35..96b3ce12a9 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -18,6 +18,15 @@ config BOOTSTAGE Calls to show_boot_progress() will also result in log entries but these will not have names.
+config SPL_BOOTSTAGE + bool "Boot timing and reported in SPL" + depends on BOOTSTAGE + help + Enable recording of boot time in SPL. To make this visible to U-Boot + proper, enable BOOTSTAGE_STASH as well. This will stash the timing + information when SPL finishes and load it when U-Boot proper starts + up. + config BOOTSTAGE_REPORT bool "Display a detailed boot timing report before booting the OS" depends on BOOTSTAGE diff --git a/common/Makefile b/common/Makefile index 14d01844ad..31b8ed1366 100644 --- a/common/Makefile +++ b/common/Makefile @@ -65,7 +65,6 @@ obj-$(CONFIG_USB_STORAGE) += usb_storage.o endif
# others -obj-$(CONFIG_BOOTSTAGE) += bootstage.o obj-$(CONFIG_CONSOLE_MUX) += iomux.o obj-$(CONFIG_MTD_NOR_FLASH) += flash.o obj-$(CONFIG_CMD_KGDB) += kgdb.o kgdb_stubs.o @@ -89,6 +88,8 @@ obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
endif # !CONFIG_SPL_BUILD
+obj-$(CONFIG_$(SPL_)BOOTSTAGE) += bootstage.o + ifdef CONFIG_SPL_BUILD obj-$(CONFIG_SPL_DFU_SUPPORT) += dfu.o obj-$(CONFIG_SPL_DFU_SUPPORT) += cli_hush.o diff --git a/common/board_f.c b/common/board_f.c index f18301996a..6c637d7cfa 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -704,11 +704,26 @@ static int jump_to_copy(void) /* Record the board_init_f() bootstage (after arch_cpu_init()) */ static int initf_bootstage(void) { +#if defined(CONFIG_SPL_BOOTSTAGE) && defined(CONFIG_BOOTSTAGE_STASH) + bool from_spl = true; +#else + bool from_spl = false; +#endif int ret;
- ret = bootstage_init(true); + ret = bootstage_init(!from_spl); if (ret) return ret; + if (from_spl) { + const void *stash = map_sysmem(CONFIG_BOOTSTAGE_STASH_ADDR, + CONFIG_BOOTSTAGE_STASH_SIZE); + + ret = bootstage_unstash(stash, CONFIG_BOOTSTAGE_STASH_SIZE); + if (ret && ret != -ENOENT) { + debug("Failed to unstash bootstage: err=%d\n", ret); + return ret; + } + }
bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
diff --git a/common/spl/spl.c b/common/spl/spl.c index 0a49766f21..f493a3ad49 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -232,6 +232,13 @@ static int spl_common_init(bool setup_malloc) gd->malloc_ptr = 0; } #endif + ret = bootstage_init(true); + if (ret) { + debug("%s: Failed to set up bootstage: ret=%d\n", __func__, + ret); + return ret; + } + bootstage_mark_name(BOOTSTAGE_ID_START_SPL, "spl"); if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) { ret = fdtdec_setup(); if (ret) { @@ -240,8 +247,10 @@ static int spl_common_init(bool setup_malloc) } } if (IS_ENABLED(CONFIG_SPL_DM)) { + bootstage_start(BOOTSTATE_ID_ACCUM_DM_SPL, "dm_spl"); /* With CONFIG_SPL_OF_PLATDATA, bring in all devices */ ret = dm_init_and_scan(!CONFIG_IS_ENABLED(OF_PLATDATA)); + bootstage_accum(BOOTSTATE_ID_ACCUM_DM_SPL); if (ret) { debug("dm_init_and_scan() returned error %d\n", ret); return ret; @@ -421,6 +430,15 @@ void board_init_r(gd_t *dummy1, ulong dummy2) }
debug("loaded - jumping to U-Boot...\n"); +#ifdef CONFIG_BOOTSTAGE_STASH + int ret; + + bootstage_mark_name(BOOTSTAGE_ID_END_SPL, "end_spl"); + ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR, + CONFIG_BOOTSTAGE_STASH_SIZE); + if (ret) + debug("Failed to stash bootstage: err=%d\n", ret); +#endif spl_board_prepare_for_boot(); jump_to_image_no_args(&spl_image); } diff --git a/include/bootstage.h b/include/bootstage.h index 5f4fffaffb..c972027ffc 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -177,6 +177,7 @@ enum bootstage_id { */ BOOTSTAGE_ID_AWAKE, BOOTSTAGE_ID_START_SPL, + BOOTSTAGE_ID_END_SPL, BOOTSTAGE_ID_START_UBOOT_F, BOOTSTAGE_ID_START_UBOOT_R, BOOTSTAGE_ID_USB_START, @@ -200,6 +201,7 @@ enum bootstage_id { BOOTSTAGE_ID_ACCUM_SPI, BOOTSTAGE_ID_ACCUM_DECOMP, BOOTSTAGE_ID_FPGA_INIT, + BOOTSTATE_ID_ACCUM_DM_SPL, BOOTSTATE_ID_ACCUM_DM_F, BOOTSTATE_ID_ACCUM_DM_R,
@@ -228,8 +230,14 @@ ulong timer_get_boot_us(void); void show_boot_progress(int val); #endif
-#if defined(CONFIG_BOOTSTAGE) && !defined(CONFIG_SPL_BUILD) && \ - !defined(USE_HOSTCC) +#if !defined(USE_HOSTCC) +#if CONFIG_IS_ENABLED(BOOTSTAGE) +#define ENABLE_BOOTSTAGE +#endif +#endif + +#ifdef ENABLE_BOOTSTAGE + /* This is the full bootstage implementation */
/** @@ -420,7 +428,8 @@ static inline int bootstage_init(bool first) { return 0; } -#endif /* CONFIG_BOOTSTAGE */ + +#endif /* ENABLE_BOOTSTAGE */
/* Helper macro for adding a bootstage to a line of code */ #define BOOTSTAGE_MARKER() \

On Mon, May 22, 2017 at 05:05:35AM -0600, Simon Glass wrote:
At present bootstage only supports U-Boot proper. But SPL can also consume boot time so it is useful to have the record start there.
Add bootstage support to SPL. Also support stashing the timing information when SPL finishes so that it can be picked up and reported by U-Boot proper. This provides a full boot time record, excluding only the time taken by the boot ROM.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

This time is interesting as a comparision with the flat device tree time. Add it to the record.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_r.c | 11 +++++++++-- include/bootstage.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 2d717e8423..5986cd7674 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -298,8 +298,15 @@ static int initr_noncached(void) #ifdef CONFIG_OF_LIVE static int initr_of_live(void) { - return of_live_build(gd->fdt_blob, - (struct device_node **)&gd->of_root); + int ret; + + bootstage_start(BOOTSTAGE_ID_ACCUM_OF_LIVE, "of_live"); + ret = of_live_build(gd->fdt_blob, (struct device_node **)&gd->of_root); + bootstage_accum(BOOTSTAGE_ID_ACCUM_OF_LIVE); + if (ret) + return ret; + + return 0; } #endif
diff --git a/include/bootstage.h b/include/bootstage.h index c972027ffc..c5d93f57fd 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -200,6 +200,7 @@ enum bootstage_id { BOOTSTAGE_ID_ACCUM_SCSI, BOOTSTAGE_ID_ACCUM_SPI, BOOTSTAGE_ID_ACCUM_DECOMP, + BOOTSTAGE_ID_ACCUM_OF_LIVE, BOOTSTAGE_ID_FPGA_INIT, BOOTSTATE_ID_ACCUM_DM_SPL, BOOTSTATE_ID_ACCUM_DM_F,

On Mon, May 22, 2017 at 05:05:36AM -0600, Simon Glass wrote:
This time is interesting as a comparision with the flat device tree time. Add it to the record.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (2)
-
Simon Glass
-
Tom Rini