From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from gallifrey.ext.pengutronix.de ([2001:67c:670:201:5054:ff:fe8d:eefb] helo=[IPv6:::1]) by metis.ext.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1j4MJD-00066b-Ag for ptxdist@pengutronix.de; Wed, 19 Feb 2020 11:10:23 +0100 References: <20200212164033.7362-1-a.fatoum@pengutronix.de> <20200214131931.GH7958@pengutronix.de> From: Ahmad Fatoum Message-ID: Date: Wed, 19 Feb 2020 11:10:23 +0100 MIME-Version: 1.0 In-Reply-To: <20200214131931.GH7958@pengutronix.de> Content-Language: en-US Subject: Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A List-Id: PTXdist Development Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: ptxdist@pengutronix.de Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ptxdist-bounces@pengutronix.de Sender: "ptxdist" To: ptxdist@pengutronix.de Hi, On 2/14/20 2:19 PM, Michael Olbrich wrote: > On Wed, Feb 12, 2020 at 05:40:33PM +0100, Ahmad Fatoum wrote: >> Trusted Firmware-A (TF-A) is a reference implementation of secure world >> software for Arm A-Profile architectures (Armv8-A and Armv7-A). >> >> Cc: Alejandro Vazquez >> Signed-off-by: Rouven Czerwinski >> Signed-off-by: Ahmad Fatoum >> --- >> v1 -> v2: >> - Made TF_A_ARCH_MAJOR configurable to support 32 bit ARMv8 (Guillermo) >> - Replaces stm32mp-specific TF_A_DTB with TF_A_EXTRA_ARGS to contain >> all board/vendor specific options >> - removed reference to no longer existing CREDITS file >> - removed TF_A_MAKE_OPT contents that are set elsewhere >> - reduced uses of += in favor of directly appending to the string >> - delete old build directory in prepare instead of compile >> - use default compile stage (Guillermo) >> - install artifacts to sysroot /usr/lib/firmware in install stage >> - install artifacts to IMAGEDIR in targetinstall >> - fix clean stage to delete proper artifacts >> --- >> platforms/tf-a.in | 138 ++++++++++++++++++++++++++++++++++++++++++++++ >> rules/tf-a.make | 114 ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 252 insertions(+) >> create mode 100644 platforms/tf-a.in >> create mode 100644 rules/tf-a.make >> >> diff --git a/platforms/tf-a.in b/platforms/tf-a.in >> new file mode 100644 >> index 000000000000..f3971c4c2a3a >> --- /dev/null >> +++ b/platforms/tf-a.in >> @@ -0,0 +1,138 @@ >> +## SECTION=bootloader >> + >> +menuconfig TF_A >> + select BOOTLOADER >> + prompt "ARM Trusted Firmware-A" > > Spaces at the end to align the '-->' ok. > >> + depends on ARCH_ARM || ARCH_ARM64 >> + bool >> + >> +if TF_A >> + >> +config TF_A_ARCH_STRING >> + string >> + default "aarch32" if ARCH_ARM >> + default "aarch64" if ARCH_ARM64 >> + >> +choice >> + prompt "TF-A Architecture" >> + default TF_A_ARM_ARCH_MAJOR_7 if ARCH_ARM >> + default TF_A_ARM_ARCH_MAJOR_8 if ARCH_ARM64 >> + help >> + Architecture version major number >> + >> + config TF_A_ARM_ARCH_MAJOR_7 >> + depends on ARCH_ARM >> + prompt "ARMv7" >> + bool >> + >> + config TF_A_ARM_ARCH_MAJOR_8_32_BIT >> + depends on ARCH_ARM >> + prompt "ARMv8 32-bit" >> + bool >> + >> + config TF_A_ARM_ARCH_MAJOR_8 >> + depends on ARCH_ARM64 >> + prompt "ARMv8" >> + bool >> + >> +endchoice >> + >> +config TF_A_ARM_ARCH_MAJOR >> + int >> + default 7 if TF_A_ARM_ARCH_MAJOR_7 >> + default 8 if TF_A_ARM_ARCH_MAJOR_8_32_BIT >> + default 8 if TF_A_ARM_ARCH_MAJOR_8 >> + >> +config TF_A_VERSION >> + string >> + default "v2.2" >> + prompt "TF-A version" >> + help >> + Enter the TF-A version you want to build. Usally something like "v2.2" > > You should mention that this is the tag name from the git repository. It can be a commit hash as well. I adjusted the description. > >> + >> +config TF_A_MD5 >> + string >> + default "bb300e5a62c911e189c80d935d497a4b" >> + prompt "TF-A source md5" > > TF_A_VERSION and TF_A_MD5 should be the first two sub options. ok. > >> + >> +config TF_A_PLATFORM >> + string >> + prompt "TF-A target platform" >> + help >> + The TF-A target platform. >> + >> +config TF_A_ARM_ARCH_MINOR >> + depends on TF_A_ARM_ARCH_MAJOR_8 || TF_A_ARM_ARCH_MAJOR_8_32_BIT >> + int >> + default 0 >> + prompt "TF-A target ARMv8.MINOR version" >> + help >> + The minor version of the ARMv8 architecture targeted. Defaults to 0. >> + >> +config TF_A_EXTRA_ARGS >> + string >> + prompt "TF-A extra build arguments" >> + help >> + Extra platform-specific build arguments to pass to the TF-A build >> + process, e.g. DTB_FILE_NAME= for the stm32mp1 >> + >> +config TF_A_ARTIFACTS >> + string >> + prompt "TF-A artifact file names" >> + default "bl2.bin" >> + help >> + A space-separated list of artifacts to copy to the image directory. >> + All file names are relative to the appropriate TF-A platform build >> + directory. > > Where does the default come from? When I tried this, the only file that > existed was 'bl31.bin'. ARM's Juno board maybe. I removed the default. The rule should always error now when it's enabled but this setting isn't configured > >> + >> +comment "Payloads" >> + >> +choice >> + prompt "BL32 Payload" >> + default TF_A_BL32_NONE >> + help >> + payload for BL32 (Secure World OS) >> + >> + config TF_A_BL32_NONE >> + prompt "None" >> + bool >> + >> + config TF_A_BL32_SP_MIN >> + depends on ARCH_ARM >> + prompt "sp_min" >> + bool >> + >> + config TF_A_BL32_TSP >> + depends on ARCH_ARM64 >> + prompt "Test Secure Payload" >> + bool >> + >> +endchoice >> + >> +if TF_A_BL32_TSP >> +choice TF_A_BL32_TSP_RAM_LOCATION >> + prompt "TSP location" >> + default TF_A_BL32_TSP_RAM_LOCATION_TSRAM >> + >> + config TF_A_BL32_TSP_RAM_LOCATION_TSRAM >> + prompt "Trusted SRAM" >> + bool >> + >> + config TF_A_BL32_TSP_RAM_LOCATION_TDRAM >> + prompt "Trusted DRAM (if available)" >> + bool >> + >> + config TF_A_BL32_TSP_RAM_LOCATION_DRAM >> + prompt "Secure DRAM region (configured by TrustZone controller)" >> + bool >> +endchoice >> + >> +config TF_A_BL32_TSP_RAM_LOCATION_STRING >> + string >> + default "tsram" if TF_A_BL32_TSP_RAM_LOCATION_TSRAM >> + default "tdram" if TF_A_BL32_TSP_RAM_LOCATION_TDRAM >> + default "dram" if TF_A_BL32_TSP_RAM_LOCATION_DRAM >> + >> +endif >> + >> +endif >> diff --git a/rules/tf-a.make b/rules/tf-a.make >> new file mode 100644 >> index 000000000000..9f895d32518d >> --- /dev/null >> +++ b/rules/tf-a.make >> @@ -0,0 +1,114 @@ >> +# -*-makefile-*- >> +# >> +# Copyright (C) 2018 by Rouven Czerwinski >> +# 2019 by Ahmad Fatoum >> +# >> +# For further information about the PTXdist project and license conditions >> +# see the README file. >> +# >> + >> +# >> +# We provide this package >> +# >> +PACKAGES-$(PTXCONF_TF_A) += tf-a >> + >> +# >> +# Paths and names >> +# >> +TF_A_VERSION := $(call remove_quotes,$(PTXCONF_TF_A_VERSION)) >> +TF_A_MD5 := $(call remove_quotes,$(PTXCONF_TF_A_MD5)) >> +TF_A := tf-a-$(TF_A_VERSION) >> +TF_A_SUFFIX := tar.gz >> +TF_A_URL := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX) >> +TF_A_SOURCE := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX) >> +TF_A_DIR := $(BUILDDIR)/$(TF_A) >> +TF_A_LICENSE := BSD-3-Clause >> + >> +# ---------------------------------------------------------------------------- >> +# Prepare >> +# ---------------------------------------------------------------------------- >> + >> +TF_A_WRAPPER_BLACKLIST := \ >> + TARGET_HARDEN_RELRO \ >> + TARGET_HARDEN_BINDNOW \ >> + TARGET_HARDEN_PIE \ >> + TARGET_DEBUG \ >> + TARGET_BUILD_ID >> + >> +TF_A_PATH := PATH=$(CROSS_PATH) >> +TF_A_MAKE_OPT := \ >> + CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \ >> + HOSTCC=$(HOSTCC) \ >> + PLAT=$(PTXCONF_TF_A_PLATFORM) \ >> + DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0) \ > > No, don't use variables that are never defined. > If the rule must be copied anyways, then editing this is simple. It's at two places, here and in the build directory. I'd rather leave it as is and move the # TF_A_DEBUG=1 above. > >> + ARCH=$(PTXCONF_TF_A_ARCH_STRING) \ >> + ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR) \ >> + $(call remove_quotes,$(PTXCONF_TF_A_EXTRA_ARGS)) \ >> + all >> + >> +ifdef PTXCONF_TF_A_BL32_TSP >> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING) >> +endif >> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR >> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR) >> +endif >> +ifdef PTXCONF_TF_A_BL32_SP_MIN >> +TF_A_MAKE_OPT += AARCH32_SP=sp_min >> +endif >> + >> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release) > > Same here. > >> +TF_A_ARTIFACTS_DEST := $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)) >> +TF_A_ARTIFACTS_SRC := $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \ >> + $(TF_A_ARTIFACTS_DEST)) >> +TF_A_CONF_TOOL := NO >> +TF_A_MAKE_ENV := $(CROSS_ENV) > > Move this variable below the prepare stage. That's for compile. Ok made a compile header and moved it there > >> +# TF_A_DEBUG=yes > > remove. See above. > >> + >> +# ---------------------------------------------------------------------------- >> +# Prepare >> +# ---------------------------------------------------------------------------- > > This is all the prepare stage. Don't add a second header. > >> +$(STATEDIR)/tf-a.prepare: >> + @$(call targetinfo) >> + @rm -rf $(TF_A_DIR)/build/ >> + @$(call touch) >> + >> +# ---------------------------------------------------------------------------- >> +# Install >> +# ---------------------------------------------------------------------------- >> + >> +$(STATEDIR)/tf-a.install: >> + @$(call targetinfo) >> +ifeq ($(TF_A_ARTIFACTS_SRC),) >> + $(warning TF_A_ARTIFACTS is empty. nothing to install.) >> +else >> + @install -m644 -D \ >> + --target-directory=$(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware \ > > Never use PTXCONF_SYSROOT_TARGET as a target. Install to TF_A_PKGDIR > instead. This will be synct to PTXCONF_SYSROOT_TARGET during install.post. > > and use '-v' to make this visible. Ok. > >> + $(TF_A_ARTIFACTS_SRC) >> +endif >> + @$(call touch) >> + >> +# ---------------------------------------------------------------------------- >> +# Target-Install >> +# ---------------------------------------------------------------------------- >> + >> +$(STATEDIR)/tf-a.targetinstall: >> + @$(call targetinfo) >> +ifeq ($(TF_A_ARTIFACTS_SRC),) >> + $(warning TF_A_ARTIFACTS is empty. nothing to install.) > > Hmm, the install stage will fail if TF_A_ARTIFACTS_SRC is empty, right? > And I don't think no output is valid in any way, so just put an '$(error )' > below the definition of TF_A_ARTIFACTS_SRC (wrapped by a 'ifdef > PTXCONF_TF_A'). done. > >> +else >> + @install -D -m644 $(TF_A_ARTIFACTS_SRC) $(IMAGEDIR) >> +endif >> + @$(call touch) >> + >> +# ---------------------------------------------------------------------------- >> +# Clean >> +# ---------------------------------------------------------------------------- >> + >> +$(STATEDIR)/tf-a.clean: >> + @$(call targetinfo) >> + @$(call clean_pkg, TF_A) >> + @rm -f $(addprefix $(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware/, \ >> + TF_A_ARTIFACTS_DEST) > > This is not necessary if you use pkgdir. dropped. > > Michael > >> + @rm -f $(addprefix $(IMAGEDIR), TF_A_ARTIFACTS_DEST) >> + >> +# vim: syntax=make >> -- >> 2.25.0 >> >> >> _______________________________________________ >> ptxdist mailing list >> ptxdist@pengutronix.de >> > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ ptxdist mailing list ptxdist@pengutronix.de