From d2bc904fbe19746c98f446e3e951684b6953a1d4 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Tue, 27 Nov 2018 18:09:19 +0100 Subject: [PATCH 1/6] mcuboot.mk: remove unnecessary override, export and :=. The previous definition of IMGTOOL was unnecessarily complicated. "abspath" is not needed. Also, the export for IMAGE_HDR_SIZE is not necessary. --- makefiles/mcuboot.mk | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/makefiles/mcuboot.mk b/makefiles/mcuboot.mk index 611c6c75672f..82f5cfda9b55 100644 --- a/makefiles/mcuboot.mk +++ b/makefiles/mcuboot.mk @@ -1,7 +1,6 @@ ifdef MCUBOOT_SLOT0_SIZE IMGTOOL ?= $(RIOTTOOLS)/mcuboot/imgtool.py -override IMGTOOL := $(abspath $(IMGTOOL)) BINFILE ?= $(BINDIR)/$(APPLICATION).bin SIGN_BINFILE = $(BINDIR)/signed-$(APPLICATION).bin @@ -10,7 +9,7 @@ MCUBOOT_BIN ?= $(BINDIR)/mcuboot.bin MCUBOOT_BIN_URL ?= http://download.riot-os.org/mynewt.mcuboot.bin MCUBOOT_BIN_MD5 ?= 0c71a0589bd3709fc2d90f07a0035ce7 -export IMAGE_HDR_SIZE ?= 512 +IMAGE_HDR_SIZE ?= 512 $(MCUBOOT_KEYFILE) $(MCUBOOT_BIN): $(filter clean, $(MAKECMDGOALS)) From 1c7c3659f7f4abe894a994036a61c2933f8a58e3 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Tue, 27 Nov 2018 18:15:05 +0100 Subject: [PATCH 2/6] mcuboot.mk: Do not define BINFILE. There is no point on defining BINFILE again. It is already done by Makefile.include. --- makefiles/mcuboot.mk | 1 - 1 file changed, 1 deletion(-) diff --git a/makefiles/mcuboot.mk b/makefiles/mcuboot.mk index 82f5cfda9b55..a14583fc062f 100644 --- a/makefiles/mcuboot.mk +++ b/makefiles/mcuboot.mk @@ -2,7 +2,6 @@ ifdef MCUBOOT_SLOT0_SIZE IMGTOOL ?= $(RIOTTOOLS)/mcuboot/imgtool.py -BINFILE ?= $(BINDIR)/$(APPLICATION).bin SIGN_BINFILE = $(BINDIR)/signed-$(APPLICATION).bin MCUBOOT_KEYFILE ?= $(BINDIR)/key.pem MCUBOOT_BIN ?= $(BINDIR)/mcuboot.bin From 38b3cbdadcf7b3e63dcc25a64e5ccc485e8af049 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Wed, 28 Nov 2018 11:15:42 +0100 Subject: [PATCH 3/6] mcuboot.mk: replace MCUBOOT_BIN by MCUBOOT_LOADER_BIN. The name MCUBOOT_BIN is confusing and not clear. It suggests it is the application binary when in reality it is just the mcuboot bootloader image. --- makefiles/mcuboot.mk | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/makefiles/mcuboot.mk b/makefiles/mcuboot.mk index a14583fc062f..19e646ca86fd 100644 --- a/makefiles/mcuboot.mk +++ b/makefiles/mcuboot.mk @@ -4,13 +4,14 @@ IMGTOOL ?= $(RIOTTOOLS)/mcuboot/imgtool.py SIGN_BINFILE = $(BINDIR)/signed-$(APPLICATION).bin MCUBOOT_KEYFILE ?= $(BINDIR)/key.pem -MCUBOOT_BIN ?= $(BINDIR)/mcuboot.bin -MCUBOOT_BIN_URL ?= http://download.riot-os.org/mynewt.mcuboot.bin -MCUBOOT_BIN_MD5 ?= 0c71a0589bd3709fc2d90f07a0035ce7 + +MCUBOOT_LOADER_BIN ?= $(BINDIR)/mcuboot.bin +MCUBOOT_LOADER_BIN_URL ?= http://download.riot-os.org/mynewt.mcuboot.bin +MCUBOOT_LOADER_BIN_MD5 ?= 0c71a0589bd3709fc2d90f07a0035ce7 IMAGE_HDR_SIZE ?= 512 -$(MCUBOOT_KEYFILE) $(MCUBOOT_BIN): $(filter clean, $(MAKECMDGOALS)) +$(MCUBOOT_KEYFILE) $(MCUBOOT_LOADER_BIN): $(filter clean, $(MAKECMDGOALS)) mcuboot-create-key: $(MCUBOOT_KEYFILE) @@ -34,14 +35,14 @@ mcuboot: mcuboot-create-key link $(COLOR_RESET)' @$(COLOR_ECHO) -$(MCUBOOT_BIN): - $(Q)$(DLCACHE) $(MCUBOOT_BIN_URL) $(MCUBOOT_BIN_MD5) $@ +$(MCUBOOT_LOADER_BIN): + $(Q)$(DLCACHE) $(MCUBOOT_LOADER_BIN_URL) $(MCUBOOT_LOADER_BIN_MD5) $@ .PHONY: mcuboot-flash-bootloader mcuboot-flash -mcuboot-flash-bootloader: FLASHFILE = $(MCUBOOT_BIN) +mcuboot-flash-bootloader: FLASHFILE = $(MCUBOOT_LOADER_BIN) mcuboot-flash-bootloader: export FLASH_ADDR = 0x0 -mcuboot-flash-bootloader: $(MCUBOOT_BIN) $(FLASHDEPS) +mcuboot-flash-bootloader: $(MCUBOOT_LOADER_BIN) $(FLASHDEPS) $(flash-recipe) mcuboot-flash: FLASHFILE = $(SIGN_BINFILE) From d325b11e126f8b6a9fd9ec95c573bd0b60f3f55a Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Tue, 27 Nov 2018 18:36:54 +0100 Subject: [PATCH 4/6] mcuboot.mk: Restructure targets to express dependencies. Add rule for MCUBOOT_ELF (do not overwrite normal elf.) Similar to ELFFILE, this depends on BASELIBS. MCUBOOT_BIN is done automatically from MCUBOOT_ELF by an implicit rule. SIGN_BINFILE (the signed binary) depends on MCUBOOT_KEYFILE and not on a phony rule. All phony rules are listed at the beginning. --- makefiles/mcuboot.mk | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/makefiles/mcuboot.mk b/makefiles/mcuboot.mk index 19e646ca86fd..66d16479c1af 100644 --- a/makefiles/mcuboot.mk +++ b/makefiles/mcuboot.mk @@ -1,3 +1,5 @@ +.PHONY: mcuboot mcuboot-create-key mcuboot-flash-bootloader mcuboot-flash + ifdef MCUBOOT_SLOT0_SIZE IMGTOOL ?= $(RIOTTOOLS)/mcuboot/imgtool.py @@ -5,6 +7,9 @@ IMGTOOL ?= $(RIOTTOOLS)/mcuboot/imgtool.py SIGN_BINFILE = $(BINDIR)/signed-$(APPLICATION).bin MCUBOOT_KEYFILE ?= $(BINDIR)/key.pem +MCUBOOT_ELF ?= $(ELFFILE:.elf=.mcuboot.elf) +MCUBOOT_BIN ?= $(MCUBOOT_ELF:.elf=.bin) + MCUBOOT_LOADER_BIN ?= $(BINDIR)/mcuboot.bin MCUBOOT_LOADER_BIN_URL ?= http://download.riot-os.org/mynewt.mcuboot.bin MCUBOOT_LOADER_BIN_MD5 ?= 0c71a0589bd3709fc2d90f07a0035ce7 @@ -21,25 +26,21 @@ $(MCUBOOT_KEYFILE): $(Q)$(IMGTOOL) keygen -k $@ -t rsa-2048 endif -mcuboot: ROM_OFFSET=$$(($(MCUBOOT_SLOT0_SIZE) + $(IMAGE_HDR_SIZE))) -mcuboot: mcuboot-create-key link - @$(COLOR_ECHO) +$(MCUBOOT_ELF): ROM_OFFSET=$$(($(MCUBOOT_SLOT0_SIZE) + $(IMAGE_HDR_SIZE))) +$(MCUBOOT_ELF): $(BASELIBS) @$(COLOR_ECHO) '$(COLOR_PURPLE)Re-linking for MCUBoot at $(MCUBOOT_SLOT0_SIZE)...$(COLOR_RESET)' - @$(COLOR_ECHO) - $(Q)$(_LINK) -o $(ELFFILE) && \ - $(OBJCOPY) $(OFLAGS) -Obinary $(ELFFILE) $(BINFILE) && \ - $(IMGTOOL) sign --key $(MCUBOOT_KEYFILE) --version $(IMAGE_VERSION) --align \ - $(MCUBOOT_IMAGE_ALIGN) -H $(IMAGE_HDR_SIZE) $(BINFILE) $(SIGN_BINFILE) - @$(COLOR_ECHO) - @$(COLOR_ECHO) '$(COLOR_PURPLE)Signed with $(MCUBOOT_KEYFILE) for version $(IMAGE_VERSION)\ - $(COLOR_RESET)' - @$(COLOR_ECHO) + $(Q)$(_LINK) -o $@ + +$(SIGN_BINFILE): $(MCUBOOT_BIN) $(MCUBOOT_KEYFILE) + @$(COLOR_ECHO) '$(COLOR_PURPLE)Signing with $(MCUBOOT_KEYFILE) for version $(IMAGE_VERSION)$(COLOR_RESET)' + $(Q)$(IMGTOOL) sign --key $(MCUBOOT_KEYFILE) --version $(IMAGE_VERSION) --align \ + $(MCUBOOT_IMAGE_ALIGN) -H $(IMAGE_HDR_SIZE) $(MCUBOOT_BIN) $(SIGN_BINFILE) + +mcuboot: $(SIGN_BINFILE) $(MCUBOOT_LOADER_BIN): $(Q)$(DLCACHE) $(MCUBOOT_LOADER_BIN_URL) $(MCUBOOT_LOADER_BIN_MD5) $@ -.PHONY: mcuboot-flash-bootloader mcuboot-flash - mcuboot-flash-bootloader: FLASHFILE = $(MCUBOOT_LOADER_BIN) mcuboot-flash-bootloader: export FLASH_ADDR = 0x0 mcuboot-flash-bootloader: $(MCUBOOT_LOADER_BIN) $(FLASHDEPS) From b439fd96367a2e3acc4f7b8ba83d29519b608800 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Wed, 28 Nov 2018 11:43:20 +0100 Subject: [PATCH 5/6] mcuboot.mk: support in-docker-container for mcuboot target. After the restructuring of mcuboot.mk building in docker was broken. This makes it work again (but only if on uses the mcuboot target.) --- makefiles/mcuboot.mk | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/makefiles/mcuboot.mk b/makefiles/mcuboot.mk index 66d16479c1af..783abc01309c 100644 --- a/makefiles/mcuboot.mk +++ b/makefiles/mcuboot.mk @@ -36,7 +36,11 @@ $(SIGN_BINFILE): $(MCUBOOT_BIN) $(MCUBOOT_KEYFILE) $(Q)$(IMGTOOL) sign --key $(MCUBOOT_KEYFILE) --version $(IMAGE_VERSION) --align \ $(MCUBOOT_IMAGE_ALIGN) -H $(IMAGE_HDR_SIZE) $(MCUBOOT_BIN) $(SIGN_BINFILE) +ifeq ($(BUILD_IN_DOCKER),1) +mcuboot: ..in-docker-container +else mcuboot: $(SIGN_BINFILE) +endif # BUILD_IN_DOCKER $(MCUBOOT_LOADER_BIN): $(Q)$(DLCACHE) $(MCUBOOT_LOADER_BIN_URL) $(MCUBOOT_LOADER_BIN_MD5) $@ From dfed9e9e6f3e3bf15865d0aaebf63fc5c42ccd66 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Mon, 10 Dec 2018 14:21:44 +0100 Subject: [PATCH 6/6] makefiles/mcuboot.mk: always define rule to make key.pem. Making the rule definition conditional does not make sense, as it should not conflict if the user overrides MCUBOOT_KEYFILE (the rule will just not get used). Factor keygen arguments into IMGTOOL_KEYFLAGS. In the future a flag can be added to generate a determnistic key for reproducibility on CI builds. --- makefiles/mcuboot.mk | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/makefiles/mcuboot.mk b/makefiles/mcuboot.mk index 783abc01309c..45018dd13c7e 100644 --- a/makefiles/mcuboot.mk +++ b/makefiles/mcuboot.mk @@ -3,6 +3,7 @@ ifdef MCUBOOT_SLOT0_SIZE IMGTOOL ?= $(RIOTTOOLS)/mcuboot/imgtool.py +IMGTOOL_KEYFLAGS ?= keygen -t rsa-2048 SIGN_BINFILE = $(BINDIR)/signed-$(APPLICATION).bin MCUBOOT_KEYFILE ?= $(BINDIR)/key.pem @@ -20,11 +21,9 @@ $(MCUBOOT_KEYFILE) $(MCUBOOT_LOADER_BIN): $(filter clean, $(MAKECMDGOALS)) mcuboot-create-key: $(MCUBOOT_KEYFILE) -ifeq ($(BINDIR)/key.pem,$(MCUBOOT_KEYFILE)) -$(MCUBOOT_KEYFILE): +$(BINDIR)/key.pem: $(Q)mkdir -p $(BINDIR) - $(Q)$(IMGTOOL) keygen -k $@ -t rsa-2048 -endif + $(Q)$(IMGTOOL) $(IMGTOOL_KEYFLAGS) -k $@ $(MCUBOOT_ELF): ROM_OFFSET=$$(($(MCUBOOT_SLOT0_SIZE) + $(IMAGE_HDR_SIZE))) $(MCUBOOT_ELF): $(BASELIBS)