coreboot-gerrit November 2019

coreboot-gerrit@coreboot.org

1 participants
2418 discussions

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is mostly optional
by Kyösti Mälkki (Code Review) 01 Nov '19

01 Nov '19

Hello Werner Zeh, Aaron Durbin, Patrick Rudolph, HAOUAS Elyes, Julius Werner, Arthur Heymans, build bot (Jenkins), Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/36527 to look at the new patch set (#5). Change subject: timestamps: COLLECT_TIMESTAMPS is mostly optional ...................................................................... timestamps: COLLECT_TIMESTAMPS is mostly optional It is a user-visible option and enabled by default for ARCH_X86, some consider it as debugging aid only. Therefore platform design should not depend on it. It must remain selected with CHROMEOS and boards are allowed to explicitly select it as well. For siemens/mc_bdx1,mc_aplX boot time will be increased due the use of get_us_since_boot() with COLLECT_TIMESTAMPS=n. When unable to determine if N seconds has elapsed from boot, this turns into a delay of N seconds. Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/soc/amd/picasso/Kconfig M src/soc/amd/stoneyridge/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/skylake/Kconfig 5 files changed, 0 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36527/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 5 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is mostly optional
by Aaron Durbin (Code Review) 01 Nov '19

01 Nov '19

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36527 ) Change subject: timestamps: COLLECT_TIMESTAMPS is mostly optional ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/36527/4//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/36527/4//COMMIT_MSG@9 PS4, Line 9: enabled by default for ARCH_X86 -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Nov 2019 18:21:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is mostly optional
by Aaron Durbin (Code Review) 01 Nov '19

01 Nov '19

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36527 ) Change subject: timestamps: COLLECT_TIMESTAMPS is mostly optional ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Nov 2019 18:20:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is mostly optional
by Kyösti Mälkki (Code Review) 01 Nov '19

01 Nov '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36527 ) Change subject: timestamps: COLLECT_TIMESTAMPS is mostly optional ...................................................................... Patch Set 4: -Code-Review (4 comments) https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG@14 PS3, Line 14: the use of get_us_since_boot() with COLLECT_TIMESTAMPS=n. > The code changes from 'has 5 seconds from power-on elapsed' to 'lets wait here for 5 seconds'. Done https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG@15 PS3, Line 15: We could implement get_us_since_boot() over monotonic timer. > Ya. It's partially my fault, I think. […] Ack https://review.coreboot.org/c/coreboot/+/36527/3/src/mainboard/intel/galile… File src/mainboard/intel/galileo/Kconfig: https://review.coreboot.org/c/coreboot/+/36527/3/src/mainboard/intel/galile… PS3, Line 109: select COLLECT_TIMESTAMPS > I'm not sure what the dependency is for CYRPTO_SHIELD necessarily. […] Done https://review.coreboot.org/c/coreboot/+/36527/3/src/vendorcode/google/chro… File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/36527/3/src/vendorcode/google/chro… PS3, Line 25: select COLLECT_TIMESTAMPS > This needs to stay. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Nov 2019 18:19:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is mostly optional
by Kyösti Mälkki (Code Review) 01 Nov '19

01 Nov '19

Hello Werner Zeh, Aaron Durbin, Patrick Rudolph, HAOUAS Elyes, Julius Werner, Arthur Heymans, build bot (Jenkins), Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/36527 to look at the new patch set (#4). Change subject: timestamps: COLLECT_TIMESTAMPS is mostly optional ...................................................................... timestamps: COLLECT_TIMESTAMPS is mostly optional It is a user-visible option and enabled by default, some consider it as debugging aid only. Therefore platform design should not depend on it. It must remain selected with CHROMEOS and boards are allowed to explicitly select it as well. For siemens/mc_bdx1,mc_aplX boot time will be increased due the use of get_us_since_boot() with COLLECT_TIMESTAMPS=n. When unable to determine if N seconds has elapsed from boot, this turns into a delay of N seconds. Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/soc/amd/picasso/Kconfig M src/soc/amd/stoneyridge/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/skylake/Kconfig 5 files changed, 0 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36527/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is always optional
by Aaron Durbin (Code Review) 01 Nov '19

01 Nov '19

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36527 ) Change subject: timestamps: COLLECT_TIMESTAMPS is always optional ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG@15 PS3, Line 15: We could implement get_us_since_boot() over monotonic timer. > I doubt if amd/stoneyridge/monotonic_timer.c version does the same. Or intel/quark/tsc_freq.c. […] Ya. It's partially my fault, I think. My point was that we could use it in that way once we remove that bit of code in src/cpu/x86/tsc/delay_tsc.c and formalize on the semantics. Current comment in timer.h: Obtain the current monotonic time. The assumption is that the time counts * up from the value 0 with value 0 being the point when the timer was * initialized. Additionally, the timer is assumed to only be valid for the * duration of the boot. It's ambiguous, I admit, but we currently make no claims about it continuing to count up across stages. We can definitely do that, though. -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Nov 2019 18:05:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is always optional
by Kyösti Mälkki (Code Review) 01 Nov '19

01 Nov '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36527 ) Change subject: timestamps: COLLECT_TIMESTAMPS is always optional ...................................................................... Patch Set 3: (2 comments) https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG@14 PS3, Line 14: the use of get_us_since_boot() with COLLECT_TIMESTAMPS=n. > Why is it that there is an increase in those conditions? The code changes from 'has 5 seconds from power-on elapsed' to 'lets wait here for 5 seconds'. https://review.coreboot.org/c/coreboot/+/36527/3//COMMIT_MSG@15 PS3, Line 15: We could implement get_us_since_boot() over monotonic timer. > Currently monotonic timer is relative to the stage. It's not global and free running. […] I doubt if amd/stoneyridge/monotonic_timer.c version does the same. Or intel/quark/tsc_freq.c. Or nvidia/tegra210/monotonic_timer.c. Probably something that was never defined? -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Nov 2019 17:59:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is always optional
by Aaron Durbin (Code Review) 01 Nov '19

01 Nov '19

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is always optional
by Aaron Durbin (Code Review) 01 Nov '19

01 Nov '19

1 0

Change in coreboot[master]: timestamps: COLLECT_TIMESTAMPS is always optional
by Aaron Durbin (Code Review) 01 Nov '19

01 Nov '19

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36527 ) Change subject: timestamps: COLLECT_TIMESTAMPS is always optional ...................................................................... Patch Set 3: > Patch Set 3: > > > Patch Set 3: > > > > Martin, Aaron, the Chromebooks' coreboot-9999.ebuild may require modifying to accommodate this? > > Not exactly. We have COLLECT_TIMESTAMPS under the CHROMEOS Kconfig option in src/vendorcode/google/chromeos/Kconfig so as long as that doesn't change we should be good. Shoot. I saw that Chrome OS default policy was changed. We can't have that. -- To view, visit https://review.coreboot.org/c/coreboot/+/36527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ee4195d266440143344781d39db9578cd8bdcb3 Gerrit-Change-Number: 36527 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Nov 2019 17:53:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2019