Change in coreboot[master]: build system: Structure and serialize INTERMEDIATE - coreboot-gerrit

12 Jan 2021

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/49358 )
Change subject: build system: Structure and serialize INTERMEDIATE
......................................................................
build system: Structure and serialize INTERMEDIATE
Target added to INTERMEDIATE all operate on coreboot.pre, each modifying
the file in some way. When running them in parallel, coreboot.pre can be
read from and written to in parallel which can corrupt the result.
Add a function to create those rules that also adds existing
INTERMEDIATE targets to enforce an order (as established by evaluation
order of Makefile.inc files).
While at it, also add the addition to the PHONY target so we don't
forget it.
BUG=chromium:1154313, b:174585424
TEST=Built a configuration with SeaBIOS + SeaBIOS config files (ps2
timeout and sercon) and saw that they were executed.
Change-Id: Ia5803806e6c33083dfe5dec8904a65c46436e756
Signed-off-by: Patrick Georgi pgeorgi@google.com
---
M Makefile.inc
M payloads/external/Makefile.inc
M src/cpu/intel/fit/Makefile.inc
M src/ec/hp/kbc1126/Makefile.inc
M src/mainboard/amd/majolica/Makefile.inc
M src/mainboard/amd/mandolin/Makefile.inc
M src/security/intel/cbnt/Makefile.inc
M src/security/intel/txt/Makefile.inc
M src/soc/amd/stoneyridge/Makefile.inc
M src/soc/intel/common/block/fast_spi/Makefile.inc
M src/southbridge/amd/pi/hudson/Makefile.inc
M src/southbridge/intel/common/firmware/Makefile.inc
12 files changed, 22 insertions(+), 57 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/49358/1

diff --git a/Makefile.inc b/Makefile.inc
index ae61c2e..058764d 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -1147,6 +1147,10 @@
 RAMSTAGE=
 endif
+add_intermediate = \
+	$(1): $(2) | $(INTERMEDIATE) \
+	$(eval INTERMEDIATE+=$(1)) $(eval PHONY+=$(1))
+
 $(obj)/coreboot.rom: $(obj)/coreboot.pre $(RAMSTAGE) $(CBFSTOOL) $$(INTERMEDIATE)
    @printf "    CBFS       $(subst $(obj)/,,$(@))\n"
 # The full ROM may be larger than the CBFS part, so create an empty
@@ -1250,7 +1254,7 @@
 ramstage-symbol-addr-cmd = $(OBJDUMP_ramstage) -t $(objcbfs)/ramstage.elf | \
    sed -n '/ $(1)$$/s/^([0-9a-fA-F]*) .*/0x\1/p'
-check-ramstage-overlaps: $(obj)/coreboot.pre
+$(call add_intermediate, check-ramstage-overlaps, $(obj)/coreboot.pre)
    programs=$$($(foreach file,$(check-ramstage-overlap-files), \
    	$(call cbfs-get-segments-cmd,$(file)) ; )) ; \
    regions=$$($(foreach region,$(check-ramstage-overlap-regions), \
@@ -1276,6 +1280,4 @@
        pstart= ; pend= ; \
    done
-INTERMEDIATE+=check-ramstage-overlaps
-PHONY+=check-ramstage-overlaps
 endif
diff --git a/payloads/external/Makefile.inc b/payloads/external/Makefile.inc
index ec0905e..9c1a569 100644
--- a/payloads/external/Makefile.inc
+++ b/payloads/external/Makefile.inc
@@ -101,9 +101,7 @@
ifneq ($(CONFIG_SEABIOS_PS2_TIMEOUT),)
 ifneq ($(CONFIG_SEABIOS_PS2_TIMEOUT),0)
-PHONY+=seabios_ps2_timeout
-INTERMEDIATE+=seabios_ps2_timeout
-seabios_ps2_timeout: $(obj)/coreboot.pre $(CBFSTOOL)
+$(call add_intermediate, seabios_ps2_timeout, $(obj)/coreboot.pre $(CBFSTOOL))
    @printf "    SeaBIOS    Wait up to $(CONFIG_SEABIOS_PS2_TIMEOUT) ms for PS/2 keyboard controller initialization\n"
    $(if $(CONFIG_UPDATE_IMAGE),-$(CBFSTOOL) $< remove -n etc/ps2-keyboard-spinup 2>/dev/null)
    $(CBFSTOOL) $< add-int -i $(CONFIG_SEABIOS_PS2_TIMEOUT) -n etc/ps2-keyboard-spinup
@@ -111,18 +109,14 @@
 endif
ifeq ($(CONFIG_SEABIOS_ADD_SERCON_PORT_FILE),y)
-PHONY+=seabios_sercon
-INTERMEDIATE+=seabios_sercon
-seabios_sercon: $(obj)/coreboot.pre $(CBFSTOOL)
+$(call add_intermediate, seabios_sercon, $(obj)/coreboot.pre $(CBFSTOOL))
    @printf "    SeaBIOS    Add sercon-port file\n"
    $(if $(CONFIG_UPDATE_IMAGE),-$(CBFSTOOL) $< remove -n etc/sercon-port 2>/dev/null)
    $(CBFSTOOL) $< add-int -i $(CONFIG_SEABIOS_SERCON_PORT_ADDR) -n etc/sercon-port
 endif
ifeq ($(CONFIG_SEABIOS_THREAD_OPTIONROMS),y)
-PHONY+=seabios_thread_optionroms
-INTERMEDIATE+=seabios_thread_optionroms
-seabios_thread_optionroms: $(obj)/coreboot.pre $(CBFSTOOL)
+$(call add_intermediate, seabios_thread_optionroms, $(obj)/coreboot.pre $(CBFSTOOL))
    @printf "    SeaBIOS    Thread optionroms\n"
    $(if $(CONFIG_UPDATE_IMAGE),-$(CBFSTOOL) $< remove -n etc/threads 2>/dev/null)
    $(CBFSTOOL) $< add-int -i 2 -n etc/threads
diff --git a/src/cpu/intel/fit/Makefile.inc b/src/cpu/intel/fit/Makefile.inc
index 20483d8..8d8f07d7 100644
--- a/src/cpu/intel/fit/Makefile.inc
+++ b/src/cpu/intel/fit/Makefile.inc
@@ -6,18 +6,14 @@
ifneq ($(CONFIG_CPU_MICROCODE_CBFS_NONE),y)
-PHONY+=add_mcu_fit
-INTERMEDIATE+=add_mcu_fit
-add_mcu_fit: $(obj)/coreboot.pre $(IFITTOOL)
+$(call add_intermediate, add_mcu_fit, $(obj)/coreboot.pre $(IFITTOOL))
    @printf "    UPDATE-FIT Microcode\n"
    $(IFITTOOL) -f $< -a -n cpu_microcode_blob.bin -t 1 -s $(CONFIG_CPU_INTEL_NUM_FIT_ENTRIES) -r COREBOOT
# Second FIT in TOP_SWAP bootblock
 ifeq ($(CONFIG_INTEL_ADD_TOP_SWAP_BOOTBLOCK),y)
-PHONY+=add_ts_mcu_fit
-INTERMEDIATE+=add_ts_mcu_fit
-add_ts_mcu_fit: $(obj)/coreboot.pre $(IFITTOOL)
+$(call add_intermediate, add_ts_mcu_fit, $(obj)/coreboot.pre $(IFITTOOL))
    @printf "    UPDATE-FIT Top Swap: Microcode\n"
 ifneq ($(FIT_ENTRY),)
    $(IFITTOOL) -f $< -A -n $(FIT_ENTRY) -t 1 -s $(CONFIG_CPU_INTEL_NUM_FIT_ENTRIES) $(TS_OPTIONS) -r COREBOOT
diff --git a/src/ec/hp/kbc1126/Makefile.inc b/src/ec/hp/kbc1126/Makefile.inc
index d767bad..92a0a72 100644
--- a/src/ec/hp/kbc1126/Makefile.inc
+++ b/src/ec/hp/kbc1126/Makefile.inc
@@ -2,7 +2,6 @@
ifeq ($(CONFIG_EC_HP_KBC1126_ECFW_IN_CBFS),y)
 KBC1126_EC_INSERT:=$(top)/util/kbc1126/kbc1126_ec_insert
-INTERMEDIATE+=kbc1126_ec_insert
ifeq ($(CONFIG_KBC1126_FIRMWARE),y)
 cbfs-files-y += ecfw1.bin
@@ -17,7 +16,7 @@
 ecfw2.bin-type := raw
 endif
-kbc1126_ec_insert: $(obj)/coreboot.pre
+$(call add_intermediate, kbc1126_ec_insert, $(obj)/coreboot.pre)
 ifeq ($(CONFIG_KBC1126_FIRMWARE),y)
    printf "    Building kbc1126_ec_insert.\n"
    $(MAKE) -C util/kbc1126
@@ -26,8 +25,6 @@
    	$(CONFIG_KBC1126_FW1_OFFSET) $(CONFIG_KBC1126_FW2_OFFSET)
 endif
-PHONY+=kbc1126_ec_insert
-
 build_complete::
 ifeq ($(CONFIG_KBC1126_FIRMWARE),)
    printf "\n** WARNING **\n"
diff --git a/src/mainboard/amd/majolica/Makefile.inc b/src/mainboard/amd/majolica/Makefile.inc
index ce3119c..7472599 100644
--- a/src/mainboard/amd/majolica/Makefile.inc
+++ b/src/mainboard/amd/majolica/Makefile.inc
@@ -1,10 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
-PHONY+=add_mchp_fw
-INTERMEDIATE+=add_mchp_fw
-
 ifeq ($(CONFIG_MAJOLICA_HAVE_MCHP_FW),y)
-add_mchp_fw: $(obj)/coreboot.pre
+$(call add_intermediate, add_mchp_fw, $(obj)/coreboot.pre)
    $(CBFSTOOL) $(obj)/coreboot.pre write -r EC -f $(CONFIG_MAJOLICA_MCHP_FW_FILE) --fill-upward
 else
 files_added:: warn_no_mchp
diff --git a/src/mainboard/amd/mandolin/Makefile.inc b/src/mainboard/amd/mandolin/Makefile.inc
index 448c12a..6644bb3 100644
--- a/src/mainboard/amd/mandolin/Makefile.inc
+++ b/src/mainboard/amd/mandolin/Makefile.inc
@@ -16,12 +16,9 @@
 APCB_SOURCES = $(MAINBOARD_BLOBS_DIR)/APCB_cereme.bin
 endif
-PHONY+=add_mchp_fw
-INTERMEDIATE+=add_mchp_fw
-
 ifeq ($(CONFIG_MANDOLIN_HAVE_MCHP_FW),y)
-add_mchp_fw: $(obj)/coreboot.pre
+$(call add_intermediate, add_mchp_fw, $(obj)/coreboot.pre)
    $(CBFSTOOL) $(obj)/coreboot.pre write -r EC -f $(CONFIG_MANDOLIN_MCHP_FW_FILE) --fill-upward
else
diff --git a/src/security/intel/cbnt/Makefile.inc b/src/security/intel/cbnt/Makefile.inc
index 349d3a9..06ff213 100644
--- a/src/security/intel/cbnt/Makefile.inc
+++ b/src/security/intel/cbnt/Makefile.inc
@@ -6,9 +6,7 @@
 boot_policy_manifest.bin-type := raw
 boot_policy_manifest.bin-align := 0x10
-PHONY+=add_bpm_fit
-INTERMEDIATE+=add_bpm_fit
-add_bpm_fit: $(obj)/coreboot.pre $(IFITTOOL)
+$(call add_intermediate, add_bpm_fit, $(obj)/coreboot.pre $(IFITTOOL))
    $(IFITTOOL) -r COREBOOT -a -n boot_policy_manifest.bin -t 12 -s $(CONFIG_CPU_INTEL_NUM_FIT_ENTRIES) -f $<
 endif
@@ -18,9 +16,7 @@
 key_manifest.bin-type := raw
 key_manifest.bin-align := 0x10
-PHONY+=add_km_fit
-INTERMEDIATE+=add_km_fit
-add_km_fit: $(obj)/coreboot.pre $(IFITTOOL)
+$(call add_intermediate, add_km_fit, $(obj)/coreboot.pre $(IFITTOOL))
    $(IFITTOOL) -r COREBOOT -a -n key_manifest.bin -t 11 -s $(CONFIG_CPU_INTEL_NUM_FIT_ENTRIES) -f $<
 endif
diff --git a/src/security/intel/txt/Makefile.inc b/src/security/intel/txt/Makefile.inc
index dee4949..a0d37fb 100644
--- a/src/security/intel/txt/Makefile.inc
+++ b/src/security/intel/txt/Makefile.inc
@@ -28,9 +28,7 @@
ifeq ($(CONFIG_CPU_INTEL_FIRMWARE_INTERFACE_TABLE),y)
-PHONY+=add_acm_fit
-INTERMEDIATE+=add_acm_fit
-add_acm_fit: $(obj)/coreboot.pre $(IFITTOOL)
+$(call add_intermediate, add_acm_fit, $(obj)/coreboot.pre $(IFITTOOL))
    $(IFITTOOL) -r COREBOOT -a -n $(CONFIG_INTEL_TXT_CBFS_BIOS_ACM) -t 2 \
         	-s $(CONFIG_CPU_INTEL_NUM_FIT_ENTRIES) -f $<
@@ -43,9 +41,7 @@
ibb-files += bootblock
-PHONY+=add_ibb_fit
-INTERMEDIATE+=add_ibb_fit
-add_ibb_fit: $(obj)/coreboot.pre $(IFITTOOL)
+$(call add_intermediate, add_ibb_fit, $(obj)/coreboot.pre $(IFITTOOL))
    $(foreach file, $(ibb-files), $(shell $(IFITTOOL) -f $< -a -n $(file) -t 7 \
                 -s $(CONFIG_CPU_INTEL_NUM_FIT_ENTRIES) -r COREBOOT)) true
diff --git a/src/soc/amd/stoneyridge/Makefile.inc b/src/soc/amd/stoneyridge/Makefile.inc
index 2ad0730..af70364 100644
--- a/src/soc/amd/stoneyridge/Makefile.inc
+++ b/src/soc/amd/stoneyridge/Makefile.inc
@@ -160,8 +160,6 @@
    	--output	$@
ifeq ($(CONFIG_AMDFW_OUTSIDE_CBFS),y)
-PHONY+=add_amdfw
-INTERMEDIATE+=add_amdfw
# Calculate firmware position inside the ROM
 STONEYRIDGE_FWM_ROM_POSITION=$(call int-add, \
@@ -169,7 +167,7 @@
    $(call int-shift-left, \
    0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000)
-add_amdfw: $(obj)/coreboot.pre $(obj)/amdfw.rom
+$(call add_intermediate, add_amdfw, $(obj)/coreboot.pre $(obj)/amdfw.rom)
    printf "    DD         Adding AMD Firmware at ROM offset 0x%x\n" \
    	"$(STONEYRIDGE_FWM_ROM_POSITION)"
    flock $< dd if=$(obj)/amdfw.rom \
diff --git a/src/soc/intel/common/block/fast_spi/Makefile.inc b/src/soc/intel/common/block/fast_spi/Makefile.inc
index d99c44f..2ee64eb 100644
--- a/src/soc/intel/common/block/fast_spi/Makefile.inc
+++ b/src/soc/intel/common/block/fast_spi/Makefile.inc
@@ -35,7 +35,7 @@
 # Check to ensure that no sections in the FMAP cross 16MiB boundary if
 # the platform supports split decode windows for BIOS region greater
 # than 16MiB.
-check-fmap-16mib-crossing: $(obj)/fmap_config.h
+$(call add_intermediate, check-fmap-16mib-crossing, $(obj)/fmap_config.h)
    	flash_offset=$$(printf "%d" $$(cat $(obj)/fmap_config.h | grep "FMAP_SECTION_FLASH_START" | awk '{print $$NF}')); \
    	for x in $$(cat $(obj)/fmap_config.h | grep "FMAP_TERMINAL_SECTIONS" | cut -d" -f2); do \
    	    start=$$(printf "%d" $$(cat $(obj)/fmap_config.h | grep "FMAP_SECTION_"$$x"_START" | awk '{print $$NF}')); \
@@ -46,9 +46,6 @@
    	done; \
    	if [ $$fail -eq 1 ]; then false; fi
-PHONY+=check-fmap-16mib-crossing
-INTERMEDIATE+=check-fmap-16mib-crossing
-
 CBFSTOOL_ADD_CMD_OPTIONS += --ext-win-base $(CONFIG_EXT_BIOS_WIN_BASE) --ext-win-size $(CONFIG_EXT_BIOS_WIN_SIZE)
endif # CONFIG_FAST_SPI_SUPPORTS_EXT_BIOS_WINDOW
diff --git a/src/southbridge/amd/pi/hudson/Makefile.inc b/src/southbridge/amd/pi/hudson/Makefile.inc
index 5eac024..5a85fb6 100644
--- a/src/southbridge/amd/pi/hudson/Makefile.inc
+++ b/src/southbridge/amd/pi/hudson/Makefile.inc
@@ -157,10 +157,7 @@
    	--output	$@
ifeq ($(CONFIG_AMDFW_OUTSIDE_CBFS),y)
-PHONY+=add_amdfw
-INTERMEDIATE+=add_amdfw
-
-add_amdfw: $(obj)/coreboot.pre $(obj)/amdfw.rom
+$(call add_intermediate, add_amdfw, $(obj)/coreboot.pre $(obj)/amdfw.rom)
    printf "    DD         Adding AMD Firmware\n"
    flock $< dd if=$(obj)/amdfw.rom \
    	of=$(obj)/coreboot.pre conv=notrunc bs=1 seek=131072 >/dev/null 2>&1
diff --git a/src/southbridge/intel/common/firmware/Makefile.inc b/src/southbridge/intel/common/firmware/Makefile.inc
index a542d7e..77f505e 100644
--- a/src/southbridge/intel/common/firmware/Makefile.inc
+++ b/src/southbridge/intel/common/firmware/Makefile.inc
@@ -7,8 +7,6 @@
 # image outside of CBFS
ifeq ($(CONFIG_HAVE_IFD_BIN),y)
-PHONY+=add_intel_firmware
-INTERMEDIATE+=add_intel_firmware
 else ifeq ($(CONFIG_INTEL_DESCRIPTOR_MODE_REQUIRED),y)
 files_added:: warn_intel_firmware
 endif
@@ -24,7 +22,7 @@
 IFDTOOL_LOCK_ME_MODE := -l
 endif
-add_intel_firmware: $(call strip_quotes,$(CONFIG_IFD_BIN_PATH))
+$(call add_intermediate, add_intel_firmware, $(call strip_quotes,$(CONFIG_IFD_BIN_PATH)))
 ifeq ($(CONFIG_HAVE_ME_BIN),y)
 add_intel_firmware: $(call strip_quotes,$(CONFIG_ME_BIN_PATH))
 endif
@@ -107,6 +105,6 @@
    printf "board's flash chip! You can use flashrom's IFD or layout\n"
    printf "parameters to flash only to the BIOS region.\n\n"
-PHONY+=add_intel_firmware warn_intel_firmware
+PHONY+=warn_intel_firmware
endif
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/49358
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia5803806e6c33083dfe5dec8904a65c46436e756
Gerrit-Change-Number: 49358
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi pgeorgi@google.com
Gerrit-MessageType: newchange



    