Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Makefile, Kconfig: Add AMD dependency for amd_blobs repo
Add Kconfig options and a target for cloning and updating the amd_blobs repo. Users should only download the repo after implicitely agreeing to AMD's License text. No formal documented agreement is required.
Users may opt to clone the repo, as well as select the git reference to use. The default is origin/master although any may be used, e.g. to support a development effort or a git bisect.
All makefile targets and dependencies are if'ed with CONFIG_USE_AMD_BLOBS to prevent accidental cloning.
Change-Id: I4ae807659db16756453dc3db2c51848291c681b8 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Makefile.inc M src/Kconfig 2 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/36416/1
diff --git a/Makefile.inc b/Makefile.inc index f7f3708..48f31ff 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -207,6 +207,29 @@ COREBOOT_EXPORTS += UPDATED_SUBMODULES endif
+#ifeq ($(CONFIG_USE_AMD_BLOBS),y) +amd_blobs_name=amd_blobs +amd_blobs_dir=3rdparty/$(amd_blobs_name) +amd_blobs_repo=https://review.coreboot.org/amd_blobs.git + +$(amd_blobs_dir): + $(warning start cd now for clone $(amd_blobs_dir)) + @echo " Cloning $(amd_blobs_name) from Git" + @git clone $(amd_blobs_repo) $(amd_blobs_dir) + +use_amd_blobs: $(amd_blobs_dir) + # if origin/master, always fetch, else fetch only if commit isn't present + if [ "$(CONFIG_AMDBLOBS_REVISION)" = "origin/master" ]; then \ + echo " Fetching new commits from the $(amd_blobs_name) Git repo"; \ + git -C $(amd_blobs_dir) fetch 2>/dev/null; \ + else \ + git -C $(amd_blobs_dir) cat-file -e $(CONFIG_AMDBLOBS_REVISION) || \ + git -C $(amd_blobs_dir) fetch 2>/dev/null; fi + @git -C $(amd_blobs_dir) checkout $(CONFIG_AMDBLOBS_REVISION) 2>/dev/null; + +PHONY+=use_amd_blobs +#endif + postcar-c-deps:=$$(OPTION_TABLE_H) ramstage-c-deps:=$$(OPTION_TABLE_H) romstage-c-deps:=$$(OPTION_TABLE_H) diff --git a/src/Kconfig b/src/Kconfig index 4c71f28..18d35b3 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -235,6 +235,26 @@ might be required for some chipsets or boards. This flag ensures that a "Free" option remains available for users.
+config USE_AMD_BLOBS + bool "Allow AMD blobs repository (with license agreement)" + help + This draws in the amd_blobs repository, which contains binary files + distributed by AMD, including VBIOS, PSP bootloaders, SMU firmwares, + etc. Selecting this item to download or clone the repo implies your + agreement to the AMD license agreement. A copy of the license text + may be reviewed by reading Documentation/soc/amd/amdblobs_license.md, + and your copy of the license is present in the repo once downloaded. + + Note that for some products, omitting PSP, SMU images, or other items + may result in a nonbooting coreboot.rom. + +config AMDBLOBS_REVISION + string "AMD blobs repo Git reference" + depends on USE_AMD_BLOBS + default "origin/master" + help + The specific commit's SHA-1 or branch name to use. + config COVERAGE bool "Code coverage support" depends on COMPILER_GCC
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Patch Set 1:
Thanks for handling the license issue with so much care.
I wonder, though, is there a reason not to integrate it as a submodule? We already have other blob repos guarded by a Kconfig. Could we handle the AMD repo in a similar way?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Patch Set 1:
Patch Set 1:
Thanks for handling the license issue with so much care.
I wonder, though, is there a reason not to integrate it as a submodule? We already have other blob repos guarded by a Kconfig. Could we handle the AMD repo in a similar way?
I agree, a submodule with update=none would fit in with the other git repos we import.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36416
to look at the new patch set (#2).
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Makefile, Kconfig: Add AMD dependency for amd_blobs repo
Add a Kconfig option for indicating agreement to use the contents of amd_blobs. Users should only download the repo after implicitely agreeing to AMD's License text. No formal documented agreement is required.
Update Makfile.inc, similar to other submodules, to initialize and checkout the submodule once the Kconfig option is selected.
Change-Id: I4ae807659db16756453dc3db2c51848291c681b8 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Makefile.inc M src/Kconfig 2 files changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/36416/2
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Thanks for handling the license issue with so much care.
I wonder, though, is there a reason not to integrate it as a submodule? We already have other blob repos guarded by a Kconfig. Could we handle the AMD repo in a similar way?
I agree, a submodule with update=none would fit in with the other git repos we import.
I didn't realize that way was so flexible. Thanks for the suggestion; that's much better than before.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Patch Set 2: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36416/2/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36416/2/Makefile.inc@205 PS2, Line 205: endif This is inside `ifeq ($(CONFIG_USE_BLOBS),y)` but Kconfig doesn't reflect this dependency, which could lead to some confusion... e.g. "Why doesn't it work? I've selected it in Kconfi!!!!111!!1"
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36416
to look at the new patch set (#3).
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Makefile, Kconfig: Add AMD dependency for amd_blobs repo
Add a Kconfig option for indicating agreement to use the contents of amd_blobs. Users should only download the repo after implicitely agreeing to AMD's License text. No formal documented agreement is required.
Update Makfile.inc, similar to other submodules, to initialize and checkout the submodule once the Kconfig option is selected.
Change-Id: I4ae807659db16756453dc3db2c51848291c681b8 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Makefile.inc M src/Kconfig 2 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/36416/3
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36416/2/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36416/2/Makefile.inc@205 PS2, Line 205: endif
This is inside `ifeq ($(CONFIG_USE_BLOBS),y)` but Kconfig doesn't reflect […]
Yes, meant to do that when I changed from PS1 to this.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/36416/2/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36416/2/Makefile.inc@205 PS2, Line 205: endif
Yes, meant to do that when I changed from PS1 to this.
We can still see how this turns out for mainboards using the AMD repo. If there will be boards that only need the AMD blobs but not the general blobs repo, we can also remove the dependency, I guess.
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36416 )
Change subject: Makefile, Kconfig: Add AMD dependency for amd_blobs repo ......................................................................
Makefile, Kconfig: Add AMD dependency for amd_blobs repo
Add a Kconfig option for indicating agreement to use the contents of amd_blobs. Users should only download the repo after implicitely agreeing to AMD's License text. No formal documented agreement is required.
Update Makfile.inc, similar to other submodules, to initialize and checkout the submodule once the Kconfig option is selected.
Change-Id: I4ae807659db16756453dc3db2c51848291c681b8 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36416 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M Makefile.inc M src/Kconfig 2 files changed, 17 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/Makefile.inc b/Makefile.inc index 3ca113b..e8a2d52 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -204,6 +204,9 @@ ifeq ($(CONFIG_PLATFORM_USES_FSP1_0)$(CONFIG_PLATFORM_USES_FSP1_1)$(CONFIG_PLATFORM_USES_FSP2_0),y) forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp)) endif +ifeq ($(CONFIG_USE_AMD_BLOBS),y) +forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/amd_blobs)) +endif endif UPDATED_SUBMODULES:=1 COREBOOT_EXPORTS += UPDATED_SUBMODULES diff --git a/src/Kconfig b/src/Kconfig index 4c71f28..0d7c934 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -235,6 +235,20 @@ might be required for some chipsets or boards. This flag ensures that a "Free" option remains available for users.
+config USE_AMD_BLOBS + bool "Allow AMD blobs repository (with license agreement)" + depends on USE_BLOBS + help + This draws in the amd_blobs repository, which contains binary files + distributed by AMD, including VBIOS, PSP bootloaders, SMU firmwares, + etc. Selecting this item to download or clone the repo implies your + agreement to the AMD license agreement. A copy of the license text + may be reviewed by reading Documentation/soc/amd/amdblobs_license.md, + and your copy of the license is present in the repo once downloaded. + + Note that for some products, omitting PSP, SMU images, or other items + may result in a nonbooting coreboot.rom. + config COVERAGE bool "Code coverage support" depends on COMPILER_GCC
-
Marshall Dawson (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)