Kyösti Mälkki submitted this change.
timestamps: Improve collection for ENV_ROMSTAGE_OR_BEFORE
Keep track of the active timestamp table location using
a CAR_GLOBAL variable. Done this way, the entire table
can be located outside _car_relocatable_data and we only
switch the pointer to CBMEM and copy the data before
CAR gets torn down.
Fix comments about requirements of timestamp_init() usage.
Remove timestamp_cache from postcar and ramstage, as CBMEM
is available early on.
Change-Id: I87370f62db23318069b6fd56ba0d1171d619cb8a
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35032
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M src/arch/x86/car.ld
M src/include/timestamp.h
M src/lib/hardwaremain.c
M src/lib/timestamp.c
4 files changed, 47 insertions(+), 72 deletions(-)
diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index 9d5ab28..2fbdb71 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -56,17 +56,16 @@
. += 32;
_epdpt = .;
#endif
- _car_relocatable_data_start = .;
- /* The timestamp implementation relies on this storage to be around
- * after migration. One of the fields indicates not to use it as the
- * backing store once cbmem comes online. Therefore, this data needs
- * to reside in the migrated area (between _car_relocatable_data_start
- * and _car_relocatable_data_end). */
+
TIMESTAMP(., 0x200)
+
+ _car_relocatable_data_start = .;
+
_car_ehci_dbg_info_start = .;
/* Reserve sizeof(struct ehci_dbg_info). */
. += 80;
_car_ehci_dbg_info_end = .;
+
/* _car_global_start and _car_global_end provide symbols to per-stage
* variables that are not shared like the timestamp and the pre-ram
* cbmem console. This is useful for clearing this area on a per-stage
diff --git a/src/include/timestamp.h b/src/include/timestamp.h
index 04d5c12..951032c 100644
--- a/src/include/timestamp.h
+++ b/src/include/timestamp.h
@@ -21,12 +21,10 @@
#if CONFIG(COLLECT_TIMESTAMPS)
/*
- * timestamp_init() needs to be called once for each of these cases:
- * 1. __PRE_RAM__ (bootblock, romstage, verstage, etc) and
- * 2. !__PRE_RAM__ (ramstage)
- * The latter is taken care of by the generic coreboot infrastructure so
- * it's up to the chipset/arch to call timestamp_init() in *one* of
- * the __PRE_RAM__ stages. If multiple calls are made timestamps will be lost.
+ * timestamp_init() needs to be called once in *one* of the ENV_ROMSTAGE_OR_BEFORE
+ * stages (bootblock, romstage, verstage, etc). It's up to the chipset/arch
+ * to make the call in the earliest stage, otherwise some timestamps will be lost.
+ * For x86 ENV_ROMSTAGE call must be made before CAR is torn down.
*/
void timestamp_init(uint64_t base);
/*
diff --git a/src/lib/hardwaremain.c b/src/lib/hardwaremain.c
index a4c154e..2ff2b8c 100644
--- a/src/lib/hardwaremain.c
+++ b/src/lib/hardwaremain.c
@@ -461,9 +461,6 @@
*/
cbmem_initialize();
- /* Record current time, try to locate timestamps in CBMEM. */
- timestamp_init(timestamp_get());
-
timestamp_add_now(TS_START_RAMSTAGE);
post_code(POST_ENTRY_RAMSTAGE);
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c
index 89152fd..bb5c114 100644
--- a/src/lib/timestamp.c
+++ b/src/lib/timestamp.c
@@ -31,7 +31,6 @@
#define MAX_BSS_TIMESTAMP_CACHE 16
struct __packed timestamp_cache {
- uint32_t cache_state;
struct timestamp_table table;
/* The struct timestamp_table has a 0 length array as its last field.
* The following 'entries' array serves as the storage space for the
@@ -41,28 +40,23 @@
DECLARE_OPTIONAL_REGION(timestamp);
-#if defined(__PRE_RAM__)
+#if ENV_ROMSTAGE_OR_BEFORE
#define USE_TIMESTAMP_REGION (REGION_SIZE(timestamp) > 0)
#else
#define USE_TIMESTAMP_REGION 0
#endif
-/* The cache location will sit in BSS when in ramstage/postcar. */
-#define TIMESTAMP_CACHE_IN_BSS (ENV_RAMSTAGE || ENV_POSTCAR)
-
-#define HAS_CBMEM (ENV_ROMSTAGE || ENV_RAMSTAGE || ENV_POSTCAR)
+/* Currently we never store timestamp cache in .bss. */
+#define TIMESTAMP_CACHE_IN_BSS 0
/*
- * Storage of cache entries during ramstage/postcar prior to cbmem coming
- * online.
+ * Storage of cache entries prior to cbmem coming online.
*/
static struct timestamp_cache timestamp_cache;
-enum {
- TIMESTAMP_CACHE_UNINITIALIZED = 0,
- TIMESTAMP_CACHE_INITIALIZED,
- TIMESTAMP_CACHE_NOT_NEEDED,
-};
+/* This points to the active timestamp_table and can change within a stage.
+ as CBMEM comes available. */
+static struct timestamp_table *glob_ts_table CAR_GLOBAL;
static void timestamp_cache_init(struct timestamp_cache *ts_cache,
uint64_t base)
@@ -70,7 +64,6 @@
ts_cache->table.num_entries = 0;
ts_cache->table.max_entries = MAX_BSS_TIMESTAMP_CACHE;
ts_cache->table.base_time = base;
- ts_cache->cache_state = TIMESTAMP_CACHE_INITIALIZED;
if (USE_TIMESTAMP_REGION)
ts_cache->table.max_entries = (REGION_SIZE(timestamp) -
@@ -87,7 +80,7 @@
} else if (USE_TIMESTAMP_REGION) {
if (REGION_SIZE(timestamp) < sizeof(*ts_cache))
BUG();
- ts_cache = car_get_var_ptr((void *)_timestamp);
+ ts_cache = (void *)_timestamp;
}
return ts_cache;
@@ -128,33 +121,26 @@
static struct timestamp_table *timestamp_table_get(void)
{
- MAYBE_STATIC_BSS struct timestamp_table *ts_table = NULL;
+ struct timestamp_table *ts_table;
struct timestamp_cache *ts_cache;
- if (ts_table != NULL)
+ ts_table = car_get_ptr(glob_ts_table);
+ if (ts_table)
return ts_table;
ts_cache = timestamp_cache_get();
+ if (ts_cache)
+ ts_table = &ts_cache->table;
- if (ts_cache == NULL) {
- if (HAS_CBMEM)
- ts_table = cbmem_find(CBMEM_ID_TIMESTAMP);
- return ts_table;
- }
-
- /* Cache is required. */
- if (ts_cache->cache_state != TIMESTAMP_CACHE_NOT_NEEDED)
- return &ts_cache->table;
-
- /* Cache shouldn't be used but there's no backing store. */
- if (!HAS_CBMEM)
- return NULL;
-
- ts_table = cbmem_find(CBMEM_ID_TIMESTAMP);
-
+ car_set_ptr(glob_ts_table, ts_table);
return ts_table;
}
+static void timestamp_table_set(struct timestamp_table *ts)
+{
+ car_set_ptr(glob_ts_table, ts);
+}
+
static const char *timestamp_name(enum timestamp_id id)
{
int i;
@@ -212,6 +198,8 @@
{
struct timestamp_cache *ts_cache;
+ assert(ENV_ROMSTAGE_OR_BEFORE);
+
if (!timestamp_should_run())
return;
@@ -222,36 +210,19 @@
return;
}
- /* Timestamps could have already been recovered.
- * In those circumstances honor the cache which sits in BSS
- * as it has already been initialized. */
- if (TIMESTAMP_CACHE_IN_BSS &&
- ts_cache->cache_state != TIMESTAMP_CACHE_UNINITIALIZED)
- return;
-
timestamp_cache_init(ts_cache, base);
+ timestamp_table_set(&ts_cache->table);
}
static void timestamp_sync_cache_to_cbmem(int is_recovery)
{
uint32_t i;
- struct timestamp_cache *ts_cache;
struct timestamp_table *ts_cache_table;
- struct timestamp_table *ts_cbmem_table = NULL;
+ struct timestamp_table *ts_cbmem_table;
if (!timestamp_should_run())
return;
- ts_cache = timestamp_cache_get();
-
- /* No timestamp cache found */
- if (ts_cache == NULL) {
- printk(BIOS_ERR, "ERROR: No timestamp cache found\n");
- return;
- }
-
- ts_cache_table = &ts_cache->table;
-
/* cbmem is being recovered. */
if (is_recovery) {
/* x86 resume path expects timestamps to be reset. */
@@ -270,6 +241,19 @@
if (ts_cbmem_table == NULL) {
printk(BIOS_ERR, "ERROR: No timestamp table allocated\n");
+ timestamp_table_set(NULL);
+ return;
+ }
+
+ /* Seed the timestamp tick frequency in ENV_PAYLOAD_LOADER. */
+ if (ENV_PAYLOAD_LOADER)
+ ts_cbmem_table->tick_freq_mhz = timestamp_tick_freq_mhz();
+
+ ts_cache_table = timestamp_table_get();
+ if (!ts_cache_table) {
+ if (ENV_ROMSTAGE)
+ printk(BIOS_ERR, "ERROR: No timestamp cache found\n");
+ timestamp_table_set(ts_cbmem_table);
return;
}
@@ -294,6 +278,7 @@
* If timestamps only get initialized in ramstage, the base_time from
* timestamp_init() will get ignored and all timestamps will be 0-based.
*/
+
for (i = 0; i < ts_cache_table->num_entries; i++) {
struct timestamp_entry *tse = &ts_cache_table->entries[i];
timestamp_add_table_entry(ts_cbmem_table, tse->entry_id,
@@ -304,13 +289,9 @@
if (ts_cbmem_table->base_time == 0)
ts_cbmem_table->base_time = ts_cache_table->base_time;
- /* Seed the timestamp tick frequency in ENV_PAYLOAD_LOADER. */
- if (ENV_PAYLOAD_LOADER)
- ts_cbmem_table->tick_freq_mhz = timestamp_tick_freq_mhz();
-
/* Cache no longer required. */
+ timestamp_table_set(ts_cbmem_table);
ts_cache_table->num_entries = 0;
- ts_cache->cache_state = TIMESTAMP_CACHE_NOT_NEEDED;
}
void timestamp_rescale_table(uint16_t N, uint16_t M)
To view, visit change 35032. To unsubscribe, or for help writing mail filters, visit settings.