Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Enable ELOG_PRERAM for all boards ......................................................................
drivers/elog: Enable ELOG_PRERAM for all boards
BUG=b:117884485, b:150502246 BRANCH=None TEST=emerge-nami coreboot
Change-Id: Id76cabc38e41e9bf79e1580a530c871a4ecef4ec Signed-off-by: Shelley Chen shchen@google.com --- M src/drivers/elog/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/45303/1
diff --git a/src/drivers/elog/Kconfig b/src/drivers/elog/Kconfig index bc25d1c..c71abbf 100644 --- a/src/drivers/elog/Kconfig +++ b/src/drivers/elog/Kconfig @@ -25,7 +25,7 @@
config ELOG_PRERAM bool - default n + default y help This option will enable event logging from the preram stage.
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Enable ELOG_PRERAM for all boards ......................................................................
Patch Set 1:
Karthik, is it ok to enable ELOG_PRERAM for all boards?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Enable ELOG_PRERAM for all boards ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45303/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45303/1//COMMIT_MSG@7 PS1, Line 7: Enable ELOG_PRERAM for all boards I think it would be better to just drop CONFIG_ELOG_PRERAM completely and use CONFIG_ELOG instead to include elog.c in all stages.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Enable ELOG_PRERAM for all boards ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45303/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45303/1//COMMIT_MSG@7 PS1, Line 7: Enable ELOG_PRERAM for all boards
I think it would be better to just drop CONFIG_ELOG_PRERAM completely and use CONFIG_ELOG instead to […]
At the time when I added ELOG_PRERAM, I wasn't sure if all the boards wanted to enable it by default. Hence added a separate config.
If we want to enable it for all boards, then +1 to what Furquan mentioned. Besides that, I don't see any issues with enabling ELOG_PRERAM for all boards.
Hello Furquan Shaikh, Julius Werner, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45303
to look at the new patch set (#2).
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
drivers/elog: Remove ELOG_PRERAM config
BUG=b:117884485, b:150502246 BRANCH=None TEST=./util/abuild/abuild -p none -t GOOGLE_NAMI -x -a -v
Change-Id: Id76cabc38e41e9bf79e1580a530c871a4ecef4ec Signed-off-by: Shelley Chen shchen@google.com --- M src/drivers/elog/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/45303/2
Hello build bot (Jenkins), Furquan Shaikh, Julius Werner, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45303
to look at the new patch set (#3).
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
drivers/elog: Remove ELOG_PRERAM config
BUG=b:117884485, b:150502246 BRANCH=None TEST=./util/abuild/abuild -p none -t GOOGLE_NAMI -x -a -v
Change-Id: Id76cabc38e41e9bf79e1580a530c871a4ecef4ec Signed-off-by: Shelley Chen shchen@google.com --- M src/drivers/elog/Kconfig M src/drivers/elog/Makefile.inc 2 files changed, 4 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/45303/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45303/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45303/3//COMMIT_MSG@8 PS3, Line 8: I think it would be helpful to have some explanation about why this change is being made.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Julius Werner, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45303
to look at the new patch set (#4).
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
drivers/elog: Remove ELOG_PRERAM config
This change is being done for the following reasons: 1. The CONFIG_ELOG_PRERAM is unused. 2. We need to pull in elog.c into romstage because we are pulling the mrc_cache_stash_data function into romstage. 3. Furquan says that we can rely on the linker to optimize out the unused 4KiB buffer in the early stages of boot, which allows us to get rid of the ELOG_PRERAM config.
BUG=b:117884485, b:150502246 BRANCH=None TEST=./util/abuild/abuild -p none -t GOOGLE_NAMI -x -a -v
Change-Id: Id76cabc38e41e9bf79e1580a530c871a4ecef4ec Signed-off-by: Shelley Chen shchen@google.com --- M src/drivers/elog/Kconfig M src/drivers/elog/Makefile.inc 2 files changed, 4 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/45303/4
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45303/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45303/3//COMMIT_MSG@8 PS3, Line 8:
I think it would be helpful to have some explanation about why this change is being made.
Done. Is the explanation ok?
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45303/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45303/1//COMMIT_MSG@7 PS1, Line 7: Enable ELOG_PRERAM for all boards
At the time when I added ELOG_PRERAM, I wasn't sure if all the boards wanted to enable it by default […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
Patch Set 4: Code-Review+2
Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
drivers/elog: Remove ELOG_PRERAM config
This change is being done for the following reasons: 1. The CONFIG_ELOG_PRERAM is unused. 2. We need to pull in elog.c into romstage because we are pulling the mrc_cache_stash_data function into romstage. 3. Furquan says that we can rely on the linker to optimize out the unused 4KiB buffer in the early stages of boot, which allows us to get rid of the ELOG_PRERAM config.
BUG=b:117884485, b:150502246 BRANCH=None TEST=./util/abuild/abuild -p none -t GOOGLE_NAMI -x -a -v
Change-Id: Id76cabc38e41e9bf79e1580a530c871a4ecef4ec Signed-off-by: Shelley Chen shchen@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45303 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/elog/Kconfig M src/drivers/elog/Makefile.inc 2 files changed, 4 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/drivers/elog/Kconfig b/src/drivers/elog/Kconfig index bc25d1c..19b3331 100644 --- a/src/drivers/elog/Kconfig +++ b/src/drivers/elog/Kconfig @@ -23,12 +23,6 @@ but it means that events added at runtime via the SMI handler will not be reflected in the CBMEM copy of the log.
-config ELOG_PRERAM - bool - default n - help - This option will enable event logging from the preram stage. - config ELOG_GSMI depends on HAVE_SMI_HANDLER bool "SMI interface to write and clear event log" diff --git a/src/drivers/elog/Makefile.inc b/src/drivers/elog/Makefile.inc index cce1c3d..370eef4 100644 --- a/src/drivers/elog/Makefile.inc +++ b/src/drivers/elog/Makefile.inc @@ -1,7 +1,7 @@ -bootblock-$(CONFIG_ELOG_PRERAM) += elog.c -verstage-$(CONFIG_ELOG_PRERAM) += elog.c -romstage-$(CONFIG_ELOG_PRERAM) += elog.c -postcar-$(CONFIG_ELOG_PRERAM) += elog.c +bootblock-$(CONFIG_ELOG) += elog.c +verstage-$(CONFIG_ELOG) += elog.c +romstage-$(CONFIG_ELOG) += elog.c +postcar-$(CONFIG_ELOG) += elog.c ramstage-$(CONFIG_ELOG) += elog.c
smm-$(CONFIG_ELOG_GSMI) += elog.c gsmi.c
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45303 )
Change subject: drivers/elog: Remove ELOG_PRERAM config ......................................................................
Patch Set 5:
Automatic boot test returned (PASS/FAIL/TOTAL): 8/1/9 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19290 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19289 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/19288 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19287 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/19286 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19294 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19293 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19292 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19291
Please note: This test is under development and might not be accurate at all!