Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
arch/x86: Use TSC_MONOTONIC_TIMER for timestamps
Implementations with monotonic_timer can take benefit of the generic implementations of timestamp_get() and timestamp_freq_tick_mhz() in lib/timestamp.c.
Change-Id: I0adfbdeb456fbd6769fbd7f39421f797369876a8 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/Kconfig M src/arch/x86/Makefile.inc M src/cpu/x86/tsc/Makefile.inc R src/cpu/x86/tsc/timestamp.c M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/monotonic_timer.c 6 files changed, 12 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/36529/1
diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index 502e774..6c1dd48 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -291,21 +291,6 @@ the acpigen function. Default is _PR.CPxx. Note that you need the \ escape character in the string.
-config COLLECT_TIMESTAMPS_NO_TSC - bool - default n - depends on COLLECT_TIMESTAMPS - help - Use a non-TSC platform-dependent source for timestamps. - -config COLLECT_TIMESTAMPS_TSC - bool - default y if !COLLECT_TIMESTAMPS_NO_TSC - default n - depends on COLLECT_TIMESTAMPS - help - Use the TSC as the timestamp source. - config PAGING_IN_CACHE_AS_RAM bool default n diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 8d00174..ffbf0f3 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -110,7 +110,6 @@
bootblock-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c bootblock-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S -bootblock-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c bootblock-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
bootblock-y += id.S @@ -198,8 +197,6 @@ # which just calls verstage(). verstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += verstage.c
-verstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c - verstage-libs ?=
$(eval $(call early_x86_assembly_entry_rule,verstage)) @@ -230,7 +227,6 @@ romstage-y += memmove.c romstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c romstage-y += postcar_loader.c -romstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c romstage-$(CONFIG_ARCH_ROMSTAGE_X86_32) += walkcbfs.S
romstage-srcs += $(wildcard $(src)/mainboard/$(MAINBOARDDIR)/romstage.c) @@ -268,7 +264,6 @@ postcar-y += memlayout.ld postcar-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c postcar-y += postcar.c -postcar-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
LDFLAGS_postcar += -Map $(objcbfs)/postcar.map
@@ -316,7 +311,6 @@ ramstage-y += tables.c ramstage-$(CONFIG_COOP_MULTITASKING) += thread.c ramstage-$(CONFIG_COOP_MULTITASKING) += thread_switch.S -ramstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c ramstage-$(CONFIG_HAVE_ACPI_RESUME) += wakeup.S
ifeq ($(CONFIG_ARCH_RAMSTAGE_X86_32),y) diff --git a/src/cpu/x86/tsc/Makefile.inc b/src/cpu/x86/tsc/Makefile.inc index b3925b5..0afd05b 100644 --- a/src/cpu/x86/tsc/Makefile.inc +++ b/src/cpu/x86/tsc/Makefile.inc @@ -4,3 +4,9 @@ verstage-$(CONFIG_UDELAY_TSC) += delay_tsc.c postcar-$(CONFIG_UDELAY_TSC) += delay_tsc.c smm-$(CONFIG_UDELAY_TSC) += delay_tsc.c + +bootblock-$(CONFIG_LAPIC_MONOTONIC_TIMER) += timestamp.c +ramstage-$(CONFIG_LAPIC_MONOTONIC_TIMER) += timestamp.c +romstage-$(CONFIG_LAPIC_MONOTONIC_TIMER) += timestamp.c +verstage-$(CONFIG_LAPIC_MONOTONIC_TIMER) += timestamp.c +postcar-$(CONFIG_LAPIC_MONOTONIC_TIMER) += timestamp.c diff --git a/src/arch/x86/timestamp.c b/src/cpu/x86/tsc/timestamp.c similarity index 86% rename from src/arch/x86/timestamp.c rename to src/cpu/x86/tsc/timestamp.c index 8cf0f96..0100021 100644 --- a/src/arch/x86/timestamp.c +++ b/src/cpu/x86/tsc/timestamp.c @@ -21,10 +21,6 @@
int timestamp_tick_freq_mhz(void) { - /* Chipsets that have a constant TSC provide this value correctly. */ - if (tsc_constant_rate()) - return tsc_freq_mhz(); - /* Filling tick_freq_mhz = 0 in timestamps-table will trigger * userspace utility to try deduce it from the running system. */ diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig index 0d6f2ff..1db6d55 100644 --- a/src/soc/amd/stoneyridge/Kconfig +++ b/src/soc/amd/stoneyridge/Kconfig @@ -46,7 +46,6 @@ select ARCH_RAMSTAGE_X86_32 select X86_AMD_FIXED_MTRRS select ACPI_AMD_HARDWARE_SLEEP_VALUES - select COLLECT_TIMESTAMPS_NO_TSC select DRIVERS_I2C_DESIGNWARE select GENERIC_GPIO_LIB select GENERIC_UDELAY diff --git a/src/soc/amd/stoneyridge/monotonic_timer.c b/src/soc/amd/stoneyridge/monotonic_timer.c index 7ea571f..00514da 100644 --- a/src/soc/amd/stoneyridge/monotonic_timer.c +++ b/src/soc/amd/stoneyridge/monotonic_timer.c @@ -15,17 +15,11 @@
#include <cpu/x86/msr.h> #include <timer.h> -#include <timestamp.h>
#define CU_PTSC_MSR 0xc0010280 #define PTSC_FREQ_MHZ 100
-void timer_monotonic_get(struct mono_time *mt) -{ - mono_time_set_usecs(mt, timestamp_get()); -} - -uint64_t timestamp_get(void) +static uint64_t ptsc_get(void) { unsigned long long val; msr_t msr; @@ -36,3 +30,8 @@
return val / PTSC_FREQ_MHZ; } + +void timer_monotonic_get(struct mono_time *mt) +{ + mono_time_set_usecs(mt, ptsc_get()); +}
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36529
to look at the new patch set (#3).
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
arch/x86: Use TSC_MONOTONIC_TIMER for timestamps
Implementations with TSC_MONOTONIC_TIMER can take benefit of the generic implementations of timestamp_get() and timestamp_freq_tick_mhz() in lib/timestamp.c.
The reasoning for keeping COLLECT_TIMESTAMPS_TSC is that family10h-15h monotonic timer and LAPIC_MONOTONIC_TIMER currently do not run in all stages.
Change-Id: I0adfbdeb456fbd6769fbd7f39421f797369876a8 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/Kconfig M src/cpu/amd/family_10h-family_15h/Kconfig M src/cpu/x86/Kconfig M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/monotonic_timer.c 5 files changed, 10 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/36529/3
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Patch Set 4: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Patch Set 4: Code-Review-2
Apparently some monotonic timers reset to zero for every stage.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36529/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36529/4//COMMIT_MSG@11 PS4, Line 11: timestamp_freq_tick_mhz() in lib/timestamp.c. I think you should also indicate that this means TSC is not used for timestamps anymore by default and only microsecond granularity timestamps are provided with out someone taking action.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Patch Set 4: -Code-Review
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36529/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36529/4//COMMIT_MSG@11 PS4, Line 11: timestamp_freq_tick_mhz() in lib/timestamp.c.
I think you should also indicate that this means TSC is not used for timestamps anymore by default a […]
Let me know if it was just bad idea to collect timestamps from monotonic timer via timer_monotonic_get(). Or if COLLECT_TIMESTAMPS for ChromeOS would forbid the granularity change.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36529/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36529/4//COMMIT_MSG@11 PS4, Line 11: timestamp_freq_tick_mhz() in lib/timestamp.c.
Let me know if it was just bad idea to collect timestamps from monotonic timer via timer_monotonic_g […]
Chrome OS wouldn't forbid it. We can get away with it, but if we are switching sources then we should note that raw TSC readings are no longer provided.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Marshall Dawson, Arthur Heymans, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36529
to look at the new patch set (#5).
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
arch/x86: Use TSC_MONOTONIC_TIMER for timestamps
Implementations with TSC_MONOTONIC_TIMER can take benefit of the generic implementations of timestamp_get() and timestamp_freq_tick_mhz() in lib/timestamp.c.
The precision of generated timestamps table is reduced to microseconds with this commit.
The reasoning for keeping COLLECT_TIMESTAMPS_TSC is that family10h-15h monotonic timer and LAPIC_MONOTONIC_TIMER currently do not run in all stages.
Change-Id: I0adfbdeb456fbd6769fbd7f39421f797369876a8 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/Kconfig M src/cpu/x86/Kconfig M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/monotonic_timer.c 4 files changed, 9 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/36529/5
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36529/5/src/arch/x86/Kconfig File src/arch/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/36529/5/src/arch/x86/Kconfig@252 PS5, Line 252: COLLECT_TIMESTAMPS_TSC I don't think anything in the tree selects this at this point. Do we want to leave it lingering as an option or remove it all together? It's pretty simple to bring back.
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36529 )
Change subject: arch/x86: Use TSC_MONOTONIC_TIMER for timestamps ......................................................................
Abandoned
-
Aaron Durbin (Code Review) -
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)