Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35280 )
Change subject: timestamps: Refactor CBMEM hook ......................................................................
timestamps: Refactor CBMEM hook
Separate timestamp_sync_cache_to_cbmem() as it is only used with ENV_ROMSTAGE.
Change-Id: Ibe18a5ecf09b2b87d8ee3e828728ad6a8a262206 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/lib/timestamp.c 1 file changed, 44 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35280/1
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c index 92bd094..3d8999c 100644 --- a/src/lib/timestamp.c +++ b/src/lib/timestamp.c @@ -200,46 +200,14 @@ timestamp_table_set(&ts_cache->table); }
-static void timestamp_sync_cache_to_cbmem(int is_recovery) +static void timestamp_sync_cache_to_cbmem(struct timestamp_table *ts_cbmem_table) { uint32_t i; struct timestamp_table *ts_cache_table; - struct timestamp_table *ts_cbmem_table; - - if (!timestamp_should_run()) - return; - - /* cbmem is being recovered. */ - if (is_recovery) { - /* x86 resume path expects timestamps to be reset. */ - if (CONFIG(ARCH_ROMSTAGE_X86_32) && ENV_ROMSTAGE) - ts_cbmem_table = timestamp_alloc_cbmem_table(); - else { - /* Find existing table in cbmem. */ - ts_cbmem_table = cbmem_find(CBMEM_ID_TIMESTAMP); - /* No existing timestamp table. */ - if (ts_cbmem_table == NULL) - ts_cbmem_table = timestamp_alloc_cbmem_table(); - } - } else - /* First time sync. Add new table. */ - ts_cbmem_table = timestamp_alloc_cbmem_table(); - - 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); + printk(BIOS_ERR, "ERROR: No timestamp cache found\n"); return; }
@@ -276,10 +244,48 @@ ts_cbmem_table->base_time = ts_cache_table->base_time;
/* Cache no longer required. */ - timestamp_table_set(ts_cbmem_table); ts_cache_table->num_entries = 0; }
+static void timestamp_reinit(int is_recovery) +{ + struct timestamp_table *ts_cbmem_table; + + if (!timestamp_should_run()) + return; + + /* cbmem is being recovered. */ + if (is_recovery) { + /* x86 resume path expects timestamps to be reset. */ + if (CONFIG(ARCH_ROMSTAGE_X86_32) && ENV_ROMSTAGE) + ts_cbmem_table = timestamp_alloc_cbmem_table(); + else { + /* Find existing table in cbmem. */ + ts_cbmem_table = cbmem_find(CBMEM_ID_TIMESTAMP); + /* No existing timestamp table. */ + if (ts_cbmem_table == NULL) + ts_cbmem_table = timestamp_alloc_cbmem_table(); + } + } else + /* First time sync. Add new table. */ + ts_cbmem_table = timestamp_alloc_cbmem_table(); + + if (ts_cbmem_table == NULL) { + printk(BIOS_ERR, "ERROR: No timestamp table allocated\n"); + timestamp_table_set(NULL); + return; + } + + if (ENV_ROMSTAGE) + timestamp_sync_cache_to_cbmem(ts_cbmem_table); + + /* Seed the timestamp tick frequency in ENV_PAYLOAD_LOADER. */ + if (ENV_PAYLOAD_LOADER) + ts_cbmem_table->tick_freq_mhz = timestamp_tick_freq_mhz(); + + timestamp_table_set(ts_cbmem_table); +} + void timestamp_rescale_table(uint16_t N, uint16_t M) { uint32_t i; @@ -321,9 +327,9 @@ return (timestamp_get() - ts->base_time) / ts->tick_freq_mhz; }
-ROMSTAGE_CBMEM_INIT_HOOK(timestamp_sync_cache_to_cbmem) -POSTCAR_CBMEM_INIT_HOOK(timestamp_sync_cache_to_cbmem) -RAMSTAGE_CBMEM_INIT_HOOK(timestamp_sync_cache_to_cbmem) +ROMSTAGE_CBMEM_INIT_HOOK(timestamp_reinit) +POSTCAR_CBMEM_INIT_HOOK(timestamp_reinit) +RAMSTAGE_CBMEM_INIT_HOOK(timestamp_reinit)
/* Provide default timestamp implementation using monotonic timer. */ uint64_t __weak timestamp_get(void)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35280 )
Change subject: timestamps: Refactor CBMEM hook ......................................................................
Patch Set 5: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35280 )
Change subject: timestamps: Refactor CBMEM hook ......................................................................
Patch Set 5: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/35280/5/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/c/coreboot/+/35280/5/src/lib/timestamp.c@274 PS5, Line 274: if (CONFIG(ARCH_ROMSTAGE_X86_32) && ENV_ROMSTAGE) nit: There's no real reason to check for x86 here. Currently no non-x86 platforms use CBMEM recovery between boots, but if they did they'd also want to reset timestamps. The whole thing could be simplified to
if (is_recovery && !ENV_ROMSTAGE) { ...find or alloc... } else { ...alloc... }
https://review.coreboot.org/c/coreboot/+/35280/5/src/lib/timestamp.c@283 PS5, Line 283: } else nit: would be nice to fix code style while you're here (needs braces)
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35280 )
Change subject: timestamps: Refactor CBMEM hook ......................................................................
timestamps: Refactor CBMEM hook
Separate timestamp_sync_cache_to_cbmem() as it is only used with ENV_ROMSTAGE.
Change-Id: Ibe18a5ecf09b2b87d8ee3e828728ad6a8a262206 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35280 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Julius Werner jwerner@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/lib/timestamp.c 1 file changed, 44 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Julius Werner: Looks good to me, approved
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c index bb5c114..4983885 100644 --- a/src/lib/timestamp.c +++ b/src/lib/timestamp.c @@ -214,46 +214,14 @@ timestamp_table_set(&ts_cache->table); }
-static void timestamp_sync_cache_to_cbmem(int is_recovery) +static void timestamp_sync_cache_to_cbmem(struct timestamp_table *ts_cbmem_table) { uint32_t i; struct timestamp_table *ts_cache_table; - struct timestamp_table *ts_cbmem_table; - - if (!timestamp_should_run()) - return; - - /* cbmem is being recovered. */ - if (is_recovery) { - /* x86 resume path expects timestamps to be reset. */ - if (CONFIG(ARCH_ROMSTAGE_X86_32) && ENV_ROMSTAGE) - ts_cbmem_table = timestamp_alloc_cbmem_table(); - else { - /* Find existing table in cbmem. */ - ts_cbmem_table = cbmem_find(CBMEM_ID_TIMESTAMP); - /* No existing timestamp table. */ - if (ts_cbmem_table == NULL) - ts_cbmem_table = timestamp_alloc_cbmem_table(); - } - } else - /* First time sync. Add new table. */ - ts_cbmem_table = timestamp_alloc_cbmem_table(); - - 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); + printk(BIOS_ERR, "ERROR: No timestamp cache found\n"); return; }
@@ -290,10 +258,48 @@ ts_cbmem_table->base_time = ts_cache_table->base_time;
/* Cache no longer required. */ - timestamp_table_set(ts_cbmem_table); ts_cache_table->num_entries = 0; }
+static void timestamp_reinit(int is_recovery) +{ + struct timestamp_table *ts_cbmem_table; + + if (!timestamp_should_run()) + return; + + /* cbmem is being recovered. */ + if (is_recovery) { + /* x86 resume path expects timestamps to be reset. */ + if (CONFIG(ARCH_ROMSTAGE_X86_32) && ENV_ROMSTAGE) + ts_cbmem_table = timestamp_alloc_cbmem_table(); + else { + /* Find existing table in cbmem. */ + ts_cbmem_table = cbmem_find(CBMEM_ID_TIMESTAMP); + /* No existing timestamp table. */ + if (ts_cbmem_table == NULL) + ts_cbmem_table = timestamp_alloc_cbmem_table(); + } + } else + /* First time sync. Add new table. */ + ts_cbmem_table = timestamp_alloc_cbmem_table(); + + if (ts_cbmem_table == NULL) { + printk(BIOS_ERR, "ERROR: No timestamp table allocated\n"); + timestamp_table_set(NULL); + return; + } + + if (ENV_ROMSTAGE) + timestamp_sync_cache_to_cbmem(ts_cbmem_table); + + /* Seed the timestamp tick frequency in ENV_PAYLOAD_LOADER. */ + if (ENV_PAYLOAD_LOADER) + ts_cbmem_table->tick_freq_mhz = timestamp_tick_freq_mhz(); + + timestamp_table_set(ts_cbmem_table); +} + void timestamp_rescale_table(uint16_t N, uint16_t M) { uint32_t i; @@ -335,9 +341,9 @@ return (timestamp_get() - ts->base_time) / ts->tick_freq_mhz; }
-ROMSTAGE_CBMEM_INIT_HOOK(timestamp_sync_cache_to_cbmem) -POSTCAR_CBMEM_INIT_HOOK(timestamp_sync_cache_to_cbmem) -RAMSTAGE_CBMEM_INIT_HOOK(timestamp_sync_cache_to_cbmem) +ROMSTAGE_CBMEM_INIT_HOOK(timestamp_reinit) +POSTCAR_CBMEM_INIT_HOOK(timestamp_reinit) +RAMSTAGE_CBMEM_INIT_HOOK(timestamp_reinit)
/* Provide default timestamp implementation using monotonic timer. */ uint64_t __weak timestamp_get(void)
-
Aaron Durbin (Code Review) -
Julius Werner (Code Review)
-
Kyösti Mälkki (Code Review)