[coreboot-gerrit] Change in coreboot[master]: timestamps: Make ENV_ROMSTAGE_OR_BEFORE improvements

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35032 )

Change subject: timestamps: Make ENV_ROMSTAGE_OR_BEFORE improvements
......................................................................

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 
---
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;
 }