[M] Change in coreboot[master]: soc/amd/common/cpu: use TSC_MONOTONIC_TIMER for SOC_AMD_COMMON_BLOCK_TSC

[Felix Held (Code Review)]

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 
Change-Id: I57b732e5c78222d278d3328b26bb8decb8f4783e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/74016
Tested-by: build bot (Jenkins) 
Reviewed-by: Martin Roth 
Reviewed-by: Paul Menzel 
---
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,