From a98122064dcc3704a54c44323ba2eda8de7358dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 24 Nov 2020 15:38:08 +0100 Subject: [PATCH 1/2] Makefile: Do not mark file targets as .PHONY target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only non-file targets should be set a .PHONY. Otherwise if file target is set as .PHONY then targets which depends on those file .PHONY targets would be always rebuilt even when their prerequisites are not changed. File target which needs to be always rebuilt can be specified in Make system via having a prerequisite on some .PHONY target, instead of marking whole target as .PHONY. In Makefile projects it is common to create empty .PHONY target named FORCE for this purpose. This patch changes all file targets which are set as .PHONY to depends on new .PHONY target FORCE, to ensure that these file targets are always rebuilt (as before). Basically they are those targets which calls external make subprocess. After FORCE target is specified in main Makefile, remove it from other Makefile files to prevent duplicate definitions. Signed-off-by: Pali Rohár Change-Id: Iee3b4e0de93879b95eb29a1745a041538412e69e --- Makefile | 18 ++++++++---------- plat/arm/board/common/board_common.mk | 1 - plat/marvell/armada/a3k/common/a3700_common.mk | 3 --- plat/marvell/armada/a8k/common/a8k_common.mk | 3 --- plat/marvell/armada/a8k/common/ble/ble.mk | 2 -- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index 2d5a5bb2e..b0399f15b 100644 --- a/Makefile +++ b/Makefile @@ -1245,8 +1245,7 @@ checkpatch: locate-checkpatch certtool: ${CRTTOOL} -.PHONY: ${CRTTOOL} -${CRTTOOL}: +${CRTTOOL}: FORCE ${Q}${MAKE} PLAT=${PLAT} USE_TBBR_DEFS=${USE_TBBR_DEFS} COT=${COT} OPENSSL_DIR=${OPENSSL_DIR} CRTTOOL=${CRTTOOL} --no-print-directory -C ${CRTTOOLPATH} @${ECHO_BLANK_LINE} @echo "Built $@ successfully" @@ -1288,8 +1287,7 @@ fiptool: ${FIPTOOL} fip: ${BUILD_PLAT}/${FIP_NAME} fwu_fip: ${BUILD_PLAT}/${FWU_FIP_NAME} -.PHONY: ${FIPTOOL} -${FIPTOOL}: +${FIPTOOL}: FORCE @${ECHO_BLANK_LINE} @echo "Building $@" ifdef UNIX_MK @@ -1302,12 +1300,10 @@ endif @${ECHO_BLANK_LINE} sptool: ${SPTOOL} -.PHONY: ${SPTOOL} -${SPTOOL}: +${SPTOOL}: FORCE ${Q}${MAKE} CPPFLAGS="-DVERSION='\"${VERSION_STRING}\"'" SPTOOL=${SPTOOL} --no-print-directory -C ${SPTOOLPATH} -.PHONY: libraries -romlib.bin: libraries +romlib.bin: libraries FORCE ${Q}${MAKE} PLAT_DIR=${PLAT_DIR} BUILD_PLAT=${BUILD_PLAT} ENABLE_BTI=${ENABLE_BTI} ARM_ARCH_MINOR=${ARM_ARCH_MINOR} INCLUDES='${INCLUDES}' DEFINES='${DEFINES}' --no-print-directory -C ${ROMLIBPATH} all # Call print_memory_map tool @@ -1320,8 +1316,7 @@ doc: enctool: ${ENCTOOL} -.PHONY: ${ENCTOOL} -${ENCTOOL}: +${ENCTOOL}: FORCE ${Q}${MAKE} PLAT=${PLAT} BUILD_INFO=0 OPENSSL_DIR=${OPENSSL_DIR} ENCTOOL=${ENCTOOL} --no-print-directory -C ${ENCTOOLPATH} @${ECHO_BLANK_LINE} @echo "Built $@ successfully" @@ -1375,3 +1370,6 @@ help: @echo "" @echo "example: build all targets for the FVP platform:" @echo " CROSS_COMPILE=aarch64-none-elf- make PLAT=fvp all" + +.PHONY: FORCE +FORCE:; diff --git a/plat/arm/board/common/board_common.mk b/plat/arm/board/common/board_common.mk index 1885a600a..6db0c0031 100644 --- a/plat/arm/board/common/board_common.mk +++ b/plat/arm/board/common/board_common.mk @@ -41,7 +41,6 @@ $(eval $(call add_define,ARM_ROTPK_LOCATION_ID)) # Force generation of the new hash if ROT_KEY is specified ifdef ROT_KEY HASH_PREREQUISITES = $(ROT_KEY) FORCE -FORCE: else HASH_PREREQUISITES = $(ROT_KEY) endif diff --git a/plat/marvell/armada/a3k/common/a3700_common.mk b/plat/marvell/armada/a3k/common/a3700_common.mk index 712b16202..2bb0f193d 100644 --- a/plat/marvell/armada/a3k/common/a3700_common.mk +++ b/plat/marvell/armada/a3k/common/a3700_common.mk @@ -211,6 +211,3 @@ mrvl_flash: $(error "Platform '${PLAT}' for target '$@' requires WTP. Please set WTP to point to the right directory") endif # WTP - -.PHONY: FORCE -FORCE:; diff --git a/plat/marvell/armada/a8k/common/a8k_common.mk b/plat/marvell/armada/a8k/common/a8k_common.mk index cf1516a28..b7c7d848d 100644 --- a/plat/marvell/armada/a8k/common/a8k_common.mk +++ b/plat/marvell/armada/a8k/common/a8k_common.mk @@ -166,6 +166,3 @@ ${DOIMAGETOOL}: FORCE .PHONY: mrvl_flash mrvl_flash: ${BUILD_PLAT}/${BOOT_IMAGE} ${DOIMAGETOOL} ${DOIMAGETOOL} ${DOIMAGE_FLAGS} ${BUILD_PLAT}/${BOOT_IMAGE} ${BUILD_PLAT}/${FLASH_IMAGE} - -.PHONY: FORCE -FORCE:; diff --git a/plat/marvell/armada/a8k/common/ble/ble.mk b/plat/marvell/armada/a8k/common/ble/ble.mk index 60fbf5f1d..78c62a010 100644 --- a/plat/marvell/armada/a8k/common/ble/ble.mk +++ b/plat/marvell/armada/a8k/common/ble/ble.mk @@ -26,7 +26,5 @@ PLAT_INCLUDES += -I$(MV_DDR_PATH) \ BLE_LINKERFILE := $(BLE_PATH)/ble.ld.S -FORCE: - $(MV_DDR_LIB): FORCE @+make -C $(MV_DDR_PATH) --no-print-directory PLAT_INCLUDES="$(PLAT_INCLUDES)" PLATFORM=$(PLAT) ARCH=AARCH64 OBJ_DIR=$(BUILD_PLAT)/ble From 4727fd1320940c2d89c75c13645864c46a24d96d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 24 Nov 2020 16:53:04 +0100 Subject: [PATCH 2/2] Makefile: Fix ${FIP_NAME} to be rebuilt only when needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently ${FIP_DEPS} as prerequisite for ${BUILD_PLAT}/${FIP_NAME} contains .PHONY targets check_$(1) and therefore ${BUILD_PLAT}/${FIP_NAME} is always rebuilt even when other file target prerequisites are not changed. These changes fix above issue and ${BUILD_PLAT}/${FIP_NAME} target is rebuilt only when its prerequisites are changed. There are 3 changes: Content of check_$(1) target is moved into check_$(1)_cmd variable so it can be easily reused. .PHONY check_$(1) targets are not put into ${FIP_DEPS} and ${FWU_FIP_DEPS} dependencies anymore and required checks which are in ${CHECK_FIP_CMD} and ${CHECK_FWU_FIP_CMD} variables are executed as part of targets ${BUILD_PLAT}/${FIP_NAME} and ${BUILD_PLAT}/${FWU_FIP_NAME} itself. To ensure that ${BUILD_PLAT}/${FIP_NAME} and ${BUILD_PLAT}/${FWU_FIP_NAME} are rebuilt even when additional dependency file image added by TOOL_ADD_IMG is changed, this file image (if exists) is added as file dependency to ${FIP_DEPS} and ${FWU_FIP_DEPS}. If it does not exist then FORCE target is added to ensure that FIP/FWU_FIP is rebuilt. Command ${CHECK_FIP_CMD}/${CHECK_FWU_FIP_CMD} will then thrown an error message if the file is required but not present. So this change ensures that if BL33 image is updated then final FIP image is updated too. And if BL33 image is not specified or does not exist and is required to be present then check_$(1)_cmd call from ${CHECK_FIP_CMD} would ensure that error message is thrown during build. Signed-off-by: Pali Rohár Change-Id: I635cf82e2b667ff57e2af83500d4aca71d235e3e --- Makefile | 2 ++ make_helpers/build_macros.mk | 15 +++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index b0399f15b..ceb5a118c 100644 --- a/Makefile +++ b/Makefile @@ -1261,6 +1261,7 @@ certificates: ${CRT_DEPS} ${CRTTOOL} endif ${BUILD_PLAT}/${FIP_NAME}: ${FIP_DEPS} ${FIPTOOL} + $(eval ${CHECK_FIP_CMD}) ${Q}${FIPTOOL} create ${FIP_ARGS} $@ ${Q}${FIPTOOL} info $@ @${ECHO_BLANK_LINE} @@ -1277,6 +1278,7 @@ fwu_certificates: ${FWU_CRT_DEPS} ${CRTTOOL} endif ${BUILD_PLAT}/${FWU_FIP_NAME}: ${FWU_FIP_DEPS} ${FIPTOOL} + $(eval ${CHECK_FWU_FIP_CMD}) ${Q}${FIPTOOL} create ${FWU_FIP_ARGS} $@ ${Q}${FIPTOOL} info $@ @${ECHO_BLANK_LINE} diff --git a/make_helpers/build_macros.mk b/make_helpers/build_macros.mk index 613fca23f..86550288c 100644 --- a/make_helpers/build_macros.mk +++ b/make_helpers/build_macros.mk @@ -214,21 +214,28 @@ define TOOL_ADD_IMG # This is the uppercase form of the first parameter $(eval _V := $(call uppercase,$(1))) + # $(check_$(1)_cmd) variable is executed in the check_$(1) target and also + # is put into the ${CHECK_$(3)FIP_CMD} variable which is executed by the + # target ${BUILD_PLAT}/${$(3)FIP_NAME}. + $(eval check_$(1)_cmd := \ + $(if $(value $(_V)),,$$$$(error "Platform '${PLAT}' requires $(_V). Please set $(_V) to point to the right file")) \ + $(if $(wildcard $(value $(_V))),,$$$$(error '$(_V)=$(value $(_V))' was specified, but '$(value $(_V))' does not exist)) \ + ) + $(3)CRT_DEPS += check_$(1) - $(3)FIP_DEPS += check_$(1) + CHECK_$(3)FIP_CMD += $$(check_$(1)_cmd) ifeq ($(4),1) $(eval ENC_BIN := ${BUILD_PLAT}/$(1)_enc.bin) $(call ENCRYPT_FW,$(value $(_V)),$(ENC_BIN)) $(call TOOL_ADD_IMG_PAYLOAD,$(1),$(value $(_V)),$(2),$(ENC_BIN),$(3), \ $(ENC_BIN)) else - $(call TOOL_ADD_IMG_PAYLOAD,$(1),$(value $(_V)),$(2),,$(3)) + $(call TOOL_ADD_IMG_PAYLOAD,$(1),$(value $(_V)),$(2),$(if $(wildcard $(value $(_V))),$(value $(_V)),FORCE),$(3)) endif .PHONY: check_$(1) check_$(1): - $$(if $(value $(_V)),,$$(error "Platform '${PLAT}' requires $(_V). Please set $(_V) to point to the right file")) - $$(if $(wildcard $(value $(_V))),,$$(error '$(_V)=$(value $(_V))' was specified, but '$(value $(_V))' does not exist)) + $(check_$(1)_cmd) endef ################################################################################