Change in coreboot[master]: [WIP] monotonic_timer: Make them SMP safe

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

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

Change subject: [WIP] monotonic_timer: Make them SMP safe
......................................................................

[WIP] monotonic_timer: Make them SMP safe

Fixes regression with commit 45ddb4344

console,boot_state: Exclude printk() from reported times

This was witnessed to cause boot regressions together with
LAPIC_MONOTONIC_TIMER=y. The code in cpu/x86/lapic/apic_timer.c
for timer_monotonic_get() is not SMP safe as LAPIC timers do not
run as synchronised as TSCs.

TBD: Time spent in printk() for APs is now incorrect in logs.

Change-Id: I1ea2c1e7172f8ab3692b42dee3f669c5942d864a
Signed-off-by: Kyösti Mälkki 
---
M src/console/printk.c
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37398/1

diff --git a/src/console/printk.c b/src/console/printk.c
index a08dd2f..a01ae93 100644
--- a/src/console/printk.c
+++ b/src/console/printk.c
@@ -32,13 +32,13 @@

static void console_time_run(void)
 {
-	if (TRACK_CONSOLE_TIME)
+	if (TRACK_CONSOLE_TIME && boot_cpu())
    	timer_monotonic_get(&mt_start);
 }

static void console_time_stop(void)
 {
-	if (TRACK_CONSOLE_TIME) {
+	if (TRACK_CONSOLE_TIME && boot_cpu()) {
    	timer_monotonic_get(&mt_stop);
    	console_usecs += mono_time_diff_microseconds(&mt_start, &mt_stop);
    }

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

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37398

to look at the new patch set (#2).

Change subject: [WIP] monotonic_timer: Make them SMP safe
......................................................................

[WIP] monotonic_timer: Make them SMP safe

Fixes regression with commit 45ddb4344

console,boot_state: Exclude printk() from reported times

This was witnessed to cause boot regressions together with
LAPIC_MONOTONIC_TIMER=y. The code in cpu/x86/lapic/apic_timer.c
for timer_monotonic_get() is not SMP safe as LAPIC timers do not
run as synchronised as TSCs.

TBD: Time spent in printk() for APs is now incorrect in logs.

Change-Id: I1ea2c1e7172f8ab3692b42dee3f669c5942d864a
Signed-off-by: Kyösti Mälkki 
---
M src/console/printk.c
1 file changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37398/2

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: [WIP] monotonic_timer: Make them SMP safe
......................................................................

Patch Set 2:

CB:37396 is revert for 4.11_branch.

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: [WIP] monotonic_timer: Make them SMP safe
......................................................................

Patch Set 2:

Here's the regression discovered in pcengines/apu2 log:

CPU_CLUSTER: 0 init ...
  ...
  CPU 0x02 would not start!
  CPU 0x02 did not initialize!
  CPU_CLUSTER: 0 init finished in 65560584 usecs  (<= should be ~130ms)
  ...
  BS: BS_DEV_INIT times (ms): entry 0 run 65530 exit 0

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: [WIP] monotonic_timer: Make them SMP safe
......................................................................

Patch Set 2: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: [WIP] monotonic_timer: Make them SMP safe
......................................................................

Patch Set 2: Code-Review+1

Change in coreboot[master]: monotonic_timer: Avoid calls from APs

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

Hello Aaron Durbin, Angel Pons, Arthur Heymans, Patrick Rudolph, Marshall Dawson, Matt DeVillier, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37398

to look at the new patch set (#3).

Change subject: monotonic_timer: Avoid calls from APs
......................................................................

monotonic_timer: Avoid calls from APs

Fixes regression with commit 45ddb4344

console,boot_state: Exclude printk() from reported times

This was witnessed to cause boot regressions together with
LAPIC_MONOTONIC_TIMER=y. The code in cpu/x86/lapic/apic_timer.c
for timer_monotonic_get() is not SMP safe as LAPIC timers do not
run as synchronised as TSCs.

TBD: Time spent in printk() for APs is now incorrect in logs.

Change-Id: I1ea2c1e7172f8ab3692b42dee3f669c5942d864a
Signed-off-by: Kyösti Mälkki 
---
M src/console/printk.c
1 file changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37398/3

Change in coreboot[master]: monotonic_timer: Avoid calls from APs

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: monotonic_timer: Avoid calls from APs
......................................................................

Patch Set 3:

Michal, can you stress-test this, grepping log for "CPU xxx did not initialize" string?

Change in coreboot[master]: monotonic_timer: Avoid calls from APs

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: monotonic_timer: Avoid calls from APs
......................................................................

Patch Set 3: Code-Review-2

Change in coreboot[master]: console,monotonic_timer: Avoid calls from APs

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

Hello Aaron Durbin, Angel Pons, Arthur Heymans, Patrick Rudolph, Marshall Dawson, Matt DeVillier, build bot (Jenkins), Nico Huber, Furquan Shaikh, Michał Żygowski,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37398

to look at the new patch set (#4).

Change subject: console,monotonic_timer: Avoid calls from APs
......................................................................

console,monotonic_timer: Avoid calls from APs

The code in cpu/x86/lapic/apic_timer.c for timer_monotonic_get()
is not SMP safe as LAPIC timers do not run as synchronised as TSCs.

The times reported for console for boot_states does not accumulate
from APs now. Also remove console time tracking from ENV_SMM.

Change-Id: I1ea2c1e7172f8ab3692b42dee3f669c5942d864a
Signed-off-by: Kyösti Mälkki 
---
M src/console/printk.c
1 file changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/37398/4

Change in coreboot[master]: console,monotonic_timer: Avoid calls from APs

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

Kyösti Mälkki has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: console,monotonic_timer: Avoid calls from APs
......................................................................

Removed Code-Review-2 by Kyösti Mälkki

Change in coreboot[master]: console,monotonic_timer: Avoid calls from APs

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: console,monotonic_timer: Avoid calls from APs
......................................................................

Patch Set 4:

Gonna run some automated tests with serial output parsing today (like 100 power cycle loops) and see how it goes.

Change in coreboot[master]: console,monotonic_timer: Avoid calls from APs

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: console,monotonic_timer: Avoid calls from APs
......................................................................

Patch Set 4:

After 6 power cycles the issue occurred. So timer is not the case

Change in coreboot[master]: console,monotonic_timer: Avoid calls from APs

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: console,monotonic_timer: Avoid calls from APs
......................................................................

Patch Set 5: Code-Review+2

Change in coreboot[master]: console,monotonic_timer: Avoid calls from APs

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37398 )

Change subject: console,monotonic_timer: Avoid calls from APs
......................................................................

console,monotonic_timer: Avoid calls from APs

The code in cpu/x86/lapic/apic_timer.c for timer_monotonic_get()
is not SMP safe as LAPIC timers do not run as synchronised as TSCs.

The times reported for console for boot_states does not accumulate
from APs now. Also remove console time tracking from ENV_SMM.

Change-Id: I1ea2c1e7172f8ab3692b42dee3f669c5942d864a
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37398
Reviewed-by: Arthur Heymans 
Tested-by: build bot (Jenkins) 
---
M src/console/printk.c
1 file changed, 3 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved

diff --git a/src/console/printk.c b/src/console/printk.c
index a08dd2f..b32fadb 100644
--- a/src/console/printk.c
+++ b/src/console/printk.c
@@ -25,20 +25,20 @@

DECLARE_SPIN_LOCK(console_lock)

-#define TRACK_CONSOLE_TIME (CONFIG(HAVE_MONOTONIC_TIMER))
+#define TRACK_CONSOLE_TIME (!ENV_SMM && CONFIG(HAVE_MONOTONIC_TIMER))

static struct mono_time mt_start, mt_stop;
 static long console_usecs;

static void console_time_run(void)
 {
-	if (TRACK_CONSOLE_TIME)
+	if (TRACK_CONSOLE_TIME && boot_cpu())
    	timer_monotonic_get(&mt_start);
 }

static void console_time_stop(void)
 {
-	if (TRACK_CONSOLE_TIME) {
+	if (TRACK_CONSOLE_TIME && boot_cpu()) {
    	timer_monotonic_get(&mt_stop);
    	console_usecs += mono_time_diff_microseconds(&mt_start, &mt_stop);
    }