Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
vc/amd/pi: Allow 00670F00 to build with no binaryPI
Make the default binaryPI image strings for all stoneyridge-based APUs depend on USE_AMD_BLOBS. Ensure the build completes without names, and without images.
Change-Id: I74a38efa2a4ad2f9f12a1f8e7fb8694d0ab9dd1e Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/vendorcode/amd/pi/00670F00/Makefile.inc M src/vendorcode/amd/pi/Kconfig 2 files changed, 28 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/37228/1
diff --git a/src/vendorcode/amd/pi/00670F00/Makefile.inc b/src/vendorcode/amd/pi/00670F00/Makefile.inc index fef7dff..796d130 100644 --- a/src/vendorcode/amd/pi/00670F00/Makefile.inc +++ b/src/vendorcode/amd/pi/00670F00/Makefile.inc @@ -116,10 +116,22 @@
#######################################################################
+warn_no_agesa: + printf "\n\t** WARNING **\n" + printf "coreboot has been built with no AGESA support. Successfully " + printf "booting is unlikely.\n\n" + +PHONY+=warn_no_amdfw + ifeq ($(CONFIG_AGESA_SPLIT_MEMORY_FILES), y)
# convert input elf to rmodule AGESA_POST_MEM_INPUT_ELF = $(call strip_quotes,$(CONFIG_AGESA_POST_MEMORY_BINARY_PI_FILE)) + +# If no post-mem file then also skip pre-mem file +ifeq ($(AGESA_POST_MEM_INPUT_ELF,)) +files_added:: warn_no_agesa +else AGESA_POST_MEM_ELF = $(objcbfs)/$(patsubst %.elf,%.debug,$(notdir $(AGESA_POST_MEM_INPUT_ELF))) AGESA_POST_MEM_ELF_RMOD = $(AGESA_POST_MEM_ELF).rmod
@@ -140,10 +152,18 @@ $(CONFIG_AGESA_POST_MEMORY_CBFS_NAME)-file := $(AGESA_POST_MEM_ELF_RMOD) $(CONFIG_AGESA_POST_MEMORY_CBFS_NAME)-type := stage $(CONFIG_AGESA_POST_MEMORY_CBFS_NAME)-compression := $(CBFS_COMPRESS_FLAG) + +endif # AGESA_POST_MEM_INPUT_ELF == "" + +else # CONFIG_AGESA_SPLIT_MEMORY_FILES + +AGESA_BINARYPI_INPUT_FILE = $(call strip_quotes,$(CONFIG_AGESA_BINARY_PI_FILE)) +ifeq ($(AGESA_BINARYPI_INPUT_FILE),) +files_added:: warn_no_agesa else
cbfs-files-$(CONFIG_CPU_AMD_AGESA_BINARY_PI) += $(CONFIG_AGESA_CBFS_NAME) -$(CONFIG_AGESA_CBFS_NAME)-file := $(CONFIG_AGESA_BINARY_PI_FILE) +$(CONFIG_AGESA_CBFS_NAME)-file := $(AGESA_BINARYPI_INPUT_FILE)
ifeq ($(CONFIG_AGESA_BINARY_PI_AS_STAGE),y) $(CONFIG_AGESA_CBFS_NAME)-type := stage @@ -156,6 +176,7 @@ $(CONFIG_AGESA_CBFS_NAME)-position := $(CONFIG_AGESA_BINARY_PI_LOCATION) endif # CONFIG_AGESA_BINARY_PI_AS_STAGE
+endif # AGESA_BINARYPI_INPUT_FILE == "" endif # CONFIG_AGESA_SPLIT_MEMORY_FILES
endif diff --git a/src/vendorcode/amd/pi/Kconfig b/src/vendorcode/amd/pi/Kconfig index 0605563..73fe4f2 100644 --- a/src/vendorcode/amd/pi/Kconfig +++ b/src/vendorcode/amd/pi/Kconfig @@ -44,10 +44,10 @@ string "AGESA PI binary file name" default "3rdparty/blobs/pi/amd/00630F01/FP3/AGESA.bin" if CPU_AMD_PI_00630F01 default "3rdparty/blobs/pi/amd/00730F01/FT3b/AGESA.bin" if CPU_AMD_PI_00730F01 - default "3rdparty/blobs/pi/amd/merlinfalcon/$(CONFIG_AMD_SOC_PACKAGE)/AGESA_CZ_FP4.bin" if SOC_AMD_MERLINFALCON && HAVE_MERLINFALCON_BINARIES - default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_MERLINFALCON && !HAVE_MERLINFALCON_BINARIES - default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FP4 - default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FT4 + default "3rdparty/blobs/pi/amd/merlinfalcon/$(CONFIG_AMD_SOC_PACKAGE)/AGESA_CZ_FP4.bin" if SOC_AMD_MERLINFALCON && HAVE_MERLINFALCON_BINARIES && USE_AMD_BLOBS + default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_MERLINFALCON && !HAVE_MERLINFALCON_BINARIES && USE_AMD_BLOBS + default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FP4 && USE_AMD_BLOBS + default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FT4 && USE_AMD_BLOBS default "3rdparty/blobs/pi/amd/00660F01/FP4/AGESA.bin" if CPU_AMD_PI_00660F01 help Specify the binary file to use for AMD platform initialization. @@ -72,7 +72,7 @@ config AGESA_PRE_MEMORY_BINARY_PI_FILE string depends on AGESA_SPLIT_MEMORY_FILES - default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_premem.elf" if SOC_AMD_STONEYRIDGE_FT4 + default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_premem.elf" if SOC_AMD_STONEYRIDGE_FT4 && USE_AMD_BLOBS help Specify the binary file to use for pre-memory AMD platform initialization. @@ -80,7 +80,7 @@ config AGESA_POST_MEMORY_BINARY_PI_FILE string depends on AGESA_SPLIT_MEMORY_FILES - default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_postmem.elf" if SOC_AMD_STONEYRIDGE_FT4 + default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_postmem.elf" if SOC_AMD_STONEYRIDGE_FT4 && USE_AMD_BLOBS help Specify the binary file to use for post-memory AMD platform initialization.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... File src/vendorcode/amd/pi/00670F00/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... PS1, Line 122: unlikely impossible?
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... File src/vendorcode/amd/pi/00670F00/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... PS1, Line 122: unlikely
impossible?
From my understanding, this is a temporary step. Some builds in the patch stack will be build without AGESA or even some blobs. Part of a transition to a new blobs location. This will not be the final code, and I suspect Marshall will remove this at the very end.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... File src/vendorcode/amd/pi/00670F00/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... PS1, Line 122: unlikely
From my understanding...
No, it's not a temporary step. This is the final condition. If you do not specify a binaryPI file, you can build an image but it won't boot. In the event you USE_BLOBS and USE_AMD_BLOBS, then there is a default path/to/file specified.
impossible...
I didn't want to split too many hairs or worry about precisely where it would stop. Assuming some die() doesn't get us, there will be no DRAM, etc. However, unlike in Intel with no IFD or ME, the x86 should function OK.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
Patch Set 1: -Code-Review
(1 comment)
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... File src/vendorcode/amd/pi/00670F00/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... PS1, Line 122: unlikely
From my understanding... […]
Not sure if maybe Arthur wanted to replace unlikely with impossible... I do understand as I saw the message coming out in one of my builds. I would agree with Arthur, the word impossible would be better as it won't come out of reset. However I'm not sure I agree with the message as a whole, as if it was only AGESA it would hang at boot init early, but with PSP it will not even come out of reset. Why not replace AGESA with "AMD binaries", as it's not specific and actually indicate all: PSP, video and AGESA. Than do replace unlikely with impossible.
Hello Arthur Heymans, Richard Spiegel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37228
to look at the new patch set (#2).
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
vc/amd/pi: Allow 00670F00 to build with no binaryPI
Make the default binaryPI image strings for all stoneyridge-based APUs depend on USE_AMD_BLOBS. Ensure the build completes without names, and without images.
Change-Id: I74a38efa2a4ad2f9f12a1f8e7fb8694d0ab9dd1e Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/vendorcode/amd/pi/00670F00/Makefile.inc M src/vendorcode/amd/pi/Kconfig 2 files changed, 28 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/37228/2
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... File src/vendorcode/amd/pi/00670F00/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/37228/1/src/vendorcode/amd/pi/00670... PS1, Line 122: unlikely
Not sure if maybe Arthur wanted to replace unlikely with impossible... […]
Done
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
Patch Set 2:
Richard, thoughts on this one?
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37228 )
Change subject: vc/amd/pi: Allow 00670F00 to build with no binaryPI ......................................................................
vc/amd/pi: Allow 00670F00 to build with no binaryPI
Make the default binaryPI image strings for all stoneyridge-based APUs depend on USE_AMD_BLOBS. Ensure the build completes without names, and without images.
Change-Id: I74a38efa2a4ad2f9f12a1f8e7fb8694d0ab9dd1e Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37228 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Richard Spiegel richard.spiegel@silverbackltd.com --- M src/vendorcode/amd/pi/00670F00/Makefile.inc M src/vendorcode/amd/pi/Kconfig 2 files changed, 28 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Richard Spiegel: Looks good to me, approved
diff --git a/src/vendorcode/amd/pi/00670F00/Makefile.inc b/src/vendorcode/amd/pi/00670F00/Makefile.inc index fef7dff..947134f 100644 --- a/src/vendorcode/amd/pi/00670F00/Makefile.inc +++ b/src/vendorcode/amd/pi/00670F00/Makefile.inc @@ -116,10 +116,22 @@
#######################################################################
+warn_no_agesa: + printf "\n\t** WARNING **\n" + printf "coreboot has been built with no AGESA support. Successfully " + printf "booting this image will be impossible.\n\n" + +PHONY+=warn_no_amdfw + ifeq ($(CONFIG_AGESA_SPLIT_MEMORY_FILES), y)
# convert input elf to rmodule AGESA_POST_MEM_INPUT_ELF = $(call strip_quotes,$(CONFIG_AGESA_POST_MEMORY_BINARY_PI_FILE)) + +# If no post-mem file then also skip pre-mem file +ifeq ($(AGESA_POST_MEM_INPUT_ELF,)) +files_added:: warn_no_agesa +else AGESA_POST_MEM_ELF = $(objcbfs)/$(patsubst %.elf,%.debug,$(notdir $(AGESA_POST_MEM_INPUT_ELF))) AGESA_POST_MEM_ELF_RMOD = $(AGESA_POST_MEM_ELF).rmod
@@ -140,10 +152,18 @@ $(CONFIG_AGESA_POST_MEMORY_CBFS_NAME)-file := $(AGESA_POST_MEM_ELF_RMOD) $(CONFIG_AGESA_POST_MEMORY_CBFS_NAME)-type := stage $(CONFIG_AGESA_POST_MEMORY_CBFS_NAME)-compression := $(CBFS_COMPRESS_FLAG) + +endif # AGESA_POST_MEM_INPUT_ELF == "" + +else # CONFIG_AGESA_SPLIT_MEMORY_FILES + +AGESA_BINARYPI_INPUT_FILE = $(call strip_quotes,$(CONFIG_AGESA_BINARY_PI_FILE)) +ifeq ($(AGESA_BINARYPI_INPUT_FILE),) +files_added:: warn_no_agesa else
cbfs-files-$(CONFIG_CPU_AMD_AGESA_BINARY_PI) += $(CONFIG_AGESA_CBFS_NAME) -$(CONFIG_AGESA_CBFS_NAME)-file := $(CONFIG_AGESA_BINARY_PI_FILE) +$(CONFIG_AGESA_CBFS_NAME)-file := $(AGESA_BINARYPI_INPUT_FILE)
ifeq ($(CONFIG_AGESA_BINARY_PI_AS_STAGE),y) $(CONFIG_AGESA_CBFS_NAME)-type := stage @@ -156,6 +176,7 @@ $(CONFIG_AGESA_CBFS_NAME)-position := $(CONFIG_AGESA_BINARY_PI_LOCATION) endif # CONFIG_AGESA_BINARY_PI_AS_STAGE
+endif # AGESA_BINARYPI_INPUT_FILE == "" endif # CONFIG_AGESA_SPLIT_MEMORY_FILES
endif diff --git a/src/vendorcode/amd/pi/Kconfig b/src/vendorcode/amd/pi/Kconfig index 0605563..73fe4f2 100644 --- a/src/vendorcode/amd/pi/Kconfig +++ b/src/vendorcode/amd/pi/Kconfig @@ -44,10 +44,10 @@ string "AGESA PI binary file name" default "3rdparty/blobs/pi/amd/00630F01/FP3/AGESA.bin" if CPU_AMD_PI_00630F01 default "3rdparty/blobs/pi/amd/00730F01/FT3b/AGESA.bin" if CPU_AMD_PI_00730F01 - default "3rdparty/blobs/pi/amd/merlinfalcon/$(CONFIG_AMD_SOC_PACKAGE)/AGESA_CZ_FP4.bin" if SOC_AMD_MERLINFALCON && HAVE_MERLINFALCON_BINARIES - default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_MERLINFALCON && !HAVE_MERLINFALCON_BINARIES - default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FP4 - default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FT4 + default "3rdparty/blobs/pi/amd/merlinfalcon/$(CONFIG_AMD_SOC_PACKAGE)/AGESA_CZ_FP4.bin" if SOC_AMD_MERLINFALCON && HAVE_MERLINFALCON_BINARIES && USE_AMD_BLOBS + default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_MERLINFALCON && !HAVE_MERLINFALCON_BINARIES && USE_AMD_BLOBS + default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FP4 && USE_AMD_BLOBS + default "3rdparty/blobs/pi/amd/00670F00/$(CONFIG_AMD_SOC_PACKAGE)/AGESA.bin" if SOC_AMD_STONEYRIDGE_FT4 && USE_AMD_BLOBS default "3rdparty/blobs/pi/amd/00660F01/FP4/AGESA.bin" if CPU_AMD_PI_00660F01 help Specify the binary file to use for AMD platform initialization. @@ -72,7 +72,7 @@ config AGESA_PRE_MEMORY_BINARY_PI_FILE string depends on AGESA_SPLIT_MEMORY_FILES - default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_premem.elf" if SOC_AMD_STONEYRIDGE_FT4 + default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_premem.elf" if SOC_AMD_STONEYRIDGE_FT4 && USE_AMD_BLOBS help Specify the binary file to use for pre-memory AMD platform initialization. @@ -80,7 +80,7 @@ config AGESA_POST_MEMORY_BINARY_PI_FILE string depends on AGESA_SPLIT_MEMORY_FILES - default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_postmem.elf" if SOC_AMD_STONEYRIDGE_FT4 + default "3rdparty/blobs/pi/amd/00670F00/FT4/AGESA_postmem.elf" if SOC_AMD_STONEYRIDGE_FT4 && USE_AMD_BLOBS help Specify the binary file to use for post-memory AMD platform initialization.
-
Arthur Heymans (Code Review) -
Marshall Dawson (Code Review)
-
Patrick Georgi (Code Review)
-
Richard Spiegel (Code Review)