Hi Ahmad, El mar., 11 feb. 2020 a las 16:22, Ahmad Fatoum (<a.fat...@pengutronix.de>) escribió: > > Hi, > > On 2/11/20 9:37 AM, Guillermo Rodriguez Garcia wrote: > > Hi Ahmad, > > > > Some other questions and comments, please see below. > > > > El lun., 10 feb. 2020 a las 17:16, Ahmad Fatoum > > (<a.fat...@pengutronix.de>) escribió: > >> > >> 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 <avazquez....@gmail.com> > >> Signed-off-by: Rouven Czerwinski <rou...@czerwinskis.de> > >> Signed-off-by: Ahmad Fatoum <a.fat...@pengutronix.de> > >> --- > >> platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++ > >> rules/tf-a.make | 123 ++++++++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 236 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..5aa4ca473c35 > >> --- /dev/null > >> +++ b/platforms/tf-a.in > >> @@ -0,0 +1,113 @@ > >> +## SECTION=bootloader > >> + > >> +menuconfig TF_A > >> + select BOOTLOADER > >> + prompt "ARM Trusted Firmware-A" > >> + 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 > >> + > >> +config TF_A_ARM_ARCH_MAJOR > >> + int > >> + default "7" if ARCH_ARM > >> + default "8" if ARCH_ARM64 > > > > Shouldn't this be configurable? > > aarch64 is always v8, but for aarch32 you can have either v7 or v8. > > Ah, indeed. Didn't know about cores like the Cortex-A32. > I will make this configurable. > > >> + > >> +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" > >> + > >> +config TF_A_MD5 > >> + string > >> + default "bb300e5a62c911e189c80d935d497a4b" > >> + prompt "TF-A source md5" > >> + > >> +config TF_A_PLATFORM > >> + string > >> + prompt "TF-A target platform" > >> + help > >> + The TF-A target platform. > >> + > >> +if ARCH_ARM64 > >> +config TF_A_ARM_ARCH_MINOR > >> + int > >> + default 0 > >> + prompt "TF-A target ARMv8.MINOR version" > >> + help > >> + The minor version of the ARMv8 architecture targeted. Defaults > >> to 0. > >> +endif > >> + > >> +config TF_A_DTB > >> + string > >> + prompt "TF-A DTB file name" > >> + help > >> + Device Tree Binary file name > >> + > >> +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. > >> + > >> +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 > > > > No way to specify other payloads? > > I don't use any others at the moment, so no. > If you want to use e.g. OP-TEE, you can amend the the tf-a.in. > A string prompt won't save you the effort of doing that, because you > would need to specify dependencies anyway. > > > > >> + > >> +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..9ee554476927 > >> --- /dev/null > >> +++ b/rules/tf-a.make > >> @@ -0,0 +1,123 @@ > >> +# -*-makefile-*- > >> +# > >> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwin...@pengutronix.de> > >> +# 2019 by Ahmad Fatoum <a.fat...@pengutronix.de> > >> +# > >> +# See CREDITS for details about who has contributed to this project. > >> +# > >> +# 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 := \ > >> + V=$(PTXDIST_VERBOSE) \ > >> + CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \ > >> + HOSTCC=$(HOSTCC) > >> +TF_A_CONF_TOOL := NO > >> +TF_A_MAKE_ENV := $(CROSS_ENV) > > > > Do you need $(CROSS_ENV) here? (not used in e.g. u-boot.make) > > Yes, u-boot's Kbuild is known to ptxdist, so we don't > need to specify it explicitly.
Uhm, here I didn't need to specify CROSS_ENV and things seemed to work just fine. > > >> +$(STATEDIR)/tf-a.prepare: > >> + @$(call targetinfo) > >> + @$(call touch) > > > > I think this does nothing and can be removed. > > Right. See tf-a.compile below > > > > >> + > >> +# > >> ---------------------------------------------------------------------------- > >> +# Compile > >> +# > >> ---------------------------------------------------------------------------- > >> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM) > >> + > >> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0) > >> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release) > >> + > >> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING) > >> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1 I think this should not be hardcoded. (Sorry, didn't spot this one before) > >> +ifdef PTXCONF_TF_A_BL32_TSP > >> +TF_A_MAKE_OPT += > >> ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING) > >> +endif > >> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR) > >> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR > >> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR) > >> +endif > >> + > >> +ifneq ($(PTXCONF_TF_A_DTB),) > >> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB) > >> +endif > >> + > >> +ifdef PTXCONF_TF_A_BL32_SP_MIN > >> +TF_A_MAKE_OPT += AARCH32_SP=sp_min > >> +endif > >> + > >> +TF_A_MAKE_OPT += all > >> + > >> +TF_A_ARTIFACTS = $(addprefix > >> $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \ > >> + $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS))) > >> + > >> +$(STATEDIR)/tf-a.compile: > >> + @$(call targetinfo) > >> + @rm -rf $(TF_A_DIR)/build/ > >> + @$(call world/compile, TF_A) > >> + @$(call touch) > > > > Can't you use the default compile rule here? > > Yes. I will move the deletion of the build directory to the prepare stage. > Seems like old versions of TF-A had problems when reconfiguring without a > clean? Not sure; I have not seen such problems myself. > > >> + > >> +# > >> ---------------------------------------------------------------------------- > >> +# Install > >> +# > >> ---------------------------------------------------------------------------- > >> + > >> +$(STATEDIR)/tf-a.install: > >> + @$(call targetinfo) > >> + > >> +ifeq ($(TF_A_ARTIFACTS),) > >> + $(warning TF_A_ARTIFACTS is empty. nothing to install.) > >> +else > >> + @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR) > >> +endif > > > > Shouldn't this (copying files to IMAGEDIR) happen in targetinstall > > rather than install? Again based on what is being done for other > > bootloaders. > > On some systems, the BootROM doesn't directly invoke the TF-A, but instead it > is > a payload embedded into another bootloader. For this to work, we need to do it > in the install stage, so the bootloader rule can depend on this and have the > artifact available. > > That was the original line of thought, but apparently, how I did it is not > completely sound. I should instead have both an install and targetinstall: > > - install installs into sysroot for use by other non-image rules > - targetinstall installs into IMAGEDIR for use by image rules > > I will incorporate these changes along with your other suggestions in a v2. Thank you, Guillermo > > Thanks! > Ahmad > > > > > Thanks, > > > > Guillermo > > > >> + > >> + @$(call touch) > >> + > >> +# > >> ---------------------------------------------------------------------------- > >> +# Target-Install > >> +# > >> ---------------------------------------------------------------------------- > >> + > >> +$(STATEDIR)/tf-a.targetinstall: > >> + @$(call targetinfo) > >> + @$(call touch) > >> + > >> +# > >> ---------------------------------------------------------------------------- > >> +# Clean > >> +# > >> ---------------------------------------------------------------------------- > >> + > >> +$(STATEDIR)/tf-a.clean: > >> + @$(call targetinfo) > >> + @$(call clean_pkg, TF_A) > >> + @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin > >> + > >> +# 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 | -- Guillermo Rodriguez Garcia guille.rodrig...@gmail.com _______________________________________________ ptxdist mailing list ptxdist@pengutronix.de