From 8c99c27df10f6c5a120e41ffb0948e357f5a2d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ky=C3=B6sti=20M=C3=A4lkki?= Date: Fri, 24 Jul 2020 15:54:31 +0300 Subject: [PATCH] lib/trace: Remove TRACE support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Looks like the option is generally not compatible with garbage collections. Nothing gets inlined, for example is_smp_boot() no longer evaluates to constant false and thus the symbols from secondary.S would need to be present for the build to pass even if we set SMP=n. Also the addresses of relocatable ramstage are currently not normalised on the logs, so util/genprof would be unable dress those. Change-Id: I0b6f310e15e6f4992cd054d288903fea8390e5cf Signed-off-by: Kyösti Mälkki Reviewed-on: https://review.coreboot.org/c/coreboot/+/45757 Tested-by: build bot (Jenkins) Reviewed-by: Angel Pons Reviewed-by: Nico Huber --- Makefile.inc | 3 - src/Kconfig | 13 +--- src/console/printk.c | 3 - src/console/vsprintf.c | 5 -- src/drivers/uart/uart8250io.c | 3 - src/include/trace.h | 28 --------- src/lib/Makefile.inc | 2 - src/lib/trace.c | 21 ------- util/genprof/.gitignore | 1 - util/genprof/Makefile | 12 ---- util/genprof/README | 31 --------- util/genprof/description.md | 1 - util/genprof/genprof.c | 114 ---------------------------------- util/genprof/log2dress | 20 ------ 14 files changed, 1 insertion(+), 256 deletions(-) delete mode 100644 src/include/trace.h delete mode 100644 src/lib/trace.c delete mode 100644 util/genprof/.gitignore delete mode 100644 util/genprof/Makefile delete mode 100644 util/genprof/README delete mode 100644 util/genprof/description.md delete mode 100644 util/genprof/genprof.c delete mode 100755 util/genprof/log2dress diff --git a/Makefile.inc b/Makefile.inc index 95846a75c7..420ce515ce 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -183,9 +183,6 @@ decompressor-generic-ccopts += -D__DECOMPRESSOR__ bootblock-generic-ccopts += -D__BOOTBLOCK__ romstage-generic-ccopts += -D__ROMSTAGE__ ramstage-generic-ccopts += -D__RAMSTAGE__ -ifeq ($(CONFIG_TRACE),y) -ramstage-c-ccopts += -finstrument-functions -endif ifeq ($(CONFIG_COVERAGE),y) ramstage-c-ccopts += -fprofile-arcs -ftest-coverage endif diff --git a/src/Kconfig b/src/Kconfig index dc98ca2c05..77d077f450 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -1113,23 +1113,12 @@ config DEBUG_INTEL_ME is present on Intel 6-series chipsets. endif -config TRACE - bool "Trace function calls" - default n - help - If enabled, every function will print information to console once - the function is entered. The syntax is ~0xaaaabbbb(0xccccdddd) - the 0xaaaabbbb is the actual function and 0xccccdddd is EIP - of calling function. Please note some printk related functions - are omitted from trace to have good looking console dumps. - config DEBUG_FUNC bool "Enable function entry and exit reporting macros" if DEFAULT_CONSOLE_LOGLEVEL_8 default n help This option enables additional function entry and exit debug messages - for select functions. If supported, this is less output than - the TRACE option. + for select functions. Note: This option will increase the size of the coreboot image. If unsure, say N. diff --git a/src/console/printk.c b/src/console/printk.c index 4a3de47832..85d9bfb1e6 100644 --- a/src/console/printk.c +++ b/src/console/printk.c @@ -10,7 +10,6 @@ #include #include #include -#include #include DECLARE_SPIN_LOCK(console_lock) @@ -81,7 +80,6 @@ int do_vprintk(int msg_level, const char *fmt, va_list args) if (log_this < CONSOLE_LOG_FAST) return 0; - DISABLE_TRACE; spin_lock(&console_lock); console_time_run(); @@ -96,7 +94,6 @@ int do_vprintk(int msg_level, const char *fmt, va_list args) console_time_stop(); spin_unlock(&console_lock); - ENABLE_TRACE; return i; } diff --git a/src/console/vsprintf.c b/src/console/vsprintf.c index d0c569bf59..06b9e494a4 100644 --- a/src/console/vsprintf.c +++ b/src/console/vsprintf.c @@ -2,7 +2,6 @@ #include #include -#include struct vsnprintf_context { char *str_buf; @@ -24,16 +23,12 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) int i; struct vsnprintf_context ctx; - DISABLE_TRACE; - ctx.str_buf = buf; ctx.buf_limit = size ? size - 1 : 0; i = vtxprintf(str_tx_byte, fmt, args, &ctx); if (size) *ctx.str_buf = '\0'; - ENABLE_TRACE; - return i; } diff --git a/src/drivers/uart/uart8250io.c b/src/drivers/uart/uart8250io.c index d0841de39c..aa8c969530 100644 --- a/src/drivers/uart/uart8250io.c +++ b/src/drivers/uart/uart8250io.c @@ -3,7 +3,6 @@ #include #include #include -#include #include "uart8250reg.h" /* Should support 8250, 16450, 16550, 16550A type UARTs */ @@ -54,7 +53,6 @@ static unsigned char uart8250_rx_byte(unsigned int base_port) static void uart8250_init(unsigned int base_port, unsigned int divisor) { - DISABLE_TRACE; /* Disable interrupts */ outb(0x0, base_port + UART8250_IER); /* Enable FIFOs */ @@ -72,7 +70,6 @@ static void uart8250_init(unsigned int base_port, unsigned int divisor) /* Set to 3 for 8N1 */ outb(CONFIG_TTYS0_LCS, base_port + UART8250_LCR); - ENABLE_TRACE; } static const unsigned int bases[] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 }; diff --git a/src/include/trace.h b/src/include/trace.h deleted file mode 100644 index ece1b2110c..0000000000 --- a/src/include/trace.h +++ /dev/null @@ -1,28 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef __TRACE_H -#define __TRACE_H - -#if !ENV_ROMSTAGE_OR_BEFORE && CONFIG(TRACE) - -void __cyg_profile_func_enter(void *, void *) - __attribute__((no_instrument_function)); - -void __cyg_profile_func_exit(void *, void *) - __attribute__((no_instrument_function)); - -extern volatile int trace_dis; - -#define DISABLE_TRACE do { trace_dis = 1; } while (0); -#define ENABLE_TRACE do { trace_dis = 0; } while (0); -#define DISABLE_TRACE_ON_FUNCTION __attribute__((no_instrument_function)); - -#else /* !CONFIG_TRACE */ - -#define DISABLE_TRACE -#define ENABLE_TRACE -#define DISABLE_TRACE_ON_FUNCTION - -#endif - -#endif diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index c228f2a9f4..6cff03dc63 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -140,8 +140,6 @@ ramstage-y += wrdd.c ramstage-$(CONFIG_CONSOLE_CBMEM) += cbmem_console.c ramstage-$(CONFIG_BOOTSPLASH) += bootsplash.c ramstage-$(CONFIG_BOOTSPLASH) += jpeg.c -ramstage-$(CONFIG_TRACE) += trace.c -postcar-$(CONFIG_TRACE) += trace.c ramstage-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c ramstage-$(CONFIG_COVERAGE) += libgcov.c ramstage-y += edid.c diff --git a/src/lib/trace.c b/src/lib/trace.c deleted file mode 100644 index a3db40b5f7..0000000000 --- a/src/lib/trace.c +++ /dev/null @@ -1,21 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include -#include - -int volatile trace_dis = 0; - -void __cyg_profile_func_enter(void *func, void *callsite) -{ - - if (trace_dis) - return; - - DISABLE_TRACE - printk(BIOS_INFO, "~%p(%p)\n", func, callsite); - ENABLE_TRACE -} - -void __cyg_profile_func_exit(void *func, void *callsite) -{ -} diff --git a/util/genprof/.gitignore b/util/genprof/.gitignore deleted file mode 100644 index 612ef67372..0000000000 --- a/util/genprof/.gitignore +++ /dev/null @@ -1 +0,0 @@ -genprof diff --git a/util/genprof/Makefile b/util/genprof/Makefile deleted file mode 100644 index 2ec77c918a..0000000000 --- a/util/genprof/Makefile +++ /dev/null @@ -1,12 +0,0 @@ -CC=gcc -CFLAGS=-O2 -Wall - -all: genprof - -genprof: genprof.o - $(CC) $(CFLAGS) -o genprof $^ - -clean: - rm -f genprof *.o *~ - -distclean: clean diff --git a/util/genprof/README b/util/genprof/README deleted file mode 100644 index d4159c2fd6..0000000000 --- a/util/genprof/README +++ /dev/null @@ -1,31 +0,0 @@ -Function tracing ----------------- - -Enable CONFIG_TRACE in debug menu. Run the compiled image on target. You will get -a log with a lot of lines like: - -... -~0x001072e8(0x00100099) -~0x00108bc0(0x0010730a) -... - -First address is address of function which was just entered, the second address -is address of functions which call that. - -You can use the log2dress to dress the log again: - -... -src/arch/x86/lib/c_start.S:85 calls /home/ruik/coreboot/src/boot/selfboot.c:367 -/home/ruik/coreboot/src/boot/selfboot.c:370 calls /home/ruik/coreboot/src/device/device.c:325 -... - -Alternatively, you can use genprof to generate a gmon.out file, which can be used -by gprof to show the call traces. You will need to install uthash library to compile -that. - -Great use is: - -make -./genprof /tmp/yourlog ; gprof ../../build/ramstage | ./gprof2dot.py -e0 -n0 | dot -Tpng -o output.png - -Which generates a PNG with a call graph. diff --git a/util/genprof/description.md b/util/genprof/description.md deleted file mode 100644 index 84618a4187..0000000000 --- a/util/genprof/description.md +++ /dev/null @@ -1 +0,0 @@ -Format function tracing logs `Bash` `C` diff --git a/util/genprof/genprof.c b/util/genprof/genprof.c deleted file mode 100644 index f4dd4cb7c1..0000000000 --- a/util/genprof/genprof.c +++ /dev/null @@ -1,114 +0,0 @@ -#include -#include -#include -#include - -#define GMON_SEC "seconds s" -uint32_t mineip = 0xffffffff; -uint32_t maxeip = 0; - -/* a hash structure to hold the arc */ -struct arec { - uint32_t eip; - uint32_t from; - uint32_t count; - UT_hash_handle hh; -}; - -struct arec *arc = NULL; - -void note_arc(uint32_t eip, uint32_t from) -{ - struct arec *s; - - HASH_FIND_INT(arc, &eip, s); - if (s == NULL) { - s = malloc(sizeof(struct arec)); - s->eip = eip; - s->from = from; - s->count = 1; - if (eip > maxeip) - maxeip = eip; - if (eip < mineip) - maxeip = eip; - - HASH_ADD_INT(arc, eip, s); - } else { - s->count++; - } -} - -int main(int argc, char* argv[]) -{ - FILE *f, *fo; - struct arec *s; - uint32_t eip, from, tmp; - uint8_t tag; - uint16_t hit; - - if (argc != 2) { - fprintf(stderr, "Please specify the coreboot trace log as parameter\n"); - return 1; - } - - f = fopen(argv[1], "r"); - if (f == NULL) { - perror("Unable to open the input file"); - return 1; - } - - fo = fopen("gmon.out", "w+"); - if (fo == NULL) { - perror("Unable to open the output file"); - fclose(f); - return 1; - } - - while (!feof(f)) { - if (fscanf(f, "~%x(%x)%*[^\n]\n", &eip, &from) == 2) { - note_arc(eip, from); - } else if (fscanf(f, "%*c~%x(%x)%*[^\n]\n", &eip, &from) == 2) { - note_arc(eip, from); - } else { - /* just drop a line */ - tmp = fscanf(f, "%*[^\n]\n"); - } - } - - /* write gprof header */ - fwrite(GMON_MAGIC, 1, sizeof(GMON_MAGIC) - 1, fo); - tmp = GMON_VERSION; - fwrite(&tmp, 1, sizeof(tmp), fo); - tmp = 0; - fwrite(&tmp, 1, sizeof(tmp), fo); - fwrite(&tmp, 1, sizeof(tmp), fo); - fwrite(&tmp, 1, sizeof(tmp), fo); - /* write fake histogram */ - tag = GMON_TAG_TIME_HIST; - fwrite(&tag, 1, sizeof(tag), fo); - fwrite(&mineip, 1, sizeof(mineip), fo); - fwrite(&maxeip, 1, sizeof(maxeip), fo); - /* size of histogram */ - tmp = 1; - fwrite(&tmp, 1, sizeof(tmp), fo); - /* prof rate */ - tmp = 1000; - fwrite(&tmp, 1, sizeof(tmp), fo); - fwrite(GMON_SEC, 1, sizeof(GMON_SEC) - 1, fo); - hit = 1; - fwrite(&hit, 1, sizeof(hit), fo); - - /* write call graph data */ - tag = GMON_TAG_CG_ARC; - for (s = arc; s != NULL; s = s->hh.next) { - fwrite(&tag, 1, sizeof(tag), fo); - fwrite(&s->from, 1, sizeof(s->from), fo); - fwrite(&s->eip, 1, sizeof(s->eip), fo); - fwrite(&s->count, 1, sizeof(s->count), fo); - } - - fclose(fo); - fclose(f); - - return 0; -} diff --git a/util/genprof/log2dress b/util/genprof/log2dress deleted file mode 100755 index a7ec4bfdbd..0000000000 --- a/util/genprof/log2dress +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env bash -#Parse a log and get back the function names and line numbers -#Provide a log file as first argument - -#Please rewrite to something more saner ! - -cat $1 | while read line ; do -A=`echo $line | cut -c 1` - -if [ "$A" = '~' ] ; then -FROM=`echo $line | tr \~ \( | tr \) \( | awk -F\( '{print $3}'` -TO=`echo $line | tr \~ \( | tr \) \(|awk -F\( '{print $2}'` -addr2line -e ../../build/cbfs/fallback/ramstage.debug "$FROM" | tr -d "\n" -echo -n " calls " -addr2line -e ../../build/cbfs/fallback/ramstage.debug "$TO" -else -echo "$line" -fi - -done