coreboot-gerrit November 2015

coreboot-gerrit@coreboot.org

1 participants
1544 discussions

Patch set updated for coreboot: build system: actually provide revision information in defconfig
by Patrick Georgi 30 Nov '15

30 Nov '15

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12558 -gerrit commit e5fbe77d7633ff2519718eb687c30fb643230705 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Fri Nov 27 12:36:28 2015 +0100 build system: actually provide revision information in defconfig The config file added to CBFS is the short version created by defconfig. The build system tried to add a header describing the version for quite a while now, but failed because it wrote to the file, then had kconfig overwrite it with the config data. While at it, rely on build.h and its version information instead of calling git manually. Change-Id: I5e4d6c857594a55432c05bf1480973fc950f4d4a Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> --- Makefile.inc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index d5cf337..04aa076 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -228,10 +228,13 @@ cbfs-files-processor-vsa= \ # arg1: input # arg2: output cbfs-files-processor-defconfig= \ - $(eval $(2): $(1) ; \ + $(eval $(2): $(1) $(obj)/build.h; \ printf " CREATE $(2) (from $(1))\n"; \ - echo "\# This image was built using git revision" `git rev-parse HEAD` > $(2).tmp && \ - $(MAKE) DOTCONFIG=$(1) DEFCONFIG=$(2).tmp savedefconfig && \ + printf "\# This image was built using coreboot " > $(2).tmp && \ + grep "\<COREBOOT_VERSION\>" $(obj)/build.h |cut -d\" -f2 >> $(2).tmp && \ + $(MAKE) DOTCONFIG=$(1) DEFCONFIG=$(2).tmp2 savedefconfig && \ + cat $(2).tmp2 >> $(2).tmp && \ + rm -f $(2).tmp2 && \ \mv -f $(2).tmp $(2)) #######################################################################

1 0

Patch set updated for coreboot: build system: drop one level of indirection
by Patrick Georgi 30 Nov '15

30 Nov '15

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12550 -gerrit commit 20c7a9bfbd45268cf2a0542148f89fc0a37da7f6 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Nov 26 17:22:38 2015 +0100 build system: drop one level of indirection coreboot.pre1 was generated then copied into coreboot.pre, now without any additional manipulation. Get rid of that extra step. Change-Id: I138567cadbc2fa1a6b6c988e34bdaae0e92d5554 Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> --- Makefile.inc | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index 3942197..9aceb18 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -613,7 +613,7 @@ prebuild-files = \ $(cbfs-add-cmd) $(if $(call extract_nth,5,$(file)),-b $(call extract_nth,5,$(file))) &&)) prebuilt-files = $(foreach file,$(cbfs-files), $(call extract_nth,1,$(file))) -$(obj)/coreboot.pre1: $(objcbfs)/bootblock.bin $$(prebuilt-files) $(FMAPTOOL) $(CBFSTOOL) $$(cpu_ucode_cbfs_file) +$(obj)/coreboot.pre: $(objcbfs)/bootblock.bin $$(prebuilt-files) $(FMAPTOOL) $(CBFSTOOL) $$(cpu_ucode_cbfs_file) $(CBFSTOOL) $@.tmp create \ -B $(objcbfs)/bootblock.bin \ $(CBFSTOOL_PRE1_OPTS) @@ -629,8 +629,8 @@ prebuild-files = \ $(if $(call extract_nth,6,$(file)),$(cbfs-add-cmd) -a $(call extract_nth,6,$(file)) &&,\ $(cbfs-add-cmd) $(if $(call extract_nth,5,$(file)),-b $(call extract_nth,5,$(file))) &&))) -.PHONY: $(obj)/coreboot.pre1 -$(obj)/coreboot.pre1: $$(prebuilt-files) $(CBFSTOOL) +.PHONY: $(obj)/coreboot.pre +$(obj)/coreboot.pre: $$(prebuilt-files) $(CBFSTOOL) mv $(obj)/coreboot.rom $@.tmp $(prebuild-files) true mv $@.tmp $@ @@ -741,11 +741,6 @@ cbfs-files-$(CONFIG_BOOTSPLASH_IMAGE) += bootsplash.jpg bootsplash.jpg-file := $(call strip_quotes,$(CONFIG_BOOTSPLASH_FILE)) bootsplash.jpg-type := bootsplash -$(obj)/coreboot.pre: $(objcbfs)/romstage.elf $(obj)/coreboot.pre1 $(CBFSTOOL) - @printf " CBFS $(subst $(obj)/,,$(@))\n" - cp $(obj)/coreboot.pre1 $@.tmp - mv $@.tmp $@ - cbfs-files-$(CONFIG_BOARD_ID_MANUAL) += board_id board_id-file := $(obj)/board_id board_id-type := raw

1 0

Patch set updated for coreboot: build system: Emit some build output while adding files to CBFS
by Patrick Georgi 30 Nov '15

30 Nov '15

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12551 -gerrit commit c7375fdab8cfec9991749ec067970020e7fd0919 Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Nov 26 18:26:15 2015 +0100 build system: Emit some build output while adding files to CBFS Change-Id: I167f570957ca7eaf71fc31e1bd84b9bbad0683eb Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> --- Makefile.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.inc b/Makefile.inc index 9aceb18..d5cf337 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -597,6 +597,7 @@ COREBOOT_ROM_DEPENDENCIES:= extract_nth=$(subst *,$(spc),$(patsubst -%-,%,$(word $(1), $(subst |,- -,-$(2)-)))) cbfs-add-cmd = \ + printf " CBFS $(call extract_nth,2,$(file))\n"; \ $(CBFSTOOL) $@.tmp \ add$(if $(filter stage,$(call extract_nth,3,$(file))),-stage)$(if $(filter payload,$(call extract_nth,3,$(file))),-payload) \ -f $(call extract_nth,1,$(file)) \

1 0

Patch set updated for coreboot: build system: make cbfs-files mechanism spaces-safe
by Patrick Georgi 30 Nov '15

30 Nov '15

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12548 -gerrit commit 5ea8c7c840e56b0e45e20b67d3c34c4b6dc1925d Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Nov 26 17:17:28 2015 +0100 build system: make cbfs-files mechanism spaces-safe Space is commonly used as separator in make variables, so escape them as * (which should be reasonably uncommon in file names and cbfstool options alike to not be a problem). Change-Id: Ia77b5559841b5eae3aa1c0c0027f2e7fb882ea2f Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> --- Makefile.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index 09cd6cc..82e7bdb 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -265,7 +265,7 @@ cbfs-files-handler= \ $(eval tmp-cbfs-file:=$(shell mkdir -p $(obj)/mainboard/$(MAINBOARDDIR); mktemp $(obj)/mainboard/$(MAINBOARDDIR)/cbfs-file.XXXXXX).out) \ $(call cbfs-files-processor-$(tmp-cbfs-method),$(tmp-old-cbfs-file),$(tmp-cbfs-file))) \ $(if $(tmp-cbfs-file), \ - $(eval cbfs-files-$(_cbfs-bucket) += $(tmp-cbfs-file)|$(2)|$($(2)-type)|$($(2)-compression)|$(strip $($(2)-position))|$($(2)-align)|$($(2)-options))) \ + $(eval cbfs-files-$(_cbfs-bucket) += $(subst $(spc),*,$(tmp-cbfs-file)|$(2)|$($(2)-type)|$($(2)-compression)|$(strip $($(2)-position))|$($(2)-align)|$($(2)-options)))) \ $(eval $(2)-name:=) \ $(eval $(2)-type:=) \ $(eval $(2)-compression:=) \ @@ -594,7 +594,7 @@ $(objcbfs)/%.elf: $(objcbfs)/%.debug COREBOOT_ROM_DEPENDENCIES:= -extract_nth=$(patsubst -%-,%,$(word $(1), $(subst |,- -,-$(2)-))) +extract_nth=$(subst *,$(spc),$(patsubst -%-,%,$(word $(1), $(subst |,- -,-$(2)-)))) cbfs-add-cmd = \ $(CBFSTOOL) $@.tmp \

1 0

Patch set updated for coreboot: build system: Add more files through cbfs-files instead of manual rules
by Patrick Georgi 30 Nov '15

30 Nov '15

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12549 -gerrit commit 4744b46f306c68b302213a056cbff07a029196da Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Nov 26 16:39:23 2015 +0100 build system: Add more files through cbfs-files instead of manual rules verstage, romstage, and payload can be added through infrastructure now. Change-Id: Ib9e612ae35fb8c0230175f5b8bca1b129f366f4b Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> --- Makefile.inc | 42 ++++++++++++++-------- src/arch/arm/Makefile.inc | 1 - src/arch/riscv/Makefile.inc | 1 - src/arch/x86/Makefile.inc | 12 ------- src/vendorcode/google/chromeos/vboot2/Makefile.inc | 10 +++++- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index 82e7bdb..3942197 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -651,19 +651,12 @@ $(REFCODE_BLOB): $(RMODTOOL) $(RMODTOOL) -i $(CONFIG_REFCODE_BLOB_FILE) -o $@ endif -$(obj)/coreboot.rom: $(obj)/coreboot.pre $(objcbfs)/ramstage.elf $(CBFSTOOL) $$(call strip_quotes,$$(COREBOOT_ROM_DEPENDENCIES)) $$(INTERMEDIATE) $(REFCODE_BLOB) $$(VERSTAGE_FILE) +$(obj)/coreboot.rom: $(obj)/coreboot.pre $(objcbfs)/ramstage.elf $(CBFSTOOL) $$(call strip_quotes,$$(COREBOOT_ROM_DEPENDENCIES)) $$(INTERMEDIATE) $(REFCODE_BLOB) @printf " CBFS $(subst $(obj)/,,$(@))\n" # 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. dd if=/dev/zero bs=$(call _toint,$(CONFIG_ROM_SIZE)) count=1 2> /dev/null | tr '\000' '\377' > $@.tmp dd if=$(obj)/coreboot.pre of=$@.tmp bs=8192 conv=notrunc 2> /dev/null -ifeq ($(CONFIG_PAYLOAD_NONE),y) - @printf " PAYLOAD none (as specified by user)\n" -endif -ifneq ($(CONFIG_PAYLOAD_FILE),) - @printf " PAYLOAD $(CONFIG_PAYLOAD_FILE) (compression: $(CBFS_PAYLOAD_COMPRESS_FLAG))\n" - $(CBFSTOOL) $@.tmp add-payload -f $(CONFIG_PAYLOAD_FILE) -n $(CONFIG_CBFS_PREFIX)/payload -c $(CBFS_PAYLOAD_COMPRESS_FLAG) $(ADDITIONAL_PAYLOAD_CONFIG) -endif ifneq ($(CONFIG_SEABIOS_PS2_TIMEOUT),) ifneq ($(CONFIG_SEABIOS_PS2_TIMEOUT),0) @printf " SeaBIOS Wait up to $(CONFIG_SEABIOS_PS2_TIMEOUT) ms for PS/2 keyboard controller initialization\n" @@ -676,9 +669,6 @@ ifeq ($(CONFIG_CPU_MICROCODE_CBFS_GENERATE),y) $(CBFSTOOL) $@.tmp update-fit -n cpu_microcode_blob.bin -x $(CONFIG_CPU_INTEL_NUM_FIT_ENTRIES) endif endif -ifeq ($(CONFIG_SEPARATE_VERSTAGE),y) - $(CBFSTOOL) $@.tmp add-stage -f $(VERSTAGE_FILE) -n $(CONFIG_CBFS_PREFIX)/verstage $(CBFSTOOL_VERSTAGE_OPTS) -endif mv $@.tmp $@ @printf " CBFSPRINT $(subst $(obj)/,,$(@))\n\n" $(CBFSTOOL) $@ print @@ -687,11 +677,37 @@ ifeq ($(CONFIG_IASL_WARNINGS_ARE_ERRORS),) @printf "***** Please fix the ASL for this platform. *****\n\n" endif +cbfs-files-y += $(CONFIG_CBFS_PREFIX)/romstage +$(CONFIG_CBFS_PREFIX)/romstage-file := $(objcbfs)/romstage.elf +$(CONFIG_CBFS_PREFIX)/romstage-type := stage +$(CONFIG_CBFS_PREFIX)/romstage-compression := none +ifeq ($(CONFIG_ARCH_ROMSTAGE_ARM),y) +$(CONFIG_CBFS_PREFIX)/romstage-options := -b 0 +endif +ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32)$(CONFIG_ARCH_ROMSTAGE_X86_64),y) +# Use a 64 byte alignment to provide a minimum alignment +# requirement for the overall romstage. While the first object within +# romstage could have a 4 byte minimum alignment that doesn't mean the linker +# won't decide the entire section should be aligned to a larger value. In the +# future cbfstool should add XIP files proper and honor the alignment +# requirements of the program segment. +# +# Make sure that segment for .car.data is ignored while adding romstage. +$(CONFIG_CBFS_PREFIX)/romstage-align := 64 +$(CONFIG_CBFS_PREFIX)/romstage-options := --xip -S .car.data -P $(CONFIG_XIP_ROM_SIZE) +endif + cbfs-files-y += $(CONFIG_CBFS_PREFIX)/ramstage $(CONFIG_CBFS_PREFIX)/ramstage-file := $(objcbfs)/ramstage.elf $(CONFIG_CBFS_PREFIX)/ramstage-type := stage $(CONFIG_CBFS_PREFIX)/ramstage-compression := $(CBFS_COMPRESS_FLAG) +cbfs-files-y += $(CONFIG_CBFS_PREFIX)/payload +$(CONFIG_CBFS_PREFIX)/payload-file := $(CONFIG_PAYLOAD_FILE) +$(CONFIG_CBFS_PREFIX)/payload-type := payload +$(CONFIG_CBFS_PREFIX)/payload-compression := $(CBFS_PAYLOAD_COMPRESS_FLAG) +$(CONFIG_CBFS_PREFIX)/payload-options := $(ADDITIONAL_PAYLOAD_CONFIG) + cbfs-files-$(CONFIG_HAVE_REFCODE_BLOB) += $(CONFIG_CBFS_PREFIX)/refcode $(CONFIG_CBFS_PREFIX)/refcode-file := $(REFCODE_BLOB) $(CONFIG_CBFS_PREFIX)/refcode-type := stage @@ -728,10 +744,6 @@ bootsplash.jpg-type := bootsplash $(obj)/coreboot.pre: $(objcbfs)/romstage.elf $(obj)/coreboot.pre1 $(CBFSTOOL) @printf " CBFS $(subst $(obj)/,,$(@))\n" cp $(obj)/coreboot.pre1 $@.tmp - $(CBFSTOOL) $@.tmp add-stage \ - -f $(objcbfs)/romstage.elf \ - -n $(CONFIG_CBFS_PREFIX)/romstage -c none \ - $(CBFSTOOL_PRE_OPTS) mv $@.tmp $@ cbfs-files-$(CONFIG_BOARD_ID_MANUAL) += board_id diff --git a/src/arch/arm/Makefile.inc b/src/arch/arm/Makefile.inc index 5af9ab8..6675360 100644 --- a/src/arch/arm/Makefile.inc +++ b/src/arch/arm/Makefile.inc @@ -23,7 +23,6 @@ ifeq ($(CONFIG_ARCH_ROMSTAGE_ARM),y) CBFSTOOL_PRE1_OPTS = -m arm -s $(CONFIG_CBFS_SIZE) -CBFSTOOL_PRE_OPTS = -b 0 endif ifeq ($(CONFIG_ARCH_ARM),y) diff --git a/src/arch/riscv/Makefile.inc b/src/arch/riscv/Makefile.inc index 49c130b..aafce1a 100644 --- a/src/arch/riscv/Makefile.inc +++ b/src/arch/riscv/Makefile.inc @@ -72,7 +72,6 @@ romstage-c-ccopts += $(riscv_flags) romstage-S-ccopts += $(riscv_asm_flags) CBFSTOOL_PRE1_OPTS = -v -m riscv -s $(CONFIG_CBFS_SIZE) -CBFSTOOL_PRE_OPTS = -v endif diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 5829983..d552aa2 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -50,15 +50,6 @@ mbi.bin-type := mbi ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32)$(CONFIG_ARCH_ROMSTAGE_X86_64),y) CBFSTOOL_PRE1_OPTS = -m x86 -s $(CONFIG_ROM_SIZE) \ -o $$(( $(CONFIG_ROM_SIZE) - $(CONFIG_CBFS_SIZE) )) -# Use a '-a 64' option to cbfstool locate to provide a minimum alignment -# requirement for the overall romstage. While the first object within -# romstage could have a 4 byte minimum alignment that doesn't mean the linker -# won't decide the entire section should be aligned to a larger value. In the -# future cbfstool should add XIP files proper and honor the alignment -# requirements of the program segment. -# -# Make sure that segment for .car.data is ignored while adding romstage. -CBFSTOOL_PRE_OPTS = -a 64 --xip -S ".car.data" -P $(CONFIG_XIP_ROM_SIZE) endif ## Calculate the base address of CBFS for later comparisons @@ -181,9 +172,6 @@ else $(eval $(call early_x86_stage,verstage,elf64-x86-64)) endif -# Verstage on x86 expected to be xip. -CBFSTOOL_VERSTAGE_OPTS = -a 64 --xip -S ".car.data" - endif # CONFIG_ARCH_VERSTAGE_X86_32 / CONFIG_ARCH_VERSTAGE_X86_64 ############################################################################### diff --git a/src/vendorcode/google/chromeos/vboot2/Makefile.inc b/src/vendorcode/google/chromeos/vboot2/Makefile.inc index 8668405..fdb0d4e 100644 --- a/src/vendorcode/google/chromeos/vboot2/Makefile.inc +++ b/src/vendorcode/google/chromeos/vboot2/Makefile.inc @@ -69,7 +69,15 @@ $(VB2_LIB): $(obj)/config.h libverstage-srcs += $(VB2_LIB) ifeq ($(CONFIG_SEPARATE_VERSTAGE),y) -VERSTAGE_FILE = $(objcbfs)/verstage.elf +cbfs-files-$(CONFIG_SEPARATE_VERSTAGE) += $(CONFIG_CBFS_PREFIX)/verstage +$(CONFIG_CBFS_PREFIX)/verstage-file := $(objcbfs)/verstage.elf +$(CONFIG_CBFS_PREFIX)/verstage-type := stage +# Verstage on x86 expected to be xip. +ifeq ($(CONFIG_ARCH_VERSTAGE_X86_32)$(CONFIG_ARCH_VERSTAGE_X86_64),y) +$(CONFIG_CBFS_PREFIX)/verstage-options := -a 64 --xip -S ".car.data" + +endif + else ifeq ($(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK),y) bootblock-srcs += $(objgenerated)/libverstage.a

1 0

Patch set updated for coreboot: build system: Allow giving additional cbfstool options to cbfs-files-y
by Patrick Georgi 30 Nov '15

30 Nov '15

Patrick Georgi (pgeorgi(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12547 -gerrit commit 95e53472009f2b2ff6da7787f17f9d80c0d134ec Author: Patrick Georgi <pgeorgi(a)chromium.org> Date: Thu Nov 26 16:37:45 2015 +0100 build system: Allow giving additional cbfstool options to cbfs-files-y This enables adding romstage, verstage, and payload, that may need additional options (eg. for XIP or for linux initrd arguments) to be added with the build system infrastructure instead of manual rules. Change-Id: Ifde4ec3ca4ab436aca9b51a3c2cc478ed493fbfb Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> --- Makefile.inc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index 4eebc71..09cd6cc 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -265,12 +265,13 @@ cbfs-files-handler= \ $(eval tmp-cbfs-file:=$(shell mkdir -p $(obj)/mainboard/$(MAINBOARDDIR); mktemp $(obj)/mainboard/$(MAINBOARDDIR)/cbfs-file.XXXXXX).out) \ $(call cbfs-files-processor-$(tmp-cbfs-method),$(tmp-old-cbfs-file),$(tmp-cbfs-file))) \ $(if $(tmp-cbfs-file), \ - $(eval cbfs-files-$(_cbfs-bucket) += $(tmp-cbfs-file)|$(2)|$($(2)-type)|$($(2)-compression)|$(strip $($(2)-position))|$($(2)-align))) \ + $(eval cbfs-files-$(_cbfs-bucket) += $(tmp-cbfs-file)|$(2)|$($(2)-type)|$($(2)-compression)|$(strip $($(2)-position))|$($(2)-align)|$($(2)-options))) \ $(eval $(2)-name:=) \ $(eval $(2)-type:=) \ $(eval $(2)-compression:=) \ $(eval $(2)-position:=) \ $(eval $(2)-required:=) \ + $(eval $(2)-options:=) \ $(eval $(2)-align:=) ####################################################################### @@ -601,7 +602,8 @@ cbfs-add-cmd = \ -f $(call extract_nth,1,$(file)) \ -n $(call extract_nth,2,$(file)) \ $(if $(filter-out stage,$(call extract_nth,3,$(file))),-t $(call extract_nth,3,$(file))) \ - $(if $(call extract_nth,4,$(file)),-c $(call extract_nth,4,$(file))) + $(if $(call extract_nth,4,$(file)),-c $(call extract_nth,4,$(file))) \ + $(call extract_nth,7,$(file)) cbfs-files=$(cbfs-files-fixed) $(cbfs-files-aligned) $(cbfs-files-regular) ifneq ($(CONFIG_UPDATE_IMAGE),y)

1 0

New patch to review for coreboot: [NOTFORMERGE] (Yet) device: Do not attempt to assign resources to large BARs
by Ben Frisch 30 Nov '15

30 Nov '15

Ben Frisch (bfrisch(a)xes-inc.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12575 -gerrit commit 077a94301f6ac8a87b8e1a9ab93e6b9486c9f583 Author: Benjamin Frisch <bfrisch(a)xes-inc.com> Date: Mon Nov 30 12:23:31 2015 -0600 [NOTFORMERGE] (Yet) device: Do not attempt to assign resources to large BARs Per the PCI Firmware Specification 3.1 and Firmware Allocation of PCI Device Resources in Windows document, firmware shall not allocate resources to devices that cannot fit in available 32-bit MMIO space. It is then assumed that 64-bit operating systems can rebalance resources as necessary (eg. adding 64-bit MMIO space in ACPI and possibly adding the pci=realloc kernel argument in in Linux). This implmentation is inspired by the error handling in the UEFI tianocore.org PCI Enumeration. If a BAR is found that cannot be satisfied, devices are disabled and skipped in the following order to attempt to make room to satisfy the remaining BAR MMIO requests: 1) Disabling the device with the largest 64-bit BARs (without OPROMs) 2) Disabling the device with the largest 32-bit BARs (without OPROMs) 3) Disabling the devices with the largest space request (with OPROMs) After the device is disabled, PCI enumeration is then repeated with the disabled device skipped. This process continues until PCI emeration completes successfully. TEST=Tested on internal coreboot port based on older upstream with device presenting 128 GB BAR with latest Linux kernel, additional testing help and feedback is appreciated. Change-Id: I0217d627ca77431075df5e24d49d6406c35893b7 Signed-off-by: Ben Frisch <bfrisch(a)xes-inc.com> --- src/device/Kconfig | 46 ++++++++++++++++ src/device/device.c | 124 ++++++++++++++++++++++++++++++++------------ src/device/device_util.c | 110 +++++++++++++++++++++++++++++++++++++++ src/device/pci_device.c | 64 ++++++++++++++++------- src/include/device/device.h | 4 ++ 5 files changed, 296 insertions(+), 52 deletions(-) diff --git a/src/device/Kconfig b/src/device/Kconfig index 0113545..b9a734d 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -389,6 +389,52 @@ config SOFTWARE_I2C I2C controller is not (yet) available. The platform code needs to provide bindings to manually toggle I2C lines. +choice SET_MAX_MEM_BAR_SIZE + prompt "Set Maximum memory BAR size to try to allocate " + optional + help + Select the maximum size PCI device memory BAR that coreboot will try + to allocate. If a BAR is found with a values above this size, coreboot + will disable the device and will not try to allocate any BARs. + + If unsure, leave this disabled, and coreboot will find and disable any + devices that are too large to be allocated by itself. The + disadvantage is a slight boot time increase if a device has to be + disabled. + +config MAX_BAR_SIZE_256MB + bool "256 MB" + help + Set the maximum size PCI device memory BAR that coreboot will try to + allocate to 256 MB. If a BAR is found with a values above this size, + coreboot will disable the device and will not try to allocate any + BARs. + +config MAX_BAR_SIZE_512MB + bool "512 MB" + help + Set the maximum size PCI device memory BAR that coreboot will try to + allocate to 512 MB. If a BAR is found with a values above this size, + coreboot will disable the device and will not try to allocate any + BARs. + +config MAX_BAR_SIZE_1GB + bool "1 GB" + help + Set the maximum size PCI device memory BAR that coreboot will try to + allocate to 1 GB. If a BAR is found with a values above this size, + coreboot will disable the device and will not try to allocate any + BARs. + +endchoice + +config MAX_MEM_BAR_SIZE_TO_ALLOCATE + hex + default 0x10000000 if MAX_BAR_SIZE_256MB + default 0x20000000 if MAX_BAR_SIZE_512MB + default 0x40000000 if MAX_BAR_SIZE_1GB + default 0 + endmenu menu "Display" diff --git a/src/device/device.c b/src/device/device.c index e23c9de..80d32f2 100644 --- a/src/device/device.c +++ b/src/device/device.c @@ -690,7 +690,7 @@ static void constrain_resources(struct device *dev, struct constraints* limits) } } -static void avoid_fixed_resources(struct device *dev) +static u8 avoid_fixed_resources(struct device *dev) { struct constraints limits; struct resource *res; @@ -743,9 +743,60 @@ static void avoid_fixed_resources(struct device *dev) if (res->flags & IORESOURCE_MEM) res->base = resource_max(res); + if (res->base < lim->base) { + /* This usually happens at the domain level, usually + * caused by a device having a BAR which is too big. + * Error out so we can retry. + */ + printk(BIOS_ERR, + "ERROR: Resource Base was lower than its limit!\n\t"); + printk(BIOS_ERR, + "%s@%02lx: base value 0x%08llx, lower limit 0x%08llx\n", + dev_path(dev), res->index, res->base, lim->base); + return 1; + } + printk(BIOS_SPEW, "%s:@%s %02lx base %08llx limit %08llx\n", __func__, dev_path(dev), res->index, res->base, res->limit); } + + return 0; +} + +static u8 avoid_all_fixed_resources(struct device *root) +{ + struct device *child; + + /* For all domains. */ + for (child = root->link_list->children; child; child = child->sibling) { + if (child->path.type == DEVICE_PATH_DOMAIN) { + if (avoid_fixed_resources(child)) { + printk( + BIOS_ERR, + "Error trying to avoid fixed resources.") + + /* Now find the device which is the largest + * and disable it. + */ + struct device *largest = find_pci_dev_max_mem(); + + printk(BIOS_SPEW, "\tDisabling Device %s\n", + dev_path(largest)); + if (largest) + largest->enabled = 0; + + /* Clear out all devices on all domains, ready + * for re-scanning. + */ + printk(BIOS_SPEW, + "\tClearing old resource computations.\n"); + clear_computed_resources(); + return 1; + } + } + } + + return 0; } device_t vga_pri = 0; @@ -1022,45 +1073,54 @@ void dev_configure(void) root = &dev_root; - /* - * Each domain should create resources which contain the entire address - * space for IO, MEM, and PREFMEM resources in the domain. The - * allocation of device resources will be done from this address space. - */ + do { + /* + * Each domain should create resources which contain the entire + * address space for IO, MEM, and PREFMEM resources in the + * domain. The allocation of device resources will be done from + * this address space. + */ - /* Read the resources for the entire tree. */ + /* Read the resources for the entire tree. */ - printk(BIOS_INFO, "Reading resources...\n"); - read_resources(root->link_list); - printk(BIOS_INFO, "Done reading resources.\n"); + printk(BIOS_INFO, "Reading resources...\n"); + read_resources(root->link_list); + printk(BIOS_INFO, "Done reading resources.\n"); - print_resource_tree(root, BIOS_SPEW, "After reading."); + print_resource_tree(root, BIOS_SPEW, "After reading."); - /* Compute resources for all domains. */ - for (child = root->link_list->children; child; child = child->sibling) { - if (!(child->path.type == DEVICE_PATH_DOMAIN)) - continue; - post_log_path(child); - for (res = child->resource_list; res; res = res->next) { - if (res->flags & IORESOURCE_FIXED) - continue; - if (res->flags & IORESOURCE_MEM) { - compute_resources(child->link_list, - res, IORESOURCE_TYPE_MASK, IORESOURCE_MEM); - continue; - } - if (res->flags & IORESOURCE_IO) { - compute_resources(child->link_list, - res, IORESOURCE_TYPE_MASK, IORESOURCE_IO); + /* Compute resources for all domains. */ + for (child = root->link_list->children; + child; + child = child->sibling) { + if (!(child->path.type == DEVICE_PATH_DOMAIN)) continue; + post_log_path(child); + for (res = child->resource_list; res; res = res->next) { + if (res->flags & IORESOURCE_FIXED) + continue; + if (res->flags & IORESOURCE_MEM) { + compute_resources( + child->link_list, + res, + IORESOURCE_TYPE_MASK, + IORESOURCE_MEM); + continue; + } + if (res->flags & IORESOURCE_IO) { + compute_resources(child->link_list, + res, + IORESOURCE_TYPE_MASK, + IORESOURCE_IO); + continue; + } } } - } - /* For all domains. */ - for (child = root->link_list->children; child; child=child->sibling) - if (child->path.type == DEVICE_PATH_DOMAIN) - avoid_fixed_resources(child); + /* Attempt to avoid all fixed resources, if this process + * succeeds then we can carry on, otherwise we should re-try. + */ + } while (avoid_all_fixed_resources(root)); /* Store the computed resource allocations into device registers ... */ printk(BIOS_INFO, "Setting resources...\n"); diff --git a/src/device/device_util.c b/src/device/device_util.c index ac18538..be08628 100644 --- a/src/device/device_util.c +++ b/src/device/device_util.c @@ -921,3 +921,113 @@ int dev_count_cpu(void) return count; } + +enum allocation_passes { + PASS_1_CHECK_64_BIT_BARS = 0, + PASS_2_CHECK_32_BIT_BARS = 1, + PASS_3_CHECK_DEVS_WITH_OPROMS = 2, + MAX_PASS = PASS_3_CHECK_DEVS_WITH_OPROMS +}; + +/** @brief finds the enabled, non root-bus, pci device using the most memory. + * + * @return pointer to the device + */ +struct device *find_pci_dev_max_mem(void) +{ + struct device *dev; + struct device *largest_dev = NULL; + struct resource *res; + uint64_t memory_used = 0; + uint64_t most_memory_used = 0; + uint8_t pass; + + for (pass = PASS_1_CHECK_64_BIT_BARS; pass <= MAX_PASS; pass++) { + + /* + * Loop through all pci devices that are not on bus 0, + * skipping them if they're disabled, checking to see + * which uses the most memory. + */ + + for (dev = &dev_root; dev; dev = dev->next) { + + /* Skip disabled, root bus, and non-pci devices */ + if ((!(dev->path.type == DEVICE_PATH_PCI)) || + (dev->bus->secondary == 0) || + (dev->enabled == 0)) + continue; + + /* Total the memory used by the device */ + memory_used = 0; + for (res = dev->resource_list; res; res = res->next) { + + /* don't check 32-bit bars on the first pass */ + if (pass == PASS_1_CHECK_64_BIT_BARS && + ((res->flags & IORESOURCE_PCI64) == 0)) + continue; + + /* skip devices with option roms until last */ + if ((pass < PASS_3_CHECK_DEVS_WITH_OPROMS) + && (res->index == PCI_ROM_ADDRESS)) { + memory_used = 0; + break; + } + + if (res->flags & IORESOURCE_MEM) + memory_used += res->size; + } + + /* Save the largest device found so far */ + if (memory_used > most_memory_used) { + most_memory_used = memory_used; + largest_dev = dev; + } + } + + /* break out of the pass loop when we find a device */ + if (largest_dev != NULL) + break; + } + + return largest_dev; +} + +/** @brief clear all non-pnp resources. + * + */ +void clear_computed_resources(void) +{ + struct device *dev; + + /* + * Loop through all non-pnp devices, clearing everything except index + * and the pointer to the next resource. This keeps them from having + * to be re-allocated. + */ + for (dev = &dev_root; dev; dev = dev->next) { + + //skip pnp devices - their resources are set early + if (dev->path.type == DEVICE_PATH_PNP) + continue; + + clear_device_resources(dev); + } +} + +/** @brief clear all resources for a specified device. Leave index and next. + * + */ +void clear_device_resources(struct device *dev) +{ + struct resource *res; + + for (res = dev->resource_list; res; res = res->next) { + res->align = 0; + res->base = 0; + res->flags = 0; + res->gran = 0; + res->limit = 0; + res->size = 0; + } +} diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 5123229..2d8e3c5 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -248,27 +248,49 @@ struct resource *pci_get_resource(struct device *dev, unsigned long index) resource->limit = 0xffff; } else { /* A Memory mapped base address. */ - attr &= PCI_BASE_ADDRESS_MEM_ATTR_MASK; - resource->flags |= IORESOURCE_MEM; - if (attr & PCI_BASE_ADDRESS_MEM_PREFETCH) - resource->flags |= IORESOURCE_PREFETCH; - attr &= PCI_BASE_ADDRESS_MEM_LIMIT_MASK; - if (attr == PCI_BASE_ADDRESS_MEM_LIMIT_32) { - /* 32bit limit. */ - resource->limit = 0xffffffffUL; - } else if (attr == PCI_BASE_ADDRESS_MEM_LIMIT_1M) { - /* 1MB limit. */ - resource->limit = 0x000fffffUL; - } else if (attr == PCI_BASE_ADDRESS_MEM_LIMIT_64) { - /* 64bit limit. */ - resource->limit = 0xffffffffffffffffULL; - resource->flags |= IORESOURCE_PCI64; + if (CONFIG_MAX_MEM_BAR_SIZE_TO_ALLOCATE && + (resource->size > CONFIG_MAX_MEM_BAR_SIZE_TO_ALLOCATE)) { + /* Disable devices that attempt to allocate a BAR beyond + the max configured size. */ + printk(BIOS_ERR, + "Device at %s requested a 0x%llx byte BAR ", + dev_path(dev), + (unsigned long long)resource->size); + printk(BIOS_ERROR, + "for register 0x%02lx.\n", + index); + printk(BIOS_ERR, + "The device exceeded largest allowed size of 0x%lx. ", + (unsigned long) + CONFIG_MAX_MEM_BAR_SIZE_TO_ALLOCATE); + printk(BIOS_ERR, "So, the device was disabled."); + dev->enabled = 0; + clear_device_resources(dev); } else { - /* Invalid value. */ - printk(BIOS_ERR, "Broken BAR with value %lx\n", attr); - printk(BIOS_ERR, " on dev %s at index %02lx\n", - dev_path(dev), index); - resource->flags = 0; + attr &= PCI_BASE_ADDRESS_MEM_ATTR_MASK; + resource->flags |= IORESOURCE_MEM; + if (attr & PCI_BASE_ADDRESS_MEM_PREFETCH) + resource->flags |= IORESOURCE_PREFETCH; + attr &= PCI_BASE_ADDRESS_MEM_LIMIT_MASK; + if (attr == PCI_BASE_ADDRESS_MEM_LIMIT_32) { + /* 32bit limit. */ + resource->limit = 0xffffffffUL; + } else if (attr == PCI_BASE_ADDRESS_MEM_LIMIT_1M) { + /* 1MB limit. */ + resource->limit = 0x000fffffUL; + } else if (attr == PCI_BASE_ADDRESS_MEM_LIMIT_64) { + /* 64bit limit. */ + resource->limit = 0xffffffffffffffffULL; + resource->flags |= IORESOURCE_PCI64; + } else { + /* Invalid value. */ + printk(BIOS_ERR, + "Broken BAR with value %lx\n", + attr); + printk(BIOS_ERR, " on dev %s at index %02lx\n", + dev_path(dev), index); + resource->flags = 0; + } } } @@ -344,6 +366,8 @@ static void pci_read_bases(struct device *dev, unsigned int howmany) (index < PCI_BASE_ADDRESS_0 + (howmany << 2));) { struct resource *resource; resource = pci_get_resource(dev, index); + if (dev->enabled == 0) + return; index += (resource->flags & IORESOURCE_PCI64) ? 8 : 4; } diff --git a/src/include/device/device.h b/src/include/device/device.h index 62460ae..c857dd2 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -234,6 +234,10 @@ void fixed_mem_resource(device_t dev, unsigned long index, void scan_smbus(device_t bus); void scan_lpc_bus(device_t bus); +struct device *find_pci_dev_max_mem(void); +void clear_computed_resources(void); +void clear_device_resources(struct device *dev); + /* It is the caller's responsibility to adjust regions such that ram_resource() * and mmio_resource() do not overlap. */

1 0

Patch merged into coreboot/master: nb/amd/amdmct/mct_ddr3: Use StopOnError to decrease training time
by gerrit＠coreboot.org 30 Nov '15

30 Nov '15

the following patch was just integrated into master: commit 309303884cc755f94aba731ad1821f49a3ea3899 Author: Timothy Pearson <tpearson(a)raptorengineeringinc.com> Date: Thu Sep 3 18:59:53 2015 -0500 nb/amd/amdmct/mct_ddr3: Use StopOnError to decrease training time There is no need to continue testing a DCT configuration after data errors have already been detected; this just wastes time during boot. Change-Id: I979e27c32a3e0b101590fba0de3d7a25d6fc44d2 Signed-off-by: Timothy Pearson <tpearson(a)raptorengineeringinc.com> Reviewed-on: https://review.coreboot.org/12066 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/12066 for details. -gerrit

1 0

Patch merged into coreboot/master: mainboard/intel: Add Little Plains
by gerrit＠coreboot.org 30 Nov '15

30 Nov '15

the following patch was just integrated into master: commit 9586dc72dbd61b56975dd4c24793ef1cdc2d012b Author: Marcin Wojciechowski <marcin.wojciechowski(a)intel.com> Date: Fri Nov 20 14:53:46 2015 +0100 mainboard/intel: Add Little Plains This adds a new mainboard: Little Plains for Intel's atom c2000 It was based on Mohon Peak board with some minor changes This board is not available as standalone product It is a managment board for Intel Ethernet Multi-host Controller FM10000 Series The FSP package is available from Intel: https://www.intel.com/fsp Change-Id: I28127a858106ed35d26e235f0c6393c20ed14350 Signed-off-by: Marcin Wojciechowski <marcin.wojciechowski(a)intel.com> Reviewed-on: https://review.coreboot.org/12503 Tested-by: build bot (Jenkins) Reviewed-by: Martin Roth <martinroth(a)google.com> See https://review.coreboot.org/12503 for details. -gerrit

1 0

New patch to review for coreboot: arch/x86/bootblock_normal: Update to use fewer registers
by Martin Roth 30 Nov '15

30 Nov '15

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12574 -gerrit commit 21befcebf1d62a940a6adebe4f1d5b27c9b3f128 Author: Martin Roth <martinroth(a)google.com> Date: Mon Nov 30 09:49:21 2015 -0700 arch/x86/bootblock_normal: Update to use fewer registers - Move initialization of entry to later in main. - Make boot_mode an unsigned char - no need to use int. - Remove unnecessary variable filenames. - Only get and try to boot fallback once. Change-Id: I823092c60dd8c2de0a36ec7fdbba3e68f6b7567a Test: compiled. Signed-off-by: Martin Roth <martinroth(a)google.com> --- src/arch/x86/bootblock_normal.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/arch/x86/bootblock_normal.c b/src/arch/x86/bootblock_normal.c index d5f03b7..a6a877c 100644 --- a/src/arch/x86/bootblock_normal.c +++ b/src/arch/x86/bootblock_normal.c @@ -10,9 +10,9 @@ static const char *get_fallback(const char *stagelist) { static void main(unsigned long bist) { - unsigned long entry; - int boot_mode; - const char *default_filenames = "normal/romstage\0fallback/romstage"; + u8 boot_mode; + const char *default_filenames = + "normal/romstage\0fallback/romstage"; if (boot_cpu()) { bootblock_mainboard_init(); @@ -30,22 +30,22 @@ static void main(unsigned long bist) boot_mode = boot_use_normal(cmos_read(RTC_BOOT_BYTE)); } - char *filenames = (char *)walkcbfs("coreboot-stages"); - if (!filenames) { - filenames = default_filenames; - } - char *normal_candidate = filenames; + char *normal_candidate = (char *)walkcbfs("coreboot-stages"); - if (boot_mode) - entry = findstage(normal_candidate); - else - entry = findstage(get_fallback(normal_candidate)); + if (!normal_candidate) + normal_candidate = default_filenames; - if (entry) call(entry, bist); + unsigned long entry; + + if (boot_mode) { + entry = findstage(normal_candidate); + if (entry) + call(entry, bist); + } - /* run fallback if normal can't be found */ entry = findstage(get_fallback(normal_candidate)); - if (entry) call(entry, bist); + if (entry) + call(entry, bist); /* duh. we're stuck */ halt();

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2015