Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Makefile: Support HAVE_BOOTBLOCK=n case
With HAVE_BOOTBLOCK=n build of bootblock-class is skipped.
Inserts an empty 64-byte bootblock-region to coreboot.rom file, cbfstool will fill in the CBFS master header relative location at the end.
Change-Id: Iaee9200f72f31175aca597865e3c74fc68bec8a6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M Makefile.inc 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/34477/1
diff --git a/Makefile.inc b/Makefile.inc index 2cad230..c275d1e 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -705,6 +705,11 @@ @printf " OBJCOPY $(notdir $(@))\n" $(OBJCOPY_bootblock) -O binary $< $@
+ifneq ($(CONFIG_HAVE_BOOTBLOCK),y) +$(objcbfs)/bootblock.bin: + dd if=/dev/zero of=$@ bs=64 count=1 +endif + $(objcbfs)/%.bin: $(objcbfs)/%.raw.bin cp $< $@
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG@13 PS1, Line 13: at the end. I thought we made this bit optional now? Sorry, I didn't dig into the details on that.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG@13 PS1, Line 13: at the end.
I thought we made this bit optional now? Sorry, I didn't dig into the details on that.
Reading src/lib/cbfs.c: cbfs_master_header_props() suggests it is not optional. Maybe not needed with VBOOT?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG@13 PS1, Line 13: at the end.
Reading src/lib/cbfs.c: cbfs_master_header_props() suggests it is not optional. […]
With fmap we don't technically need it. But I'm not surprised there's still a dependency. thanks for digging.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1: Code-Review+1
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG@13 PS1, Line 13: at the end.
With fmap we don't technically need it. But I'm not surprised there's still a dependency. […]
Also CB:32327 comments discuss further dependency in payloads. Like no FMAP support in SeaBIOS.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1:
Ping. Was there some action requested to make this worth +2 ?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1: Code-Review+2
Patch Set 1:
Ping. Was there some action requested to make this worth +2 ?
I thought others would chime in.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1: Code-Review+2
Sorry for the delay.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34477/1//COMMIT_MSG@13 PS1, Line 13: at the end.
Also CB:32327 comments discuss further dependency in payloads. Like no FMAP support in SeaBIOS.
Ack
Marshall Dawson has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34477 )
Change subject: Makefile: Support HAVE_BOOTBLOCK=n case ......................................................................
Makefile: Support HAVE_BOOTBLOCK=n case
With HAVE_BOOTBLOCK=n build of bootblock-class is skipped.
Inserts an empty 64-byte bootblock-region to coreboot.rom file, cbfstool will fill in the CBFS master header relative location at the end.
Change-Id: Iaee9200f72f31175aca597865e3c74fc68bec8a6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34477 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com --- M Makefile.inc 1 file changed, 5 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Aaron Durbin: Looks good to me, approved Marshall Dawson: Looks good to me, approved
diff --git a/Makefile.inc b/Makefile.inc index 2cad230..c275d1e 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -705,6 +705,11 @@ @printf " OBJCOPY $(notdir $(@))\n" $(OBJCOPY_bootblock) -O binary $< $@
+ifneq ($(CONFIG_HAVE_BOOTBLOCK),y) +$(objcbfs)/bootblock.bin: + dd if=/dev/zero of=$@ bs=64 count=1 +endif + $(objcbfs)/%.bin: $(objcbfs)/%.raw.bin cp $< $@
-
Aaron Durbin (Code Review) -
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Paul Menzel (Code Review)