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.