Change in coreboot[master]: Revert "src/security/vboot: Add option to skip display init with vboo...
Hello Amol N Sukerkar, Aaron Durbin, Julius Werner, Subrata Banik, Sachin Agrawal, Lean Sheng Tan, build bot (Jenkins), Martin Roth,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34308
to review the following change.
Change subject: Revert "src/security/vboot: Add option to skip display init with vboot 2.0" ......................................................................
Revert "src/security/vboot: Add option to skip display init with vboot 2.0"
This reverts commit 598af2e2c2785c00eb4290cdcefe1082b2a6f858.
Reason for revert: This commit breaks every board with VBOOT enabled if the platform is apollolake, broadwell, skylake, baswell, baytrails or icelake. The reason is, that the SoC selects VBOOT_MUST_REQUEST_DISPLAY by default, and this has a dependency now on VBOOT_MAY_SKIP_DISPLAY_INIT. This will only be auto-selected if it is a CHROMEOS platform.
Change-Id: I3872d9aa993326ded135d8a5d950d5b1b1eddf34 --- M src/lib/bootmode.c M src/security/vboot/Kconfig 2 files changed, 2 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/34308/1
diff --git a/src/lib/bootmode.c b/src/lib/bootmode.c index 51bbbe5..737dcf9 100644 --- a/src/lib/bootmode.c +++ b/src/lib/bootmode.c @@ -2,7 +2,6 @@ * This file is part of the coreboot project. * * Copyright (C) 2011 The ChromiumOS Authors. All rights reserved. - * Copyright (C) 2019 Intel Corporation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -34,7 +33,8 @@
int display_init_required(void) { - if (CONFIG(VBOOT_MAY_SKIP_DISPLAY_INIT)) { + /* For vboot, always honor VBOOT_WD_FLAG_DISPLAY_INIT. */ + if (CONFIG(VBOOT)) { /* Must always select MUST_REQUEST_DISPLAY when using this function. */ if (!CONFIG(VBOOT_MUST_REQUEST_DISPLAY)) diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index fa98935..ea1f738 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -154,21 +154,10 @@ reboots caused after vboot verification is run. e.g. reboots caused by FSP components on Intel platforms.
-config VBOOT_MAY_SKIP_DISPLAY_INIT - bool "Skip display initialization in normal mode" - default y if CHROMEOS - default n - help - Set this option to indicate that coreboot should skip display - initialization on a normal (non-recovery, non-developer) boot. - This is useful for platforms that do not support firmware - user-interface in normal mode. - config VBOOT_MUST_REQUEST_DISPLAY bool default y if VGA_ROM_RUN default n - depends on VBOOT_MAY_SKIP_DISPLAY_INIT help Set this option to indicate to vboot that this platform will skip its display initialization on a normal (non-recovery, non-developer) boot.
Hello Patrick Rudolph, Amol N Sukerkar, Aaron Durbin, Julius Werner, Subrata Banik, Sachin Agrawal, Lean Sheng Tan, Philipp Deppenwiese, build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34308
to look at the new patch set (#2).
Change subject: Revert "src/security/vboot: Add option to skip display init with vboot 2.0" ......................................................................
Revert "src/security/vboot: Add option to skip display init with vboot 2.0"
This reverts commit 598af2e2c2785c00eb4290cdcefe1082b2a6f858.
Reason for revert: This commit breaks every board with VBOOT enabled if the platform is apollolake, broadwell, skylake, baswell, baytrails or icelake. The reason is, that the SoC selects VBOOT_MUST_REQUEST_DISPLAY by default, and this has a dependency now on VBOOT_MAY_SKIP_DISPLAY_INIT. This will only be auto-selected if it is a CHROMEOS platform.
Change-Id: I3872d9aa993326ded135d8a5d950d5b1b1eddf34 Signed-off-by: Christian Walter christian.walter@9elements.com --- M src/lib/bootmode.c M src/security/vboot/Kconfig 2 files changed, 2 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/34308/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34308 )
Change subject: Revert "src/security/vboot: Add option to skip display init with vboot 2.0" ......................................................................
Patch Set 2: Code-Review+2
Patrick Rudolph has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34308 )
Change subject: Revert "src/security/vboot: Add option to skip display init with vboot 2.0" ......................................................................
Revert "src/security/vboot: Add option to skip display init with vboot 2.0"
This reverts commit 598af2e2c2785c00eb4290cdcefe1082b2a6f858.
Reason for revert: This commit breaks every board with VBOOT enabled if the platform is apollolake, broadwell, skylake, baswell, baytrails or icelake. The reason is, that the SoC selects VBOOT_MUST_REQUEST_DISPLAY by default, and this has a dependency now on VBOOT_MAY_SKIP_DISPLAY_INIT. This will only be auto-selected if it is a CHROMEOS platform.
Change-Id: I3872d9aa993326ded135d8a5d950d5b1b1eddf34 Signed-off-by: Christian Walter christian.walter@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34308 Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/lib/bootmode.c M src/security/vboot/Kconfig 2 files changed, 2 insertions(+), 13 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/lib/bootmode.c b/src/lib/bootmode.c index 083fd9d..2465966 100644 --- a/src/lib/bootmode.c +++ b/src/lib/bootmode.c @@ -2,7 +2,6 @@ * This file is part of the coreboot project. * * Copyright (C) 2011 The ChromiumOS Authors. All rights reserved. - * Copyright (C) 2019 Intel Corporation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -34,7 +33,8 @@
int display_init_required(void) { - if (CONFIG(VBOOT_MAY_SKIP_DISPLAY_INIT)) { + /* For vboot, always honor VBOOT_WD_FLAG_DISPLAY_INIT. */ + if (CONFIG(VBOOT)) { /* Must always select MUST_REQUEST_DISPLAY when using this function. */ if (!CONFIG(VBOOT_MUST_REQUEST_DISPLAY)) diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index fa98935..ea1f738 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -154,21 +154,10 @@ reboots caused after vboot verification is run. e.g. reboots caused by FSP components on Intel platforms.
-config VBOOT_MAY_SKIP_DISPLAY_INIT - bool "Skip display initialization in normal mode" - default y if CHROMEOS - default n - help - Set this option to indicate that coreboot should skip display - initialization on a normal (non-recovery, non-developer) boot. - This is useful for platforms that do not support firmware - user-interface in normal mode. - config VBOOT_MUST_REQUEST_DISPLAY bool default y if VGA_ROM_RUN default n - depends on VBOOT_MAY_SKIP_DISPLAY_INIT help Set this option to indicate to vboot that this platform will skip its display initialization on a normal (non-recovery, non-developer) boot.
-
Christian Walter (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)