Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74016 )
(
5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/amd/common/cpu: use TSC_MONOTONIC_TIMER for SOC_AMD_COMMON_BLOCK_TSC ......................................................................
soc/amd/common/cpu: use TSC_MONOTONIC_TIMER for SOC_AMD_COMMON_BLOCK_TSC
All AMD SoCs with Zen-based CPU cores are already using timestamps based on the TSC counter, so use the existing common infrastructure instead of reimplementing it in a similar way.
The behavior of the code changes slightly, but results in identical timestamps. The timestamp_get implementation in soc/amd/common/block/cpu divided the result of rdtscll() in timestamp_get by the result of tsc_freq_mhz() and didn't override the weak timestamp_tick_freq_mhz implementation that returns 1. The non AMD specific code returns the result of rdtscll() in timestamp_get, but returns tsc_freq_mhz() instead of 1 in timestamp_tick_freq_mhz, so we still get the correct timestamps.
TEST=The raw timestamps printed on the serial console are now multiplied by the expected factor of the TSC frequency in MHz.
TEST=Normalized timestamps printed on the serial console by the x86 code don't change significantly on Mandolin when comparing before and after this patch. A slight variation in the timestamps is expected. An example would be:
Before: CPU_CLUSTER: 0 init finished in 630 msecs After: CPU_CLUSTER: 0 init finished in 629 msecs
TEST=The calculations of the time spent in verstage on PSP before entering the bootblock on Guybrush result in similar times when multiplying the value before the patch with the TSC frequency in the case with the patch applied. The raw values printed on the serial console by the verstage on PSP use the 1us time base, but the timestamp logs that end up in CBMEM will be fixed up to use the same time base as the x86 part of coreboot.
Signed-off-by: Felix Held felix-coreboot@felixheld.de Change-Id: I57b732e5c78222d278d3328b26bb8decb8f4783e Reviewed-on: https://review.coreboot.org/c/coreboot/+/74016 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Martin Roth martin.roth@amd.corp-partner.google.com Reviewed-by: Paul Menzel paulepanter@mailbox.org --- M src/soc/amd/common/block/cpu/Kconfig M src/soc/amd/common/block/cpu/noncar/bootblock.c M src/soc/amd/common/block/cpu/tsc/Makefile.inc D src/soc/amd/common/block/cpu/tsc/monotonic_timer.c M src/soc/amd/common/vboot/vboot_bootblock.c 5 files changed, 66 insertions(+), 33 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Martin Roth: Looks good to me, approved
diff --git a/src/soc/amd/common/block/cpu/Kconfig b/src/soc/amd/common/block/cpu/Kconfig index 5a1f86a..f7946c3 100644 --- a/src/soc/amd/common/block/cpu/Kconfig +++ b/src/soc/amd/common/block/cpu/Kconfig @@ -98,9 +98,9 @@
config SOC_AMD_COMMON_BLOCK_TSC bool - select COLLECT_TIMESTAMPS_NO_TSC # selected use SoC-specific timestamp function select TSC_SYNC_LFENCE select UDELAY_TSC + select TSC_MONOTONIC_TIMER help Select this option to add the common functions for getting the TSC frequency of AMD family 17h, 19h and 1Ah CPUs/APUs and to provide diff --git a/src/soc/amd/common/block/cpu/noncar/bootblock.c b/src/soc/amd/common/block/cpu/noncar/bootblock.c index 13bac18..9ef899e 100644 --- a/src/soc/amd/common/block/cpu/noncar/bootblock.c +++ b/src/soc/amd/common/block/cpu/noncar/bootblock.c @@ -16,12 +16,6 @@ write_resume_eip(); enable_pci_mmconf();
- /* - * base_timestamp is raw tsc value. We need to divide by tsc_freq_mhz - * to get micro-seconds granularity. - */ - base_timestamp /= tsc_freq_mhz(); - if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) boot_with_psp_timestamp(base_timestamp);
diff --git a/src/soc/amd/common/block/cpu/tsc/Makefile.inc b/src/soc/amd/common/block/cpu/tsc/Makefile.inc index 67bd6fb..ade4970 100644 --- a/src/soc/amd/common/block/cpu/tsc/Makefile.inc +++ b/src/soc/amd/common/block/cpu/tsc/Makefile.inc @@ -23,18 +23,13 @@ ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_TSC),y)
bootblock-y += tsc_freq.c -bootblock-y += monotonic_timer.c
verstage_x86-y += tsc_freq.c -verstage_x86-y += monotonic_timer.c
romstage-y += tsc_freq.c -romstage-y += monotonic_timer.c
ramstage-y += tsc_freq.c -ramstage-y += monotonic_timer.c
smm-y += tsc_freq.c -smm-y += monotonic_timer.c
endif # CONFIG_SOC_AMD_COMMON_BLOCK_TSC diff --git a/src/soc/amd/common/block/cpu/tsc/monotonic_timer.c b/src/soc/amd/common/block/cpu/tsc/monotonic_timer.c deleted file mode 100644 index 4cedc3d..0000000 --- a/src/soc/amd/common/block/cpu/tsc/monotonic_timer.c +++ /dev/null @@ -1,15 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <cpu/x86/tsc.h> -#include <timer.h> -#include <timestamp.h> - -void timer_monotonic_get(struct mono_time *mt) -{ - mono_time_set_usecs(mt, timestamp_get()); -} - -uint64_t timestamp_get(void) -{ - return rdtscll() / tsc_freq_mhz(); -} diff --git a/src/soc/amd/common/vboot/vboot_bootblock.c b/src/soc/amd/common/vboot/vboot_bootblock.c index 1375e98..07039fd 100644 --- a/src/soc/amd/common/vboot/vboot_bootblock.c +++ b/src/soc/amd/common/vboot/vboot_bootblock.c @@ -2,9 +2,22 @@
#include <bootblock_common.h> #include <console/console.h> +#include <cpu/x86/tsc.h> #include <soc/psp_transfer.h> #include <symbols.h> #include <timestamp.h> +#include <types.h> + +/* + * Verstage on PSP uses the weak timestamp_tick_freq_mhz implementation returning 1, so the + * unit of the timestamps in the transfer buffer is microseconds. The x86 side uses the TSC + * rate as reference for the timestamps, so the values from the PSP transfer buffer need to be + * multiplied by the TSC frequency in MHz. + */ +static uint64_t timestamp_from_usec(uint64_t usec) +{ + return usec * tsc_freq_mhz(); +}
void boot_with_psp_timestamp(uint64_t base_timestamp) { @@ -20,14 +33,14 @@ * info->timestamp is PSP's timestamp (in microseconds) * when x86 processor is released. */ - uint64_t psp_last_ts = info->timestamp; + uint64_t psp_last_ts_usec = info->timestamp;
int i; struct timestamp_table *psp_ts_table = (struct timestamp_table *)(void *) ((uintptr_t)_transfer_buffer + info->timestamp_offset); /* new base_timestamp will be offset for all PSP timestamps. */ - base_timestamp -= psp_last_ts; + base_timestamp -= timestamp_from_usec(psp_last_ts_usec);
for (i = 0; i < psp_ts_table->num_entries; i++) { struct timestamp_entry *tse = &psp_ts_table->entries[i]; @@ -37,11 +50,12 @@ * to make it absolute then add base_timestamp again since * it'll be a new base_time. * - * We don't need to convert unit since both PSP and coreboot - * will use 1us granularity. - * + * Verstage on PSP uses a 1 microsecond timestamp granularity while the x86 + * part of coreboot uses the TSC tick time as granularity, so this needs to be + * converted. */ - tse->entry_stamp += psp_ts_table->base_time + base_timestamp; + tse->entry_stamp += timestamp_from_usec(psp_ts_table->base_time) + + base_timestamp; }
bootblock_main_with_timestamp(base_timestamp, psp_ts_table->entries,