Change in coreboot[master]: soc/amd/picasso: Add Kconfig option for the PSP bootloader filename
Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41436 )
Change subject: soc/amd/picasso: Add Kconfig option for the PSP bootloader filename ......................................................................
soc/amd/picasso: Add Kconfig option for the PSP bootloader filename
Add option to change bootloader file.
BUG=b:149934526 TEST=Change option and verify new bootloader file is used. Using the amd_blobs I can only boot using PspBootLoader_test_RV_dbg.sbin.
Change-Id: Ib6597f7d4ffa0d48aead6974bd7111c987418f20 Signed-off-by: Martin Roth martinroth@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Signed-off-by: Raul E Rangel rrangel@chromium.org --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc 2 files changed, 15 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/41436/1
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 1d49f2a..d56270c 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -377,10 +377,20 @@ If unsure, answer 'n'
config PSP_WHITELIST_FILE - string "Debug whitelist file name" + string "Debug whitelist file path" depends on HAVE_PSP_WHITELIST_FILE default "3rdparty/amd_blobs/picasso/PSP/wtl-rvn.sbin"
+config PSP_BOOTLOADER_FILE + string "Specify the PSP Bootloader file path" + default "3rdparty/amd_blobs/picasso/PSP/PspBootLoader_WL_RV.sbin" if HAVE_PSP_WHITELIST_FILE + default "3rdparty/amd_blobs/picasso/PSP/PspBootLoader_prod_RV.sbin" + help + Supply the name of the PSP bootloader file. + + Note that this option may conflict with the whitelist file if a + different PSP bootloader binary is specified. + config PSP_UNLOCK_SECURE_DEBUG bool "Unlock secure debug" default n diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index e015fbe..51c0855 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -104,11 +104,11 @@ FIRMWARE_LOCATE=$(dir $(call strip_quotes, $(CONFIG_AMD_PUBKEY_FILE)))
# type = 0x1 -ifeq ($(CONFIG_HAVE_PSP_WHITELIST_FILE),y) -PSPBTLDR_FILE=$(top)/$(FIRMWARE_LOCATE)/PspBootLoader_WL_RV.sbin -else -PSPBTLDR_FILE=$(top)/$(FIRMWARE_LOCATE)/PspBootLoader_prod_RV.sbin +ifeq ($(CONFIG_PSP_BOOTLOADER_FILE),) +$(error CONFIG_PSP_BOOTLOADER_FILE was not defined) endif +PSPBTLDR_FILE=$(realpath $(call strip_quotes, $(CONFIG_PSP_BOOTLOADER_FILE))) +$(info Adding PSP $(shell md5sum $(PSPBTLDR_FILE)))
# types = 0x8 and 0x12 PSP_SMUFW1_SUB1_FILE=$(top)/$(FIRMWARE_LOCATE)/SmuFirmwareRV2.csbin
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41436 )
Change subject: soc/amd/picasso: Add Kconfig option for the PSP bootloader filename ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/41436/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/41436/1/src/soc/amd/picasso/Kconfig... PS1, Line 386: PspBootLoader_WL_RV This file doesn't really exist in amd_blobs repo. Shouldn't this and PSP_WHITELIST_FILE be explicitly set by the mainboard when using it rather than having a default here?
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41436 )
Change subject: soc/amd/picasso: Add Kconfig option for the PSP bootloader filename ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41436/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/41436/1/src/soc/amd/picasso/Kconfig... PS1, Line 386: PspBootLoader_WL_RV
This file doesn't really exist in amd_blobs repo. […]
I would rather leave it. It's one less config for folks working with HDT to have to mess with. Unless you feel strongly about it?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41436 )
Change subject: soc/amd/picasso: Add Kconfig option for the PSP bootloader filename ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41436/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/41436/1/src/soc/amd/picasso/Kconfig... PS1, Line 386: PspBootLoader_WL_RV
I would rather leave it. It's one less config for folks working with HDT to have to mess with. […]
No, I think that is fine.
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41436 )
Change subject: soc/amd/picasso: Add Kconfig option for the PSP bootloader filename ......................................................................
soc/amd/picasso: Add Kconfig option for the PSP bootloader filename
Add option to change bootloader file.
BUG=b:149934526 TEST=Change option and verify new bootloader file is used. Using the amd_blobs I can only boot using PspBootLoader_test_RV_dbg.sbin.
Change-Id: Ib6597f7d4ffa0d48aead6974bd7111c987418f20 Signed-off-by: Martin Roth martinroth@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Signed-off-by: Raul E Rangel rrangel@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/41436 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc 2 files changed, 15 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 71630b8..2578b46 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -341,10 +341,20 @@ If unsure, answer 'n'
config PSP_WHITELIST_FILE - string "Debug whitelist file name" + string "Debug whitelist file path" depends on HAVE_PSP_WHITELIST_FILE default "3rdparty/amd_blobs/picasso/PSP/wtl-rvn.sbin"
+config PSP_BOOTLOADER_FILE + string "Specify the PSP Bootloader file path" + default "3rdparty/amd_blobs/picasso/PSP/PspBootLoader_WL_RV.sbin" if HAVE_PSP_WHITELIST_FILE + default "3rdparty/amd_blobs/picasso/PSP/PspBootLoader_prod_RV.sbin" + help + Supply the name of the PSP bootloader file. + + Note that this option may conflict with the whitelist file if a + different PSP bootloader binary is specified. + config PSP_UNLOCK_SECURE_DEBUG bool "Unlock secure debug" default n diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 6fc834e..ef2b6b1 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -105,11 +105,11 @@ FIRMWARE_LOCATE=$(dir $(call strip_quotes, $(CONFIG_AMD_PUBKEY_FILE)))
# type = 0x1 -ifeq ($(CONFIG_HAVE_PSP_WHITELIST_FILE),y) -PSPBTLDR_FILE=$(top)/$(FIRMWARE_LOCATE)/PspBootLoader_WL_RV.sbin -else -PSPBTLDR_FILE=$(top)/$(FIRMWARE_LOCATE)/PspBootLoader_prod_RV.sbin +ifeq ($(CONFIG_PSP_BOOTLOADER_FILE),) +$(error CONFIG_PSP_BOOTLOADER_FILE was not defined) endif +PSPBTLDR_FILE=$(realpath $(call strip_quotes, $(CONFIG_PSP_BOOTLOADER_FILE))) +$(info Adding PSP $(shell md5sum $(PSPBTLDR_FILE)))
# types = 0x8 and 0x12 PSP_SMUFW1_SUB1_FILE=$(top)/$(FIRMWARE_LOCATE)/SmuFirmwareRV2.csbin
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41436 )
Change subject: soc/amd/picasso: Add Kconfig option for the PSP bootloader filename ......................................................................
Patch Set 2:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3578 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3577 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3576 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3575
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Felix Held (Code Review)
-
Furquan Shaikh (Code Review)
-
Raul Rangel (Code Review)