Change in coreboot[master]: soc/amd/picasso: Reboot for recovery if no psp workbuf is found
Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
soc/amd/picasso: Reboot for recovery if no psp workbuf is found
Instead of halting if the vboot workbuf is not passed to coreboot by the PSP, reset and reboot into recovery mode.
This process is made more difficult because if the workbuf isn't available, we can't reboot directly into recovery - the workbuf is needed for that process. To overcome this, we set a CMOS flag, and reset the system. Psp_verstage checks for this flag so it will update vbnv and reset the system after generating the workbuf.
BUG=b:152638343 TEST=Simulate the workbuf not being present and verify the reboot process.
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I049db956a5209904b274747be28ff226ce542316 --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/bootblock/bootblock.c M src/soc/amd/picasso/include/soc/psp_transfer.h M src/soc/amd/picasso/psp_verstage/psp_verstage.c M src/soc/amd/picasso/psp_verstage/psp_verstage.h 6 files changed, 46 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/44538/1
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 1b83000..5258f67 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -505,6 +505,15 @@ Runs verstage on the PSP. Only available on certain Chrome OS branded parts from AMD.
+config CMOS_RECOVERY_BYTE + hex + default 0x6F + depends on VBOOT_STARTS_BEFORE_BOOTBLOCK + help + If the workbuf is not passed from the PSP to coreboot, set the + recovery flag and reboot. The PSP will pick up this flag, mark the + recovery request in VBNV, and reset the system into recovery mode. + if VBOOT_SLOTS_RW_AB && VBOOT_STARTS_BEFORE_BOOTBLOCK
config RWA_REGION_ONLY diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index bbb064f..07bb71c 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -22,6 +22,7 @@ bootblock-y += gpio.c bootblock-y += smi_util.c bootblock-y += config.c +bootblock-y += reset.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 556fbad..fdd97f4 100644 --- a/src/soc/amd/picasso/bootblock/bootblock.c +++ b/src/soc/amd/picasso/bootblock/bootblock.c @@ -2,6 +2,7 @@
#include <stdint.h> #include <symbols.h> +#include <amdblocks/reset.h> #include <bootblock_common.h> #include <console/console.h> #include <cpu/x86/cache.h> @@ -9,6 +10,8 @@ #include <cpu/amd/msr.h> #include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> +#include <pc80/mc146818rtc.h> +#include <soc/psp_transfer.h> #include <soc/southbridge.h> #include <soc/i2c.h> #include <amdblocks/amd_pci_mmconf.h> @@ -135,7 +138,11 @@
printk(BIOS_DEBUG, "Signature: %#08x\n", *(uint32_t *)_vboot2_work);
- die("Halting.\n"); + cmos_init(0); + cmos_write(CMOS_RECOVERY_FLAG, CMOS_RECOVERY_BYTE); + warm_reset(); + } else { + cmos_write(0x00, CMOS_RECOVERY_BYTE); } #endif
diff --git a/src/soc/amd/picasso/include/soc/psp_transfer.h b/src/soc/amd/picasso/include/soc/psp_transfer.h index f377e65..0e1139d 100644 --- a/src/soc/amd/picasso/include/soc/psp_transfer.h +++ b/src/soc/amd/picasso/include/soc/psp_transfer.h @@ -3,6 +3,14 @@ #ifndef PSP_VERSTAGE_PSP_TRANSFER_H #define PSP_VERSTAGE_PSP_TRANSFER_H
+# if (CONFIG_CMOS_RECOVERY_BYTE != 0) +# define CMOS_RECOVERY_BYTE CONFIG_CMOS_RECOVERY_BYTE +# elif CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) +# error "Must set CONFIG_CMOS_RECOVERY_BYTE" +# endif + +#define CMOS_RECOVERY_FLAG 0x96 + #define TRANSFER_INFO_SIZE 64 #define TIMESTAMP_BUFFER_SIZE 0x200
diff --git a/src/soc/amd/picasso/psp_verstage/psp_verstage.c b/src/soc/amd/picasso/psp_verstage/psp_verstage.c index 11b39df..676bc3d 100644 --- a/src/soc/amd/picasso/psp_verstage/psp_verstage.c +++ b/src/soc/amd/picasso/psp_verstage/psp_verstage.c @@ -5,10 +5,11 @@ #include <bl_uapp/bl_syscall_public.h> #include <boot_device.h> #include <cbfs.h> -#include <commonlib/region.h> #include <console/console.h> #include <fmap.h> +#include <pc80/mc146818rtc.h> #include <soc/psp_transfer.h> +#include <security/vboot/vbnv.h> #include <security/vboot/misc.h> #include <security/vboot/symbols.h> #include <security/vboot/vboot_common.h> @@ -35,6 +36,21 @@ vboot_reboot(); }
+static uint32_t check_cmos_recovery_flag(void) +{ + /* Only reset if cmos is valid */ + if (vbnv_cmos_failed()) + return 0; + + /* If flag is set, clear it, then return error to reboot */ + if (cmos_read(CMOS_RECOVERY_BYTE) == CMOS_RECOVERY_FLAG) { + cmos_write(0x00, CMOS_RECOVERY_BYTE); + return POSTCODE_CMOS_RECOVERY; + } + + return 0; +} + static uintptr_t locate_amdfw(const char *name, struct region_device *rdev) { struct cbfsf fh; @@ -214,6 +230,8 @@
vb2api_relocate(_vboot2_work, _vboot2_work, VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE, &ctx); + retval = check_cmos_recovery_flag(); + if (retval)
post_code(POSTCODE_SAVE_BUFFERS); retval = save_buffers(&ctx); diff --git a/src/soc/amd/picasso/psp_verstage/psp_verstage.h b/src/soc/amd/picasso/psp_verstage/psp_verstage.h index 3c7574d..ef5c452 100644 --- a/src/soc/amd/picasso/psp_verstage/psp_verstage.h +++ b/src/soc/amd/picasso/psp_verstage/psp_verstage.h @@ -31,6 +31,7 @@ #define POSTCODE_UPDATE_PSP_BIOS_DIR_ERROR 0xC7 #define POSTCODE_FMAP_REGION_MISSING 0xC8 #define POSTCODE_AMD_FW_MISSING 0xC9 +#define POSTCODE_CMOS_RECOVERY 0xCA
#define POSTCODE_UNMAP_SPI_ROM 0xF0 #define POSTCODE_UNMAP_FCH_DEVICES 0xF1
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... PS1, Line 234: if (retval) suspect code indent for conditional statements (8, 8)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44538/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44538/1//COMMIT_MSG@14 PS1, Line 14: needed for that process you should elaborate as to why
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig... PS1, Line 510: 0x6F How was this offset chosen?
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... File src/soc/amd/picasso/include/soc/psp_transfer.h:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... PS1, Line 12: 0x96 What is the significance of this value? And why is the macro name have a _FLAG in it? This is a value of a byte we're setting.
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... PS1, Line 234: if (retval) goto err? Seems like it should be a post_code(retval) based on what I see. But a vboot error isn't set for retval.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig... PS1, Line 510: 0x6F
How was this offset chosen?
I looked at the cmos values that were currently in use to avoid those. We use 0x70 to save VBNV, so I thought that the byte before that was an appropriate place to add the signal byte.
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... File src/soc/amd/picasso/include/soc/psp_transfer.h:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... PS1, Line 12: 0x96
What is the significance of this value? And why is the macro name have a _FLAG in it? This is a valu […]
The value is completely arbitrary. I needed a value to say that we needed to tell vboot to go into recovery. If you'd prefer a different value - 0xAD, we could use that instead. I don't think 0x00, 0x01 or 0xFF are good values, but other than those.
The word FLAG is used here in the same way that you'd use SIGNAL. Is CMOS_RECOVERY_MAGIC_VAL better?
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... PS1, Line 234: if (retval)
goto err? Seems like it should be a post_code(retval) based on what I see. […]
Thanks. Yes, there should be a "goto err;" there. That must have gotten removed in my testing and I didn't notice that it wasn't included in the patch.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44538/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44538/1//COMMIT_MSG@14 PS1, Line 14: needed for that process
you should elaborate as to why
Done
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig... PS1, Line 510: 0x6F
I looked at the cmos values that were currently in use to avoid those. […]
Sorry, we use 0x52 as the default location of VBNV. I grabbed the wrong section that we were using. The block at 0x70 is used for saving the CMOS POST values. Changed to 0x51 and added a comment about why that value is used.
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... File src/soc/amd/picasso/include/soc/psp_transfer.h:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... PS1, Line 12: 0x96
The value is completely arbitrary. […]
Changed macro name to CMOS_RECOVERY_MAGIC_VAL. I left it at 0x96 unless there's a value that you'd prefer.
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/psp_ver... PS1, Line 234: if (retval)
Thanks. Yes, there should be a "goto err;" there. […]
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/Kconfig... PS1, Line 510: 0x6F
Sorry, we use 0x52 as the default location of VBNV. I grabbed the wrong section that we were using. […]
sgtm
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... File src/soc/amd/picasso/include/soc/psp_transfer.h:
https://review.coreboot.org/c/coreboot/+/44538/1/src/soc/amd/picasso/include... PS1, Line 12: 0x96
Changed macro name to CMOS_RECOVERY_MAGIC_VAL. […]
sgtm. I wanted to understand the significance, if any, for 0x96.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 1:
push the updated patchset?
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44538
to look at the new patch set (#2).
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
soc/amd/picasso: Reboot for recovery if no psp workbuf is found
Instead of halting if the vboot workbuf is not passed to coreboot by the PSP, reset and reboot into recovery mode.
This process is made more difficult because if the workbuf isn't available, we can't reboot directly into recovery - the workbuf is needed for that process to be done through the regular calls, and we don't want to go around the vboot API and just write into VBNV directly. To overcome this, we set a CMOS flag, and reset the system. PSP_verstage checks for this flag so it will update VBNV and reset the system after generating the workbuf.
BUG=b:152638343 TEST=Simulate the workbuf not being present and verify the reboot process.
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I049db956a5209904b274747be28ff226ce542316 --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/bootblock/bootblock.c M src/soc/amd/picasso/include/soc/psp_transfer.h M src/soc/amd/picasso/psp_verstage/psp_verstage.c M src/soc/amd/picasso/psp_verstage/psp_verstage.h 6 files changed, 51 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/44538/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44538/2/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/44538/2/src/soc/amd/picasso/psp_ver... PS2, Line 48: printk(BIOS_DEBUG,"Reboot into recovery requested by coreboot\n"); space required after that ',' (ctx:VxV)
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44538
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
soc/amd/picasso: Reboot for recovery if no psp workbuf is found
Instead of halting if the vboot workbuf is not passed to coreboot by the PSP, reset and reboot into recovery mode.
This process is made more difficult because if the workbuf isn't available, we can't reboot directly into recovery - the workbuf is needed for that process to be done through the regular calls, and we don't want to go around the vboot API and just write into VBNV directly. To overcome this, we set a CMOS flag, and reset the system. PSP_verstage checks for this flag so it will update VBNV and reset the system after generating the workbuf.
BUG=b:152638343 TEST=Simulate the workbuf not being present and verify the reboot process.
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I049db956a5209904b274747be28ff226ce542316 --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/bootblock/bootblock.c M src/soc/amd/picasso/include/soc/psp_transfer.h M src/soc/amd/picasso/psp_verstage/psp_verstage.c M src/soc/amd/picasso/psp_verstage/psp_verstage.h 6 files changed, 51 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/44538/3
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.h:
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... PS3, Line 34: POSTCODE_CMOS_RECOVERY Everything in this patch looks good. However, these values are for post_code(). However, this one is not used for that. It's used as a return value for a vboot error. This should be renamed or we should be using VB_ERROR_X defines.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.h:
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... PS3, Line 34: POSTCODE_CMOS_RECOVERY
Everything in this patch looks good. However, these values are for post_code(). […]
All of these postcode values that represent errors are used for both the postcode and the vboot subcode. I added a comment in the code where they're used.
Do you still want me to change anything?
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... PS3, Line 29: subcode += PSP_VBOOT_ERROR_SUBCODE; : svc_write_postcode(subcode); For all of the postcode errors, they're used as both the postcode and the bottom 8 bits of the vboot subcode.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... File src/soc/amd/picasso/psp_verstage/psp_verstage.h:
https://review.coreboot.org/c/coreboot/+/44538/3/src/soc/amd/picasso/psp_ver... PS3, Line 34: POSTCODE_CMOS_RECOVERY
All of these postcode values that represent errors are used for both the postcode and the vboot subc […]
OK. It wasn't clear to me these were errors we propagate up.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
Patch Set 3: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44538 )
Change subject: soc/amd/picasso: Reboot for recovery if no psp workbuf is found ......................................................................
soc/amd/picasso: Reboot for recovery if no psp workbuf is found
Instead of halting if the vboot workbuf is not passed to coreboot by the PSP, reset and reboot into recovery mode.
This process is made more difficult because if the workbuf isn't available, we can't reboot directly into recovery - the workbuf is needed for that process to be done through the regular calls, and we don't want to go around the vboot API and just write into VBNV directly. To overcome this, we set a CMOS flag, and reset the system. PSP_verstage checks for this flag so it will update VBNV and reset the system after generating the workbuf.
BUG=b:152638343 TEST=Simulate the workbuf not being present and verify the reboot process.
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I049db956a5209904b274747be28ff226ce542316 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44538 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/bootblock/bootblock.c M src/soc/amd/picasso/include/soc/psp_transfer.h M src/soc/amd/picasso/psp_verstage/psp_verstage.c M src/soc/amd/picasso/psp_verstage/psp_verstage.h 6 files changed, 51 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 1b83000..642935f 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -505,6 +505,18 @@ Runs verstage on the PSP. Only available on certain Chrome OS branded parts from AMD.
+config CMOS_RECOVERY_BYTE + hex + default 0x51 + depends on VBOOT_STARTS_BEFORE_BOOTBLOCK + help + If the workbuf is not passed from the PSP to coreboot, set the + recovery flag and reboot. The PSP will read this byte, mark the + recovery request in VBNV, and reset the system into recovery mode. + + This is the byte before the default first byte used by VBNV + (0x26 + 0x0E - 1) + if VBOOT_SLOTS_RW_AB && VBOOT_STARTS_BEFORE_BOOTBLOCK
config RWA_REGION_ONLY diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index f11d895..c12b1af 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -22,6 +22,7 @@ bootblock-y += gpio.c bootblock-y += smi_util.c bootblock-y += config.c +bootblock-y += reset.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 556fbad..4700027 100644 --- a/src/soc/amd/picasso/bootblock/bootblock.c +++ b/src/soc/amd/picasso/bootblock/bootblock.c @@ -2,6 +2,7 @@
#include <stdint.h> #include <symbols.h> +#include <amdblocks/reset.h> #include <bootblock_common.h> #include <console/console.h> #include <cpu/x86/cache.h> @@ -9,6 +10,8 @@ #include <cpu/amd/msr.h> #include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> +#include <pc80/mc146818rtc.h> +#include <soc/psp_transfer.h> #include <soc/southbridge.h> #include <soc/i2c.h> #include <amdblocks/amd_pci_mmconf.h> @@ -135,7 +138,11 @@
printk(BIOS_DEBUG, "Signature: %#08x\n", *(uint32_t *)_vboot2_work);
- die("Halting.\n"); + cmos_init(0); + cmos_write(CMOS_RECOVERY_MAGIC_VAL, CMOS_RECOVERY_BYTE); + warm_reset(); + } else { + cmos_write(0x00, CMOS_RECOVERY_BYTE); } #endif
diff --git a/src/soc/amd/picasso/include/soc/psp_transfer.h b/src/soc/amd/picasso/include/soc/psp_transfer.h index 6a43b55..0996f64 100644 --- a/src/soc/amd/picasso/include/soc/psp_transfer.h +++ b/src/soc/amd/picasso/include/soc/psp_transfer.h @@ -3,6 +3,14 @@ #ifndef PSP_VERSTAGE_PSP_TRANSFER_H #define PSP_VERSTAGE_PSP_TRANSFER_H
+# if (CONFIG_CMOS_RECOVERY_BYTE != 0) +# define CMOS_RECOVERY_BYTE CONFIG_CMOS_RECOVERY_BYTE +# elif CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) +# error "Must set CONFIG_CMOS_RECOVERY_BYTE" +# endif + +#define CMOS_RECOVERY_MAGIC_VAL 0x96 + #define TRANSFER_INFO_SIZE 64 #define TIMESTAMP_BUFFER_SIZE 0x200
diff --git a/src/soc/amd/picasso/psp_verstage/psp_verstage.c b/src/soc/amd/picasso/psp_verstage/psp_verstage.c index 5568797..005c8b0 100644 --- a/src/soc/amd/picasso/psp_verstage/psp_verstage.c +++ b/src/soc/amd/picasso/psp_verstage/psp_verstage.c @@ -5,10 +5,11 @@ #include <bl_uapp/bl_syscall_public.h> #include <boot_device.h> #include <cbfs.h> -#include <commonlib/region.h> #include <console/console.h> #include <fmap.h> +#include <pc80/mc146818rtc.h> #include <soc/psp_transfer.h> +#include <security/vboot/vbnv.h> #include <security/vboot/misc.h> #include <security/vboot/symbols.h> #include <security/vboot/vboot_common.h> @@ -35,6 +36,22 @@ vboot_reboot(); }
+static uint32_t check_cmos_recovery(void) +{ + /* Only reset if cmos is valid */ + if (vbnv_cmos_failed()) + return 0; + + /* If the byte is set, clear it, then return error to reboot */ + if (cmos_read(CMOS_RECOVERY_BYTE) == CMOS_RECOVERY_MAGIC_VAL) { + cmos_write(0x00, CMOS_RECOVERY_BYTE); + printk(BIOS_DEBUG, "Reboot into recovery requested by coreboot\n"); + return POSTCODE_CMOS_RECOVERY; + } + + return 0; +} + static uintptr_t locate_amdfw(const char *name, struct region_device *rdev) { struct cbfsf fh; @@ -216,6 +233,9 @@
vb2api_relocate(_vboot2_work, _vboot2_work, VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE, &ctx); + retval = check_cmos_recovery(); + if (retval) + goto err;
post_code(POSTCODE_SAVE_BUFFERS); retval = save_buffers(&ctx); diff --git a/src/soc/amd/picasso/psp_verstage/psp_verstage.h b/src/soc/amd/picasso/psp_verstage/psp_verstage.h index 3c7574d..ef5c452 100644 --- a/src/soc/amd/picasso/psp_verstage/psp_verstage.h +++ b/src/soc/amd/picasso/psp_verstage/psp_verstage.h @@ -31,6 +31,7 @@ #define POSTCODE_UPDATE_PSP_BIOS_DIR_ERROR 0xC7 #define POSTCODE_FMAP_REGION_MISSING 0xC8 #define POSTCODE_AMD_FW_MISSING 0xC9 +#define POSTCODE_CMOS_RECOVERY 0xCA
#define POSTCODE_UNMAP_SPI_ROM 0xF0 #define POSTCODE_UNMAP_FCH_DEVICES 0xF1
-
Aaron Durbin (Code Review) -
build bot (Jenkins) (Code Review)
-
Martin Roth (Code Review)