Zoltan Baldaszti has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
payloads/external: add support for BOOTBOOT payload
Change-Id: I8692cde0730338026a7760a293c1e37f66004bc0 Signed-off-by: Zoltan Baldaszti bztemail@gmail.com --- A payloads/external/BOOTBOOT/Kconfig A payloads/external/BOOTBOOT/Kconfig.name A payloads/external/BOOTBOOT/Makefile M payloads/external/Makefile.inc 4 files changed, 69 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45482/1
diff --git a/payloads/external/BOOTBOOT/Kconfig b/payloads/external/BOOTBOOT/Kconfig new file mode 100644 index 0000000..c9d7133 --- /dev/null +++ b/payloads/external/BOOTBOOT/Kconfig @@ -0,0 +1,6 @@ +if PAYLOAD_BOOTBOOT + +config PAYLOAD_FILE + default "payloads/external/BOOTBOOT/bootboot/dist/bootbootcb.elf" + +endif diff --git a/payloads/external/BOOTBOOT/Kconfig.name b/payloads/external/BOOTBOOT/Kconfig.name new file mode 100644 index 0000000..082a9b1 --- /dev/null +++ b/payloads/external/BOOTBOOT/Kconfig.name @@ -0,0 +1,8 @@ +config PAYLOAD_BOOTBOOT + bool "BOOTBOOT" + depends on ARCH_X86 || ARCH_ARM64 + help + Select this option if you want to build a coreboot image + with a BOOTBOOT Protocol payload. + + See https://gitlab.com/bztsrc/bootboot for more information. diff --git a/payloads/external/BOOTBOOT/Makefile b/payloads/external/BOOTBOOT/Makefile new file mode 100644 index 0000000..5cd716a --- /dev/null +++ b/payloads/external/BOOTBOOT/Makefile @@ -0,0 +1,49 @@ +project_git_repo=https://gitlab.com/bztsrc/bootboot.git +project_dir=bootboot +ifeq ($(CONFIG_COREBOOT_BUILD),) +include ../../../.config +endif +ifeq ($(CONFIG_ARCH_ARM64),y) +loader_dir=$(project_dir)/aarch64-cb +else +loader_dir=$(project_dir)/x86_64-cb +endif + +unexport KCONFIG_AUTOHEADER +unexport KCONFIG_AUTOCONFIG +unexport KCONFIG_DEPENDENCIES +unexport KCONFIG_SPLITCONFIG +unexport KCONFIG_TRISTATE +unexport KCONFIG_NEGATIVES + +all: bootboot + +checkout: + echo " GIT BOOTBOOT $(loader_dir)" + @test -d ../../../../bootboot && ln -s ../../../../bootboot 2>/dev/null || true + test -L $(project_dir) || ( \ + ( test -d $(project_dir) || \ + git clone $(project_git_repo) $(project_dir) ) && \ + ( cd $(project_dir) && \ + git checkout master && \ + git remote update ) ) + +bootboot: libpayload + echo " MAKE $(loader_dir)" + $(MAKE) -C $(loader_dir) LIBCONFIG_PATH=../../../libpayload + +libpayload: checkout + cp $(loader_dir)/lib.config ../../libpayload/.config + cd ../../libpayload && $(MAKE) oldconfig && \ + $(MAKE) && $(MAKE) DESTDIR=../external/BOOTBOOT/$(loader_dir) install + +clean: + test -d $(loader_dir) && $(MAKE) -C $(loader_dir) clean || exit 0 + +distclean: + rm -rf $(project_dir) + +print-repo-info: + echo "$(project_git_repo) $(project_dir)" + +.PHONY: checkout bootboot libpayload clean distclean print-repo-info diff --git a/payloads/external/Makefile.inc b/payloads/external/Makefile.inc index ef0990c..15d7348 100644 --- a/payloads/external/Makefile.inc +++ b/payloads/external/Makefile.inc @@ -303,3 +303,9 @@ CONFIG_YABITS_MASTER=$(CONFIG_YABITS_MASTER) \ CONFIG_YABITS_STABLE=$(CONFIG_YABITS_STABLE) \ MFLAGS= MAKEFLAGS= + +# BOOTBOOT + +payloads/external/BOOTBOOT/bootboot/dist/bootbootcb.elf: + $(MAKE) -C payloads/external/BOOTBOOT all +
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45482/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/1//COMMIT_MSG@8 PS1, Line 8: Can it be easily tested with QEMU?
Zoltan Baldaszti has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 1:
(1 comment)
Thank you for checking out! Yes, I test all versions of BOOTBOOT in qemu all the time. The coreboot payload version is no exception.
https://review.coreboot.org/c/coreboot/+/45482/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/1//COMMIT_MSG@8 PS1, Line 8:
Can it be easily tested with QEMU?
Yes, of course! qemu-system-x86_64 -bios coreboot.rom -serial stdio
Detailed compilation instructions: https://gitlab.com/bztsrc/bootboot/-/tree/master/x86_64-cb
Example pre-compiled coreboot.rom with this payload (and a make rule to run it with in QEMU): https://gitlab.com/bztsrc/bootboot/-/tree/master/images see "make coreboot", loads the initrd from a disk with GPT and ESP.
Further note: you don't need to have a disk image either, you can put the initrd into the ROM using cbfstool. Example command on the first link.
Without an initrd, it should print "BOOTBOOT-PANIC: No initrd found."
Plus one: I've fixed the newline ending issue, but accidentally created a new PR, sorry about that! https://review.coreboot.org/c/coreboot/+/45483
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45482
to look at the new patch set (#2).
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
payloads/external: add support for BOOTBOOT payload
Change-Id: I8692cde0730338026a7760a293c1e37f66004bc0 Signed-off-by: Zoltan Baldaszti bztemail@gmail.com --- A payloads/external/BOOTBOOT/Kconfig A payloads/external/BOOTBOOT/Kconfig.name A payloads/external/BOOTBOOT/Makefile M payloads/external/Makefile.inc 4 files changed, 68 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45482/2
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG@8 PS2, Line 8: Could we get some more detail in the commit. What is BOOTBOOT?
Zoltan Baldaszti has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG@8 PS2, Line 8:
Could we get some more detail in the commit. […]
Yes, on the provided link there's a detailed description and also a PDF documentation, as well as several examples. Here's the link again: https://gitlab.com/bztsrc/bootboot
"The protocol describes how to boot an ELF64 or PE32+ executable inside an initial ram disk image into clean 64 bit mode, without using any configuration or even knowing the file system of initrd."
In short, BOOTBOOT is a multi-platform, architecture agnostic, 64-bit only boot protocol with many reference implementations (legacy BIOS, Multiboot, UEFI, ARM RPi, etc.). Coreboot payload is one of the implementations, which fits in BOOTBOOT's philosophy very well.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG@8 PS2, Line 8:
Yes, on the provided link there's a detailed description and also a PDF documentation, as well as se […]
What Marc means is that you should place this information in the commit message. The commit message is the first thing one reads about a change, and explains what this change is about, and why it's a good idea. Without a description, a commit feels like an advert that only says "Buy FooBar toothpaste!".
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45482
to look at the new patch set (#3).
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
payloads/external: add support for BOOTBOOT payload
BOOTBOOT is a multi-platform, architecture agnostic boot protocol. For more information see https://gitlab.com/bztsrc/bootboot
Change-Id: I8692cde0730338026a7760a293c1e37f66004bc0 Signed-off-by: Zoltan Baldaszti bztemail@gmail.com --- A payloads/external/BOOTBOOT/Kconfig A payloads/external/BOOTBOOT/Kconfig.name A payloads/external/BOOTBOOT/Makefile M payloads/external/Makefile.inc 4 files changed, 68 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45482/3
Zoltan Baldaszti has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG@8 PS2, Line 8:
What Marc means is that you should place this information in the commit message. […]
Oh, I see! I've updated the commit message with a brief description, and also provided a link for those who wants to know more.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG@8 PS2, Line 8:
Oh, I see! I've updated the commit message with a brief description, and also provided a link for th […]
You could add some more less obvious information. For instance, I was happy to see that you based it on libpayload. My next question would be from where / how you load the kernel? given that there are no file- system drivers in libpayload and the BOOTBOOT descriptions says it is out of scope. This particular implementation however, has to do it some way and I'm very curious :)
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 3:
(2 comments)
Looks good but for some of the git foo.
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... File payloads/external/BOOTBOOT/Makefile:
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... PS2, Line 23: @test -d ../../../../bootboot && ln -s ../../../../bootboot 2>/dev/null || true This looks like a very local solution, maybe not suited for upstreaming?
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... PS2, Line 29: git remote update ) ) Doing this as the last step looks odd. It wouldn't do anything to the checked out files, even if there are updates. Maybe start with a simple clone and then we can look into updates later? I've wasted some time on the topic myself (for other payloads) and still haven't come up with a fool-proof solution.
Zoltan Baldaszti has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... File payloads/external/BOOTBOOT/Makefile:
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... PS2, Line 23: @test -d ../../../../bootboot && ln -s ../../../../bootboot 2>/dev/null || true
This looks like a very local solution, maybe not suited for upstreaming?
This is the case when the bootboot git repository is cloned to the same directory as the coreboot repository. Then there's no point in cloning it again under payloads/BOOTBOOT too. It's a local thing sure, but I think having multiple git repositories in the same directory is common enough. I can remove this if you'd like, but I don't think it hurts anyone.
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... PS2, Line 29: git remote update ) )
Doing this as the last step looks odd. It wouldn't do anything to the […]
Actually this part comes from the FILO external payload's Makefile. I'm okay if the repo is not updated, because a) I don't think it's going to change that much b) I don't think it is the compilation's job to refresh the code anyway (it is okay to clone the code at first, but keeping up-to-date is totally different thing IMHO). Let me know if you decide to remove this.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45482
to look at the new patch set (#4).
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
payloads/external: add support for BOOTBOOT payload
BOOTBOOT is a multi-platform, architecture agnostic boot protocol. The protocol describes how to boot an ELF64 or PE32+ executable inside an initial ram disk image into clean 64 bit mode. This version uses libpayload to do that. Depending on the lib's configuration, initrd can be in ROM as a cbfs file or a Flashmap partition; on disk a GPT partition or a file on a FAT formatted ESP partition. For more information see https://gitlab.com/bztsrc/bootboot
Change-Id: I8692cde0730338026a7760a293c1e37f66004bc0 Signed-off-by: Zoltan Baldaszti bztemail@gmail.com --- A payloads/external/BOOTBOOT/Kconfig A payloads/external/BOOTBOOT/Kconfig.name A payloads/external/BOOTBOOT/Makefile M payloads/external/Makefile.inc 4 files changed, 68 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45482/4
Zoltan Baldaszti has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45482/2//COMMIT_MSG@8 PS2, Line 8:
You could add some more less obvious information. For instance, I was […]
Ok, I've added more description to the commit message. But to answer your question adequately (which would be too long for a commit message I think), the place where initrd can be loaded from mostly depends on libpayload's configuration. - can be in ROM as a Flashmap partition (this is the default) - if libpayload has cbfs support, then can be in ROM as a cbfs file - if libpayload has serial port support, can be loaded over serial - if libpayload has storage or USB disk support, can be on disk For that latter, initrd can be on a GPT partition, or can be a file on ESP (for that the implementation has a GPT and a FAT16/32 parser). As for the initrd, statically linked executable, tar, cpio and some other formats supported, with optional gzip compression (and if libpayload compiled with LZMA or LZ4, then those too). If the format of initrd is not recognized, then it is scanned for the first executable compiled for the architecture (therefore it can load kernels from unknown image formats as well, provided the kernel is contiguous in them).
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45482
to look at the new patch set (#5).
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
payloads/external: add support for BOOTBOOT payload
BOOTBOOT is a multi-platform, architecture agnostic boot protocol. The protocol describes how to boot an ELF64 or PE32+ executable inside an initial ram disk image into clean 64 bit mode. This version uses libpayload to do that. Depending on the lib's configuration, initrd can be in ROM as a cbfs file or a Flashmap partition; on disk a GPT partition or a file on a FAT formatted ESP partition. For more information see https://gitlab.com/bztsrc/bootboot
Change-Id: I8692cde0730338026a7760a293c1e37f66004bc0 Signed-off-by: Zoltan Baldaszti bztemail@gmail.com --- A payloads/external/BOOTBOOT/Kconfig A payloads/external/BOOTBOOT/Kconfig.name A payloads/external/BOOTBOOT/Makefile M payloads/external/Makefile.inc 4 files changed, 63 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45482/5
Zoltan Baldaszti has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 5:
(2 comments)
removed commands from checkout rule, except for the git clone
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... File payloads/external/BOOTBOOT/Makefile:
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... PS2, Line 23: @test -d ../../../../bootboot && ln -s ../../../../bootboot 2>/dev/null || true
This is the case when the bootboot git repository is cloned to the same directory as the coreboot re […]
Ok, I've just removed it, that's the safest.
https://review.coreboot.org/c/coreboot/+/45482/2/payloads/external/BOOTBOOT/... PS2, Line 29: git remote update ) )
Actually this part comes from the FILO external payload's Makefile. […]
Although it was copied from FILO, I've removed this too. I only left the git clone command, but I've kept the check for a symlink. Hope this is okay now.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 5: Code-Review+2
Thanks for the updates, Zoltan.
If there are no objections, I think we can merge it like this.
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
payloads/external: add support for BOOTBOOT payload
BOOTBOOT is a multi-platform, architecture agnostic boot protocol. The protocol describes how to boot an ELF64 or PE32+ executable inside an initial ram disk image into clean 64 bit mode. This version uses libpayload to do that. Depending on the lib's configuration, initrd can be in ROM as a cbfs file or a Flashmap partition; on disk a GPT partition or a file on a FAT formatted ESP partition. For more information see https://gitlab.com/bztsrc/bootboot
Change-Id: I8692cde0730338026a7760a293c1e37f66004bc0 Signed-off-by: Zoltan Baldaszti bztemail@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45482 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- A payloads/external/BOOTBOOT/Kconfig A payloads/external/BOOTBOOT/Kconfig.name A payloads/external/BOOTBOOT/Makefile M payloads/external/Makefile.inc 4 files changed, 63 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/payloads/external/BOOTBOOT/Kconfig b/payloads/external/BOOTBOOT/Kconfig new file mode 100644 index 0000000..c9d7133 --- /dev/null +++ b/payloads/external/BOOTBOOT/Kconfig @@ -0,0 +1,6 @@ +if PAYLOAD_BOOTBOOT + +config PAYLOAD_FILE + default "payloads/external/BOOTBOOT/bootboot/dist/bootbootcb.elf" + +endif diff --git a/payloads/external/BOOTBOOT/Kconfig.name b/payloads/external/BOOTBOOT/Kconfig.name new file mode 100644 index 0000000..082a9b1 --- /dev/null +++ b/payloads/external/BOOTBOOT/Kconfig.name @@ -0,0 +1,8 @@ +config PAYLOAD_BOOTBOOT + bool "BOOTBOOT" + depends on ARCH_X86 || ARCH_ARM64 + help + Select this option if you want to build a coreboot image + with a BOOTBOOT Protocol payload. + + See https://gitlab.com/bztsrc/bootboot for more information. diff --git a/payloads/external/BOOTBOOT/Makefile b/payloads/external/BOOTBOOT/Makefile new file mode 100644 index 0000000..2460c18 --- /dev/null +++ b/payloads/external/BOOTBOOT/Makefile @@ -0,0 +1,44 @@ +project_git_repo=https://gitlab.com/bztsrc/bootboot.git +project_dir=bootboot +ifeq ($(CONFIG_COREBOOT_BUILD),) +include ../../../.config +endif +ifeq ($(CONFIG_ARCH_ARM64),y) +loader_dir=$(project_dir)/aarch64-cb +else +loader_dir=$(project_dir)/x86_64-cb +endif + +unexport KCONFIG_AUTOHEADER +unexport KCONFIG_AUTOCONFIG +unexport KCONFIG_DEPENDENCIES +unexport KCONFIG_SPLITCONFIG +unexport KCONFIG_TRISTATE +unexport KCONFIG_NEGATIVES + +all: bootboot + +checkout: + echo " GIT BOOTBOOT $(loader_dir)" + test -L $(project_dir) || test -d $(project_dir) || \ + git clone $(project_git_repo) $(project_dir) + +bootboot: libpayload + echo " MAKE $(loader_dir)" + $(MAKE) -C $(loader_dir) LIBCONFIG_PATH=../../../libpayload + +libpayload: checkout + cp $(loader_dir)/lib.config ../../libpayload/.config + cd ../../libpayload && $(MAKE) oldconfig && \ + $(MAKE) && $(MAKE) DESTDIR=../external/BOOTBOOT/$(loader_dir) install + +clean: + test -d $(loader_dir) && $(MAKE) -C $(loader_dir) clean || exit 0 + +distclean: + rm -rf $(project_dir) + +print-repo-info: + echo "$(project_git_repo) $(project_dir)" + +.PHONY: checkout bootboot libpayload clean distclean print-repo-info diff --git a/payloads/external/Makefile.inc b/payloads/external/Makefile.inc index ef0990c..da199e4 100644 --- a/payloads/external/Makefile.inc +++ b/payloads/external/Makefile.inc @@ -303,3 +303,8 @@ CONFIG_YABITS_MASTER=$(CONFIG_YABITS_MASTER) \ CONFIG_YABITS_STABLE=$(CONFIG_YABITS_STABLE) \ MFLAGS= MAKEFLAGS= + +# BOOTBOOT + +payloads/external/BOOTBOOT/bootboot/dist/bootbootcb.elf: + $(MAKE) -C payloads/external/BOOTBOOT all
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45482 )
Change subject: payloads/external: add support for BOOTBOOT payload ......................................................................
Patch Set 6:
Nice. Thank you for the reviews and improvements.
Zoltan, I recommend to announce the new payload on the coreboot mailing list, and also add an entry to the release notes for coreboot 4.13.
-
Angel Pons (Code Review) -
Marc Jones (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Zoltan Baldaszti (Code Review)