CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for cbf... [chromiumos/third_party/coreboot : chromeos-2013.04]

34 views
Skip to first unread message

Julius Werner (Gerrit)

unread,
Nov 14, 2014, 9:26:50 PM11/14/14
to Hung-Te Lin, Stefan Reinauer, David Hendricks, Aaron Durbin, Patrick Georgi, Vadim Bendebury
Hello Hung-Te Lin, Stefan Reinauer, David Hendricks, Aaron Durbin, Patrick
Georgi, Vadim Bendebury,

I'd like you to do a code review. Please visit

https://chromium-review.googlesource.com/229974

to review the following change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................

CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for cbfstool

Some projects (like ChromeOS) put more content than described by CBFS
onto their image. For top-aligned images (read: x86), this has
traditionally been achieved with a CBFS_SIZE Kconfig (which denotes the
area actually managed by CBFS, as opposed to ROM_SIZE) that is used to
calculate the CBFS entry start offset. On bottom-aligned boards, many
define a fake (smaller) ROM_SIZE for only the CBFS part, which is not
consistently done and can be an issue because ROM_SIZE is expected to be
a power of two.

This patch changes all non-x86 boards to describe their actual
(physical) ROM size via one of the BOARD_ROMSIZE_KB_xxx options as a
mainboard Kconfig select (which is the correct place to declare
unchangeable physical properties of the board). It also changes the
cbfstool create invocation to use CBFS_SIZE as the -s parameter for
those architectures, which defaults to ROM_SIZE but gets overridden for
special use cases like ChromeOS. This has the advantage that cbfstool
has a consistent idea of where the area it is responsible for ends,
which offers better bounds-checking and is needed for a subsequent fix.

Also change the FMAP offset to default to right behind the (now
consistently known) CBFS region for non-x86 boards, which has emerged as
a de-facto standard on those architectures and allows us to reduce the
amount of custom configuration. In the future, the nightmare that is
ChromeOS's image build system could be redesigned to enforce this
automatically, and also confirm that it doesn't overwrite any space used
by CBFS (which is now consistently defined as the file size of
coreboot.rom on non-x86).

BRANCH=None
BUG=None
TEST=Built and booted on Veyron_Pinky.

Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Signed-off-by: Julius Werner <jwe...@chromium.org>
---
M Makefile.inc
M configs/config.arm64-generic
M configs/config.cosmos
M configs/config.daisy
M configs/config.nyan
M configs/config.nyan_big
M configs/config.nyan_blaze
M configs/config.peach_pit
M configs/config.rush
M configs/config.rush_ryu
M configs/config.storm
M configs/config.veyron_jerry
M configs/config.veyron_pinky
M src/Kconfig
M src/arch/arm/Makefile.inc
M src/arch/arm64/Makefile.inc
M src/arch/mips/Makefile.inc
M src/arch/x86/Makefile.inc
M src/mainboard/google/cosmos/Kconfig
M src/mainboard/google/nyan/Kconfig
M src/mainboard/google/nyan_big/Kconfig
M src/mainboard/google/nyan_blaze/Kconfig
M src/mainboard/google/rush/Kconfig
M src/mainboard/google/rush_ryu/Kconfig
M src/mainboard/google/storm/Kconfig
M src/mainboard/google/veyron_jerry/Kconfig
M src/mainboard/google/veyron_pinky/Kconfig
M src/soc/qualcomm/Kconfig
M src/vendorcode/google/chromeos/Kconfig
29 files changed, 42 insertions(+), 37 deletions(-)



diff --git a/Makefile.inc b/Makefile.inc
index 25a0807..de15fd5 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -410,7 +410,7 @@
prebuilt-files = $(foreach file,$(cbfs-files), $(call
extract_nth,1,$(file)))

$(obj)/coreboot.pre1: $(objcbfs)/bootblock.bin $$(prebuilt-files)
$(CBFSTOOL) $$(cpu_ucode_cbfs_file)
- $(CBFSTOOL) $@.tmp create -s $(CONFIG_COREBOOT_ROMSIZE_KB)K \
+ $(CBFSTOOL) $@.tmp create \
-B $(objcbfs)/bootblock.bin -a 64 \
$(CBFSTOOL_PRE1_OPTS)
$(prebuild-files) true
diff --git a/configs/config.arm64-generic b/configs/config.arm64-generic
index 971f40d..168f854 100644
--- a/configs/config.arm64-generic
+++ b/configs/config.arm64-generic
@@ -1,4 +1,4 @@
CONFIG_VENDOR_EMULATION=y
CONFIG_BOARD_EMULATION_FOUNDATION_ARMV8=y
-CONFIG_COREBOOT_ROMSIZE_KB_4096=y
+CONFIG_CBFS_SIZE=0x100000
CONFIG_CONSOLE_SERIAL=y
diff --git a/configs/config.cosmos b/configs/config.cosmos
index 04ac4d3..d2033d1 100644
--- a/configs/config.cosmos
+++ b/configs/config.cosmos
@@ -1,6 +1,5 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_COSMOS=y
-CONFIG_COREBOOT_ROMSIZE_KB_512=y
-CONFIG_FLASHMAP_OFFSET=0x00080000
+CONFIG_CBFS_SIZE=0x80000
CONFIG_VBOOT2_VERIFY_FIRMWARE=y
-# CONFIG_CONSOLE_SERIAL is not set
\ No newline at end of file
+# CONFIG_CONSOLE_SERIAL is not set
diff --git a/configs/config.daisy b/configs/config.daisy
index 57f3bb9..37d04d6 100644
--- a/configs/config.daisy
+++ b/configs/config.daisy
@@ -1,5 +1,5 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_DAISY=y
-CONFIG_COREBOOT_ROMSIZE_KB_1024=y
+CONFIG_CBFS_SIZE=0x100000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_COLLECT_TIMESTAMPS=y
diff --git a/configs/config.nyan b/configs/config.nyan
index bbeedd2..9b61ea8 100644
--- a/configs/config.nyan
+++ b/configs/config.nyan
@@ -1,10 +1,9 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_NYAN=y
-CONFIG_COREBOOT_ROMSIZE_KB_1024=y
+CONFIG_CBFS_SIZE=0x100000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_CONSOLE_CBMEM=y
CONFIG_VBOOT_VERIFY_FIRMWARE=y
-CONFIG_FLASHMAP_OFFSET=0x00100000
CONFIG_ELOG=y
CONFIG_DRIVERS_AS3722_RTC=y
CONFIG_DRIVERS_AS3722_RTC_BUS=4
diff --git a/configs/config.nyan_big b/configs/config.nyan_big
index c9d09d0..1763e98 100644
--- a/configs/config.nyan_big
+++ b/configs/config.nyan_big
@@ -1,10 +1,9 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_NYAN_BIG=y
-CONFIG_COREBOOT_ROMSIZE_KB_1024=y
+CONFIG_CBFS_SIZE=0x100000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_CONSOLE_CBMEM=y
CONFIG_VBOOT_VERIFY_FIRMWARE=y
-CONFIG_FLASHMAP_OFFSET=0x00100000
CONFIG_ELOG=y
CONFIG_DRIVERS_AS3722_RTC=y
CONFIG_DRIVERS_AS3722_RTC_BUS=4
diff --git a/configs/config.nyan_blaze b/configs/config.nyan_blaze
index 7c49ddf..45f09e7 100644
--- a/configs/config.nyan_blaze
+++ b/configs/config.nyan_blaze
@@ -1,10 +1,9 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_NYAN_BLAZE=y
-CONFIG_COREBOOT_ROMSIZE_KB_1024=y
+CONFIG_CBFS_SIZE=0x100000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_CONSOLE_CBMEM=y
CONFIG_VBOOT_VERIFY_FIRMWARE=y
-CONFIG_FLASHMAP_OFFSET=0x00100000
CONFIG_ELOG=y
CONFIG_DRIVERS_AS3722_RTC=y
CONFIG_DRIVERS_AS3722_RTC_BUS=4
diff --git a/configs/config.peach_pit b/configs/config.peach_pit
index ef84d7c..d0dac7c 100644
--- a/configs/config.peach_pit
+++ b/configs/config.peach_pit
@@ -1,5 +1,5 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_PEACH_PIT=y
-CONFIG_COREBOOT_ROMSIZE_KB_1024=y
+CONFIG_CBFS_SIZE=0x100000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_COLLECT_TIMESTAMPS=y
diff --git a/configs/config.rush b/configs/config.rush
index 602714a..6e9c365 100644
--- a/configs/config.rush
+++ b/configs/config.rush
@@ -1,8 +1,7 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_RUSH=y
-CONFIG_COREBOOT_ROMSIZE_KB_4096=y
+CONFIG_CBFS_SIZE=0x200000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_CONSOLE_CBMEM=y
CONFIG_MTS_DIRECTORY="3rdparty/mainboard/google/rush"
CONFIG_VBOOT2_VERIFY_FIRMWARE=y
-CONFIG_FLASHMAP_OFFSET=0x00200000
diff --git a/configs/config.rush_ryu b/configs/config.rush_ryu
index bdf8cd2..b86e9db 100644
--- a/configs/config.rush_ryu
+++ b/configs/config.rush_ryu
@@ -1,11 +1,10 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_RUSH_RYU=y
-CONFIG_COREBOOT_ROMSIZE_KB_8192=y
+CONFIG_CBFS_SIZE=0x200000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_CONSOLE_CBMEM=y
CONFIG_MTS_DIRECTORY="3rdparty/mainboard/google/rush_ryu"
CONFIG_VBOOT2_VERIFY_FIRMWARE=y
-CONFIG_FLASHMAP_OFFSET=0x00200000
CONFIG_ELOG=y
CONFIG_DRIVERS_TI_TPS65913_RTC=y
CONFIG_DRIVERS_TI_TPS65913_RTC_BUS=4
diff --git a/configs/config.storm b/configs/config.storm
index df94174..062abc3 100644
--- a/configs/config.storm
+++ b/configs/config.storm
@@ -1,6 +1,5 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_STORM=y
-CONFIG_COREBOOT_ROMSIZE_KB_4096=y
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_COLLECT_TIMESTAMPS=y
CONFIG_CONSOLE_CBMEM=y
diff --git a/configs/config.veyron_jerry b/configs/config.veyron_jerry
index b331633..0723960 100644
--- a/configs/config.veyron_jerry
+++ b/configs/config.veyron_jerry
@@ -1,9 +1,8 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_VEYRON_JERRY=y
-CONFIG_COREBOOT_ROMSIZE_KB_1024=y
+CONFIG_CBFS_SIZE=0x100000
# CONFIG_CONSOLE_SERIAL is not set
CONFIG_COLLECT_TIMESTAMPS=y
# CONFIG_ELOG=y
# CONFIG_CONSOLE_CBMEM=y
-CONFIG_FLASHMAP_OFFSET=0x00100000
CONFIG_VBOOT2_VERIFY_FIRMWARE=y
diff --git a/configs/config.veyron_pinky b/configs/config.veyron_pinky
index 7192c3b..2b39dad 100644
--- a/configs/config.veyron_pinky
+++ b/configs/config.veyron_pinky
@@ -1,8 +1,7 @@
CONFIG_VENDOR_GOOGLE=y
CONFIG_BOARD_GOOGLE_VEYRON_PINKY=y
-CONFIG_COREBOOT_ROMSIZE_KB_1024=y
+CONFIG_CBFS_SIZE=0x100000
# CONFIG_CONSOLE_SERIAL is not set
# CONFIG_ELOG=y
# CONFIG_CONSOLE_CBMEM=y
-CONFIG_FLASHMAP_OFFSET=0x00100000
CONFIG_VBOOT2_VERIFY_FIRMWARE=y
diff --git a/src/Kconfig b/src/Kconfig
index f926f2e..23f14b4 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -430,8 +430,14 @@
default n

config CBFS_SIZE
- hex
+ hex "Size of CBFS filesystem in ROM"
default ROM_SIZE
+ help
+ This is the part of the ROM actually managed by CBFS, located at the
+ end of the ROM (passed through cbfstool -o) on x86 and at at the start
+ of the ROM (passed through cbfstool -s) everywhere else. Defaults to
+ span the whole ROM but can be overwritten to make coreboot live
+ alongside other components (like ChromeOS's vboot/FMAP).

config CACHE_ROM_SIZE
hex
diff --git a/src/arch/arm/Makefile.inc b/src/arch/arm/Makefile.inc
index 58c9c95..a7d85fe 100644
--- a/src/arch/arm/Makefile.inc
+++ b/src/arch/arm/Makefile.inc
@@ -33,7 +33,9 @@

###############################################################################

ifeq ($(CONFIG_ARCH_ROMSTAGE_ARM),y)
-CBFSTOOL_PRE1_OPTS = -m arm -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) -H
$(CONFIG_CBFS_HEADER_ROM_OFFSET) -o $(CONFIG_CBFS_ROM_OFFSET)
+CBFSTOOL_PRE1_OPTS = -m arm -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) \
+ -H $(CONFIG_CBFS_HEADER_ROM_OFFSET) \
+ -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
endif


###############################################################################
diff --git a/src/arch/arm64/Makefile.inc b/src/arch/arm64/Makefile.inc
index 8f99a46..93db9e2 100644
--- a/src/arch/arm64/Makefile.inc
+++ b/src/arch/arm64/Makefile.inc
@@ -34,7 +34,9 @@

################################################################################

ifeq ($(CONFIG_ARCH_ROMSTAGE_ARM64),y)
-CBFSTOOL_PRE1_OPTS = -m arm64 -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) -H
$(CONFIG_CBFS_HEADER_ROM_OFFSET) -o $(CONFIG_CBFS_ROM_OFFSET)
+CBFSTOOL_PRE1_OPTS = -m arm64 -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) \
+ -H $(CONFIG_CBFS_HEADER_ROM_OFFSET) \
+ -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
endif


################################################################################
diff --git a/src/arch/mips/Makefile.inc b/src/arch/mips/Makefile.inc
index 5446803..39bb05e 100644
--- a/src/arch/mips/Makefile.inc
+++ b/src/arch/mips/Makefile.inc
@@ -24,8 +24,9 @@

###############################################################################

ifeq ($(CONFIG_ARCH_ROMSTAGE_MIPS),y)
-CBFSTOOL_PRE1_OPTS = -m mips -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) -H
$(CONFIG_CBFS_HEADER_ROM_OFFSET) -o $(CONFIG_CBFS_ROM_OFFSET)
-CBFSTOOL_PRE_OPTS = -b 0
+CBFSTOOL_PRE1_OPTS = -m mips -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) \
+ -H $(CONFIG_CBFS_HEADER_ROM_OFFSET) \
+ -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
endif


###############################################################################
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 28f8105..0c7d71d 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -58,7 +58,8 @@

################################################################################

ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32),y)
-CBFSTOOL_PRE1_OPTS = -m x86 -o $$(( $(CONFIG_ROM_SIZE) -
$(CONFIG_CBFS_SIZE) ))
+CBFSTOOL_PRE1_OPTS = -m x86 -s $(CONFIG_ROM_SIZE) \
+ -o $$(( $(CONFIG_ROM_SIZE) - $(CONFIG_CBFS_SIZE) ))
# Make sure that segment for .car.data is ignored while adding romstage.
CBFSTOOL_PRE_OPTS = -b $(shell cat $(objcbfs)/base_xip.txt) -S ".car.data"
endif
diff --git a/src/mainboard/google/cosmos/Kconfig
b/src/mainboard/google/cosmos/Kconfig
index 56ab368..895fd05 100644
--- a/src/mainboard/google/cosmos/Kconfig
+++ b/src/mainboard/google/cosmos/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_2048
select CHROMEOS
select CHROMEOS_VBNV_FLASH
select COMMON_CBFS_SPI_WRAPPER
diff --git a/src/mainboard/google/nyan/Kconfig
b/src/mainboard/google/nyan/Kconfig
index 19eb6a1..8a39bb7 100644
--- a/src/mainboard/google/nyan/Kconfig
+++ b/src/mainboard/google/nyan/Kconfig
@@ -21,6 +21,7 @@

config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
+ select BOARD_ROMSIZE_KB_4096
select CHROMEOS
select CHROMEOS_VBNV_EC
select EC_GOOGLE_CHROMEEC
diff --git a/src/mainboard/google/nyan_big/Kconfig
b/src/mainboard/google/nyan_big/Kconfig
index b01dd84..9105076 100644
--- a/src/mainboard/google/nyan_big/Kconfig
+++ b/src/mainboard/google/nyan_big/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_4096
select CHROMEOS
select CHROMEOS_VBNV_EC
select EC_GOOGLE_CHROMEEC
diff --git a/src/mainboard/google/nyan_blaze/Kconfig
b/src/mainboard/google/nyan_blaze/Kconfig
index e85a633..5b74497 100644
--- a/src/mainboard/google/nyan_blaze/Kconfig
+++ b/src/mainboard/google/nyan_blaze/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_4096
select CHROMEOS
select CHROMEOS_VBNV_EC
select EC_GOOGLE_CHROMEEC
diff --git a/src/mainboard/google/rush/Kconfig
b/src/mainboard/google/rush/Kconfig
index f27b658..486c7cf 100644
--- a/src/mainboard/google/rush/Kconfig
+++ b/src/mainboard/google/rush/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_4096
select CHROMEOS
select CHROMEOS_VBNV_EC
select EC_GOOGLE_CHROMEEC
diff --git a/src/mainboard/google/rush_ryu/Kconfig
b/src/mainboard/google/rush_ryu/Kconfig
index 93f5eea..24d0c04 100644
--- a/src/mainboard/google/rush_ryu/Kconfig
+++ b/src/mainboard/google/rush_ryu/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_8192
select CHROMEOS
select CHROMEOS_VBNV_EC
select EC_GOOGLE_CHROMEEC
diff --git a/src/mainboard/google/storm/Kconfig
b/src/mainboard/google/storm/Kconfig
index 7d75d7a..d2626c5 100644
--- a/src/mainboard/google/storm/Kconfig
+++ b/src/mainboard/google/storm/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_8192
select CHROMEOS
select COMMON_CBFS_SPI_WRAPPER
select MAINBOARD_HAS_BOOTBLOCK_INIT
diff --git a/src/mainboard/google/veyron_jerry/Kconfig
b/src/mainboard/google/veyron_jerry/Kconfig
index 4d250d3..8a48a45 100644
--- a/src/mainboard/google/veyron_jerry/Kconfig
+++ b/src/mainboard/google/veyron_jerry/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_4096
select CHROMEOS
select CHROMEOS_VBNV_EC
select EC_GOOGLE_CHROMEEC
diff --git a/src/mainboard/google/veyron_pinky/Kconfig
b/src/mainboard/google/veyron_pinky/Kconfig
index aac4b3c..7c93cbf 100644
--- a/src/mainboard/google/veyron_pinky/Kconfig
+++ b/src/mainboard/google/veyron_pinky/Kconfig
@@ -22,6 +22,7 @@
config BOARD_SPECIFIC_OPTIONS # dummy
def_bool y
select BOARD_ID_SUPPORT
+ select BOARD_ROMSIZE_KB_4096
select CHROMEOS
select CHROMEOS_VBNV_EC
select EC_GOOGLE_CHROMEEC
diff --git a/src/soc/qualcomm/Kconfig b/src/soc/qualcomm/Kconfig
index 918093b..b7a12d4 100644
--- a/src/soc/qualcomm/Kconfig
+++ b/src/soc/qualcomm/Kconfig
@@ -1,9 +1 @@
source src/soc/qualcomm/ipq806x/Kconfig
-
-config CBFS_SIZE
- hex "Size of CBFS filesystem in ROM"
- default ROM_SIZE
- help
- CBFS size needs to match the size of memory allocated to the
- coreboot blob elsewhere in the system. Make sure this config option
- is fine tuned in the board config file.
diff --git a/src/vendorcode/google/chromeos/Kconfig
b/src/vendorcode/google/chromeos/Kconfig
index 64c3f38..be63a52 100644
--- a/src/vendorcode/google/chromeos/Kconfig
+++ b/src/vendorcode/google/chromeos/Kconfig
@@ -82,6 +82,7 @@
hex "Flash Map Offset"
default 0x00670000 if NORTHBRIDGE_INTEL_SANDYBRIDGE
default 0x00610000 if NORTHBRIDGE_INTEL_IVYBRIDGE
+ default CBFS_SIZE if !ARCH_X86
default 0
help
Offset of flash map in firmware image
@@ -150,4 +151,4 @@
is sent during an S3 resume.

source src/vendorcode/google/chromeos/vboot1/Kconfig
-source src/vendorcode/google/chromeos/vboot2/Kconfig
\ No newline at end of file
+source src/vendorcode/google/chromeos/vboot2/Kconfig

--
To view, visit https://chromium-review.googlesource.com/229974
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>

Aaron Durbin (Gerrit)

unread,
Nov 14, 2014, 10:21:43 PM11/14/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi
Aaron Durbin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

(4 comments)

https://chromium-review.googlesource.com/#/c/229974/1//COMMIT_MSG
Commit Message:

Line 21: unchangeable physical properties of the board). It also changes the
And is the same thing as x86, right?


Line 24: special use cases like ChromeOS. This has the advantage that
cbfstool
Where is this override? I don't think I'm seeing/understanding it.


https://chromium-review.googlesource.com/#/c/229974/1/configs/config.arm64-generic
File configs/config.arm64-generic:

Line 3: CONFIG_CBFS_SIZE=0x100000
Can you split out the configs/* changes? Just CQ-DEPEND the parent-child
together. It should ease upstreaming.


https://chromium-review.googlesource.com/#/c/229974/1/src/arch/arm/Makefile.inc
File src/arch/arm/Makefile.inc:

Line 38: -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
I'm still confused by this (-s CONFIG_CBFS_SIZE). Why can't we add a proper
limit to cbfstool so that it doesn't go beyond a given offset? The problem
with this is that it doesn't properly create a full-sized coreboot.rom to
flash onto the device. Wouldn't adding that limit make the semantics
consistent?
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Julius Werner (Gerrit)

unread,
Nov 14, 2014, 10:45:36 PM11/14/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Aaron Durbin, Patrick Georgi
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

(4 comments)

https://chromium-review.googlesource.com/#/c/229974/1//COMMIT_MSG
Commit Message:

Line 21: unchangeable physical properties of the board). It also changes the
> And is the same thing as x86, right?
Yes, all the x86 already use BOARD_ROMSIZE_KB_xxx


Line 24: special use cases like ChromeOS. This has the advantage that
cbfstool
> Where is this override? I don't think I'm seeing/understanding it.
In the config.<board> files.


https://chromium-review.googlesource.com/#/c/229974/1/configs/config.arm64-generic
File configs/config.arm64-generic:

Line 3: CONFIG_CBFS_SIZE=0x100000
> Can you split out the configs/* changes? Just CQ-DEPEND the parent-child
> to
Uhh... sure, will do that next iteration.


https://chromium-review.googlesource.com/#/c/229974/1/src/arch/arm/Makefile.inc
File src/arch/arm/Makefile.inc:

Line 38: -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
> I'm still confused by this (-s CONFIG_CBFS_SIZE). Why can't we add a
> proper
Well... I thought this was the simplest way to add it and is also
essentially what we are already doing. The values passed through -s don't
change (except for rush_ryu and storm), the number (1MB) in it just comes
from a different variable now which I think describes reality better.

We could add another flag and more logic to cbfstool to model the
difference between CBFS size and physical image size, but I don't really
see much value in that. If you want to add something else to the image you
will need to postprocess it with another tool anyway (which can do the job
of padding it out), and if you don't want to do that and flash coreboot.rom
directly instead you should have just left the CBFS_SIZE option at it's
default. The other nice thing is that this communicates the size of the
CBFS part clearly (via the file size of coreboot.rom) to whatever other
tool will extend it (we currently don't make use of that, but we could).

And one more issue is that the current cros_bundle_firmware implementation
is so godawful hacky that it actually requires the coreboot.rom file to be
exactly 1MB in size (otherwise it will silently assemble an image filled
with 0xff). I'd rather not touch that with a ten foot pole right now, I'm
just trying to get rid of those hardcoded header offsets in the next patch
(one step at a time).
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Aaron Durbin (Gerrit)

unread,
Nov 14, 2014, 11:07:04 PM11/14/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi
Aaron Durbin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

(3 comments)

https://chromium-review.googlesource.com/#/c/229974/1//COMMIT_MSG
Commit Message:

Line 24: special use cases like ChromeOS. This has the advantage that
cbfstool
> In the config.<board> files.
Duh. Thank you.


https://chromium-review.googlesource.com/#/c/229974/1/src/arch/arm/Makefile.inc
File src/arch/arm/Makefile.inc:

Line 38: -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
> Well... I thought this was the simplest way to add it and is also
> essential
Fwiw, the reason I ask the question is that it's very helpful when doing
initial bringup to have coreboot proper spit out a coreboot.rom that is the
romsize. I understand what you are saying in that -s is really only for
cbfs. But that's just not true on x86. I think it's important to be
consistent. To do that I think the only changes to cbfstool is adding a
total file size option.

I guess the question is does coreboot.rom mean the final image or just
cbfs? Your approach is taking the stance that coreboot.rom means cbfs only
which may or may not be the whole final rom size.

I still need to think about it some. Not saying this approach is bad or
anything. Just need to think about it while not being a Friday night. :)


https://chromium-review.googlesource.com/#/c/229974/1/src/arch/x86/Makefile.inc
File src/arch/x86/Makefile.inc:

Line 62: -o $$(( $(CONFIG_ROM_SIZE) - $(CONFIG_CBFS_SIZE) ))
The combination of -s and -o here implies cbfs size (CONFIG_CBFS_SIZE)
because it hits the end of the file (-s). From a consistency standpoint, it
seems like you should just be passing CONFIG_CBFS_SIZE to -s and leaving -o
off.

I don't actually think we should, but I think just adding a proper
--cbfs_size which is different than -s (which would be file size or
whatever) handles all those cases.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Julius Werner (Gerrit)

unread,
Nov 15, 2014, 3:06:09 AM11/15/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Aaron Durbin, Patrick Georgi
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

(2 comments)

I think I see where you're coming from Aaron, but I don't really see a good
solution here that's perfectly clean and consistent. Every method I can
think of ends up having ugly edge cases attached to it that still make it
no clearer than this patch in the end.

My main goal here is really just what the next patch does. This CL is only
meant to create the prerequisites (essentially just removing the storm/ryu
special case so cbfstool has a consistent view of the image) while changing
what we have as little as possible.

https://chromium-review.googlesource.com/#/c/229974/1/src/arch/arm/Makefile.inc
File src/arch/arm/Makefile.inc:

Line 38: -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
> Fwiw, the reason I ask the question is that it's very helpful when doing
> in
One more thing to consider is that cbfstool needs a way to know the size of
the CBFS part (for bounds checking when adding entries). If we don't want
to change the master header format, the only field we have for that is
header->romsize, so that inevitably has to be different between x86 (where
it's the whole image size) and others (where it's only the CBFS part). I
think that is an inconsistency we cannot really remove, and once we allow
that the effect of a --total-image-size would really only be to pad it once
on create and then forget about it. It might be simpler to just do the same
thing with dd from the top-level Makefile.inc or our ebuild.


https://chromium-review.googlesource.com/#/c/229974/1/src/arch/x86/Makefile.inc
File src/arch/x86/Makefile.inc:

Line 62: -o $$(( $(CONFIG_ROM_SIZE) - $(CONFIG_CBFS_SIZE) ))
> The combination of -s and -o here implies cbfs size (CONFIG_CBFS_SIZE)
> beca
Okay, I see what you're saying. I didn't even consider that we could also
use a smaller -s here, because it would change all the offsets. But I guess
since the image is top-aligned that would actually still work in coreboot.

However, we would still have issues if we later prepended the other 7MB in
front of that and tried to use cbfstool on the resulting complete
image.bin, since it expects that all offsets are calculated in relation to
the start of the file right now (we could change that, but it would be a
lot more work and potential for new bugs).
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Hung-Te Lin (Gerrit)

unread,
Nov 19, 2014, 5:13:55 AM11/19/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, David Hendricks, Aaron Durbin, Patrick Georgi
Hung-Te Lin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

We thought about adding a "cbfs size" field in CBFS header (so people
adding bits can still figure out the right size after build procedure is
done.

Maybe you want to add that at the same time?
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: No

Aaron Durbin (Gerrit)

unread,
Nov 19, 2014, 10:19:32 AM11/19/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi
Aaron Durbin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

(1 comment)

" Every method I can think of ends up having ugly edge cases attached to it
that still make it no clearer than this patch in the end."

I don't follow. Is that using the supposition of not touching the master
header? If so, I think we should add the necessary option to cbfstool as
well as add a field to the master header.

https://chromium-review.googlesource.com/#/c/229974/1/src/arch/arm/Makefile.inc
File src/arch/arm/Makefile.inc:

Line 38: -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
> One more thing to consider is that cbfstool needs a way to know the size
> of
I think we should change the master header format. We've got the pad field
to work with. I think the issue of overloading the 'romsize' definition is
that (I believe) it's used as a definitive actual rom size.

I don't understand the comment about not being able to remove the
inconsistency. The only reason the inconsistency exists is because it was
codified as being inconsistent from x86. Can we not rectify that? I feel
like we can and should by utilizing the pad[1] field to place a barrier for
end of cbfs content.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Aaron Durbin (Gerrit)

unread,
Nov 19, 2014, 10:23:21 AM11/19/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi
Aaron Durbin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

Forgot to add this:

If one didn't have to support any legacy anything, how would you solve the
issues of sub region usage within a super region? And would all
offsets/markers be relative? Any use for having self-discovery by the
contents of the media?
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: No

Julius Werner (Gerrit)

unread,
Nov 19, 2014, 5:28:05 PM11/19/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Aaron Durbin, Patrick Georgi
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

(1 comment)

> If one didn't have to support any legacy anything, how would you solve
> the issues of sub region usage within a super region? And would all
> offsets/markers be relative? Any use for having self-discovery by the
> contents of the media?

Uhh... is this a philosophical question? ;) I think if I were to redesign
everything, I would merge FMAP and CBFS into a single layout system used by
both coreboot and vboot, with a directory in a single location (like FMAP,
so RO/RW split works), and a common way to find that directory at runtime
(like CBFS's master header pointer, so we can bootstrap it on both x86 and
others). It should be possible to be embedded into itself for cases like
SeaBIOS (maybe like a subdirectory, but in a way that code can chose to
interpret the part in the same way as the whole, so I guess offsets would
need to be relative). It should also make considerations for vboot
encryption of both individual parts and consecutive sections right from the
start of its design, and either embed something like our vblocks straight
in the directory or at least make them easy to associate with the content
areas they belong to.

But, alas, considering how many tendrils are hanging off FMAP I don't see
us having time for such a big overhaul in any near or medium term future...
so I think we should make the best we can out of our dual layout system
mashup.

https://chromium-review.googlesource.com/#/c/229974/1/src/arch/arm/Makefile.inc
File src/arch/arm/Makefile.inc:

Line 38: -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
> I think we should change the master header format. We've got the pad field
Yes, my previous comment was assuming that we didn't want to change the
CBFS header. You're right that we could just add the other size value and
then hide some more of the differences in cbfstool.

However, that would blow up this series to be way larger than I intended
to. There's a lot of things in cbfstool that all work on the size of the
image somehow which would need to be changed, and I don't really know it
well enough to be confident that I could find and resolve all of them
correctly. The current patches only change cbfstool superficially, altering
where some parameters come from and how defaults are calculated but not
needing to touch any of the actual, tricky entry generation and placement
code.

Could we maybe keep your suggestion as an idea for the unspecified future,
but go with something like this solution as a stopgap for the time being? I
think it's still going halfway in the right direction (at least more
consistent than the status quo) and solves a real problem without too much
effort. I have grepped for the use of romsize in coreboot and am pretty
sure that it's not relied on to mean the whole image size in any
non-x86-specific code (otherwise some boards like Nyan would've already
been broken), so this should be safe.

I can throw the dd to pad coreboot.rom up to ROM_SIZE into this patch to
solve your flashrom problem if you want.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Aaron Durbin (Gerrit)

unread,
Nov 21, 2014, 9:51:52 AM11/21/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi
Aaron Durbin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 1:

(1 comment)

I'll wait for your patch updates.

https://chromium-review.googlesource.com/#/c/229974/1/src/arch/arm/Makefile.inc
File src/arch/arm/Makefile.inc:

Line 38: -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
> Yes, my previous comment was assuming that we didn't want to change the
> CBF
Where did all your ambition go, Julius? I'm so disappointed.

Kidding aside, I think we can go with this in the interim. I think the
issue of self discovery of cbfs location is a tricky matter. The one issue
is that we can't perform cbstool add operations on an image that is larger
than CBFS_SIZE. That's unfortunate, but I think we can live with it for now.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Julius Werner (Gerrit)

unread,
Nov 22, 2014, 4:24:46 AM11/22/14
to Stefan Reinauer, Hung-Te Lin, David Hendricks, Aaron Durbin, Patrick Georgi, Vadim Bendebury
Julius Werner has uploaded a new patch set (#2).
CQ-DEPEND=TODO,TODO
BRANCH=None
BUG=chromium:422501
TEST=Built and booted on Veyron_Pinky.

Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Signed-off-by: Julius Werner <jwe...@chromium.org>
---
M Makefile.inc
M src/Kconfig
M src/arch/arm/Makefile.inc
M src/arch/arm64/Makefile.inc
M src/arch/mips/Makefile.inc
M src/arch/x86/Makefile.inc
M src/mainboard/google/cosmos/Kconfig
M src/mainboard/google/nyan/Kconfig
M src/mainboard/google/nyan_big/Kconfig
M src/mainboard/google/nyan_blaze/Kconfig
M src/mainboard/google/rush/Kconfig
M src/mainboard/google/rush_ryu/Kconfig
M src/mainboard/google/storm/Kconfig
M src/mainboard/google/veyron_jerry/Kconfig
M src/mainboard/google/veyron_pinky/Kconfig
M src/soc/qualcomm/Kconfig
M src/vendorcode/google/chromeos/Kconfig
17 files changed, 34 insertions(+), 17 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>

Julius Werner (Gerrit)

unread,
Nov 22, 2014, 4:27:34 AM11/22/14
to Hung-Te Lin, Stefan Reinauer, David Hendricks, Aaron Durbin, Patrick Georgi, Vadim Bendebury
Hello Hung-Te Lin, Stefan Reinauer, David Hendricks, Aaron Durbin, Patrick
Georgi, Vadim Bendebury,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/229974

to look at the new patch set (#3).
CQ-DEPEND=CL:231576,CL:231475
BRANCH=None
BUG=chromium:422501
TEST=Built and booted on Veyron_Pinky.

Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Signed-off-by: Julius Werner <jwe...@chromium.org>
---
M Makefile.inc
M src/Kconfig
M src/arch/arm/Makefile.inc
M src/arch/arm64/Makefile.inc
M src/arch/mips/Makefile.inc
M src/arch/x86/Makefile.inc
M src/mainboard/google/cosmos/Kconfig
M src/mainboard/google/nyan/Kconfig
M src/mainboard/google/nyan_big/Kconfig
M src/mainboard/google/nyan_blaze/Kconfig
M src/mainboard/google/rush/Kconfig
M src/mainboard/google/rush_ryu/Kconfig
M src/mainboard/google/storm/Kconfig
M src/mainboard/google/veyron_jerry/Kconfig
M src/mainboard/google/veyron_pinky/Kconfig
M src/soc/qualcomm/Kconfig
M src/vendorcode/google/chromeos/Kconfig
17 files changed, 34 insertions(+), 17 deletions(-)


Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>

Julius Werner (Gerrit)

unread,
Nov 24, 2014, 2:36:32 PM11/24/14
to Stefan Reinauer, Hung-Te Lin, David Hendricks, Aaron Durbin, Patrick Georgi, Vadim Bendebury
Julius Werner has uploaded a new patch set (#4).
CQ-DEPEND=CL:231576,CL:231475
BRANCH=None
BUG=chromium:422501
TEST=Built and booted on Veyron_Pinky.

Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Signed-off-by: Julius Werner <jwe...@chromium.org>
---
M Makefile.inc
M src/Kconfig
M src/arch/arm/Makefile.inc
M src/arch/arm64/Makefile.inc
M src/arch/mips/Makefile.inc
M src/arch/x86/Makefile.inc
M src/mainboard/google/cosmos/Kconfig
M src/mainboard/google/nyan/Kconfig
M src/mainboard/google/nyan_big/Kconfig
M src/mainboard/google/nyan_blaze/Kconfig
M src/mainboard/google/rush/Kconfig
M src/mainboard/google/rush_ryu/Kconfig
M src/mainboard/google/storm/Kconfig
M src/mainboard/google/veyron_jerry/Kconfig
M src/mainboard/google/veyron_mighty/Kconfig
M src/mainboard/google/veyron_pinky/Kconfig
M src/soc/qualcomm/Kconfig
M src/vendorcode/google/chromeos/Kconfig
18 files changed, 35 insertions(+), 17 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>

Aaron Durbin (Gerrit)

unread,
Nov 24, 2014, 4:48:48 PM11/24/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi
Aaron Durbin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 4: Code-Review+2

(1 comment)

https://chromium-review.googlesource.com/#/c/229974/4/Makefile.inc
File Makefile.inc:

Line 446: dd if=$(obj)/coreboot.pre of=$@.tmp bs=8192 conv=notrunc 2>
/dev/null
In the case of CBFS_SIZE != ROMSIZE don't we lose out on the goal of
limiting cbfs size? cbfstool isn't honoring romsize in the header field.
It's using the file size as the proxy for where the end of the cbfs is.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Julius Werner (Gerrit)

unread,
Nov 24, 2014, 8:28:19 PM11/24/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 4:

(1 comment)

https://chromium-review.googlesource.com/#/c/229974/4/Makefile.inc
File Makefile.inc:

Line 446: dd if=$(obj)/coreboot.pre of=$@.tmp bs=8192 conv=notrunc 2>
/dev/null
> In the case of CBFS_SIZE != ROMSIZE don't we lose out on the goal of
> limiti
It's not?

Okay, I went to the bottom of this and it turns out that it neither works
exactly like I assumed nor like you just said. It doesn't use romsize, but
it doesn't use the image file size either (for anything other than trying
to find the master header pointer).

Instead, all free space in the CBFS is marked with "empty" entries. On
image creation, it creates one big empty entry according to the -s
parameter (so it's the correct size in our case). Then, later on it just
breaks these empty entries down into smaller ones but always retains that
total size. I also tested adding a file to a padded image and it really
fails if you go over the end of the CBFS area.

I guess this means that ->romsize isn't as sacred as I though before, and
we could maybe really make it span the physical image size. But there are
several other cryptic uses throughout cbfstool that I'm too lazy to think
through right now, so I'd rather keep it the way it works in this patch for
now.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Aaron Durbin (Gerrit)

unread,
Nov 25, 2014, 10:25:49 AM11/25/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi
Aaron Durbin has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 4:

(1 comment)

https://chromium-review.googlesource.com/#/c/229974/4/Makefile.inc
File Makefile.inc:

Line 446: dd if=$(obj)/coreboot.pre of=$@.tmp bs=8192 conv=notrunc 2>
/dev/null
> It's not?
haha. good to know. Thanks for the legwork.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: Yes

Julius Werner (Gerrit)

unread,
Nov 25, 2014, 2:43:35 PM11/25/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 4: Commit-Queue+1 Verified+1

Okay, looks like it's time to get this in... thanks again for all the
reviews, Aaron!
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-HasComments: No

Julius Werner (Gerrit)

unread,
Nov 25, 2014, 6:54:53 PM11/25/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, chrome-internal-fetch, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 4: -Commit-Queue

Sure, why not. I think Stefan is OOO this week(?) but this can wait until
next.
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-Reviewer: chrome-internal-fetch <chrome-int...@google.com>
Gerrit-HasComments: No

Julius Werner (Gerrit)

unread,
Dec 1, 2014, 3:40:07 PM12/1/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, chrome-internal-fetch, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 4: Commit-Queue+1
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>

Julius Werner (Gerrit)

unread,
Dec 2, 2014, 2:29:27 PM12/2/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, chrome-internal-fetch, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 4: Commit-Queue+1
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>

Julius Werner (Gerrit)

unread,
Dec 2, 2014, 3:44:12 PM12/2/14
to Aaron Durbin, Stefan Reinauer, Hung-Te Lin, David Hendricks, chrome-internal-fetch, Patrick Georgi, Vadim Bendebury
Hello Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/229974

to look at the new patch set (#5).
CQ-DEPEND=CL:231576,CL:231475
BRANCH=None
BUG=chromium:422501
TEST=Built and booted on Veyron_Pinky.

Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Signed-off-by: Julius Werner <jwe...@chromium.org>
---
M Makefile.inc
M src/Kconfig
M src/arch/arm/Makefile.inc
M src/arch/arm64/Makefile.inc
M src/arch/mips/Makefile.inc
M src/arch/x86/Makefile.inc
M src/mainboard/google/cosmos/Kconfig
M src/mainboard/google/nyan/Kconfig
M src/mainboard/google/nyan_big/Kconfig
M src/mainboard/google/nyan_blaze/Kconfig
M src/mainboard/google/rush/Kconfig
M src/mainboard/google/rush_ryu/Kconfig
M src/mainboard/google/storm/Kconfig
M src/mainboard/google/veyron_jerry/Kconfig
M src/mainboard/google/veyron_mighty/Kconfig
M src/mainboard/google/veyron_pinky/Kconfig
M src/soc/qualcomm/Kconfig
M src/vendorcode/google/chromeos/Kconfig
18 files changed, 35 insertions(+), 17 deletions(-)


Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-Reviewer: chrome-internal-fetch <chrome-int...@google.com>

Julius Werner (Gerrit)

unread,
Dec 2, 2014, 3:45:20 PM12/2/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, chrome-internal-fetch, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 5: Commit-Queue+1 Verified+1
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>

Julius Werner (Gerrit)

unread,
Dec 2, 2014, 4:19:39 PM12/2/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, chrome-internal-fetch, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 5: Commit-Queue+1
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>

Julius Werner (Gerrit)

unread,
Dec 2, 2014, 6:03:07 PM12/2/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, chrome-internal-fetch, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 5: Commit-Queue+1

Okay CQ, at this point it's really not funny anymore...
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>

Julius Werner (Gerrit)

unread,
Dec 2, 2014, 10:49:06 PM12/2/14
to Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, chrome-internal-fetch, Aaron Durbin
Julius Werner has posted comments on this change.

Change subject: CBFS: Correct ROM_SIZE for ARM boards, use CBFS_SIZE for
cbfstool
......................................................................


Patch Set 5: Commit-Queue+1
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>

chrome-internal-fetch (Gerrit)

unread,
Dec 3, 2014, 1:09:47 AM12/3/14
to Julius Werner, Vadim Bendebury, Stefan Reinauer, Hung-Te Lin, David Hendricks, Patrick Georgi, Aaron Durbin
chrome-internal-fetch has submitted this change and it was merged.
CQ-DEPEND=CL:231576,CL:231475
BRANCH=None
BUG=chromium:422501
TEST=Built and booted on Veyron_Pinky.

Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Signed-off-by: Julius Werner <jwe...@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/229974
Reviewed-by: Aaron Durbin <adu...@chromium.org>
---
M Makefile.inc
M src/Kconfig
M src/arch/arm/Makefile.inc
M src/arch/arm64/Makefile.inc
M src/arch/mips/Makefile.inc
M src/arch/x86/Makefile.inc
M src/mainboard/google/cosmos/Kconfig
M src/mainboard/google/nyan/Kconfig
M src/mainboard/google/nyan_big/Kconfig
M src/mainboard/google/nyan_blaze/Kconfig
M src/mainboard/google/rush/Kconfig
M src/mainboard/google/rush_ryu/Kconfig
M src/mainboard/google/storm/Kconfig
M src/mainboard/google/veyron_jerry/Kconfig
M src/mainboard/google/veyron_mighty/Kconfig
M src/mainboard/google/veyron_pinky/Kconfig
M src/soc/qualcomm/Kconfig
M src/vendorcode/google/chromeos/Kconfig
18 files changed, 35 insertions(+), 17 deletions(-)

Approvals:
Julius Werner: Ready; Verified
Aaron Durbin: Looks good to me, approved



diff --git a/Makefile.inc b/Makefile.inc
index 25a0807..d82de69 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -410,7 +410,7 @@
prebuilt-files = $(foreach file,$(cbfs-files), $(call
extract_nth,1,$(file)))

$(obj)/coreboot.pre1: $(objcbfs)/bootblock.bin $$(prebuilt-files)
$(CBFSTOOL) $$(cpu_ucode_cbfs_file)
- $(CBFSTOOL) $@.tmp create -s $(CONFIG_COREBOOT_ROMSIZE_KB)K \
+ $(CBFSTOOL) $@.tmp create \
-B $(objcbfs)/bootblock.bin -a 64 \
$(CBFSTOOL_PRE1_OPTS)
$(prebuild-files) true
@@ -440,7 +440,10 @@

$(obj)/coreboot.rom: $(obj)/coreboot.pre $(objcbfs)/ramstage.elf
$(CBFSTOOL) $(call strip_quotes,$(COREBOOT_ROM_DEPENDENCIES))
$$(INTERMEDIATE) $$(VBOOT_STUB) $(REFCODE_BLOB)
@printf " CBFS $(subst $(obj)/,,$(@))\n"
- cp $(obj)/coreboot.pre $@.tmp
+# The full ROM may be larger than the CBFS part, so create an empty
+# file (filled with \377 = 0xff) and copy the CBFS image over it.
+ tr '\000' '\377' < /dev/zero 2> /dev/null | dd of=$@.tmp bs=8192
iflag=fullblock count=$$(($(CONFIG_ROM_SIZE) / 8192)) 2> /dev/null
+ dd if=$(obj)/coreboot.pre of=$@.tmp bs=8192 conv=notrunc 2> /dev/null
$(CBFSTOOL) $@.tmp add-stage -f $(objcbfs)/ramstage.elf -n
$(CONFIG_CBFS_PREFIX)/ramstage -c $(CBFS_COMPRESS_FLAG)
ifeq ($(CONFIG_PAYLOAD_NONE),y)
@printf " PAYLOAD none (as specified by user)\n"
diff --git a/src/Kconfig b/src/Kconfig
index 8d3d831..da5196d 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -422,8 +422,14 @@
index bf1ba27..e1e8ee7 100644
index 3623f2e..b9b6011 100644
diff --git a/src/mainboard/google/veyron_mighty/Kconfig
b/src/mainboard/google/veyron_mighty/Kconfig
index ee61ddb..041fab3 100644
--- a/src/mainboard/google/veyron_mighty/Kconfig
+++ b/src/mainboard/google/veyron_mighty/Kconfig
index d0fae0a..2cf7ec2 100644
--- a/src/vendorcode/google/chromeos/Kconfig
+++ b/src/vendorcode/google/chromeos/Kconfig
@@ -87,6 +87,7 @@
hex "Flash Map Offset"
default 0x00670000 if NORTHBRIDGE_INTEL_SANDYBRIDGE
default 0x00610000 if NORTHBRIDGE_INTEL_IVYBRIDGE
+ default CBFS_SIZE if !ARCH_X86
default 0
help
Offset of flash map in firmware image
@@ -161,4 +162,4 @@
Whether this platform has a physical recovery switch

source src/vendorcode/google/chromeos/vboot1/Kconfig
-source src/vendorcode/google/chromeos/vboot2/Kconfig
\ No newline at end of file
+source src/vendorcode/google/chromeos/vboot2/Kconfig

--
To view, visit https://chromium-review.googlesource.com/229974
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4fce5a56a8d72f4c4dd3a08c129025f1565351cc
Gerrit-PatchSet: 6
Gerrit-Project: chromiumos/third_party/coreboot
Gerrit-Branch: chromeos-2013.04
Gerrit-Owner: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adu...@chromium.org>
Gerrit-Reviewer: David Hendricks <dhen...@chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hun...@chromium.org>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pge...@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <rein...@chromium.org>
Gerrit-Reviewer: Vadim Bendebury <vbe...@chromium.org>
Gerrit-Reviewer: chrome-internal-fetch <chrome-int...@google.com>
Reply all
Reply to author
Forward
0 new messages