From 4aa5053da5fa0729d30bce5df9a0a037a391f2f3 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 15 Jan 2023 14:15:42 -0700 Subject: timer: Tidy up use of notrace Tracing is typically enabled by the time driver model starts up, so there is no point in adding a 'notrace' to the timer-init function. However, once the driver model timer is enabled, we do need to be able to access the timer's private data when reading the timer, so add it to the core function needed for that. Update the function's documentation while we are here. Signed-off-by: Simon Glass --- include/timer.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/timer.h b/include/timer.h index d33a26e28fe..311ce6b2c3a 100644 --- a/include/timer.h +++ b/include/timer.h @@ -9,11 +9,16 @@ #define timer_get_ops(dev) ((struct timer_ops *)(dev)->driver->ops) /** - * dm_timer_init() - initialize a timer for time keeping. On success - * initializes gd->timer so that lib/timer can use it for future - * referrence. + * dm_timer_init() - set up a timer for time keeping * - * Return: 0 on success or error number + * Sets up gd->timer if the device is not already bound, making sure it is + * probed and ready for use + * + * On success, inits gd->timer so that lib/timer can use it for future reference + * + * Returns: 0 on success, -EAGAIN if driver model is not ready yet, -ENODEV if + * no timer could be found, other error if the timer could not be bound or + * probed */ int dm_timer_init(void); -- cgit v1.3.1 From d9044e5363bb0b32a7aff00adf47c227f78f0f32 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 15 Jan 2023 14:15:46 -0700 Subject: trace: Update the file header It seems better to put the TEXT_BASE value in the file header rather than in an entry record. While it is true that there is a separate base for pre-relocation, this can be handled by using offsets in the file. It is useful to have a version number in case we need to change the trace format again. Update the header to make these changes. Signed-off-by: Simon Glass --- include/trace.h | 12 ++++++++++-- lib/trace.c | 19 ++++--------------- 2 files changed, 14 insertions(+), 17 deletions(-) (limited to 'include') diff --git a/include/trace.h b/include/trace.h index e7aee024f03..2be8d81b515 100644 --- a/include/trace.h +++ b/include/trace.h @@ -6,6 +6,8 @@ #ifndef __TRACE_H #define __TRACE_H +/* this file is included from a tool so uses uint32_t instead of u32, etc. */ + enum { /* * This affects the granularity of our trace. We can bin function @@ -23,6 +25,8 @@ enum { * this value. */ FUNC_SITE_SIZE = 4, /* distance between function sites */ + + TRACE_VERSION = 1, }; enum trace_chunk_type { @@ -39,7 +43,11 @@ struct trace_output_func { /* A header at the start of the trace output buffer */ struct trace_output_hdr { enum trace_chunk_type type; /* Record type */ - size_t rec_count; /* Number of records */ + uint32_t version; /* Version (TRACE_VERSION) */ + uint32_t rec_count; /* Number of records */ + uint32_t spare; /* 0 */ + uint64_t text_base; /* Value of CONFIG_TEXT_BASE */ + uint64_t spare2; /* 0 */ }; /* Print statistics about traced function calls */ @@ -63,7 +71,7 @@ int trace_list_functions(void *buff, size_t buff_size, size_t *needed); enum ftrace_flags { FUNCF_EXIT = 0UL << 30, FUNCF_ENTRY = 1UL << 30, - FUNCF_TEXTBASE = 2UL << 30, + /* two more values are available */ FUNCF_TIMESTAMP_MASK = 0x3fffffff, }; diff --git a/lib/trace.c b/lib/trace.c index b9dc6d2e4b5..2e2c1bed54f 100644 --- a/lib/trace.c +++ b/lib/trace.c @@ -118,18 +118,6 @@ static void notrace add_ftrace(void *func_ptr, void *caller, ulong flags) hdr->ftrace_count++; } -static void notrace add_textbase(void) -{ - if (hdr->ftrace_count < hdr->ftrace_size) { - struct trace_call *rec = &hdr->ftrace[hdr->ftrace_count]; - - rec->func = CONFIG_TEXT_BASE; - rec->caller = 0; - rec->flags = FUNCF_TEXTBASE; - } - hdr->ftrace_count++; -} - /** * __cyg_profile_func_enter() - record function entry * @@ -278,8 +266,11 @@ int trace_list_calls(void *buff, size_t buff_size, size_t *needed) /* Update the header */ if (output_hdr) { + memset(output_hdr, '\0', sizeof(*output_hdr)); output_hdr->rec_count = upto; output_hdr->type = TRACE_CHUNK_CALLS; + output_hdr->version = TRACE_VERSION; + output_hdr->text_base = CONFIG_TEXT_BASE; } /* Work out how must of the buffer we used */ @@ -385,10 +376,9 @@ int notrace trace_init(void *buff, size_t buff_size) /* Use any remaining space for the timed function trace */ hdr->ftrace = (struct trace_call *)(buff + needed); hdr->ftrace_size = (buff_size - needed) / sizeof(*hdr->ftrace); - add_textbase(); + hdr->depth_limit = CONFIG_TRACE_CALL_DEPTH_LIMIT; puts("trace: enabled\n"); - hdr->depth_limit = CONFIG_TRACE_CALL_DEPTH_LIMIT; trace_enabled = 1; trace_inited = 1; @@ -426,7 +416,6 @@ int notrace trace_early_init(void) /* Use any remaining space for the timed function trace */ hdr->ftrace = (struct trace_call *)((char *)hdr + needed); hdr->ftrace_size = (buff_size - needed) / sizeof(*hdr->ftrace); - add_textbase(); hdr->depth_limit = CONFIG_TRACE_EARLY_CALL_DEPTH_LIMIT; printf("trace: early enable at %08x\n", CONFIG_TRACE_EARLY_ADDR); -- cgit v1.3.1 From c3d91812a2b69fbc73eccdcec70d7ea510d7c8bd Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 15 Jan 2023 14:15:47 -0700 Subject: trace: Reduce the number of function sites Given that the compiler adds two function calls into each function, the current spacing is overkill. Drop it down to 16 bytes per function, which is still plenty. This saves some space in the trace buffer. Also move the calculation into a function, so it is common code. Add a check for gd->mon_len being unset, which breaks tracing. Signed-off-by: Simon Glass --- include/trace.h | 6 ++++-- lib/trace.c | 19 +++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/trace.h b/include/trace.h index 2be8d81b515..763d6d1255a 100644 --- a/include/trace.h +++ b/include/trace.h @@ -17,14 +17,16 @@ enum { * * The value here assumes a minimum instruction size of 4 bytes, * or that instructions are 2 bytes but there are at least 2 of - * them in every function. + * them in every function. Given that each function needs a call to + * __cyg_profile_func_enter() and __cyg_profile_func_exit() as well, + * we cannot have functions smaller that 16 bytes. * * Increasing this value reduces the number of functions we can * resolve, but reduces the size of the uintptr_t array used for * our function list, which is the length of the code divided by * this value. */ - FUNC_SITE_SIZE = 4, /* distance between function sites */ + FUNC_SITE_SIZE = 16, /* distance between function sites */ TRACE_VERSION = 1, }; diff --git a/lib/trace.c b/lib/trace.c index 2e2c1bed54f..12dae2089a4 100644 --- a/lib/trace.c +++ b/lib/trace.c @@ -321,6 +321,17 @@ void notrace trace_set_enabled(int enabled) trace_enabled = enabled != 0; } +static int get_func_count(void) +{ + /* Detect no support for mon_len since this means tracing cannot work */ + if (IS_ENABLED(CONFIG_SANDBOX) && !gd->mon_len) { + puts("Tracing is not supported on this board\n"); + return -ENOTSUPP; + } + + return gd->mon_len / FUNC_SITE_SIZE; +} + /** * trace_init() - initialize the tracing system and enable it * @@ -330,10 +341,12 @@ void notrace trace_set_enabled(int enabled) */ int notrace trace_init(void *buff, size_t buff_size) { - ulong func_count = gd->mon_len / FUNC_SITE_SIZE; + int func_count = get_func_count(); size_t needed; int was_disabled = !trace_enabled; + if (func_count < 0) + return func_count; trace_save_gd(); if (!was_disabled) { @@ -393,10 +406,12 @@ int notrace trace_init(void *buff, size_t buff_size) */ int notrace trace_early_init(void) { - ulong func_count = gd->mon_len / FUNC_SITE_SIZE; + int func_count = get_func_count(); size_t buff_size = CONFIG_TRACE_EARLY_SIZE; size_t needed; + if (func_count < 0) + return func_count; /* We can ignore additional calls to this function */ if (trace_enabled) return 0; -- cgit v1.3.1