Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32262
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add field |declares_display| to vboot_working_data struct * Add functions to get andset |declares_display|
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/common.c M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 22 files changed, 46 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/1
diff --git a/src/lib/bootmode.c b/src/lib/bootmode.c index 052eb8f..1b39031 100644 --- a/src/lib/bootmode.c +++ b/src/lib/bootmode.c @@ -35,11 +35,13 @@
int display_init_required(void) { - /* For vboot always honor vboot_handoff_skip_display_init(). */ + /* For vboot always honor vboot_declares_display(). */ if (CONFIG(VBOOT)) { - /* Must always select OPROM_MATTERS when using this function. */ - if (!CONFIG(VBOOT_OPROM_MATTERS)) + /* Must always select MUST_REQUEST_DISPLAY when using this + * function. */ + if (!CONFIG(VBOOT_MUST_REQUEST_DISPLAY)) dead_code(); + return vboot_declares_display(); return !vboot_handoff_skip_display_init(); }
diff --git a/src/northbridge/intel/haswell/Kconfig b/src/northbridge/intel/haswell/Kconfig index 082f2d6..c251b7d 100644 --- a/src/northbridge/intel/haswell/Kconfig +++ b/src/northbridge/intel/haswell/Kconfig @@ -26,7 +26,7 @@ if NORTHBRIDGE_INTEL_HASWELL
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_NORTHBRIDGE_INIT diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 4815cb1..66bcc1e 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -169,17 +169,15 @@ reboots caused after vboot verification is run. e.g. reboots caused by FSP components on Intel platforms.
-config VBOOT_OPROM_MATTERS +config VBOOT_MUST_REQUEST_DISPLAY bool default y if VGA_ROM_RUN default n help Set this option to indicate to vboot that this platform will skip its display initialization on a normal (non-recovery, non-developer) boot. - Vboot calls this "oprom matters" because on x86 devices this - traditionally meant that the video option ROM will not be loaded, but - it works functionally the same for other platforms that can skip their - native display initialization code instead. + Unless display is specifically requested, the video option ROM is not + loaded, and any other native display initialization code is not run.
config VBOOT_HAS_REC_HASH_SPACE bool diff --git a/src/security/vboot/common.c b/src/security/vboot/common.c index 8f8165a..b26b7e5 100644 --- a/src/security/vboot/common.c +++ b/src/security/vboot/common.c @@ -117,6 +117,16 @@ return reg->size > 0; }
+int vboot_set_declares_display(int declared) +{ + vboot_get_working_data()->declares_display = declared; +} + +int vboot_declares_display(void) +{ + return vboot_get_working_data()->declares_display; +} + #if CONFIG(VBOOT_MIGRATE_WORKING_DATA) /* * For platforms that do not employ VBOOT_STARTS_IN_ROMSTAGE, vboot diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index 27317ad..8ed6038 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -34,6 +34,7 @@ */ struct vboot_working_data { struct selected_region selected_region; + uint8_t declares_display; /* offset of the buffer from the start of this struct */ uint32_t buffer_offset; uint32_t buffer_size; @@ -53,6 +54,9 @@ void vboot_set_selected_region(const struct region *region); int vboot_is_slot_selected(void);
+int vboot_set_declares_display(int declared); +int vboot_declares_display(void); + /* * Source: security/vboot/vboot_handoff.c */ diff --git a/src/security/vboot/vboot_common.c b/src/security/vboot/vboot_common.c index 5e95c62..3dddc76 100644 --- a/src/security/vboot/vboot_common.c +++ b/src/security/vboot/vboot_common.c @@ -88,11 +88,6 @@ return !!(vbho->init_params.out_flags & flag); }
-int vboot_handoff_skip_display_init(void) -{ - return !vboot_get_handoff_flag(VB_INIT_OUT_ENABLE_DISPLAY); -} - int vboot_handoff_check_developer_flag(void) { return vboot_get_handoff_flag(VB_INIT_OUT_ENABLE_DEVELOPER); diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 8c92437..768b29d 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -60,7 +60,6 @@ * Returns 0 for flag if false * Returns value read for other fields */ -int vboot_handoff_skip_display_init(void); int vboot_handoff_check_recovery_flag(void); int vboot_handoff_check_developer_flag(void); int vboot_handoff_get_recovery_reason(void); diff --git a/src/security/vboot/vboot_handoff.c b/src/security/vboot/vboot_handoff.c index 2f239e6..42562ce 100644 --- a/src/security/vboot/vboot_handoff.c +++ b/src/security/vboot/vboot_handoff.c @@ -67,29 +67,25 @@ vb_sd->flags |= VBSD_BOOT_REC_SWITCH_ON; *oflags |= VB_INIT_OUT_ENABLE_RECOVERY; *oflags |= VB_INIT_OUT_CLEAR_RAM; - *oflags |= VB_INIT_OUT_ENABLE_DISPLAY; *oflags |= VB_INIT_OUT_ENABLE_USB_STORAGE; } if (vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) { *oflags |= VB_INIT_OUT_ENABLE_DEVELOPER; *oflags |= VB_INIT_OUT_CLEAR_RAM; - *oflags |= VB_INIT_OUT_ENABLE_DISPLAY; *oflags |= VB_INIT_OUT_ENABLE_USB_STORAGE; vb_sd->flags |= VBSD_BOOT_DEV_SWITCH_ON; vb_sd->flags |= VBSD_LF_DEV_SWITCH_ON; } - /* TODO: Set these in depthcharge */ - if (CONFIG(VBOOT_OPROM_MATTERS)) { + /* Inform vboot if the display was requested by vboot kernel phase + * or enabled by dev/rec mode. */ + if (vboot_wants_oprom() || vb2_sd->recovery_reason || + vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) { + vboot_set_declares_display(1); + vb_sd->flags |= VBSD_OPROM_LOADED; + } + /* TODO: Remove when depthcharge no longer reads this flag. */ + if (CONFIG(VBOOT_MUST_REQUEST_DISPLAY)) { vb_sd->flags |= VBSD_OPROM_MATTERS; - /* - * Inform vboot if the display was enabled by dev/rec - * mode or was requested by vboot kernel phase. - */ - if ((*oflags & VB_INIT_OUT_ENABLE_DISPLAY) || - vboot_wants_oprom()) { - vb_sd->flags |= VBSD_OPROM_LOADED; - *oflags |= VB_INIT_OUT_ENABLE_DISPLAY; - } }
/* In vboot1, VBSD_FWB_TRIED is diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index d715f39..35d383d 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -118,7 +118,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/intel/baytrail/Kconfig b/src/soc/intel/baytrail/Kconfig index a83a7e9..03ad31d 100644 --- a/src/soc/intel/baytrail/Kconfig +++ b/src/soc/intel/baytrail/Kconfig @@ -43,7 +43,7 @@ select CPU_HAS_L2_ENABLE_MSR
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT diff --git a/src/soc/intel/braswell/Kconfig b/src/soc/intel/braswell/Kconfig index 8db4795..a0c0708 100644 --- a/src/soc/intel/braswell/Kconfig +++ b/src/soc/intel/braswell/Kconfig @@ -53,7 +53,7 @@ select SOUTHBRIDGE_INTEL_COMMON_SMBUS
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig index 5f503da..149cf9a 100644 --- a/src/soc/intel/broadwell/Kconfig +++ b/src/soc/intel/broadwell/Kconfig @@ -65,7 +65,7 @@ default y
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 9796546..46271d6 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -228,7 +228,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 2e9e3e2..2fbc6da 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -151,7 +151,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 5116f3a..fcfe2b6 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -106,7 +106,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/mediatek/mt8173/Kconfig b/src/soc/mediatek/mt8173/Kconfig index b0d6fa0..e966b8e 100644 --- a/src/soc/mediatek/mt8173/Kconfig +++ b/src/soc/mediatek/mt8173/Kconfig @@ -16,7 +16,7 @@ if SOC_MEDIATEK_MT8173
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE
diff --git a/src/soc/mediatek/mt8183/Kconfig b/src/soc/mediatek/mt8183/Kconfig index 6582e4e..f96282c 100644 --- a/src/soc/mediatek/mt8183/Kconfig +++ b/src/soc/mediatek/mt8183/Kconfig @@ -14,7 +14,7 @@ if SOC_MEDIATEK_MT8183
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE
diff --git a/src/soc/nvidia/tegra124/Kconfig b/src/soc/nvidia/tegra124/Kconfig index eecb59b..475d50e 100644 --- a/src/soc/nvidia/tegra124/Kconfig +++ b/src/soc/nvidia/tegra124/Kconfig @@ -18,7 +18,7 @@ if SOC_NVIDIA_TEGRA124
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE
diff --git a/src/soc/nvidia/tegra210/Kconfig b/src/soc/nvidia/tegra210/Kconfig index 8883baa..6750aa9 100644 --- a/src/soc/nvidia/tegra210/Kconfig +++ b/src/soc/nvidia/tegra210/Kconfig @@ -18,7 +18,7 @@ config VBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY
config MAINBOARD_DO_DSI_INIT bool "Use dsi graphics interface" diff --git a/src/soc/qualcomm/sdm845/Kconfig b/src/soc/qualcomm/sdm845/Kconfig index c0e3294..ba06488 100644 --- a/src/soc/qualcomm/sdm845/Kconfig +++ b/src/soc/qualcomm/sdm845/Kconfig @@ -16,7 +16,7 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK
config SDM845_QSPI diff --git a/src/soc/rockchip/rk3288/Kconfig b/src/soc/rockchip/rk3288/Kconfig index 53666c2..38d8752 100644 --- a/src/soc/rockchip/rk3288/Kconfig +++ b/src/soc/rockchip/rk3288/Kconfig @@ -34,7 +34,7 @@ if SOC_ROCKCHIP_RK3288
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE diff --git a/src/soc/rockchip/rk3399/Kconfig b/src/soc/rockchip/rk3399/Kconfig index 6e45df3..83fc437 100644 --- a/src/soc/rockchip/rk3399/Kconfig +++ b/src/soc/rockchip/rk3399/Kconfig @@ -19,7 +19,7 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK
config PMIC_BUS
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32262/1/src/security/vboot/vboot_handoff.c File src/security/vboot/vboot_handoff.c:
https://review.coreboot.org/#/c/32262/1/src/security/vboot/vboot_handoff.c@8... PS1, Line 87: if (CONFIG(VBOOT_MUST_REQUEST_DISPLAY)) { braces {} are not necessary for single statement blocks
Hello Randall Spangler, Patrick Rudolph, Aaron Durbin, Simon Glass, Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32262
to look at the new patch set (#2).
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add field |declares_display| to vboot_working_data struct * Add functions to get andset |declares_display|
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/common.c M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 22 files changed, 46 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/32262/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32262/2//COMMIT_MSG@20 PS2, Line 20: andset and set
https://review.coreboot.org/#/c/32262/2/src/lib/bootmode.c File src/lib/bootmode.c:
https://review.coreboot.org/#/c/32262/2/src/lib/bootmode.c@41 PS2, Line 41: * Please remove the asterisk.
https://review.coreboot.org/#/c/32262/2/src/lib/bootmode.c@45 PS2, Line 45: return !vboot_handoff_skip_display_init(); This return line can now be removed?
https://review.coreboot.org/#/c/32262/2/src/security/vboot/vboot_handoff.c File src/security/vboot/vboot_handoff.c:
https://review.coreboot.org/#/c/32262/2/src/security/vboot/vboot_handoff.c@8... PS2, Line 80: * Remove for concise multi-line comment.
Hello Randall Spangler, Patrick Rudolph, Aaron Durbin, Simon Glass, Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32262
to look at the new patch set (#3).
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add field |declares_display| to vboot_working_data struct * Add functions to get and set |declares_display|
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/common.c M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 22 files changed, 46 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/3
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/#/c/32262/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32262/2//COMMIT_MSG@20 PS2, Line 20: andset
and set
Done
https://review.coreboot.org/#/c/32262/2/src/lib/bootmode.c File src/lib/bootmode.c:
https://review.coreboot.org/#/c/32262/2/src/lib/bootmode.c@41 PS2, Line 41: *
Please remove the asterisk.
Done
https://review.coreboot.org/#/c/32262/2/src/lib/bootmode.c@45 PS2, Line 45: return !vboot_handoff_skip_display_init();
This return line can now be removed?
Done
https://review.coreboot.org/#/c/32262/2/src/security/vboot/vboot_handoff.c File src/security/vboot/vboot_handoff.c:
https://review.coreboot.org/#/c/32262/2/src/security/vboot/vboot_handoff.c@8... PS2, Line 80: *
Remove for concise multi-line comment.
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 3: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c@125 PS3, Line 125: int vboot_declares_display(void) Sorry, I'm still not really happy with the name "declares display". What's wrong with "display needed" or "display required"? That feels way more natural as to what this is to me. (Or just call it init_display or something that would probably be good enough too.)
Also, I'm not sure if we really need full getter and setter accessors here, we're not in Java 101. ;) I think for a simple flags field, it's okay if code manipulates it directly.
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h@37 PS3, Line 37: uint8_t declares_display; Let's make it a uint32_t flags in case we come up with other one-bit stuff we need to track here later. (You could also make it a bit-field for ease of access.)
Hello Randall Spangler, Patrick Rudolph, Aaron Durbin, Simon Glass, Julius Werner, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32262
to look at the new patch set (#4).
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add |flags| field to vboot_working_data struct * Create VBOOT_FLAG_DISPLAY_REQUESTED and set in vboot_handoff
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 21 files changed, 40 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/4
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c@125 PS3, Line 125: int vboot_declares_display(void)
Sorry, I'm still not really happy with the name "declares display". […]
Well, if we want to have a function that fulfills both needs, it could be written like:
*uint32_t flags; vboot_get_flags(flags); *flags |= VBOOT_FLAG_DISPLAY_REQUIRED;
But maybe we should just directly use it:
vboot_get_working_data()->flags |= VBOOT_FLAG_DISPLAY_REQUIRED;
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h@37 PS3, Line 37: uint8_t declares_display;
Let's make it a uint32_t flags in case we come up with other one-bit stuff we need to track here lat […]
What's the most acceptable format? I haven't really come across any bit-fields in working with coreboot/vboot/depthcharge so far.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c@125 PS3, Line 125: int vboot_declares_display(void)
Well, if we want to have a function that fulfills both needs, it could be written like: […]
Yes, I think the latter option is reasonable.
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size; nit: maybe change these two to u16 so the whole thing can still fit 16 bytes (which we align the work buffer behind it to)?
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@47 PS4, Line 47: #define VBOOT_FLAG_DISPLAY_REQUESTED (1 << 0) Maybe prefix this VB_WD_FLAG instead to have clearer differentiation from the dozens of other kinds of flags vboot has in various places?
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h@37 PS3, Line 37: uint8_t declares_display;
What's the most acceptable format? I haven't really come across any bit-fields in working with core […]
I think either is fine. We tend to not use bitfields for hardware registers (with a few exceptions like libpayload's USB stacks), but for flags that are handled purely in software they should be okay. It's just a personal preference thing.
Hello Randall Spangler, Patrick Rudolph, Aaron Durbin, Simon Glass, Julius Werner, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32262
to look at the new patch set (#5).
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add |flags| field to vboot_working_data struct * Create VBOOT_FLAG_DISPLAY_REQUESTED and set in vboot_handoff
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 21 files changed, 40 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/5
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c@125 PS3, Line 125: int vboot_declares_display(void)
Yes, I think the latter option is reasonable.
Ack
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
nit: maybe change these two to u16 so the whole thing can still fit 16 bytes (which we align the wor […]
You're talking about buffer_offset and buffer_size? I'm not so sure what you mean by "so the whole thing can still fit 16 bytes" ...
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@47 PS4, Line 47: #define VBOOT_FLAG_DISPLAY_REQUESTED (1 << 0)
Maybe prefix this VB_WD_FLAG instead to have clearer differentiation from the dozens of other kinds […]
Sure. I'd still like to keep the front as VBOOT to differentiate from vboot internal stuff (just like we moved vb2_working_data to vboot_working_data).
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h@37 PS3, Line 37: uint8_t declares_display;
I think either is fine. […]
Ack
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
You're talking about buffer_offset and buffer_size? I'm not so sure what you mean by "so the whole […]
Yes. Also, I'm a moron. buffer_offset may need to be larger than 64K. Don't listen to me. (I guess we could make the flags u16 and the size u16 and then squeeze it together so that it fits, but maybe it's not worth it.)
Right now, as written, this struct is 20 bytes long. vboot_init_work_context sets the workbuf to ALIGN_UP(sizeof(*wd), 16), so that would be 32 with this and you're wasting 28 bytes. No big deal.
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@47 PS4, Line 47: #define VBOOT_FLAG_DISPLAY_REQUESTED (1 << 0)
Sure. […]
Sounds good.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
Yes. Also, I'm a moron. buffer_offset may need to be larger than 64K. Don't listen to me. […]
Shit, did I submit this already? I shouldn't try to get stuff done while having to jump off the bus.
buffer_offset is of course always 16 right now and will very easily fit in a u16.
Hello Randall Spangler, Patrick Rudolph, Aaron Durbin, Simon Glass, Julius Werner, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32262
to look at the new patch set (#6).
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add |flags| field to vboot_working_data struct * Create VBOOT_FLAG_DISPLAY_REQUESTED and set in vboot_handoff
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 21 files changed, 41 insertions(+), 45 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/6
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
Shit, did I submit this already? I shouldn't try to get stuff done while having to jump off the bus. […]
Well you still managed to correct yourself extremely quickly =)
Not sure if it's worth optimizing for wasting N<16 bytes, since we may want to add more members to this structure in the future.
But you're quite right that buffer_offset fits easily enough in u16. I can change it. (Although in theory we don't even need buffer_offset at all -- workbuf location ALIGN_UP(xyz) can be calculated easily enough...)
While we're on the topic -- do you have any thoughts on `struct region` in commonlib/region.h? Is there a good reason for it using size_t, or would there be any harm in modifying that code to use u16 or u32? Then we can get rid of selected_region altogether and just use the vanilla struct.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h@40 PS6, Line 40: uint32_t buffer_size; Well, this doesn't achieve anything since the compiler just adds 16 byte padding (and in fact we should always write serializable data structures in a way that won't generate implicit padding, just to be sure). I meant reducing buffer_size as well (or not do it at all).
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
While we're on the topic -- do you have any thoughts on `struct region` in commonlib/region.h? Is there a good reason for it using size_t, or would there be any harm in modifying that code to use u16 or u32? Then we can get rid of selected_region altogether and just use the vanilla struct.
That's maybe a better question for Aaron than for me. You could try uploading a patch to change it and see what he thinks. It definitely needs to be 32 there because it is used for regions in memory or on SPI flash which can both be larger than 64K. Whether 32 vs. 64 matters I'm not sure... we always have a sort of implicit rule to never put anything in memory above 4G in coreboot since there is probably some old code assuming pointers to be 32 bit buried somewhere, but whether we should be adding new code cementing that to a core API is a different question.
My guess is that if you propose this, people will just tell you to use u64 to be safe. And then my heart will weep for all those poor 2-3 extra cycles we waste per region operation on 32-bit CPUs, although in practice it won't make a difference.
(And yes, we could get rid of buffer_offset completely too.)
Hello Randall Spangler, Patrick Rudolph, Aaron Durbin, Simon Glass, Julius Werner, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32262
to look at the new patch set (#7).
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add |flags| field to vboot_working_data struct * Create VBOOT_FLAG_DISPLAY_REQUESTED and set in vboot_handoff
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 21 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/7
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
While we're on the topic -- do you have any thoughts on `struct region` in commonlib/region.h? Is there a good reason for it using size_t, or would there be any harm in modifying that code to use u16 or u32? Then we can get rid of selected_region altogether and just use the vanilla struct.
That's maybe a better question for Aaron than for me. You could try uploading a patch to change it and see what he thinks. It definitely needs to be 32 there because it is used for regions in memory or on SPI flash which can both be larger than 64K. Whether 32 vs. 64 matters I'm not sure... we always have a sort of implicit rule to never put anything in memory above 4G in coreboot since there is probably some old code assuming pointers to be 32 bit buried somewhere, but whether we should be adding new code cementing that to a core API is a different question.
My guess is that if you propose this, people will just tell you to use u64 to be safe. And then my heart will weep for all those poor 2-3 extra cycles we waste per region operation on 32-bit CPUs, although in practice it won't make a difference.
(And yes, we could get rid of buffer_offset completely too.)
struct region was never intended to be serialized and I weep for fixed size fields in things that aren't serialized. :)
More broadly struct region sort of conflates two things: pointer addressability and boot media. Since our boot media has traditionally been small the same underlying type works (even on 32-bit machines): 4GiB max addressibility. If you wanted to cover everything then you'd need to implement the equivalent of loff_t to cover large spans.
I'm not sure I see much of a gain of trying to serialize struct region. Once one tries to reuse types like that people don't know all the uses and then if someone makes a change then it suddenly breaks an implicit assumption of size and type -- not very stable. So my preference would be to keep the types the same.
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(2 comments)
Should be ready for commit now.
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h@40 PS6, Line 40: uint32_t buffer_size;
Well, this doesn't achieve anything since the compiler just adds 16 byte padding (and in fact we sho […]
Okay, I see what you're saying. So if we make both buffer_offset and buffer_size 16-bit, then they should share the same 32-bit word?
But seems like member alignment depends on the compiler implementation -- how can we be certain that this is the case?
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/4/src/security/vboot/misc.h@40 PS4, Line 40: uint32_t buffer_size;
While we're on the topic -- do you have any thoughts on `struct region` in commonlib/region. […]
OK, thanks for the feedback, Aaron. Let's prevent the tears from flowing and keep `struct selected_region` separate.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h@40 PS6, Line 40: uint32_t buffer_size;
Okay, I see what you're saying. […]
I don't remember the exact rules in the standard, but for the compilers we use (GCC/clang) you can generally assume that they won't add padding as long as all members are already naturally aligned (and that's why we should always make sure that's the case in all serialized structures, adding explicit 'u8 reserved' members if needed... the other option is using __packed, but that comes bundled with some stupid other effects we don't want so it's better to avoid it where possible).
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig@180 PS7, Line 180: and any other native display initialization code is not run This doesn't seem to reflect the code on older x86 platforms. AFAIUI, the first native graphics init for Intel was always run and in case of failure, the option ROM was run conditionally. I don't know if the NGI was run unconditionally by accident, but we have still a lot code that reflects that. I guess somebody should check at least all the Intel platforms that select this, if they comply with this description.
Some NGI instances were replaced with libgfxinit by now. But I'm rather sure that we maintained the old -- probably wrong -- behavior.
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c File src/security/vboot/vboot_handoff.c:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c@8... PS7, Line 84: VBSD_OPROM_LOADED out of curiosity, I assume `VBSD_OPROM_LOADED` implies more than what the name says? it's not that we always have an oprom to load.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig@180 PS7, Line 180: and any other native display initialization code is not run
This doesn't seem to reflect the code on older x86 platforms. AFAIUI, […]
The second sentence describes how things are supposed to work if the platform sets this flag. The platforms you're describing (which always init display unconditionally, whether that's via option ROM or native) would not have this flag set. All this does is provide a hint to vboot whether it can assume the display is always enabled or it needs to reboot with a special request in NVRAM to get it turned on.
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c File src/security/vboot/vboot_handoff.c:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c@8... PS7, Line 84: VBSD_OPROM_LOADED
out of curiosity, I assume `VBSD_OPROM_LOADED` implies more than what the […]
Yes, the whole "oprom" name isn't really accurate anymore, it's a general "we have enabled the display (whatever that takes)" flag. I think the plan is to slowly rename everything to a more generic "display init/available". (This particular flag isn't renamed here because it's going to be eliminated soon anyway, after a few more changes on the vboot side.)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
Looks like this needs a rebase before submission, Joel.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig@180 PS7, Line 180: and any other native display initialization code is not run
The second sentence describes how things are supposed to work if the platform sets this flag. […]
Sorry, I forgot to mention, I had Haswell in mind. Which selects it. Maybe I overestimated the problem and that's the only one.
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c File src/security/vboot/vboot_handoff.c:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c@8... PS7, Line 84: VBSD_OPROM_LOADED
Yes, the whole "oprom" name isn't really accurate anymore, it's a general "we have enabled the displ […]
Ack
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig@180 PS7, Line 180: and any other native display initialization code is not run
Sorry, I forgot to mention, I had Haswell in mind. […]
Yeah, I just added that there recently... why did I do that? Oh, because we used to set it in depthcharge for Haswell when the option was still there. Hmmm. Quite possible that this no longer applies nowadays. Did all the Haswell boards switch to libgfxinit? If so, we can probably remove this there. (Then again, nobody really cares...)
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig@180 PS7, Line 180: and any other native display initialization code is not run
Yeah, I just added that there recently... […]
I'm not sure if anybody still builds vboot with these old platforms. For Haswell, libgfxinit is the only alternative to VGA_ROM_RUN for some time. But you are probably right, maybe nobody who uses it cares. I guess it would be best to make MUST_REQUEST_DISPLAY the default (unless a platform really requires it to run unconditionally, e.g. OS can't bring it up). I'll try to keep that in mind if I touch the code again :)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig@180 PS7, Line 180: and any other native display initialization code is not run
I guess it would be best to make MUST_REQUEST_DISPLAY the default
I've thought about that too, or getting rid of the option entirely, I think my remaining concern was that we'd then forget to disable it for devices that don't have a display at all. The problem with defaults is that nobody would ever override them unless there's a visible issue. With the way it's done here, it's unset by default but you'll realize very quickly if you need to set it because of the assertion in display_init_required().
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
(3 comments)
Ready for merge
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/Kconfig@180 PS7, Line 180: and any other native display initialization code is not run
I guess it would be best to make MUST_REQUEST_DISPLAY the default […]
The other option would be to have MUST_REQUEST_DISPLAY depend on the other relevant options that select for OPROM, libgfxinit, VGA_ROM_RUN, etc. But not totally sure how this would work -- the spiderweb of display-related options is pretty confusing.
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/6/src/security/vboot/misc.h@40 PS6, Line 40: uint32_t buffer_size;
I don't remember the exact rules in the standard, but for the compilers we use (GCC/clang) you can g […]
Ack
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c File src/security/vboot/vboot_handoff.c:
https://review.coreboot.org/#/c/32262/7/src/security/vboot/vboot_handoff.c@8... PS7, Line 84: VBSD_OPROM_LOADED
Ack
Yup, that's right. Currently we are slated to have two flags:
vb2_context: VB2_CONTEXT_DISPLAY_INIT vb2_shared_data: VB2_SD_FLAG_DISPLAY_AVAILABLE
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 7:
Ready for merge
See above... there's something out of sync with this CL, I think it needs a rebase (the Gerrit UI won't let me merge it as is).
Hello Randall Spangler, Aaron Durbin, Patrick Rudolph, Simon Glass, Julius Werner, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32262
to look at the new patch set (#8).
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add |flags| field to vboot_working_data struct * Create VBOOT_FLAG_DISPLAY_REQUESTED and set in vboot_handoff
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 21 files changed, 42 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32262/8
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 8:
Rebased -- there was some merging needed in the haswell Kconfig.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 8: Code-Review+2
Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
vboot: refactor OPROM code
The name OPROM is somewhat inaccurate, since other steps to bring up display and graphics are needed depending on mainboard/SoC. This patch cleans up OPROM code nomenclature, and works towards the goal of deprecating vboot1:
* Rename CONFIG_VBOOT_OPROM_MATTERS to CONFIG_VBOOT_MUST_REQUEST_DISPLAY and clarify Kconfig description * Remove function vboot_handoff_skip_display_init * Remove use of the VbInit oflag VB_INIT_OUT_ENABLE_DISPLAY * Add |flags| field to vboot_working_data struct * Create VBOOT_FLAG_DISPLAY_REQUESTED and set in vboot_handoff
BUG=b:124141368, b:124192753, chromium:948529 TEST=make clean && make test-abuild TEST=build and flash eve device; attempt loading dev/rec modes BRANCH=none
Change-Id: Idf111a533c3953448b4b9084885a9a65a2432a8b Signed-off-by: Joel Kitching kitching@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32262 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/lib/bootmode.c M src/northbridge/intel/haswell/Kconfig M src/security/vboot/Kconfig M src/security/vboot/misc.h M src/security/vboot/vboot_common.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_handoff.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/baytrail/Kconfig M src/soc/intel/braswell/Kconfig M src/soc/intel/broadwell/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/mediatek/mt8173/Kconfig M src/soc/mediatek/mt8183/Kconfig M src/soc/nvidia/tegra124/Kconfig M src/soc/nvidia/tegra210/Kconfig M src/soc/qualcomm/sdm845/Kconfig M src/soc/rockchip/rk3288/Kconfig M src/soc/rockchip/rk3399/Kconfig 21 files changed, 42 insertions(+), 46 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/lib/bootmode.c b/src/lib/bootmode.c index 052eb8f..18f6d5d 100644 --- a/src/lib/bootmode.c +++ b/src/lib/bootmode.c @@ -35,12 +35,14 @@
int display_init_required(void) { - /* For vboot always honor vboot_handoff_skip_display_init(). */ + /* For vboot, always honor VBOOT_WD_FLAG_DISPLAY_INIT. */ if (CONFIG(VBOOT)) { - /* Must always select OPROM_MATTERS when using this function. */ - if (!CONFIG(VBOOT_OPROM_MATTERS)) + /* Must always select MUST_REQUEST_DISPLAY when using this + function. */ + if (!CONFIG(VBOOT_MUST_REQUEST_DISPLAY)) dead_code(); - return !vboot_handoff_skip_display_init(); + return vboot_get_working_data()->flags + & VBOOT_WD_FLAG_DISPLAY_INIT; }
/* By default always initialize display. */ diff --git a/src/northbridge/intel/haswell/Kconfig b/src/northbridge/intel/haswell/Kconfig index 49466bc..8c1e0b1 100644 --- a/src/northbridge/intel/haswell/Kconfig +++ b/src/northbridge/intel/haswell/Kconfig @@ -44,7 +44,7 @@ and back to the RW region after the binary is done.
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE if !HASWELL_VBOOT_IN_BOOTBLOCK
config VGA_BIOS_ID diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 4815cb1..66bcc1e 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -169,17 +169,15 @@ reboots caused after vboot verification is run. e.g. reboots caused by FSP components on Intel platforms.
-config VBOOT_OPROM_MATTERS +config VBOOT_MUST_REQUEST_DISPLAY bool default y if VGA_ROM_RUN default n help Set this option to indicate to vboot that this platform will skip its display initialization on a normal (non-recovery, non-developer) boot. - Vboot calls this "oprom matters" because on x86 devices this - traditionally meant that the video option ROM will not be loaded, but - it works functionally the same for other platforms that can skip their - native display initialization code instead. + Unless display is specifically requested, the video option ROM is not + loaded, and any other native display initialization code is not run.
config VBOOT_HAS_REC_HASH_SPACE bool diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index 27317ad..b4fae19 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -34,12 +34,19 @@ */ struct vboot_working_data { struct selected_region selected_region; + uint32_t flags; /* offset of the buffer from the start of this struct */ - uint32_t buffer_offset; - uint32_t buffer_size; + uint16_t buffer_offset; + uint16_t buffer_size; };
/* + * Definitions for vboot_working_data.flags values. + */ +/* vboot requests display initialization from coreboot. */ +#define VBOOT_WD_FLAG_DISPLAY_INIT (1 << 0) + +/* * Source: security/vboot/common.c */ struct vboot_working_data * const vboot_get_working_data(void); diff --git a/src/security/vboot/vboot_common.c b/src/security/vboot/vboot_common.c index 5e95c62..3dddc76 100644 --- a/src/security/vboot/vboot_common.c +++ b/src/security/vboot/vboot_common.c @@ -88,11 +88,6 @@ return !!(vbho->init_params.out_flags & flag); }
-int vboot_handoff_skip_display_init(void) -{ - return !vboot_get_handoff_flag(VB_INIT_OUT_ENABLE_DISPLAY); -} - int vboot_handoff_check_developer_flag(void) { return vboot_get_handoff_flag(VB_INIT_OUT_ENABLE_DEVELOPER); diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 8c92437..768b29d 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -60,7 +60,6 @@ * Returns 0 for flag if false * Returns value read for other fields */ -int vboot_handoff_skip_display_init(void); int vboot_handoff_check_recovery_flag(void); int vboot_handoff_check_developer_flag(void); int vboot_handoff_get_recovery_reason(void); diff --git a/src/security/vboot/vboot_handoff.c b/src/security/vboot/vboot_handoff.c index ff938e1..178877d 100644 --- a/src/security/vboot/vboot_handoff.c +++ b/src/security/vboot/vboot_handoff.c @@ -61,30 +61,25 @@ vb_sd->flags |= VBSD_BOOT_REC_SWITCH_ON; *oflags |= VB_INIT_OUT_ENABLE_RECOVERY; *oflags |= VB_INIT_OUT_CLEAR_RAM; - *oflags |= VB_INIT_OUT_ENABLE_DISPLAY; *oflags |= VB_INIT_OUT_ENABLE_USB_STORAGE; } if (vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) { *oflags |= VB_INIT_OUT_ENABLE_DEVELOPER; *oflags |= VB_INIT_OUT_CLEAR_RAM; - *oflags |= VB_INIT_OUT_ENABLE_DISPLAY; *oflags |= VB_INIT_OUT_ENABLE_USB_STORAGE; vb_sd->flags |= VBSD_BOOT_DEV_SWITCH_ON; vb_sd->flags |= VBSD_LF_DEV_SWITCH_ON; } - /* TODO: Set these in depthcharge */ - if (CONFIG(VBOOT_OPROM_MATTERS)) { - vb_sd->flags |= VBSD_OPROM_MATTERS; - /* - * Inform vboot if the display was enabled by dev/rec - * mode or was requested by vboot kernel phase. - */ - if ((*oflags & VB_INIT_OUT_ENABLE_DISPLAY) || - vboot_wants_oprom()) { - vb_sd->flags |= VBSD_OPROM_LOADED; - *oflags |= VB_INIT_OUT_ENABLE_DISPLAY; - } + /* Inform vboot if the display was requested by vboot kernel phase + or enabled by dev/rec mode. */ + if (vboot_wants_oprom() || vb2_sd->recovery_reason || + vb2_sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) { + vboot_get_working_data()->flags |= VBOOT_WD_FLAG_DISPLAY_INIT; + vb_sd->flags |= VBSD_OPROM_LOADED; } + /* TODO: Remove when depthcharge no longer reads this flag. */ + if (CONFIG(VBOOT_MUST_REQUEST_DISPLAY)) + vb_sd->flags |= VBSD_OPROM_MATTERS;
/* In vboot1, VBSD_FWB_TRIED is * set only if B is booted as explicitly requested. Therefore, if B is diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 98fdb02..217d1ea 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -119,7 +119,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/intel/baytrail/Kconfig b/src/soc/intel/baytrail/Kconfig index a83a7e9..03ad31d 100644 --- a/src/soc/intel/baytrail/Kconfig +++ b/src/soc/intel/baytrail/Kconfig @@ -43,7 +43,7 @@ select CPU_HAS_L2_ENABLE_MSR
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT diff --git a/src/soc/intel/braswell/Kconfig b/src/soc/intel/braswell/Kconfig index 8db4795..a0c0708 100644 --- a/src/soc/intel/braswell/Kconfig +++ b/src/soc/intel/braswell/Kconfig @@ -53,7 +53,7 @@ select SOUTHBRIDGE_INTEL_COMMON_SMBUS
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig index 25984f1..83ccf7b 100644 --- a/src/soc/intel/broadwell/Kconfig +++ b/src/soc/intel/broadwell/Kconfig @@ -66,7 +66,7 @@ default y
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index b730b7a..6759b7f 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -231,7 +231,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 2e9e3e2..2fbc6da 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -151,7 +151,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 5116f3a..fcfe2b6 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -106,7 +106,7 @@
config VBOOT select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS diff --git a/src/soc/mediatek/mt8173/Kconfig b/src/soc/mediatek/mt8173/Kconfig index b0d6fa0..e966b8e 100644 --- a/src/soc/mediatek/mt8173/Kconfig +++ b/src/soc/mediatek/mt8173/Kconfig @@ -16,7 +16,7 @@ if SOC_MEDIATEK_MT8173
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE
diff --git a/src/soc/mediatek/mt8183/Kconfig b/src/soc/mediatek/mt8183/Kconfig index 6582e4e..f96282c 100644 --- a/src/soc/mediatek/mt8183/Kconfig +++ b/src/soc/mediatek/mt8183/Kconfig @@ -14,7 +14,7 @@ if SOC_MEDIATEK_MT8183
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE
diff --git a/src/soc/nvidia/tegra124/Kconfig b/src/soc/nvidia/tegra124/Kconfig index eecb59b..475d50e 100644 --- a/src/soc/nvidia/tegra124/Kconfig +++ b/src/soc/nvidia/tegra124/Kconfig @@ -18,7 +18,7 @@ if SOC_NVIDIA_TEGRA124
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE
diff --git a/src/soc/nvidia/tegra210/Kconfig b/src/soc/nvidia/tegra210/Kconfig index 8883baa..6750aa9 100644 --- a/src/soc/nvidia/tegra210/Kconfig +++ b/src/soc/nvidia/tegra210/Kconfig @@ -18,7 +18,7 @@ config VBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY
config MAINBOARD_DO_DSI_INIT bool "Use dsi graphics interface" diff --git a/src/soc/qualcomm/sdm845/Kconfig b/src/soc/qualcomm/sdm845/Kconfig index c0e3294..ba06488 100644 --- a/src/soc/qualcomm/sdm845/Kconfig +++ b/src/soc/qualcomm/sdm845/Kconfig @@ -16,7 +16,7 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK
config SDM845_QSPI diff --git a/src/soc/rockchip/rk3288/Kconfig b/src/soc/rockchip/rk3288/Kconfig index 53666c2..38d8752 100644 --- a/src/soc/rockchip/rk3288/Kconfig +++ b/src/soc/rockchip/rk3288/Kconfig @@ -34,7 +34,7 @@ if SOC_ROCKCHIP_RK3288
config VBOOT - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE diff --git a/src/soc/rockchip/rk3399/Kconfig b/src/soc/rockchip/rk3399/Kconfig index fb95499..897a597 100644 --- a/src/soc/rockchip/rk3399/Kconfig +++ b/src/soc/rockchip/rk3399/Kconfig @@ -20,7 +20,7 @@ select VBOOT_MIGRATE_WORKING_DATA select VBOOT_SEPARATE_VERSTAGE select VBOOT_RETURN_FROM_VERSTAGE - select VBOOT_OPROM_MATTERS + select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK
config PMIC_BUS
-
Aaron Durbin (Code Review) -
build bot (Jenkins) (Code Review)
-
Joel Kitching (Code Review)
-
Julius Werner (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)