Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38780 )
Change subject: vboot: remove VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT option ......................................................................
vboot: remove VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT option
With CL:1940398, this option is no longer needed. Recovery requests are not cleared until kernel verification stage is reached. If the FSP triggers any reboots, recovery requests will be preserved. In particular:
- Manual requests will be preserved via recovery switch state, whose behaviour is modified in CB:XXXXXX. - Other recovery requests will remain in nvdata across reboot.
BUG=b:124141368, b:35576380 TEST=make clean && make test-abuild BRANCH=none
Change-Id: I52d17a3c6730be5c04c3c0ae020368d11db6ca3c Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/Kconfig M src/security/vboot/bootmode.c M src/security/vboot/misc.h M src/security/vboot/vbnv.c M src/security/vboot/vbnv.h M src/security/vboot/vboot_logic.c M src/soc/amd/stoneyridge/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/intel/tigerlake/Kconfig 12 files changed, 2 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/38780/1
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index ea70e65..54e88dd 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -156,14 +156,6 @@ reused by the succeeding stage. This is useful if a RAM space is too small to fit both the verstage and the succeeding stage.
-config VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT - bool - default n - help - This option ensures that the recovery request is not lost because of - reboots caused after vboot verification is run. e.g. reboots caused by - FSP components on Intel platforms. - config VBOOT_MUST_REQUEST_DISPLAY bool default y if VGA_ROM_RUN diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c index 61b3d76..5f15790 100644 --- a/src/security/vboot/bootmode.c +++ b/src/security/vboot/bootmode.c @@ -50,62 +50,21 @@ return sd->recovery_reason; }
-void vboot_save_recovery_reason_vbnv(void) -{ - if (!CONFIG(VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT)) - return; - - int reason = vboot_get_recovery_reason_shared_data(); - if (!reason) - return; - - set_recovery_mode_into_vbnv(reason); -} - -static void vboot_clear_recovery_reason_vbnv(void *unused) -{ - if (!CONFIG(VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT)) - return; - - set_recovery_mode_into_vbnv(0); -} - -/* - * Recovery reason stored in VBNV needs to be cleared before the state of VBNV - * is backed-up anywhere or jumping to the payload (whichever occurs - * first). Currently, vbnv_cmos.c backs up VBNV on POST_DEVICE. Thus, we need to - * make sure that the stored recovery reason is cleared off before that - * happens. - * IMPORTANT: Any reboot occurring after BS_DEV_INIT state will cause loss of - * recovery reason on reboot. Until now, we have seen reboots occurring on x86 - * only in FSP stages which run before BS_DEV_INIT. - */ -BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, - vboot_clear_recovery_reason_vbnv, NULL); - /* * vb2_check_recovery_request looks up different components to identify if there * is a recovery request and returns appropriate reason code: * 1. Checks if recovery mode is initiated by EC. If yes, returns * VB2_RECOVERY_RO_MANUAL. - * 2. Checks if recovery request is present in VBNV and returns the code read - * from it. - * 3. Checks if vboot verification is done. If yes, return the reason code from + * 2. Checks if vboot verification is done. If yes, return the reason code from * shared data. - * 4. If nothing applies, return 0 indicating no recovery request. + * 3. If nothing applies, return 0 indicating no recovery request. */ int vboot_check_recovery_request(void) { - int reason = 0; - /* EC-initiated recovery. */ if (get_recovery_mode_switch()) return VB2_RECOVERY_RO_MANUAL;
- /* Recovery request in VBNV. */ - if ((reason = get_recovery_mode_from_vbnv()) != 0) - return reason; - /* Identify if vboot verification is already complete. */ if (vboot_logic_executed()) return vboot_get_recovery_reason_shared_data(); diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index 2d5b084..c629ca5 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -52,11 +52,6 @@ int vboot_locate_firmware(struct vb2_context *ctx, struct region_device *fw);
/* - * Source: security/vboot/bootmode.c - */ -void vboot_save_recovery_reason_vbnv(void); - -/* * The stage loading code is compiled and entered from multiple stages. The * helper functions below attempt to provide more clarity on when certain * code should be called. They are implemented inline for better compile-time diff --git a/src/security/vboot/vbnv.c b/src/security/vboot/vbnv.c index be598ac..a5a7806 100644 --- a/src/security/vboot/vbnv.c +++ b/src/security/vboot/vbnv.c @@ -101,26 +101,6 @@ vbnv_initialized = 0; }
-/* Save a recovery reason into VBNV. */ -void set_recovery_mode_into_vbnv(int recovery_reason) -{ - uint8_t vbnv_copy[VBOOT_VBNV_BLOCK_SIZE]; - - read_vbnv(vbnv_copy); - - vbnv_copy[RECOVERY_OFFSET] = recovery_reason; - vbnv_copy[CRC_OFFSET] = crc8_vbnv(vbnv_copy, CRC_OFFSET); - - save_vbnv(vbnv_copy); -} - -/* Read the recovery reason from VBNV. */ -int get_recovery_mode_from_vbnv(void) -{ - vbnv_setup(); - return vbnv[RECOVERY_OFFSET]; -} - /* Read the USB Device Controller(UDC) enable flag from VBNV. */ int vbnv_udc_enable_flag(void) { diff --git a/src/security/vboot/vbnv.h b/src/security/vboot/vbnv.h index a2f0b4c..7d288d5 100644 --- a/src/security/vboot/vbnv.h +++ b/src/security/vboot/vbnv.h @@ -23,8 +23,6 @@ void save_vbnv(const uint8_t *vbnv_copy); int verify_vbnv(uint8_t *vbnv_copy); void regen_vbnv_crc(uint8_t *vbnv_copy); -int get_recovery_mode_from_vbnv(void); -void set_recovery_mode_into_vbnv(int recovery_reason);
/* Read the USB Device Controller(UDC) enable flag from VBNV. */ int vbnv_udc_enable_flag(void); diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index eabf8f7..465c85b 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -429,8 +429,5 @@ vboot_is_firmware_slot_a(ctx) ? 'A' : 'B');
verstage_main_exit: - /* Save recovery reason in case of unexpected reboots on x86. */ - vboot_save_recovery_reason_vbnv(); - timestamp_add_now(TS_END_VBOOT); } diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig index c3fcad9..7d69a92 100644 --- a/src/soc/amd/stoneyridge/Kconfig +++ b/src/soc/amd/stoneyridge/Kconfig @@ -93,7 +93,6 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_STARTS_IN_BOOTBLOCK - select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_VBNV_CMOS select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 0d69da2..6c90294 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -113,7 +113,6 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_MUST_REQUEST_DISPLAY - select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index d098785..b68e93d 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -260,7 +260,6 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_MUST_REQUEST_DISPLAY - select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 15e895f..210fd0d 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -165,7 +165,6 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_MUST_REQUEST_DISPLAY - select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 6277cea..7382df0 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -93,7 +93,6 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_MUST_REQUEST_DISPLAY - select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index cef1fd0..56a0d74 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -189,7 +189,6 @@ config VBOOT select VBOOT_SEPARATE_VERSTAGE select VBOOT_MUST_REQUEST_DISPLAY - select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_VBNV_CMOS select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH