Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46112 )
Change subject: soc/amd/picasso: Refactor workbuf check into separate function ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46112/1/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/46112/1/src/soc/amd/picasso/bootblo... PS1, Line 115: : #if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)
I'm not crazy about this, but there are things that aren't defined if VBOOT_STARTS_BEFORE_BOOTBLOCK […]
If you add a vboot_bootblock.c or some such, you can just move all this into that file and conditionally ad it: bootblock-$(VBOOT) += vboot_bootblock.c
The only call that would be required is the conditional you have below in bootblock_soc_init(). I whipped a patch up for you.
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 1e9ba4a124..b7b6cfe9ad 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -23,6 +23,8 @@ bootblock-y += gpio.c bootblock-y += smi_util.c bootblock-y += config.c bootblock-y += reset.c +bootblock-y += pmutil.c +bootblock-$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK) += vboot_transfer.c
romstage-y += i2c.c romstage-y += romstage.c diff --git a/src/soc/amd/picasso/bootblock/bootblock.c b/src/soc/amd/picasso/bootblock/bootblock.c index 470002789f..2ef589762c 100644 --- a/src/soc/amd/picasso/bootblock/bootblock.c +++ b/src/soc/amd/picasso/bootblock/bootblock.c @@ -132,19 +132,8 @@ void bootblock_soc_init(void) u32 val = cpuid_eax(1); printk(BIOS_DEBUG, "Family_Model: %08x\n", val);
-#if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) - if (*(uint32_t *)_vboot2_work != VB2_SHARED_DATA_MAGIC) { - printk(BIOS_ERR, "ERROR: VBOOT workbuf not valid.\n"); - - printk(BIOS_DEBUG, "Signature: %#08x\n", *(uint32_t *)_vboot2_work); - - cmos_init(0); - cmos_write(CMOS_RECOVERY_MAGIC_VAL, CMOS_RECOVERY_BYTE); - warm_reset(); - } else { - cmos_write(0x00, CMOS_RECOVERY_BYTE); - } -#endif + if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) + verify_psp_transfer_buf();
fch_early_init(); } diff --git a/src/soc/amd/picasso/include/soc/psp_transfer.h b/src/soc/amd/picasso/include/soc/psp_transfer.h index be88ce876d..fdd7bd1fd9 100644 --- a/src/soc/amd/picasso/include/soc/psp_transfer.h +++ b/src/soc/amd/picasso/include/soc/psp_transfer.h @@ -41,6 +41,10 @@ struct transfer_info_struct {
_Static_assert(sizeof(struct transfer_info_struct) == TRANSFER_INFO_SIZE, \ "TRANSFER_INFO_SIZE is incorrect"); + +/* Make sure the PSP transferred information over to x86 side. */ +void verify_psp_transfer_buf(void); + #endif
#endif /* PSP_VERSTAGE_PSP_TRANSFER_H */ diff --git a/src/soc/amd/picasso/vboot_transfer.c b/src/soc/amd/picasso/vboot_transfer.c new file mode 100644 index 0000000000..666c5b1479 --- /dev/null +++ b/src/soc/amd/picasso/vboot_transfer.c @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <amdblocks/reset.h> +#include <console/console.h> +#include <pc80/mc146818rtc.h> +#include <security/vboot/vbnv.h> +#include <security/vboot/symbols.h> +#include <soc/psp_transfer.h> +#include <2struct.h> + +void verify_psp_transfer_buf(void) +{ + if (*(uint32_t *)_vboot2_work == VB2_SHARED_DATA_MAGIC) { + cmos_write(0x00, CMOS_RECOVERY_BYTE); + return; + } + + /* + * If CMOS is valid and the system has already been rebooted once, but + * still returns here, instead of rebooting to verstage again, assume + * that the system is in a reboot loop and halt. + */ + if (cmos_read(RTC_CLK_ALTCENTURY) != 0xff && + cmos_read(CMOS_RECOVERY_BYTE) == CMOS_RECOVERY_MAGIC_VAL) + die("Error: Reboot into recovery was unsuccessful. Halting."); + + printk(BIOS_ERR, "ERROR: VBOOT workbuf not valid.\n"); + printk(BIOS_DEBUG, "Signature: %#08x\n", *(uint32_t *)_vboot2_work); + cmos_init(0); + cmos_write(CMOS_RECOVERY_MAGIC_VAL, CMOS_RECOVERY_BYTE); + warm_reset(); +}