[coreboot-gerrit] Change in coreboot[master]: Remove libverstage as separate library and source file class

Julius Werner (Code Review) gerrit at coreboot.org
Tue Mar 28 22:19:02 CEST 2017


Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/18302 )

Change subject: Remove libverstage as separate library and source file class
......................................................................


Remove libverstage as separate library and source file class

In builds without CONFIG_VBOOT_SEPARATE_VERSTAGE, verstage files are
linked directly into the bootblock or the romstage. However, they're
still compiled with a separate "libverstage" source file class, linked
into an intermediate library and then linked into the final destination
stage.

There is no obvious benefit to doing it this way and it's unclear why it
was chosen in the first place... there are, however, obvious
disadvantages: it can result in code that is used by both libverstage
and the host stage to occur twice in the output binary. It also means
that libverstage files have their separate compiler flags that are not
necessarily aligned with the host stage, which can lead to weird effects
like <rules.h> macros not being set the way you would expect. In fact,
VBOOT_STARTS_IN_ROMSTAGE configurations are currently broken on x86
because their libverstage code that gets compiled into the romstage sets
ENV_VERSTAGE, but CAR migration code expects all ENV_VERSTAGE code to
run pre-migration.

This patch resolves these problems by removing the separate library.
There is no more difference between the 'verstage' and 'libverstage'
classes, and the source files added to them are just treated the same
way a bootblock or romstage source files in configurations where the
verstage is linked into either of these respective stages (allowing for
the normal object code deduplication and causing those files to be
compiled with the same flags as the host stage's files).

Tested this whole series by booting a Kevin, an Elm (both with and
without SEPARATE_VERSTAGE) and a Falco in normal and recovery mode.

Change-Id: I6bb84a9bf1cd54f2e02ca1f665740a9c88d88df4
Signed-off-by: Julius Werner <jwerner at chromium.org>
Reviewed-on: https://review.coreboot.org/18302
Tested-by: build bot (Jenkins)
Reviewed-by: Aaron Durbin <adurbin at chromium.org>
---
M Makefile
M Makefile.inc
M src/arch/arm/Makefile.inc
M src/arch/arm/armv4/Makefile.inc
M src/arch/arm/armv7/Makefile.inc
M src/arch/arm64/Makefile.inc
M src/arch/arm64/armv8/Makefile.inc
M src/arch/x86/Makefile.inc
M src/drivers/i2c/ww_ring/Makefile.inc
M src/drivers/intel/fsp1_1/Makefile.inc
M src/lib/Makefile.inc
M src/soc/qualcomm/ipq40xx/Makefile.inc
M src/soc/qualcomm/ipq806x/Makefile.inc
M src/soc/rockchip/rk3288/Makefile.inc
M src/vboot/Makefile.inc
M toolchain.inc
16 files changed, 37 insertions(+), 48 deletions(-)

Approvals:
  Aaron Durbin: Looks good to me, approved
  build bot (Jenkins): Verified



diff --git a/Makefile b/Makefile
index 36bb650..701a146 100644
--- a/Makefile
+++ b/Makefile
@@ -243,11 +243,15 @@
 
 # collect all object files eligible for building
 subdirs:=$(TOPLEVEL)
+postinclude-hooks :=
 $(eval $(call evaluate_subdirs))
 ifeq ($(FAILBUILD),1)
 $(error cannot continue build)
 endif
 
+# Run hooks registered by subdirectories that need to be evaluated after all files have been parsed
+$(eval $(postinclude-hooks))
+
 # Eliminate duplicate mentions of source files in a class
 $(foreach class,$(classes),$(eval $(class)-srcs:=$(sort $($(class)-srcs))))
 
diff --git a/Makefile.inc b/Makefile.inc
index 2267f32..5ab3524 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -97,7 +97,7 @@
 
 #######################################################################
 # Add source classes and their build options
-classes-y := ramstage romstage bootblock postcar smm smmstub cpu_microcode libverstage verstage
+classes-y := ramstage romstage bootblock postcar smm smmstub cpu_microcode verstage
 
 # Add dynamic classes for rmodules
 $(foreach supported_arch,$(ARCH_SUPPORTED), \
@@ -200,7 +200,6 @@
 
 ramstage-c-deps:=$$(OPTION_TABLE_H)
 romstage-c-deps:=$$(OPTION_TABLE_H)
-libverstage-c-deps:=$$(OPTION_TABLE_H)
 verstage-c-deps:=$$(OPTION_TABLE_H)
 bootblock-c-deps:=$$(OPTION_TABLE_H)
 $(foreach type,ads adb, \
@@ -527,10 +526,6 @@
 romstage-y+=$(DEVICETREE_STATIC_C)
 verstage-y+=$(DEVICETREE_STATIC_C)
 bootblock-y+=$(DEVICETREE_STATIC_C)
-
-$(objgenerated)/libverstage.a: $$(libverstage-objs)
-	rm -f $@
-	$(AR_libverstage) rcsT $@ $^
 
 #######################################################################
 # Clean up rules
diff --git a/src/arch/arm/Makefile.inc b/src/arch/arm/Makefile.inc
index 2742414..013a4dd 100644
--- a/src/arch/arm/Makefile.inc
+++ b/src/arch/arm/Makefile.inc
@@ -68,9 +68,9 @@
 
 ifeq ($(CONFIG_ARCH_VERSTAGE_ARM),y)
 
-$(objcbfs)/verstage.debug: $(objgenerated)/libverstage.a $$(verstage-objs)
+$(objcbfs)/verstage.debug: $$(verstage-objs)
 	@printf "    LINK       $(subst $(obj)/,,$(@))\n"
-	$(LD_verstage) $(LDFLAGS_verstage) -o $@ -L$(obj) -T $(call src-to-obj,verstage,src/mainboard/$(MAINBOARDDIR)/memlayout.ld) --whole-archive --start-group $(filter-out %.ld,$(verstage-objs)) $(objgenerated)/libverstage.a --end-group
+	$(LD_verstage) $(LDFLAGS_verstage) -o $@ -L$(obj) -T $(call src-to-obj,verstage,src/mainboard/$(MAINBOARDDIR)/memlayout.ld) --whole-archive --start-group $(filter-out %.ld,$(verstage-objs)) --end-group
 
 verstage-y += boot.c
 verstage-y += div0.c
diff --git a/src/arch/arm/armv4/Makefile.inc b/src/arch/arm/armv4/Makefile.inc
index 1b91961..e8e49a6 100644
--- a/src/arch/arm/armv4/Makefile.inc
+++ b/src/arch/arm/armv4/Makefile.inc
@@ -39,7 +39,6 @@
 ################################################################################
 
 ifeq ($(CONFIG_ARCH_VERSTAGE_ARMV4),y)
-libverstage-generic-ccopts += $(armv4_flags)
 verstage-generic-ccopts += $(armv4_flags)
 
 verstage-y += cache.c
diff --git a/src/arch/arm/armv7/Makefile.inc b/src/arch/arm/armv7/Makefile.inc
index d978f00..fe0b446 100644
--- a/src/arch/arm/armv7/Makefile.inc
+++ b/src/arch/arm/armv7/Makefile.inc
@@ -71,8 +71,6 @@
 ################################################################################
 
 ifeq ($(CONFIG_ARCH_VERSTAGE_ARMV7),y)
-libverstage-generic-ccopts += $(armv7-a_flags)
-libverstage-S-ccopts += $(armv7_asm_flags)
 verstage-generic-ccopts += $(armv7-a_flags)
 verstage-S-ccopts += $(armv7_asm_flags)
 
@@ -83,14 +81,10 @@
 verstage-y += mmu.c
 
 else ifeq ($(CONFIG_ARCH_VERSTAGE_ARMV7_M),y)
-libverstage-generic-ccopts += $(armv7-m_flags)
-libverstage-S-ccopts += $(armv7_asm_flags)
 verstage-generic-ccopts += $(armv7-m_flags)
 verstage-S-ccopts += $(armv7_asm_flags)
 
 else ifeq ($(CONFIG_ARCH_VERSTAGE_ARMV7_R),y)
-libverstage-generic-ccopts += $(armv7-r_flags)
-libverstage-S-ccopts += $(armv7-r_asm_flags)
 verstage-generic-ccopts += $(armv7-r_flags)
 verstage-S-ccopts += $(armv7-r_asm_flags)
 
diff --git a/src/arch/arm64/Makefile.inc b/src/arch/arm64/Makefile.inc
index 7f3ce3f..ad3941b 100644
--- a/src/arch/arm64/Makefile.inc
+++ b/src/arch/arm64/Makefile.inc
@@ -65,9 +65,9 @@
 
 ifeq ($(CONFIG_ARCH_VERSTAGE_ARM64),y)
 
-$(objcbfs)/verstage.debug: $(objgenerated)/libverstage.a $$(verstage-objs)
+$(objcbfs)/verstage.debug: $$(verstage-objs)
 	@printf "    LINK       $(subst $(obj)/,,$(@))\n"
-	$(LD_verstage) $(LDFLAGS_verstage) -o $@ -L$(obj) --whole-archive --start-group $(filter-out %.ld,$(verstage-objs)) $(objgenerated)/libverstage.a --end-group -T $(call src-to-obj,verstage,src/mainboard/$(MAINBOARDDIR)/memlayout.ld)
+	$(LD_verstage) $(LDFLAGS_verstage) -o $@ -L$(obj) --whole-archive --start-group $(filter-out %.ld,$(verstage-objs)) --end-group -T $(call src-to-obj,verstage,src/mainboard/$(MAINBOARDDIR)/memlayout.ld)
 
 verstage-y += boot.c
 verstage-y += div0.c
diff --git a/src/arch/arm64/armv8/Makefile.inc b/src/arch/arm64/armv8/Makefile.inc
index f026a99..a7a6b19 100644
--- a/src/arch/arm64/armv8/Makefile.inc
+++ b/src/arch/arm64/armv8/Makefile.inc
@@ -53,7 +53,6 @@
 verstage-y += cache_helpers.S
 verstage-y += exception.c
 
-libverstage-generic-ccopts += $(armv8_flags)
 verstage-generic-ccopts += $(armv8_flags)
 
 endif
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 332e8ec..5f184c5 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -186,7 +186,7 @@
 
 verstage-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c
 
-verstage-libs += $(objgenerated)/libverstage.a
+verstage-libs ?=
 
 $(eval $(call early_x86_assembly_entry_rule,verstage))
 
diff --git a/src/drivers/i2c/ww_ring/Makefile.inc b/src/drivers/i2c/ww_ring/Makefile.inc
index 09f2e93..ea0d1a8 100644
--- a/src/drivers/i2c/ww_ring/Makefile.inc
+++ b/src/drivers/i2c/ww_ring/Makefile.inc
@@ -1,5 +1,5 @@
-libverstage-$(CONFIG_DRIVERS_I2C_WW_RING) += ww_ring.c
-libverstage-$(CONFIG_DRIVERS_I2C_WW_RING) += ww_ring_programs.c
+verstage-$(CONFIG_DRIVERS_I2C_WW_RING) += ww_ring.c
+verstage-$(CONFIG_DRIVERS_I2C_WW_RING) += ww_ring_programs.c
 
 ramstage-$(CONFIG_DRIVERS_I2C_WW_RING) += ww_ring.c
 ramstage-$(CONFIG_DRIVERS_I2C_WW_RING) += ww_ring_programs.c
diff --git a/src/drivers/intel/fsp1_1/Makefile.inc b/src/drivers/intel/fsp1_1/Makefile.inc
index e2f75ee..960df74 100644
--- a/src/drivers/intel/fsp1_1/Makefile.inc
+++ b/src/drivers/intel/fsp1_1/Makefile.inc
@@ -18,7 +18,7 @@
 
 verstage-y += car.c
 verstage-y += fsp_util.c
-verstage-y += verstage.c
+verstage-$(CONFIG_SEPARATE_VERSTAGE) += verstage.c
 
 bootblock-y += bootblock.c
 bootblock-y += fsp_util.c
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 4576006..8f92b29 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -50,9 +50,9 @@
 verstage-y += boot_device.c
 verstage-$(CONFIG_CONSOLE_CBMEM) += cbmem_console.c
 
-libverstage-$(CONFIG_TPM) += tlcl.c
-libverstage-$(CONFIG_TPM2) += tpm2_marshaling.c
-libverstage-$(CONFIG_TPM2) += tpm2_tlcl.c
+verstage-$(CONFIG_TPM) += tlcl.c
+verstage-$(CONFIG_TPM2) += tpm2_marshaling.c
+verstage-$(CONFIG_TPM2) += tpm2_tlcl.c
 
 ifeq ($(CONFIG_VBOOT_SEPARATE_VERSTAGE),y)
 romstage-$(CONFIG_TPM) += tlcl.c
diff --git a/src/soc/qualcomm/ipq40xx/Makefile.inc b/src/soc/qualcomm/ipq40xx/Makefile.inc
index 568b0e3..08be720 100644
--- a/src/soc/qualcomm/ipq40xx/Makefile.inc
+++ b/src/soc/qualcomm/ipq40xx/Makefile.inc
@@ -24,10 +24,10 @@
 
 verstage-y += clock.c
 verstage-y += gpio.c
-libverstage-y += blsp.c
-libverstage-y += i2c.c
-libverstage-y += qup.c
-libverstage-y += spi.c
+verstage-y += blsp.c
+verstage-y += i2c.c
+verstage-y += qup.c
+verstage-y += spi.c
 verstage-y += timer.c
 verstage-$(CONFIG_DRIVERS_UART) += uart.c
 
diff --git a/src/soc/qualcomm/ipq806x/Makefile.inc b/src/soc/qualcomm/ipq806x/Makefile.inc
index 42d28e4..624b633 100644
--- a/src/soc/qualcomm/ipq806x/Makefile.inc
+++ b/src/soc/qualcomm/ipq806x/Makefile.inc
@@ -23,10 +23,10 @@
 
 verstage-y += clock.c
 verstage-y += gpio.c
-libverstage-y += gsbi.c
-libverstage-y += i2c.c
-libverstage-y += qup.c
-libverstage-y += spi.c
+verstage-y += gsbi.c
+verstage-y += i2c.c
+verstage-y += qup.c
+verstage-y += spi.c
 verstage-y += timer.c
 verstage-$(CONFIG_DRIVERS_UART) += uart.c
 
diff --git a/src/soc/rockchip/rk3288/Makefile.inc b/src/soc/rockchip/rk3288/Makefile.inc
index b29038d..75c2e24 100644
--- a/src/soc/rockchip/rk3288/Makefile.inc
+++ b/src/soc/rockchip/rk3288/Makefile.inc
@@ -37,7 +37,7 @@
 verstage-y += ../common/gpio.c
 verstage-y += gpio.c
 verstage-y += clock.c
-libverstage-y += crypto.c
+verstage-y += crypto.c
 verstage-y += ../common/i2c.c
 verstage-$(CONFIG_SOFTWARE_I2C) += software_i2c.c
 
diff --git a/src/vboot/Makefile.inc b/src/vboot/Makefile.inc
index 56a3bac..0d6ce57 100644
--- a/src/vboot/Makefile.inc
+++ b/src/vboot/Makefile.inc
@@ -21,7 +21,6 @@
 verstage-y += bootmode.c
 postcar-y += bootmode.c
 
-libverstage-generic-ccopts += -D__PRE_RAM__ -D__VERSTAGE__
 verstage-generic-ccopts += -D__PRE_RAM__ -D__VERSTAGE__
 
 bootblock-y += vbnv.c
@@ -62,14 +61,14 @@
 postcar-y += vboot_common.c
 
 bootblock-y += common.c
-libverstage-y += vboot_logic.c
+verstage-y += vboot_logic.c
 verstage-y += common.c
-verstage-y += verstage.c
+verstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += verstage.c
 ifeq (${CONFIG_VBOOT_MOCK_SECDATA},y)
-libverstage-y += secdata_mock.c
+verstage-y += secdata_mock.c
 romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_mock.c
 else
-libverstage-y += secdata_tpm.c
+verstage-y += secdata_tpm.c
 romstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += secdata_tpm.c
 endif
 romstage-y += vboot_handoff.c common.c
@@ -88,9 +87,9 @@
 endif # CONFIG_VBOOT_SEPARATE_VERSTAGE
 
 VB2_LIB = $(obj)/external/vboot_reference/vboot_fw20.a
-VBOOT_CFLAGS += $(patsubst -I%,-I$(top)/%, $(filter-out -I$(obj), $(filter-out -include $(src)/include/kconfig.h, $(CPPFLAGS_libverstage))))
-VBOOT_CFLAGS += $(CFLAGS_libverstage)
-VBOOT_CFLAGS += $(libverstage-c-ccopts)
+VBOOT_CFLAGS += $(patsubst -I%,-I$(top)/%, $(filter-out -I$(obj), $(filter-out -include $(src)/include/kconfig.h, $(CPPFLAGS_verstage))))
+VBOOT_CFLAGS += $(CFLAGS_verstage)
+VBOOT_CFLAGS += $(verstage-c-ccopts)
 VBOOT_CFLAGS += -I$(abspath $(obj)) -include $(top)/src/include/kconfig.h -Wno-missing-prototypes
 VBOOT_CFLAGS += -DVBOOT_DEBUG
 
@@ -104,7 +103,7 @@
 		V=$(V) \
 		fwlib20
 
-libverstage-srcs += $(VB2_LIB)
+verstage-srcs += $(VB2_LIB)
 
 ifeq ($(CONFIG_VBOOT_SEPARATE_VERSTAGE),y)
 
@@ -131,11 +130,11 @@
 
 endif
 
-else
+else # CONFIG_VBOOT_SEPARATE_VERSTAGE
 ifeq ($(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK),y)
-bootblock-srcs += $(objgenerated)/libverstage.a
+postinclude-hooks += $$(eval bootblock-srcs += $$(verstage-srcs))
 else
-romstage-srcs += $(objgenerated)/libverstage.a
+postinclude-hooks += $$(eval romstage-srcs += $$(verstage-srcs))
 endif
 endif # CONFIG_VBOOT_SEPARATE_VERSTAGE
 
diff --git a/toolchain.inc b/toolchain.inc
index 9b95215..5501c32 100644
--- a/toolchain.inc
+++ b/toolchain.inc
@@ -47,8 +47,7 @@
 ROMCC=CCC_CC="$(ROMCC_BIN)" $(CC)
 endif
 
-COREBOOT_STANDARD_STAGES := bootblock libverstage verstage romstage ramstage
-MAP-libverstage := verstage
+COREBOOT_STANDARD_STAGES := bootblock verstage romstage ramstage
 
 ARCHDIR-i386	:= x86
 ARCHDIR-x86_32	:= x86

-- 
To view, visit https://review.coreboot.org/18302
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6bb84a9bf1cd54f2e02ca1f665740a9c88d88df4
Gerrit-PatchSet: 16
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)



More information about the coreboot-gerrit mailing list