mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
* [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
@ 2020-02-12 16:40 Ahmad Fatoum
  2020-02-13 12:59 ` Guillermo Rodriguez Garcia
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-12 16:40 UTC (permalink / raw)
  To: ptxdist; +Cc: Alejandro Vazquez, Ahmad Fatoum

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.dev@gmail.com>
Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
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"
+	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"
+
+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.
+
+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.
+
+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 <r.czerwinski@pengutronix.de>
+#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
+#
+# 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) \
+	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)
+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)
+# TF_A_DEBUG=yes
+
+# ----------------------------------------------------------------------------
+# Prepare
+# ----------------------------------------------------------------------------
+$(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 \
+		$(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.)
+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)
+	@rm -f $(addprefix $(IMAGEDIR), TF_A_ARTIFACTS_DEST)
+
+# vim: syntax=make
-- 
2.25.0


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-12 16:40 [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A Ahmad Fatoum
@ 2020-02-13 12:59 ` Guillermo Rodriguez Garcia
  2020-02-13 13:30   ` Ahmad Fatoum
  2020-02-14 13:19 ` Michael Olbrich
  2020-02-17 16:25 ` Michael Tretter
  2 siblings, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-13 12:59 UTC (permalink / raw)
  To: ptxdist; +Cc: Alejandro Vazquez, Ahmad Fatoum

Hi Ahmad,

El mié., 12 feb. 2020 a las 17:40, Ahmad Fatoum
(<a.fatoum@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.dev@gmail.com>
> Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> 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"
> +       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"
> +
> +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.
> +
> +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.
> +
> +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 <r.czerwinski@pengutronix.de>
> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> +#
> +# 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) \
> +       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)
> +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)
> +# TF_A_DEBUG=yes
> +
> +# ----------------------------------------------------------------------------
> +# Prepare
> +# ----------------------------------------------------------------------------
> +$(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 \

This doesn't look right.
Shouldn't the install stage install things to the package install
directory only?
And, in case you want to install something somewhere else, shouldn't
the actual target directory at least be configurable?
For example there is no /usr/lib/firmware in my platform.

Guillermo

> +               $(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.)
> +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)
> +       @rm -f $(addprefix $(IMAGEDIR), TF_A_ARTIFACTS_DEST)
> +
> +# vim: syntax=make
> --
> 2.25.0
>
>
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de



-- 
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 12:59 ` Guillermo Rodriguez Garcia
@ 2020-02-13 13:30   ` Ahmad Fatoum
  2020-02-13 14:25     ` Guillermo Rodriguez Garcia
  0 siblings, 1 reply; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-13 13:30 UTC (permalink / raw)
  To: Guillermo Rodriguez Garcia, ptxdist; +Cc: Alejandro Vazquez

On 2/13/20 1:59 PM, Guillermo Rodriguez Garcia wrote:
> Hi Ahmad,
> 
> El mié., 12 feb. 2020 a las 17:40, Ahmad Fatoum
> (<a.fatoum@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.dev@gmail.com>
>> Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> 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"
>> +       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"
>> +
>> +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.
>> +
>> +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.
>> +
>> +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 <r.czerwinski@pengutronix.de>
>> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
>> +#
>> +# 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) \
>> +       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)
>> +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)
>> +# TF_A_DEBUG=yes
>> +
>> +# ----------------------------------------------------------------------------
>> +# Prepare
>> +# ----------------------------------------------------------------------------
>> +$(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 \
> 
> This doesn't look right.
> Shouldn't the install stage install things to the package install
> directory only?
> And, in case you want to install something somewhere else, shouldn't
> the actual target directory at least be configurable?
> For example there is no /usr/lib/firmware in my platform.

That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
on some i.MX8). It's not installed in the target rootfs.

> 
> Guillermo
> 
>> +               $(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.)
>> +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)
>> +       @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                       | https://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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 13:30   ` Ahmad Fatoum
@ 2020-02-13 14:25     ` Guillermo Rodriguez Garcia
  2020-02-13 14:28       ` Ahmad Fatoum
  0 siblings, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-13 14:25 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Alejandro Vazquez, ptxdist

El jue., 13 feb. 2020 a las 14:30, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> On 2/13/20 1:59 PM, Guillermo Rodriguez Garcia wrote:
> > Hi Ahmad,
> >
> > El mié., 12 feb. 2020 a las 17:40, Ahmad Fatoum
> > (<a.fatoum@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.dev@gmail.com>
> >> Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >> 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"
> >> +       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"
> >> +
> >> +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.
> >> +
> >> +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.
> >> +
> >> +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 <r.czerwinski@pengutronix.de>
> >> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> +#
> >> +# 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) \
> >> +       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)
> >> +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)
> >> +# TF_A_DEBUG=yes
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Prepare
> >> +# ----------------------------------------------------------------------------
> >> +$(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 \
> >
> > This doesn't look right.
> > Shouldn't the install stage install things to the package install
> > directory only?
> > And, in case you want to install something somewhere else, shouldn't
> > the actual target directory at least be configurable?
> > For example there is no /usr/lib/firmware in my platform.
>
> That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
> on some i.MX8). It's not installed in the target rootfs.

Ok but that's specific to that particular configuration, and the
choice of directory also seems to be arbitrary.
Does this belong in a generic rules file ?

Guillermo

>
> >
> > Guillermo
> >
> >> +               $(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.)
> >> +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)
> >> +       @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                       | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



-- 
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 14:25     ` Guillermo Rodriguez Garcia
@ 2020-02-13 14:28       ` Ahmad Fatoum
  2020-02-13 14:32         ` Ahmad Fatoum
  0 siblings, 1 reply; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-13 14:28 UTC (permalink / raw)
  To: Guillermo Rodriguez Garcia; +Cc: Alejandro Vazquez, ptxdist

On 2/13/20 3:25 PM, Guillermo Rodriguez Garcia wrote:
> El jue., 13 feb. 2020 a las 14:30, Ahmad Fatoum
> (<a.fatoum@pengutronix.de>) escribió:

>>>> +# ----------------------------------------------------------------------------
>>>> +# 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 \
>>>
>>> This doesn't look right.
>>> Shouldn't the install stage install things to the package install
>>> directory only?
>>> And, in case you want to install something somewhere else, shouldn't
>>> the actual target directory at least be configurable?
>>> For example there is no /usr/lib/firmware in my platform.
>>
>> That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
>> on some i.MX8). It's not installed in the target rootfs.
> 
> Ok but that's specific to that particular configuration, and the
> choice of directory also seems to be arbitrary.

You got to standardize on something. If this is good, a $(PTXCONF_SYSROOT_FIRMWARE)
might be a good thing to agree on.

> Does this belong in a generic rules file ?

How would that look like?


Cheers
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 14:28       ` Ahmad Fatoum
@ 2020-02-13 14:32         ` Ahmad Fatoum
  2020-02-13 14:35           ` Guillermo Rodriguez Garcia
  0 siblings, 1 reply; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-13 14:32 UTC (permalink / raw)
  To: Guillermo Rodriguez Garcia; +Cc: Alejandro Vazquez, ptxdist

On 2/13/20 3:28 PM, Ahmad Fatoum wrote:
> On 2/13/20 3:25 PM, Guillermo Rodriguez Garcia wrote:
>> El jue., 13 feb. 2020 a las 14:30, Ahmad Fatoum
>> (<a.fatoum@pengutronix.de>) escribió:
> 
>>>>> +# ----------------------------------------------------------------------------
>>>>> +# 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 \
>>>>
>>>> This doesn't look right.
>>>> Shouldn't the install stage install things to the package install
>>>> directory only?
>>>> And, in case you want to install something somewhere else, shouldn't
>>>> the actual target directory at least be configurable?
>>>> For example there is no /usr/lib/firmware in my platform.
>>>
>>> That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
>>> on some i.MX8). It's not installed in the target rootfs.
>>
>> Ok but that's specific to that particular configuration, and the
>> choice of directory also seems to be arbitrary.
> 
> You got to standardize on something. If this is good, a $(PTXCONF_SYSROOT_FIRMWARE)
> might be a good thing to agree on.
> 
>> Does this belong in a generic rules file ?
> 
> How would that look like?

Ah, you mean this is too specific that it shouldn't be in tf-a.make?

Well, Some Zynqs do this as well. If you are embedding OPTEE in TF-A, you will need
to save optee somewhere too. It's not a niche use case. The split between install
and target-install is just because so many vendors have their own idea of where the TF-A
comes from. If most of these can be made to work by installing the file to sysroot along
with the IMAGEDIR, so why not do so?

Cheers
Ahmad

> 
> 
> Cheers
> Ahmad
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 14:32         ` Ahmad Fatoum
@ 2020-02-13 14:35           ` Guillermo Rodriguez Garcia
  2020-02-13 14:44             ` Ahmad Fatoum
  0 siblings, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-13 14:35 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Alejandro Vazquez, ptxdist

El jue., 13 feb. 2020 a las 15:32, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> On 2/13/20 3:28 PM, Ahmad Fatoum wrote:
> > On 2/13/20 3:25 PM, Guillermo Rodriguez Garcia wrote:
> >> El jue., 13 feb. 2020 a las 14:30, Ahmad Fatoum
> >> (<a.fatoum@pengutronix.de>) escribió:
> >
> >>>>> +# ----------------------------------------------------------------------------
> >>>>> +# 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 \
> >>>>
> >>>> This doesn't look right.
> >>>> Shouldn't the install stage install things to the package install
> >>>> directory only?
> >>>> And, in case you want to install something somewhere else, shouldn't
> >>>> the actual target directory at least be configurable?
> >>>> For example there is no /usr/lib/firmware in my platform.
> >>>
> >>> That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
> >>> on some i.MX8). It's not installed in the target rootfs.
> >>
> >> Ok but that's specific to that particular configuration, and the
> >> choice of directory also seems to be arbitrary.
> >
> > You got to standardize on something. If this is good, a $(PTXCONF_SYSROOT_FIRMWARE)
> > might be a good thing to agree on.
> >
> >> Does this belong in a generic rules file ?
> >
> > How would that look like?
>
> Ah, you mean this is too specific that it shouldn't be in tf-a.make?

Yes.

>
> Well, Some Zynqs do this as well. If you are embedding OPTEE in TF-A, you will need
> to save optee somewhere too. It's not a niche use case. The split between install

For cases like this (optee) you're going to need to amend tf-a.in
anyway because you need to add dependencies (you mentioned this
yourself in a previous email)

> and target-install is just because so many vendors have their own idea of where the TF-A
> comes from. If most of these can be made to work by installing the file to sysroot along
> with the IMAGEDIR, so why not do so?

If it really covers "most of these" use cases, then I guess it may make sense.

Still -- shouldn't it at least be optional?

Guillermo

>
> Cheers
> Ahmad
>
> >
> >
> > Cheers
> > Ahmad
> >
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



-- 
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 14:35           ` Guillermo Rodriguez Garcia
@ 2020-02-13 14:44             ` Ahmad Fatoum
  2020-02-13 14:49               ` Ahmad Fatoum
  2020-02-13 15:05               ` Guillermo Rodriguez Garcia
  0 siblings, 2 replies; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-13 14:44 UTC (permalink / raw)
  To: Guillermo Rodriguez Garcia; +Cc: Alejandro Vazquez, ptxdist

On 2/13/20 3:35 PM, Guillermo Rodriguez Garcia wrote:
> El jue., 13 feb. 2020 a las 15:32, Ahmad Fatoum
> (<a.fatoum@pengutronix.de>) escribió:
>>
>> On 2/13/20 3:28 PM, Ahmad Fatoum wrote:
>>> On 2/13/20 3:25 PM, Guillermo Rodriguez Garcia wrote:
>>>> El jue., 13 feb. 2020 a las 14:30, Ahmad Fatoum
>>>> (<a.fatoum@pengutronix.de>) escribió:
>>>
>>>>>>> +# ----------------------------------------------------------------------------
>>>>>>> +# 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 \
>>>>>>
>>>>>> This doesn't look right.
>>>>>> Shouldn't the install stage install things to the package install
>>>>>> directory only?
>>>>>> And, in case you want to install something somewhere else, shouldn't
>>>>>> the actual target directory at least be configurable?
>>>>>> For example there is no /usr/lib/firmware in my platform.
>>>>>
>>>>> That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
>>>>> on some i.MX8). It's not installed in the target rootfs.
>>>>
>>>> Ok but that's specific to that particular configuration, and the
>>>> choice of directory also seems to be arbitrary.
>>>
>>> You got to standardize on something. If this is good, a $(PTXCONF_SYSROOT_FIRMWARE)
>>> might be a good thing to agree on.
>>>
>>>> Does this belong in a generic rules file ?
>>>
>>> How would that look like?
>>
>> Ah, you mean this is too specific that it shouldn't be in tf-a.make?
> 
> Yes.
> 
>>
>> Well, Some Zynqs do this as well. If you are embedding OPTEE in TF-A, you will need
>> to save optee somewhere too. It's not a niche use case. The split between install
> 
> For cases like this (optee) you're going to need to amend tf-a.in
> anyway because you need to add dependencies (you mentioned this
> yourself in a previous email)
> 
>> and target-install is just because so many vendors have their own idea of where the TF-A
>> comes from. If most of these can be made to work by installing the file to sysroot along
>> with the IMAGEDIR, so why not do so?
> 
> If it really covers "most of these" use cases, then I guess it may make sense.

if it's consumed by image rules, it's in IMAGEDIR. If it's consumed by "normal" rules,
it's in the sysroot at a fixed place. Not sure if there is another use case I am missing.

> 
> Still -- shouldn't it at least be optional?

Why should the .install stage be behind an option, but the .targetinstall not?
Should there be options for enabling/disabling each one of them?

And why? Just to save a few hundred kilobytes extra in the _build_ directory?
Doesn't sound like a good enough reason to me.

Cheers
Ahmad

> 
> Guillermo
> 
>>
>> Cheers
>> Ahmad
>>
>>>
>>>
>>> Cheers
>>> Ahmad
>>>
>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 14:44             ` Ahmad Fatoum
@ 2020-02-13 14:49               ` Ahmad Fatoum
  2020-02-13 15:05               ` Guillermo Rodriguez Garcia
  1 sibling, 0 replies; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-13 14:49 UTC (permalink / raw)
  To: ptxdist

(Sorry missed this one)

On 2/13/20 3:44 PM, Ahmad Fatoum wrote:
> For cases like this (optee) you're going to need to amend tf-a.in
> anyway because you need to add dependencies (you mentioned this
> yourself in a previous email)

I mean that the already existing optee rule would need to have
an install stage as well, if tf-a wants to use it, so installation
to build sysroot, but not IMAGEDIR is not something that's specific
to only tf-a.make introduced here.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 14:44             ` Ahmad Fatoum
  2020-02-13 14:49               ` Ahmad Fatoum
@ 2020-02-13 15:05               ` Guillermo Rodriguez Garcia
  2020-02-13 15:08                 ` Ahmad Fatoum
  1 sibling, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-13 15:05 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Alejandro Vazquez, ptxdist

El jue., 13 feb. 2020 a las 15:44, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> On 2/13/20 3:35 PM, Guillermo Rodriguez Garcia wrote:
> > El jue., 13 feb. 2020 a las 15:32, Ahmad Fatoum
> > (<a.fatoum@pengutronix.de>) escribió:
> >>
> >> On 2/13/20 3:28 PM, Ahmad Fatoum wrote:
> >>> On 2/13/20 3:25 PM, Guillermo Rodriguez Garcia wrote:
> >>>> El jue., 13 feb. 2020 a las 14:30, Ahmad Fatoum
> >>>> (<a.fatoum@pengutronix.de>) escribió:
> >>>
> >>>>>>> +# ----------------------------------------------------------------------------
> >>>>>>> +# 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 \
> >>>>>>
> >>>>>> This doesn't look right.
> >>>>>> Shouldn't the install stage install things to the package install
> >>>>>> directory only?
> >>>>>> And, in case you want to install something somewhere else, shouldn't
> >>>>>> the actual target directory at least be configurable?
> >>>>>> For example there is no /usr/lib/firmware in my platform.
> >>>>>
> >>>>> That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
> >>>>> on some i.MX8). It's not installed in the target rootfs.
> >>>>
> >>>> Ok but that's specific to that particular configuration, and the
> >>>> choice of directory also seems to be arbitrary.
> >>>
> >>> You got to standardize on something. If this is good, a $(PTXCONF_SYSROOT_FIRMWARE)
> >>> might be a good thing to agree on.
> >>>
> >>>> Does this belong in a generic rules file ?
> >>>
> >>> How would that look like?
> >>
> >> Ah, you mean this is too specific that it shouldn't be in tf-a.make?
> >
> > Yes.
> >
> >>
> >> Well, Some Zynqs do this as well. If you are embedding OPTEE in TF-A, you will need
> >> to save optee somewhere too. It's not a niche use case. The split between install
> >
> > For cases like this (optee) you're going to need to amend tf-a.in
> > anyway because you need to add dependencies (you mentioned this
> > yourself in a previous email)
> >
> >> and target-install is just because so many vendors have their own idea of where the TF-A
> >> comes from. If most of these can be made to work by installing the file to sysroot along
> >> with the IMAGEDIR, so why not do so?
> >
> > If it really covers "most of these" use cases, then I guess it may make sense.
>
> if it's consumed by image rules, it's in IMAGEDIR. If it's consumed by "normal" rules,
> it's in the sysroot at a fixed place. Not sure if there is another use case I am missing.
>
> >
> > Still -- shouldn't it at least be optional?
>
> Why should the .install stage be behind an option, but the .targetinstall not?
> Should there be options for enabling/disabling each one of them?
>
> And why? Just to save a few hundred kilobytes extra in the _build_ directory?
> Doesn't sound like a good enough reason to me.

OK this makes sense.

But, you should at least make sure
$(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware exists before trying to
copy anything into it.

Guillermo

>
> Cheers
> Ahmad
>
> >
> > Guillermo
> >
> >>
> >> Cheers
> >> Ahmad
> >>
> >>>
> >>>
> >>> Cheers
> >>> Ahmad
> >>>
> >>
> >>
> >> --
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >
> >
> >
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



-- 
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 15:05               ` Guillermo Rodriguez Garcia
@ 2020-02-13 15:08                 ` Ahmad Fatoum
  2020-02-13 15:24                   ` Guillermo Rodriguez Garcia
  0 siblings, 1 reply; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-13 15:08 UTC (permalink / raw)
  To: Guillermo Rodriguez Garcia; +Cc: Alejandro Vazquez, ptxdist

On 2/13/20 4:05 PM, Guillermo Rodriguez Garcia wrote:

>>>>>
>>>>>>>>> +# ----------------------------------------------------------------------------
>>>>>>>>> +# 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 \
>>>>>>>>

> OK this makes sense.
> 
> But, you should at least make sure
> $(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware exists before trying to
> copy anything into it.

That's why -D is there. From man install(1):

-D     create all leading components of DEST except the last, or all components of
	--target-directory, then copy SOURCE to DEST

Cheers
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 15:08                 ` Ahmad Fatoum
@ 2020-02-13 15:24                   ` Guillermo Rodriguez Garcia
  2020-02-13 15:27                     ` Ahmad Fatoum
  0 siblings, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-13 15:24 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Alejandro Vazquez, ptxdist

El jue., 13 feb. 2020 a las 16:08, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> On 2/13/20 4:05 PM, Guillermo Rodriguez Garcia wrote:
>
> >>>>>
> >>>>>>>>> +# ----------------------------------------------------------------------------
> >>>>>>>>> +# 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 \
> >>>>>>>>
>
> > OK this makes sense.
> >
> > But, you should at least make sure
> > $(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware exists before trying to
> > copy anything into it.
>
> That's why -D is there. From man install(1):
>
> -D     create all leading components of DEST except the last, or all components of
>         --target-directory, then copy SOURCE to DEST

Yes but the effect of -D combined with --target-directory does not
seem to be standard; my version of install does not support it.
The man page for my version says:

-D     create all leading components of DEST except the last, then
copy SOURCE to DEST

And a quick test yields the following:

$ touch test
$ install -D --target-directory a/b/c test
install: failed to access 'a/b/c': No such file or director

I guess it's better to avoid relying on that behaviour and create the
directory explicitly instead.

BR,

Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 15:24                   ` Guillermo Rodriguez Garcia
@ 2020-02-13 15:27                     ` Ahmad Fatoum
  2020-02-13 15:33                       ` Guillermo Rodriguez Garcia
  0 siblings, 1 reply; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-13 15:27 UTC (permalink / raw)
  To: Guillermo Rodriguez Garcia; +Cc: Alejandro Vazquez, ptxdist

On 2/13/20 4:24 PM, Guillermo Rodriguez Garcia wrote:
> El jue., 13 feb. 2020 a las 16:08, Ahmad Fatoum
> (<a.fatoum@pengutronix.de>) escribió:
>>
>> On 2/13/20 4:05 PM, Guillermo Rodriguez Garcia wrote:
>>
>>>>>>>
>>>>>>>>>>> +# ----------------------------------------------------------------------------
>>>>>>>>>>> +# 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 \
>>>>>>>>>>
>>
>>> OK this makes sense.
>>>
>>> But, you should at least make sure
>>> $(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware exists before trying to
>>> copy anything into it.
>>
>> That's why -D is there. From man install(1):
>>
>> -D     create all leading components of DEST except the last, or all components of
>>         --target-directory, then copy SOURCE to DEST
> 
> Yes but the effect of -D combined with --target-directory does not
> seem to be standard; my version of install does not support it.
> The man page for my version says:
> 
> -D     create all leading components of DEST except the last, then
> copy SOURCE to DEST
> 
> And a quick test yields the following:
> 
> $ touch test
> $ install -D --target-directory a/b/c test
> install: failed to access 'a/b/c': No such file or director
> 
> I guess it's better to avoid relying on that behaviour and create the
> directory explicitly instead.

I see. Will replace with separate install -d in a v3 then.

> 
> BR,
> 
> Guillermo Rodriguez Garcia
> guille.rodriguez@gmail.com
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 15:27                     ` Ahmad Fatoum
@ 2020-02-13 15:33                       ` Guillermo Rodriguez Garcia
  2020-02-14 10:57                         ` Alex Vazquez
  0 siblings, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-13 15:33 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Alejandro Vazquez, ptxdist

El jue., 13 feb. 2020 a las 16:27, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> On 2/13/20 4:24 PM, Guillermo Rodriguez Garcia wrote:
> > El jue., 13 feb. 2020 a las 16:08, Ahmad Fatoum
> > (<a.fatoum@pengutronix.de>) escribió:
> >>
> >> On 2/13/20 4:05 PM, Guillermo Rodriguez Garcia wrote:
> >>
> >>>>>>>
> >>>>>>>>>>> +# ----------------------------------------------------------------------------
> >>>>>>>>>>> +# 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 \
> >>>>>>>>>>
> >>
> >>> OK this makes sense.
> >>>
> >>> But, you should at least make sure
> >>> $(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware exists before trying to
> >>> copy anything into it.
> >>
> >> That's why -D is there. From man install(1):
> >>
> >> -D     create all leading components of DEST except the last, or all components of
> >>         --target-directory, then copy SOURCE to DEST
> >
> > Yes but the effect of -D combined with --target-directory does not
> > seem to be standard; my version of install does not support it.
> > The man page for my version says:
> >
> > -D     create all leading components of DEST except the last, then
> > copy SOURCE to DEST
> >
> > And a quick test yields the following:
> >
> > $ touch test
> > $ install -D --target-directory a/b/c test
> > install: failed to access 'a/b/c': No such file or director
> >
> > I guess it's better to avoid relying on that behaviour and create the
> > directory explicitly instead.
>
> I see. Will replace with separate install -d in a v3 then.

You might also want to update the targetinstall stage:

- If IMAGEDIR is guaranteed to exist when the targetinstall rule runs,
then the -D option can be dropped
- If IMAGEDIR is not guaranteed to exist, then -D will not create the
directory properly in the way the command is currently written, and
you probably want also explicitly create the directory

Thx,

Guillermo

>
> >
> > BR,
> >
> > Guillermo Rodriguez Garcia
> > guille.rodriguez@gmail.com
> >
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



-- 
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-13 15:33                       ` Guillermo Rodriguez Garcia
@ 2020-02-14 10:57                         ` Alex Vazquez
  0 siblings, 0 replies; 22+ messages in thread
From: Alex Vazquez @ 2020-02-14 10:57 UTC (permalink / raw)
  To: Guillermo Rodriguez Garcia; +Cc: ptxdist, Ahmad Fatoum

Hi, Ahmad.

I took a look at your TF-A rules and I've checked that it generates
the binaries correctly.

Thanks! ;)

Cheers.
Alejandro.



El jue., 13 feb. 2020 a las 16:33, Guillermo Rodriguez Garcia
(<guille.rodriguez@gmail.com>) escribió:
>
> El jue., 13 feb. 2020 a las 16:27, Ahmad Fatoum
> (<a.fatoum@pengutronix.de>) escribió:
> >
> > On 2/13/20 4:24 PM, Guillermo Rodriguez Garcia wrote:
> > > El jue., 13 feb. 2020 a las 16:08, Ahmad Fatoum
> > > (<a.fatoum@pengutronix.de>) escribió:
> > >>
> > >> On 2/13/20 4:05 PM, Guillermo Rodriguez Garcia wrote:
> > >>
> > >>>>>>>
> > >>>>>>>>>>> +# ----------------------------------------------------------------------------
> > >>>>>>>>>>> +# 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 \
> > >>>>>>>>>>
> > >>
> > >>> OK this makes sense.
> > >>>
> > >>> But, you should at least make sure
> > >>> $(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware exists before trying to
> > >>> copy anything into it.
> > >>
> > >> That's why -D is there. From man install(1):
> > >>
> > >> -D     create all leading components of DEST except the last, or all components of
> > >>         --target-directory, then copy SOURCE to DEST
> > >
> > > Yes but the effect of -D combined with --target-directory does not
> > > seem to be standard; my version of install does not support it.
> > > The man page for my version says:
> > >
> > > -D     create all leading components of DEST except the last, then
> > > copy SOURCE to DEST
> > >
> > > And a quick test yields the following:
> > >
> > > $ touch test
> > > $ install -D --target-directory a/b/c test
> > > install: failed to access 'a/b/c': No such file or director
> > >
> > > I guess it's better to avoid relying on that behaviour and create the
> > > directory explicitly instead.
> >
> > I see. Will replace with separate install -d in a v3 then.
>
> You might also want to update the targetinstall stage:
>
> - If IMAGEDIR is guaranteed to exist when the targetinstall rule runs,
> then the -D option can be dropped
> - If IMAGEDIR is not guaranteed to exist, then -D will not create the
> directory properly in the way the command is currently written, and
> you probably want also explicitly create the directory
>
> Thx,
>
> Guillermo
>
> >
> > >
> > > BR,
> > >
> > > Guillermo Rodriguez Garcia
> > > guille.rodriguez@gmail.com
> > >
> >
> >
> > --
> > Pengutronix e.K.                           |                             |
> > Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
> > 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
>
>
> --
> Guillermo Rodriguez Garcia
> guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-12 16:40 [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A Ahmad Fatoum
  2020-02-13 12:59 ` Guillermo Rodriguez Garcia
@ 2020-02-14 13:19 ` Michael Olbrich
  2020-02-19 10:10   ` Ahmad Fatoum
  2020-02-17 16:25 ` Michael Tretter
  2 siblings, 1 reply; 22+ messages in thread
From: Michael Olbrich @ 2020-02-14 13:19 UTC (permalink / raw)
  To: ptxdist

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 <avazquez.dev@gmail.com>
> Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> 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 '-->'

> +	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.

> +
> +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.

> +
> +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'.

> +
> +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 <r.czerwinski@pengutronix.de>
> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> +#
> +# 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.

> +	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.

> +# TF_A_DEBUG=yes

remove. 

> +
> +# ----------------------------------------------------------------------------
> +# 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.

> +		$(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').

> +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.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-12 16:40 [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A Ahmad Fatoum
  2020-02-13 12:59 ` Guillermo Rodriguez Garcia
  2020-02-14 13:19 ` Michael Olbrich
@ 2020-02-17 16:25 ` Michael Tretter
  2020-02-17 16:33   ` Guillermo Rodriguez Garcia
  2 siblings, 1 reply; 22+ messages in thread
From: Michael Tretter @ 2020-02-17 16:25 UTC (permalink / raw)
  To: ptxdist

On Wed, 12 Feb 2020 17:40:33 +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).

I successfully built the TF-A BL31 for the ZynqMP using this rule.

However, I saw that the TF-A build uses the current git commitish for
BUILD_STRING, which will be written into the binary. If I build the
TF-A with this rule, this ends up to be the commitish of the BSP. I'm
not sure if I actually want this, but I don't know what to put there
instead.

Michael

> 
> Cc: Alejandro Vazquez <avazquez.dev@gmail.com>
> Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> 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

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-17 16:25 ` Michael Tretter
@ 2020-02-17 16:33   ` Guillermo Rodriguez Garcia
  2020-02-18  7:35     ` Michael Olbrich
  0 siblings, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-17 16:33 UTC (permalink / raw)
  To: ptxdist

Hi Michael,

El lun., 17 feb. 2020 a las 17:26, Michael Tretter
(<m.tretter@pengutronix.de>) escribió:
>
> On Wed, 12 Feb 2020 17:40:33 +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).
>
> I successfully built the TF-A BL31 for the ZynqMP using this rule.
>
> However, I saw that the TF-A build uses the current git commitish for
> BUILD_STRING, which will be written into the binary. If I build the
> TF-A with this rule, this ends up to be the commitish of the BSP. I'm
> not sure if I actually want this, but I don't know what to put there
> instead.

Since we finally agreed to keep TF_A_EXTRA_ARGS in the final TF-A rule
files, you can actually set BUILD_STRING to anything you want. Just
include BUILD_STRING=whatever in the TF_A_EXTRA_ARGS parameter.

Best,

Guillermo

>
> Michael
>
> >
> > Cc: Alejandro Vazquez <avazquez.dev@gmail.com>
> > Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> > 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
>
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de



-- 
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-17 16:33   ` Guillermo Rodriguez Garcia
@ 2020-02-18  7:35     ` Michael Olbrich
  2020-02-18  9:02       ` Guillermo Rodriguez Garcia
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Olbrich @ 2020-02-18  7:35 UTC (permalink / raw)
  To: ptxdist

On Mon, Feb 17, 2020 at 05:33:57PM +0100, Guillermo Rodriguez Garcia wrote:
> El lun., 17 feb. 2020 a las 17:26, Michael Tretter
> (<m.tretter@pengutronix.de>) escribió:
> >
> > On Wed, 12 Feb 2020 17:40:33 +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).
> >
> > I successfully built the TF-A BL31 for the ZynqMP using this rule.
> >
> > However, I saw that the TF-A build uses the current git commitish for
> > BUILD_STRING, which will be written into the binary. If I build the
> > TF-A with this rule, this ends up to be the commitish of the BSP. I'm
> > not sure if I actually want this, but I don't know what to put there
> > instead.
> 
> Since we finally agreed to keep TF_A_EXTRA_ARGS in the final TF-A rule
> files, you can actually set BUILD_STRING to anything you want. Just
> include BUILD_STRING=whatever in the TF_A_EXTRA_ARGS parameter.

There should always be a default BUILD_STRING that does not depend on the
BSP commit-ish. $(TF_A_VERSION) probably.

Michael

-- 
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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-18  7:35     ` Michael Olbrich
@ 2020-02-18  9:02       ` Guillermo Rodriguez Garcia
  2020-02-19  7:48         ` Michael Tretter
  0 siblings, 1 reply; 22+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-18  9:02 UTC (permalink / raw)
  To: ptxdist

El mar., 18 feb. 2020 a las 8:36, Michael Olbrich
(<m.olbrich@pengutronix.de>) escribió:
>
> On Mon, Feb 17, 2020 at 05:33:57PM +0100, Guillermo Rodriguez Garcia wrote:
> > El lun., 17 feb. 2020 a las 17:26, Michael Tretter
> > (<m.tretter@pengutronix.de>) escribió:
> > >
> > > On Wed, 12 Feb 2020 17:40:33 +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).
> > >
> > > I successfully built the TF-A BL31 for the ZynqMP using this rule.
> > >
> > > However, I saw that the TF-A build uses the current git commitish for
> > > BUILD_STRING, which will be written into the binary. If I build the
> > > TF-A with this rule, this ends up to be the commitish of the BSP. I'm
> > > not sure if I actually want this, but I don't know what to put there
> > > instead.
> >
> > Since we finally agreed to keep TF_A_EXTRA_ARGS in the final TF-A rule
> > files, you can actually set BUILD_STRING to anything you want. Just
> > include BUILD_STRING=whatever in the TF_A_EXTRA_ARGS parameter.
>
> There should always be a default BUILD_STRING that does not depend on the
> BSP commit-ish. $(TF_A_VERSION) probably.

OK, then we could add this to the .make file:

TF_A_MAKE_OPT        := \
    CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
    HOSTCC=$(HOSTCC) \
    PLAT=$(PTXCONF_TF_A_PLATFORM) \
    ARCH=$(PTXCONF_TF_A_ARCH_STRING) \
    ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR) \
    BUILD_STRING=$(PTXCONF_TF_A_VERSION) \
    $(call remove_quotes,$(PTXCONF_TF_A_EXTRA_ARGS)) \
    all

BUILD_STRING can still be overriden in TF_A_EXTRA_ARGS if needed.

Guillermo

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-18  9:02       ` Guillermo Rodriguez Garcia
@ 2020-02-19  7:48         ` Michael Tretter
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Tretter @ 2020-02-19  7:48 UTC (permalink / raw)
  To: ptxdist

On Tue, 18 Feb 2020 10:02:57 +0100, Guillermo Rodriguez Garcia wrote:
> El mar., 18 feb. 2020 a las 8:36, Michael Olbrich
> (<m.olbrich@pengutronix.de>) escribió:
> >
> > On Mon, Feb 17, 2020 at 05:33:57PM +0100, Guillermo Rodriguez Garcia wrote:  
> > > El lun., 17 feb. 2020 a las 17:26, Michael Tretter
> > > (<m.tretter@pengutronix.de>) escribió:  
> > > >
> > > > On Wed, 12 Feb 2020 17:40:33 +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).  
> > > >
> > > > I successfully built the TF-A BL31 for the ZynqMP using this rule.
> > > >
> > > > However, I saw that the TF-A build uses the current git commitish for
> > > > BUILD_STRING, which will be written into the binary. If I build the
> > > > TF-A with this rule, this ends up to be the commitish of the BSP. I'm
> > > > not sure if I actually want this, but I don't know what to put there
> > > > instead.  
> > >
> > > Since we finally agreed to keep TF_A_EXTRA_ARGS in the final TF-A rule
> > > files, you can actually set BUILD_STRING to anything you want. Just
> > > include BUILD_STRING=whatever in the TF_A_EXTRA_ARGS parameter.  
> >
> > There should always be a default BUILD_STRING that does not depend on the
> > BSP commit-ish. $(TF_A_VERSION) probably.  
> 
> OK, then we could add this to the .make file:
> 
> TF_A_MAKE_OPT        := \
>     CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
>     HOSTCC=$(HOSTCC) \
>     PLAT=$(PTXCONF_TF_A_PLATFORM) \
>     ARCH=$(PTXCONF_TF_A_ARCH_STRING) \
>     ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR) \
>     BUILD_STRING=$(PTXCONF_TF_A_VERSION) \

The version string that is printed during the boot will then be
something like "v2.2(Release):v2.2". The first version is the hard
coded version in the makefile and the second is the git commitish that
was used to build the tf-a. Seems OK to me.

Michael

>     $(call remove_quotes,$(PTXCONF_TF_A_EXTRA_ARGS)) \
>     all
> 
> BUILD_STRING can still be overriden in TF_A_EXTRA_ARGS if needed.
> 
> Guillermo
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A
  2020-02-14 13:19 ` Michael Olbrich
@ 2020-02-19 10:10   ` Ahmad Fatoum
  0 siblings, 0 replies; 22+ messages in thread
From: Ahmad Fatoum @ 2020-02-19 10:10 UTC (permalink / raw)
  To: ptxdist

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 <avazquez.dev@gmail.com>
>> Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> 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 <r.czerwinski@pengutronix.de>
>> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
>> +#
>> +# 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-02-19 10:10 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-12 16:40 [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A Ahmad Fatoum
2020-02-13 12:59 ` Guillermo Rodriguez Garcia
2020-02-13 13:30   ` Ahmad Fatoum
2020-02-13 14:25     ` Guillermo Rodriguez Garcia
2020-02-13 14:28       ` Ahmad Fatoum
2020-02-13 14:32         ` Ahmad Fatoum
2020-02-13 14:35           ` Guillermo Rodriguez Garcia
2020-02-13 14:44             ` Ahmad Fatoum
2020-02-13 14:49               ` Ahmad Fatoum
2020-02-13 15:05               ` Guillermo Rodriguez Garcia
2020-02-13 15:08                 ` Ahmad Fatoum
2020-02-13 15:24                   ` Guillermo Rodriguez Garcia
2020-02-13 15:27                     ` Ahmad Fatoum
2020-02-13 15:33                       ` Guillermo Rodriguez Garcia
2020-02-14 10:57                         ` Alex Vazquez
2020-02-14 13:19 ` Michael Olbrich
2020-02-19 10:10   ` Ahmad Fatoum
2020-02-17 16:25 ` Michael Tretter
2020-02-17 16:33   ` Guillermo Rodriguez Garcia
2020-02-18  7:35     ` Michael Olbrich
2020-02-18  9:02       ` Guillermo Rodriguez Garcia
2020-02-19  7:48         ` Michael Tretter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox