Frans Hendriks has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
configs/config.facebookfbg1710: Add config file
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A configs/config.facebookfbg1710 1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/34343/1
diff --git a/configs/config.facebookfbg1710 b/configs/config.facebookfbg1710 new file mode 100644 index 0000000..be2e6d8 --- /dev/null +++ b/configs/config.facebookfbg1710 @@ -0,0 +1,10 @@ +CONFIG_VENDOR_FACEBOOK=y +CONFIG_C_ENV_BOOTBLOCK_SIZE=0x6000 +CONFIG_ONBOARD_SAMSUNG_MEM=y +CONFIG_CPU_MICROCODE_CBFS_LOC=0xFFF8B000 +CONFIG_VENDORCODE_ELTAN_OEM_MANIFEST_LOC=0xfffe9000 +CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS=y +CONFIG_RUN_FSP_GOP=y +CONFIG_DISPLAY_HOBS=y +CONFIG_DEFAULT_CONSOLE_LOGLEVEL_8=y +CONFIG_PAYLOAD_LINUX=y
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... PS1, Line 6: CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS=y this means that this config won't add microcode, right? Is that intentional?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 1:
I think that needs to be "config.facebook_fbg1710" or "config.facebook_fbg1710_some_description_here", as I don't see it in the Jenkins build log. It's always good to give a short summary in the file, where it's different from default.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... PS1, Line 6: CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS=y
this means that this config won't add microcode, right? Is that intentional?
Enabling will include external BIN with uCode. Config CPU_UCODE_BINARIES will be used for path to 3rdparty/intel-microcode bin.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... PS1, Line 6: CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS=y
Enabling will include external BIN with uCode. […]
right, and that path is empty by default and there's no override here. So I guess the binaries come from somewhere else?
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... PS1, Line 6: CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS=y
right, and that path is empty by default and there's no override here. […]
The CPU_UCODE_BINARIES should be available in this config. Will check.
Hello Patrick Rudolph, Felix Held, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34343
to look at the new patch set (#2).
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
configs/config.facebookfbg1710: Add config file
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A configs/config.facebookfbg1710 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/34343/2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
https://review.coreboot.org/c/coreboot/+/34343/1/configs/config.facebookfbg1... PS1, Line 6: CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS=y
The CPU_UCODE_BINARIES should be available in this config. […]
Done
https://review.coreboot.org/c/coreboot/+/34343/2/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
PS2: It's still missing in the build tests on https://qa.coreboot.org/job/coreboot-gerrit/98551/testReport/(root)/board/ because of the naming scheme
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/2/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
PS2:
It's still missing in the build tests on https://qa.coreboot. […]
What is ment with 'naming scheme'?
Is there a way to retrieve the used config settings?
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/2/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
PS2:
What is ment with 'naming scheme'? […]
Noticed naming of other config files.
Hello Patrick Rudolph, Felix Held, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34343
to look at the new patch set (#3).
Change subject: configs/config.facebookfbg1710: Add config file ......................................................................
configs/config.facebookfbg1710: Add config file
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A configs/config.facebook_fbg1710 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/34343/3
Hello Patrick Rudolph, Felix Held, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34343
to look at the new patch set (#4).
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
configs/config.facebook_fbg1701: Add config file
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A configs/config.facebook_fbg1710 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/34343/4
Hello Patrick Rudolph, Felix Held, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34343
to look at the new patch set (#5).
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
configs/config.facebook_fbg1701: Add config file
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A configs/config.facebook_fbg1701 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/34343/5
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/2/configs/config.facebookfbg1... File configs/config.facebookfbg1710:
PS2:
Noticed naming of other config files.
Done
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
Patch Set 6: Code-Review+1
Hello Patrick Rudolph, Wim Vervoorn, Felix Held, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34343
to look at the new patch set (#7).
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
configs/config.facebook_fbg1701: Add config file
Enable vendorcode measured and verified boot.
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A configs/config.facebook_fbg1701 1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/34343/7
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/7/configs/config.facebook_fbg... File configs/config.facebook_fbg1701:
https://review.coreboot.org/c/coreboot/+/34343/7/configs/config.facebook_fbg... PS7, Line 8: CONFIG_VENDORCODE_ELTAN_VBOOT=y Enabling vboot also requires the public key to be available in order to build. We should either make this configurable or provide a dummy key.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/7/configs/config.facebook_fbg... File configs/config.facebook_fbg1701:
https://review.coreboot.org/c/coreboot/+/34343/7/configs/config.facebook_fbg... PS7, Line 8: CONFIG_VENDORCODE_ELTAN_VBOOT=y
Enabling vboot also requires the public key to be available in order to build. […]
Please provide a "dev" key. Ideally something at the absolutely bottom end of the parameter space (e.g. smallest possible key size) to make sure using that key in a real build looks as ridiculous as it is :-)
Another idea I had is to build a transient key on every build, but that interferes with reproducible builds which can create issues down the road (there are ideas to be able to assert that a commit doesn't change the resulting binaries and have jenkins test that)
Hello Kyösti Mälkki, Patrick Rudolph, Wim Vervoorn, Felix Held, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34343
to look at the new patch set (#8).
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
configs/config.facebook_fbg1701: Add config file
Enable vendorcode measured and verified boot. Use VBOOT test key for VENDORCODE_ELTAN_VBOOT_KEY_FILE
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com --- A configs/config.facebook_fbg1701 1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/34343/8
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34343/7/configs/config.facebook_fbg... File configs/config.facebook_fbg1701:
https://review.coreboot.org/c/coreboot/+/34343/7/configs/config.facebook_fbg... PS7, Line 8: CONFIG_VENDORCODE_ELTAN_VBOOT=y
Please provide a "dev" key. […]
Used a VBOOT dev-key.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
Patch Set 8: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/34343 )
Change subject: configs/config.facebook_fbg1701: Add config file ......................................................................
configs/config.facebook_fbg1701: Add config file
Enable vendorcode measured and verified boot. Use VBOOT test key for VENDORCODE_ELTAN_VBOOT_KEY_FILE
BUG=N/A TEST=booting Embedded Linux 4.20 kernel on Facebook FBG1701
Change-Id: Ia2cb3bb873b2d5e7e9031e5b249d86605d8e0945 Signed-off-by: Frans Hendriks fhendriks@eltan.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34343 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- A configs/config.facebook_fbg1701 1 file changed, 12 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/configs/config.facebook_fbg1701 b/configs/config.facebook_fbg1701 new file mode 100644 index 0000000..b372bbe --- /dev/null +++ b/configs/config.facebook_fbg1701 @@ -0,0 +1,12 @@ +CONFIG_VENDOR_FACEBOOK=y +CONFIG_C_ENV_BOOTBLOCK_SIZE=0x6000 +CONFIG_ONBOARD_SAMSUNG_MEM=y +CONFIG_CPU_MICROCODE_CBFS_LOC=0xFFF8B000 +CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS=y +CONFIG_CPU_UCODE_BINARIES="3rdparty/intel-microcode/intel-ucode/06-4c-04" +CONFIG_VENDORCODE_ELTAN_MBOOT=y +CONFIG_VENDORCODE_ELTAN_VBOOT=y +CONFIG_VENDORCODE_ELTAN_VBOOT_KEY_FILE="3rdparty/vboot/tests/devkeys-acc/key_hadoken.vbpubk2" +CONFIG_RUN_FSP_GOP=y +CONFIG_DISPLAY_HOBS=y +CONFIG_DEFAULT_CONSOLE_LOGLEVEL_8=y