Change in coreboot[master]: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO
Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44046 )
Change subject: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO ......................................................................
soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO
When booting from the RO region of a VBOOT enabled ROM, there shouldn't be a reliance on anything outside of the RO section. This includes the APOB_NV region (similar to the MRC cache region). By skipping the region when setting up the BIOS Directory table, the PSP won't try to use the region when booting.
The APOB_NV region is still used for the VBOOT RW sections.
BUG=b:158363448 TEST=Build RO with no APOB_NV region. Dump the BDT and verify that it's not in RO, but is in RW_A & RW_B. Boot into recovery.
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I13c35ba8a2331492744d2acf257db15e4a53102a --- M src/soc/amd/picasso/Makefile.inc 1 file changed, 11 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/44046/1
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 4c1d726..ff2db4a 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -323,6 +323,11 @@ OPT_APOB_NV_SIZE=$(call add_opt_prefix, $(APOB_NV_SIZE), --apob-nv-size) OPT_APOB_NV_BASE=$(call add_opt_prefix, $(APOB_NV_BASE),--apob-nv-base)
+ifeq ($(CONFIG_VBOOT),) +OPT_APOB0_NV_SIZE=$(OPT_APOB_NV_SIZE) +OPT_APOB0_NV_BASE=$(OPT_APOB_NV_BASE) +endif + AMDFW_COMMON_ARGS=$(OPT_AMD_PUBKEY_FILE) \ $(OPT_PSPBTLDR_FILE) \ $(OPT_PSPSCUREOS_FILE) \ @@ -333,8 +338,6 @@ $(OPT_SMUFW2_SUB1_FILE) \ $(OPT_PSP_APCB_FILES) \ $(OPT_APOB_ADDR) \ - $(OPT_APOB_NV_SIZE) \ - $(OPT_APOB_NV_BASE) \ $(OPT_PSP_BIOSBIN_FILE) \ $(OPT_PSP_BIOSBIN_DEST) \ $(OPT_PSP_BIOSBIN_SIZE) \ @@ -458,6 +461,8 @@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ $(AMDFW_COMMON_ARGS) \ + $(OPT_APOB0_NV_SIZE) \ + $(OPT_APOB0_NV_BASE) \ --location $(shell printf "%#x" $(PICASSO_FWM_POSITION)) \ --output $@
@@ -472,6 +477,8 @@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ $(AMDFW_COMMON_ARGS) \ + $(OPT_APOB_NV_SIZE) \ + $(OPT_APOB_NV_BASE) \ --location $(shell printf "%#x" $(CONFIG_PICASSO_FW_A_POSITION)) \ --anywhere \ --output $@ @@ -481,6 +488,8 @@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ $(AMDFW_COMMON_ARGS) \ + $(OPT_APOB_NV_SIZE) \ + $(OPT_APOB_NV_BASE) \ --location $(shell printf "%#x" $(CONFIG_PICASSO_FW_B_POSITION)) \ --anywhere \ --output $@
Martin Roth has removed Patrick Georgi from this change. ( https://review.coreboot.org/c/coreboot/+/44046 )
Change subject: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO ......................................................................
Removed reviewer Patrick Georgi.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44046 )
Change subject: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO ......................................................................
Patch Set 1: Code-Review+2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44046 )
Change subject: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44046 )
Change subject: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO ......................................................................
Patch Set 1: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44046 )
Change subject: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO ......................................................................
soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO
When booting from the RO region of a VBOOT enabled ROM, there shouldn't be a reliance on anything outside of the RO section. This includes the APOB_NV region (similar to the MRC cache region). By skipping the region when setting up the BIOS Directory table, the PSP won't try to use the region when booting.
The APOB_NV region is still used for the VBOOT RW sections.
BUG=b:158363448 TEST=Build RO with no APOB_NV region. Dump the BDT and verify that it's not in RO, but is in RW_A & RW_B. Boot into recovery.
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I13c35ba8a2331492744d2acf257db15e4a53102a Reviewed-on: https://review.coreboot.org/c/coreboot/+/44046 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/Makefile.inc 1 file changed, 11 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index e29dbc2..f11d895 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -326,6 +326,11 @@ OPT_EFS_SPI_SPEED=$(call add_opt_prefix, $(CONFIG_EFS_SPI_SPEED), --spi-speed) OPT_EFS_SPI_MICRON_FLAG=$(call add_opt_prefix, $(CONFIG_EFS_SPI_MICRON_FLAG), --spi-micron-flag)
+ifeq ($(CONFIG_VBOOT),) +OPT_APOB0_NV_SIZE=$(OPT_APOB_NV_SIZE) +OPT_APOB0_NV_BASE=$(OPT_APOB_NV_BASE) +endif + AMDFW_COMMON_ARGS=$(OPT_AMD_PUBKEY_FILE) \ $(OPT_PSPBTLDR_FILE) \ $(OPT_PSPSCUREOS_FILE) \ @@ -336,8 +341,6 @@ $(OPT_SMUFW2_SUB1_FILE) \ $(OPT_PSP_APCB_FILES) \ $(OPT_APOB_ADDR) \ - $(OPT_APOB_NV_SIZE) \ - $(OPT_APOB_NV_BASE) \ $(OPT_PSP_BIOSBIN_FILE) \ $(OPT_PSP_BIOSBIN_DEST) \ $(OPT_PSP_BIOSBIN_SIZE) \ @@ -465,6 +468,8 @@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ $(AMDFW_COMMON_ARGS) \ + $(OPT_APOB0_NV_SIZE) \ + $(OPT_APOB0_NV_BASE) \ --location $(shell printf "%#x" $(PICASSO_FWM_POSITION)) \ --output $@
@@ -479,6 +484,8 @@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ $(AMDFW_COMMON_ARGS) \ + $(OPT_APOB_NV_SIZE) \ + $(OPT_APOB_NV_BASE) \ --location $(shell printf "%#x" $(CONFIG_PICASSO_FW_A_POSITION)) \ --anywhere \ --output $@ @@ -488,6 +495,8 @@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ $(AMDFW_COMMON_ARGS) \ + $(OPT_APOB_NV_SIZE) \ + $(OPT_APOB_NV_BASE) \ --location $(shell printf "%#x" $(CONFIG_PICASSO_FW_B_POSITION)) \ --anywhere \ --output $@
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44046 )
Change subject: soc/amd/picasso: If using VBOOT, skip the APOB_NV region for RO ......................................................................
Patch Set 4:
Automatic boot test returned (PASS/FAIL/TOTAL): 6/1/7 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/16111 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16110 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/16109 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16108 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/16107 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/16113 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16112
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Raul Rangel (Code Review)