Kyösti Mälkki has uploaded this change for review.

View Change

timestamps: Make ENV_ROMSTAGE_OR_BEFORE improvements

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 just
switch the pointer to CBMEM and copy the data before
CAR gets torn down.

TBD: This probably breaks FSP1_0 which returns to coreboot
proper only after tearing down CAR. I have an idea how to
fix it though.

Change-Id: I87370f62db23318069b6fd56ba0d1171d619cb8a
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
---
M src/arch/x86/car.ld
M src/include/timestamp.h
M src/lib/timestamp.c
3 files changed, 24 insertions(+), 29 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/35032/1
diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index d19818c..25b667c 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -60,17 +60,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_ehci_dbg_info_start = .;
/* Reserve sizeof(struct ehci_dbg_info). */
. += 80;
_car_ehci_dbg_info_end = .;
+
+ _car_relocatable_data_start = .;
+
/* _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..1fa568c 100644
--- a/src/include/timestamp.h
+++ b/src/include/timestamp.h
@@ -22,11 +22,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)
+ * 1. ENV_ROMSTAGE_OR_BEFORE (bootblock, romstage, verstage, etc) and
+ * 2. ENV_RAMSTAGE (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.
+ * it's up to the chipset/arch to call timestamp_init() in the former stages.
*/
void timestamp_init(uint64_t base);
/*
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c
index 89152fd..3e9914e 100644
--- a/src/lib/timestamp.c
+++ b/src/lib/timestamp.c
@@ -41,7 +41,7 @@

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
@@ -58,6 +58,10 @@
*/
static struct timestamp_cache timestamp_cache;

+/* 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;
+
enum {
TIMESTAMP_CACHE_UNINITIALIZED = 0,
TIMESTAMP_CACHE_INITIALIZED,
@@ -87,7 +91,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,30 +132,21 @@

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;

+ ts_table = car_get_var(glob_ts_table);
if (ts_table != NULL)
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;
- }
+ if (ts_table == NULL && HAS_CBMEM)
+ ts_table = cbmem_find(CBMEM_ID_TIMESTAMP);

- /* 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_var(glob_ts_table, ts_table);
return ts_table;
}

@@ -237,7 +232,7 @@
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;
@@ -270,6 +265,7 @@

if (ts_cbmem_table == NULL) {
printk(BIOS_ERR, "ERROR: No timestamp table allocated\n");
+ car_set_var(glob_ts_table, ts_cbmem_table);
return;
}

@@ -309,6 +305,7 @@
ts_cbmem_table->tick_freq_mhz = timestamp_tick_freq_mhz();

/* Cache no longer required. */
+ car_set_var(glob_ts_table, ts_cbmem_table);
ts_cache_table->num_entries = 0;
ts_cache->cache_state = TIMESTAMP_CACHE_NOT_NEEDED;
}

To view, visit change 35032. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I87370f62db23318069b6fd56ba0d1171d619cb8a
Gerrit-Change-Number: 35032
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-MessageType: newchange