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

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>
---
Cc: Guillermo Rodriguez Garcia <guille.rodriguez@gmail.com>

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 (Guillermo)
 - 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 (Guillermo)
 - fix clean stage to delete proper artifacts (Guillermo)
 - remove LOAD_IMAGE_V2=1 make option (Guillermo)
---
 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] 14+ messages in thread

* Re: [ptxdist] [PATCH] tf-a: new package for ARM trusted firmware A
  2020-02-12 16:36 [ptxdist] [PATCH] tf-a: new package for ARM trusted firmware A Ahmad Fatoum
@ 2020-02-12 16:38 ` Ahmad Fatoum
  0 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2020-02-12 16:38 UTC (permalink / raw)
  To: ptxdist; +Cc: Alejandro Vazquez, Guillermo Rodriguez Garcia

On 2/12/20 5:36 PM, 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>
> ---
> Cc: Guillermo Rodriguez Garcia <guille.rodriguez@gmail.com>
> 
> v1 -> v2:

That's v2.. I'll resend.


-- 
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] 14+ messages in thread

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

El mié., 12 feb. 2020 a las 10:28, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> Hi,
>
> On 2/12/20 10:07 AM, Guillermo Rodriguez Garcia wrote:
> >>> Uhm, here I didn't need to specify CROSS_ENV and things seemed to work
> >>> just fine.
> >>
> >> CROSS_ENV sets stuff like CC and LD. I took a look inside the v2.2 TF-A Makefile
> >> and on line 808 there is a reference to $(CC) before it's defined,
> >
> > That would be a bug in the Makefile, and if this was the case,
> > shouldn't we add a
> > patch to fix the Makefile (and possibly submit it to upstream),
> > instead of trying to
> > work around the bug by setting CROSS_ENV ?
> >
> > Anyway I am not sure this is actually a bug -- I see that CC is
> > defined in line 160
> > in v2.2 of the Makefile; that should happen before the usage in line 808.
>
> Sorry, see line 93, which happens before the definition.

Ah, yes. That indeed looks like a bug.
>
> >
> >> so in that case it
> >> would come from the environment. It's probably unintended, but to account for
> >> any other scripts that may reference LD, CC and the like without defining them
> >> first, I guess it's better to have CROSS_ENV in. Do you agree?
> >
> > I guess keeping it can not do any harm. However it can also hide bugs,
> > so I am not
> > sure what is best.
>
> yes, it's a bug and should be fixed. But this might not be the only instance,
> so we can either:
>
> - Not do anything
> - Give CC and friends a proper value

Yes, or:

3. Add a patch to fix the Makefile (as it is done in many other
ptxdist packages)

But anyway, as I said above, I don't have a strong opinion on this.
Setting CROSS_ENV works around the bug, which is both good and bad
(good for obvious reasons, bad because it sidesteps this kind of
issues rather than fixing them).

So, looking forward to v2 :-)

Best,

Guillermo

>
> I think #2 is reasonable.
>
> Cheers
> Ahmad
>
> >
> > Guillermo
> >
> >>
> >>>
> >>>>
> >>>>>> +$(STATEDIR)/tf-a.prepare:
> >>>>>> +       @$(call targetinfo)
> >>>>>> +       @$(call touch)
> >>>>>
> >>>>> I think this does nothing and can be removed.
> >>>>
> >>>> Right. See tf-a.compile below
> >>>>
> >>>>>
> >>>>>> +
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +# Compile
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
> >>>>>> +
> >>>>>> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
> >>>>>> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
> >>>>>> +
> >>>>>> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
> >>>>>> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
> >>>
> >>> I think this should not be hardcoded.
> >>>
> >>> (Sorry, didn't spot this one before)
> >>
> >> Will drop it.
> >>
> >>>
> >>>>>> +ifdef PTXCONF_TF_A_BL32_TSP
> >>>>>> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
> >>>>>> +endif
> >>>>>> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
> >>>>>> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
> >>>>>> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
> >>>>>> +endif
> >>>>>> +
> >>>>>> +ifneq ($(PTXCONF_TF_A_DTB),)
> >>>>>> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
> >>>>>> +endif
> >>>>>> +
> >>>>>> +ifdef PTXCONF_TF_A_BL32_SP_MIN
> >>>>>> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
> >>>>>> +endif
> >>>>>> +
> >>>>>> +TF_A_MAKE_OPT += all
> >>>>>> +
> >>>>>> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
> >>>>>> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
> >>>>>> +
> >>>>>> +$(STATEDIR)/tf-a.compile:
> >>>>>> +       @$(call targetinfo)
> >>>>>> +       @rm -rf $(TF_A_DIR)/build/
> >>>>>> +       @$(call world/compile, TF_A)
> >>>>>> +       @$(call touch)
> >>>>>
> >>>>> Can't you use the default compile rule here?
> >>>>
> >>>> Yes. I will move the deletion of the build directory to the prepare stage.
> >>>> Seems like old versions of TF-A had problems when reconfiguring without a clean?
> >>>
> >>> Not sure; I have not seen such problems myself.
> >>
> >> I've sent a question to the colleague who first added the line.
> >> Will keep it in for now.
> >>
> >>>
> >>>>
> >>>>>> +
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +# Install
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +
> >>>>>> +$(STATEDIR)/tf-a.install:
> >>>>>> +       @$(call targetinfo)
> >>>>>> +
> >>>>>> +ifeq ($(TF_A_ARTIFACTS),)
> >>>>>> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
> >>>>>> +else
> >>>>>> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
> >>>>>> +endif
> >>>>>
> >>>>> Shouldn't this (copying files to IMAGEDIR) happen in targetinstall
> >>>>> rather than install? Again based on what is being done for other
> >>>>> bootloaders.
> >>>>
> >>>> On some systems, the BootROM doesn't directly invoke the TF-A, but instead it is
> >>>> a payload embedded into another bootloader. For this to work, we need to do it
> >>>> in the install stage, so the bootloader rule can depend on this and have the
> >>>> artifact available.
> >>>>
> >>>> That was the original line of thought, but apparently, how I did it is not
> >>>> completely sound. I should instead have both an install and targetinstall:
> >>>>
> >>>> - install installs into sysroot for use by other non-image rules
> >>>> - targetinstall installs into IMAGEDIR for use by image rules
> >>>>
> >>>> I will incorporate these changes along with your other suggestions in a v2.
> >>>
> >>> Thank you,
> >>>
> >>> Guillermo
> >>>
> >>>>
> >>>> Thanks!
> >>>> Ahmad
> >>>>
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Guillermo
> >>>>>
> >>>>>> +
> >>>>>> +       @$(call touch)
> >>>>>> +
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +# Target-Install
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +
> >>>>>> +$(STATEDIR)/tf-a.targetinstall:
> >>>>>> +       @$(call targetinfo)
> >>>>>> +       @$(call touch)
> >>>>>> +
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +# Clean
> >>>>>> +# ----------------------------------------------------------------------------
> >>>>>> +
> >>>>>> +$(STATEDIR)/tf-a.clean:
> >>>>>> +       @$(call targetinfo)
> >>>>>> +       @$(call clean_pkg, TF_A)
> >>>>>> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
> >>>>>> +
> >>>>>> +# vim: syntax=make
> >>>>>> --
> >>>>>> 2.25.0
> >>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> ptxdist mailing list
> >>>>>> ptxdist@pengutronix.de
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>> --
> >>>> Pengutronix e.K.                           |                             |
> >>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>>
> >>>
> >>>
> >>
> >> --
> >> 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 |
> >
> >
> >
>
>
> --
> 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] 14+ messages in thread

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

Hi,

On 2/12/20 10:07 AM, Guillermo Rodriguez Garcia wrote:
>>> Uhm, here I didn't need to specify CROSS_ENV and things seemed to work
>>> just fine.
>>
>> CROSS_ENV sets stuff like CC and LD. I took a look inside the v2.2 TF-A Makefile
>> and on line 808 there is a reference to $(CC) before it's defined,
> 
> That would be a bug in the Makefile, and if this was the case,
> shouldn't we add a
> patch to fix the Makefile (and possibly submit it to upstream),
> instead of trying to
> work around the bug by setting CROSS_ENV ?
> 
> Anyway I am not sure this is actually a bug -- I see that CC is
> defined in line 160
> in v2.2 of the Makefile; that should happen before the usage in line 808.

Sorry, see line 93, which happens before the definition.

> 
>> so in that case it
>> would come from the environment. It's probably unintended, but to account for
>> any other scripts that may reference LD, CC and the like without defining them
>> first, I guess it's better to have CROSS_ENV in. Do you agree?
> 
> I guess keeping it can not do any harm. However it can also hide bugs,
> so I am not
> sure what is best.

yes, it's a bug and should be fixed. But this might not be the only instance,
so we can either:

- Not do anything
- Give CC and friends a proper value

I think #2 is reasonable.

Cheers
Ahmad

> 
> Guillermo
> 
>>
>>>
>>>>
>>>>>> +$(STATEDIR)/tf-a.prepare:
>>>>>> +       @$(call targetinfo)
>>>>>> +       @$(call touch)
>>>>>
>>>>> I think this does nothing and can be removed.
>>>>
>>>> Right. See tf-a.compile below
>>>>
>>>>>
>>>>>> +
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +# Compile
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
>>>>>> +
>>>>>> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
>>>>>> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
>>>>>> +
>>>>>> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
>>>>>> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
>>>
>>> I think this should not be hardcoded.
>>>
>>> (Sorry, didn't spot this one before)
>>
>> Will drop it.
>>
>>>
>>>>>> +ifdef PTXCONF_TF_A_BL32_TSP
>>>>>> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
>>>>>> +endif
>>>>>> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
>>>>>> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
>>>>>> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
>>>>>> +endif
>>>>>> +
>>>>>> +ifneq ($(PTXCONF_TF_A_DTB),)
>>>>>> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
>>>>>> +endif
>>>>>> +
>>>>>> +ifdef PTXCONF_TF_A_BL32_SP_MIN
>>>>>> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
>>>>>> +endif
>>>>>> +
>>>>>> +TF_A_MAKE_OPT += all
>>>>>> +
>>>>>> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
>>>>>> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
>>>>>> +
>>>>>> +$(STATEDIR)/tf-a.compile:
>>>>>> +       @$(call targetinfo)
>>>>>> +       @rm -rf $(TF_A_DIR)/build/
>>>>>> +       @$(call world/compile, TF_A)
>>>>>> +       @$(call touch)
>>>>>
>>>>> Can't you use the default compile rule here?
>>>>
>>>> Yes. I will move the deletion of the build directory to the prepare stage.
>>>> Seems like old versions of TF-A had problems when reconfiguring without a clean?
>>>
>>> Not sure; I have not seen such problems myself.
>>
>> I've sent a question to the colleague who first added the line.
>> Will keep it in for now.
>>
>>>
>>>>
>>>>>> +
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +# Install
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +
>>>>>> +$(STATEDIR)/tf-a.install:
>>>>>> +       @$(call targetinfo)
>>>>>> +
>>>>>> +ifeq ($(TF_A_ARTIFACTS),)
>>>>>> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
>>>>>> +else
>>>>>> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
>>>>>> +endif
>>>>>
>>>>> Shouldn't this (copying files to IMAGEDIR) happen in targetinstall
>>>>> rather than install? Again based on what is being done for other
>>>>> bootloaders.
>>>>
>>>> On some systems, the BootROM doesn't directly invoke the TF-A, but instead it is
>>>> a payload embedded into another bootloader. For this to work, we need to do it
>>>> in the install stage, so the bootloader rule can depend on this and have the
>>>> artifact available.
>>>>
>>>> That was the original line of thought, but apparently, how I did it is not
>>>> completely sound. I should instead have both an install and targetinstall:
>>>>
>>>> - install installs into sysroot for use by other non-image rules
>>>> - targetinstall installs into IMAGEDIR for use by image rules
>>>>
>>>> I will incorporate these changes along with your other suggestions in a v2.
>>>
>>> Thank you,
>>>
>>> Guillermo
>>>
>>>>
>>>> Thanks!
>>>> Ahmad
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Guillermo
>>>>>
>>>>>> +
>>>>>> +       @$(call touch)
>>>>>> +
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +# Target-Install
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +
>>>>>> +$(STATEDIR)/tf-a.targetinstall:
>>>>>> +       @$(call targetinfo)
>>>>>> +       @$(call touch)
>>>>>> +
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +# Clean
>>>>>> +# ----------------------------------------------------------------------------
>>>>>> +
>>>>>> +$(STATEDIR)/tf-a.clean:
>>>>>> +       @$(call targetinfo)
>>>>>> +       @$(call clean_pkg, TF_A)
>>>>>> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
>>>>>> +
>>>>>> +# vim: syntax=make
>>>>>> --
>>>>>> 2.25.0
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> ptxdist mailing list
>>>>>> ptxdist@pengutronix.de
>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>> Pengutronix e.K.                           |                             |
>>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>
>>>
>>>
>>
>> --
>> 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 |
> 
> 
> 


-- 
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] 14+ messages in thread

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

Hi Ahmad,

El mar., 11 feb. 2020 a las 17:53, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> Hello Guillermo,
>
> On 2/11/20 5:27 PM, Guillermo Rodriguez Garcia wrote:
> > Hi Ahmad,
> >
> > El mar., 11 feb. 2020 a las 16:22, Ahmad Fatoum
> > (<a.fatoum@pengutronix.de>) escribió:
> >>
> >> Hi,
> >>
> >> On 2/11/20 9:37 AM, Guillermo Rodriguez Garcia wrote:
> >>> Hi Ahmad,
> >>>
> >>> Some other questions and comments, please see below.
> >>>
> >>> El lun., 10 feb. 2020 a las 17:16, Ahmad Fatoum
> >>> (<a.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>
> >>>> ---
> >>>>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
> >>>>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  2 files changed, 236 insertions(+)
> >>>>  create mode 100644 platforms/tf-a.in
> >>>>  create mode 100644 rules/tf-a.make
> >>>>
> >>>> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
> >>>> new file mode 100644
> >>>> index 000000000000..5aa4ca473c35
> >>>> --- /dev/null
> >>>> +++ b/platforms/tf-a.in
> >>>> @@ -0,0 +1,113 @@
> >>>> +## SECTION=bootloader
> >>>> +
> >>>> +menuconfig TF_A
> >>>> +       select BOOTLOADER
> >>>> +       prompt "ARM Trusted Firmware-A"
> >>>> +       depends on ARCH_ARM || ARCH_ARM64
> >>>> +       bool
> >>>> +
> >>>> +if TF_A
> >>>> +
> >>>> +config TF_A_ARCH_STRING
> >>>> +        string
> >>>> +        default "aarch32" if ARCH_ARM
> >>>> +        default "aarch64" if ARCH_ARM64
> >>>> +
> >>>> +config TF_A_ARM_ARCH_MAJOR
> >>>> +        int
> >>>> +        default "7" if ARCH_ARM
> >>>> +        default "8" if ARCH_ARM64
> >>>
> >>> Shouldn't this be configurable?
> >>> aarch64 is always v8, but for aarch32 you can have either v7 or v8.
> >>
> >> Ah, indeed. Didn't know about cores like the Cortex-A32.
> >> I will make this configurable.
> >>
> >>>> +
> >>>> +config TF_A_VERSION
> >>>> +       string
> >>>> +       default "v2.2"
> >>>> +       prompt "TF-A version"
> >>>> +       help
> >>>> +         Enter the TF-A version you want to build. Usally something like "v2.2"
> >>>> +
> >>>> +config TF_A_MD5
> >>>> +       string
> >>>> +       default "bb300e5a62c911e189c80d935d497a4b"
> >>>> +       prompt "TF-A source md5"
> >>>> +
> >>>> +config TF_A_PLATFORM
> >>>> +       string
> >>>> +       prompt "TF-A target platform"
> >>>> +       help
> >>>> +         The TF-A target platform.
> >>>> +
> >>>> +if ARCH_ARM64
> >>>> +config TF_A_ARM_ARCH_MINOR
> >>>> +       int
> >>>> +       default 0
> >>>> +       prompt "TF-A target ARMv8.MINOR version"
> >>>> +       help
> >>>> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
> >>>> +endif
> >>>> +
> >>>> +config TF_A_DTB
> >>>> +       string
> >>>> +       prompt "TF-A DTB file name"
> >>>> +       help
> >>>> +         Device Tree Binary file name
> >>>> +
> >>>> +config TF_A_ARTIFACTS
> >>>> +       string
> >>>> +       prompt "TF-A artifact file names"
> >>>> +       default "bl2.bin"
> >>>> +       help
> >>>> +         A space-separated list of artifacts to copy to the image directory.
> >>>> +         All file names are relative to the appropriate TF-A platform build
> >>>> +         directory.
> >>>> +
> >>>> +comment "Payloads"
> >>>> +
> >>>> +choice
> >>>> +       prompt "BL32 Payload"
> >>>> +       default TF_A_BL32_NONE
> >>>> +       help
> >>>> +         payload for BL32 (Secure World OS)
> >>>> +
> >>>> +       config TF_A_BL32_NONE
> >>>> +               prompt "None"
> >>>> +               bool
> >>>> +
> >>>> +       config TF_A_BL32_SP_MIN
> >>>> +               depends on ARCH_ARM
> >>>> +               prompt "sp_min"
> >>>> +               bool
> >>>> +
> >>>> +       config TF_A_BL32_TSP
> >>>> +               depends on ARCH_ARM64
> >>>> +               prompt "Test Secure Payload"
> >>>> +               bool
> >>>
> >>> No way to specify other payloads?
> >>
> >> I don't use any others at the moment, so no.
> >> If you want to use e.g. OP-TEE, you can amend the the tf-a.in.
> >> A string prompt won't save you the effort of doing that, because you
> >> would need to specify dependencies anyway.
> >>
> >>>
> >>>> +
> >>>> +endchoice
> >>>> +
> >>>> +if TF_A_BL32_TSP
> >>>> +choice TF_A_BL32_TSP_RAM_LOCATION
> >>>> +       prompt "TSP location"
> >>>> +       default TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> >>>> +
> >>>> +       config TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> >>>> +               prompt "Trusted SRAM"
> >>>> +               bool
> >>>> +
> >>>> +       config TF_A_BL32_TSP_RAM_LOCATION_TDRAM
> >>>> +               prompt "Trusted DRAM (if available)"
> >>>> +               bool
> >>>> +
> >>>> +       config TF_A_BL32_TSP_RAM_LOCATION_DRAM
> >>>> +               prompt "Secure DRAM region (configured by TrustZone controller)"
> >>>> +               bool
> >>>> +endchoice
> >>>> +
> >>>> +config TF_A_BL32_TSP_RAM_LOCATION_STRING
> >>>> +        string
> >>>> +        default "tsram" if TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> >>>> +        default "tdram" if TF_A_BL32_TSP_RAM_LOCATION_TDRAM
> >>>> +        default "dram"  if TF_A_BL32_TSP_RAM_LOCATION_DRAM
> >>>> +
> >>>> +endif
> >>>> +
> >>>> +endif
> >>>> diff --git a/rules/tf-a.make b/rules/tf-a.make
> >>>> new file mode 100644
> >>>> index 000000000000..9ee554476927
> >>>> --- /dev/null
> >>>> +++ b/rules/tf-a.make
> >>>> @@ -0,0 +1,123 @@
> >>>> +# -*-makefile-*-
> >>>> +#
> >>>> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
> >>>> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>>> +#
> >>>> +# See CREDITS for details about who has contributed to this project.
> >>>> +#
> >>>> +# For further information about the PTXdist project and license conditions
> >>>> +# see the README file.
> >>>> +#
> >>>> +
> >>>> +#
> >>>> +# We provide this package
> >>>> +#
> >>>> +PACKAGES-$(PTXCONF_TF_A) += tf-a
> >>>> +
> >>>> +#
> >>>> +# Paths and names
> >>>> +#
> >>>> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
> >>>> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
> >>>> +TF_A           := tf-a-$(TF_A_VERSION)
> >>>> +TF_A_SUFFIX    := tar.gz
> >>>> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
> >>>> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
> >>>> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
> >>>> +TF_A_LICENSE    := BSD-3-Clause
> >>>> +
> >>>> +# ----------------------------------------------------------------------------
> >>>> +# Prepare
> >>>> +# ----------------------------------------------------------------------------
> >>>> +
> >>>> +TF_A_WRAPPER_BLACKLIST := \
> >>>> +       TARGET_HARDEN_RELRO \
> >>>> +       TARGET_HARDEN_BINDNOW \
> >>>> +       TARGET_HARDEN_PIE \
> >>>> +       TARGET_DEBUG \
> >>>> +       TARGET_BUILD_ID
> >>>> +
> >>>> +TF_A_PATH      := PATH=$(CROSS_PATH)
> >>>> +TF_A_MAKE_OPT  := \
> >>>> +       V=$(PTXDIST_VERBOSE) \
> >>>> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
> >>>> +       HOSTCC=$(HOSTCC)
> >>>> +TF_A_CONF_TOOL := NO
> >>>> +TF_A_MAKE_ENV  := $(CROSS_ENV)
> >>>
> >>> Do you need $(CROSS_ENV) here? (not used in e.g. u-boot.make)
> >>
> >> Yes, u-boot's Kbuild is known to ptxdist, so we don't
> >> need to specify it explicitly.
> >
> > Uhm, here I didn't need to specify CROSS_ENV and things seemed to work
> > just fine.
>
> CROSS_ENV sets stuff like CC and LD. I took a look inside the v2.2 TF-A Makefile
> and on line 808 there is a reference to $(CC) before it's defined,

That would be a bug in the Makefile, and if this was the case,
shouldn't we add a
patch to fix the Makefile (and possibly submit it to upstream),
instead of trying to
work around the bug by setting CROSS_ENV ?

Anyway I am not sure this is actually a bug -- I see that CC is
defined in line 160
in v2.2 of the Makefile; that should happen before the usage in line 808.

> so in that case it
> would come from the environment. It's probably unintended, but to account for
> any other scripts that may reference LD, CC and the like without defining them
> first, I guess it's better to have CROSS_ENV in. Do you agree?

I guess keeping it can not do any harm. However it can also hide bugs,
so I am not
sure what is best.

Guillermo

>
> >
> >>
> >>>> +$(STATEDIR)/tf-a.prepare:
> >>>> +       @$(call targetinfo)
> >>>> +       @$(call touch)
> >>>
> >>> I think this does nothing and can be removed.
> >>
> >> Right. See tf-a.compile below
> >>
> >>>
> >>>> +
> >>>> +# ----------------------------------------------------------------------------
> >>>> +# Compile
> >>>> +# ----------------------------------------------------------------------------
> >>>> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
> >>>> +
> >>>> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
> >>>> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
> >>>> +
> >>>> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
> >>>> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
> >
> > I think this should not be hardcoded.
> >
> > (Sorry, didn't spot this one before)
>
> Will drop it.
>
> >
> >>>> +ifdef PTXCONF_TF_A_BL32_TSP
> >>>> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
> >>>> +endif
> >>>> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
> >>>> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
> >>>> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
> >>>> +endif
> >>>> +
> >>>> +ifneq ($(PTXCONF_TF_A_DTB),)
> >>>> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
> >>>> +endif
> >>>> +
> >>>> +ifdef PTXCONF_TF_A_BL32_SP_MIN
> >>>> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
> >>>> +endif
> >>>> +
> >>>> +TF_A_MAKE_OPT += all
> >>>> +
> >>>> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
> >>>> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
> >>>> +
> >>>> +$(STATEDIR)/tf-a.compile:
> >>>> +       @$(call targetinfo)
> >>>> +       @rm -rf $(TF_A_DIR)/build/
> >>>> +       @$(call world/compile, TF_A)
> >>>> +       @$(call touch)
> >>>
> >>> Can't you use the default compile rule here?
> >>
> >> Yes. I will move the deletion of the build directory to the prepare stage.
> >> Seems like old versions of TF-A had problems when reconfiguring without a clean?
> >
> > Not sure; I have not seen such problems myself.
>
> I've sent a question to the colleague who first added the line.
> Will keep it in for now.
>
> >
> >>
> >>>> +
> >>>> +# ----------------------------------------------------------------------------
> >>>> +# Install
> >>>> +# ----------------------------------------------------------------------------
> >>>> +
> >>>> +$(STATEDIR)/tf-a.install:
> >>>> +       @$(call targetinfo)
> >>>> +
> >>>> +ifeq ($(TF_A_ARTIFACTS),)
> >>>> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
> >>>> +else
> >>>> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
> >>>> +endif
> >>>
> >>> Shouldn't this (copying files to IMAGEDIR) happen in targetinstall
> >>> rather than install? Again based on what is being done for other
> >>> bootloaders.
> >>
> >> On some systems, the BootROM doesn't directly invoke the TF-A, but instead it is
> >> a payload embedded into another bootloader. For this to work, we need to do it
> >> in the install stage, so the bootloader rule can depend on this and have the
> >> artifact available.
> >>
> >> That was the original line of thought, but apparently, how I did it is not
> >> completely sound. I should instead have both an install and targetinstall:
> >>
> >> - install installs into sysroot for use by other non-image rules
> >> - targetinstall installs into IMAGEDIR for use by image rules
> >>
> >> I will incorporate these changes along with your other suggestions in a v2.
> >
> > Thank you,
> >
> > Guillermo
> >
> >>
> >> Thanks!
> >> Ahmad
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Guillermo
> >>>
> >>>> +
> >>>> +       @$(call touch)
> >>>> +
> >>>> +# ----------------------------------------------------------------------------
> >>>> +# Target-Install
> >>>> +# ----------------------------------------------------------------------------
> >>>> +
> >>>> +$(STATEDIR)/tf-a.targetinstall:
> >>>> +       @$(call targetinfo)
> >>>> +       @$(call touch)
> >>>> +
> >>>> +# ----------------------------------------------------------------------------
> >>>> +# Clean
> >>>> +# ----------------------------------------------------------------------------
> >>>> +
> >>>> +$(STATEDIR)/tf-a.clean:
> >>>> +       @$(call targetinfo)
> >>>> +       @$(call clean_pkg, TF_A)
> >>>> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
> >>>> +
> >>>> +# vim: syntax=make
> >>>> --
> >>>> 2.25.0
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> ptxdist mailing list
> >>>> ptxdist@pengutronix.de
> >>>
> >>>
> >>>
> >>
> >> --
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >
> >
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

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

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

Hello Guillermo,

On 2/11/20 5:27 PM, Guillermo Rodriguez Garcia wrote:
> Hi Ahmad,
> 
> El mar., 11 feb. 2020 a las 16:22, Ahmad Fatoum
> (<a.fatoum@pengutronix.de>) escribió:
>>
>> Hi,
>>
>> On 2/11/20 9:37 AM, Guillermo Rodriguez Garcia wrote:
>>> Hi Ahmad,
>>>
>>> Some other questions and comments, please see below.
>>>
>>> El lun., 10 feb. 2020 a las 17:16, Ahmad Fatoum
>>> (<a.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>
>>>> ---
>>>>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
>>>>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 236 insertions(+)
>>>>  create mode 100644 platforms/tf-a.in
>>>>  create mode 100644 rules/tf-a.make
>>>>
>>>> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
>>>> new file mode 100644
>>>> index 000000000000..5aa4ca473c35
>>>> --- /dev/null
>>>> +++ b/platforms/tf-a.in
>>>> @@ -0,0 +1,113 @@
>>>> +## SECTION=bootloader
>>>> +
>>>> +menuconfig TF_A
>>>> +       select BOOTLOADER
>>>> +       prompt "ARM Trusted Firmware-A"
>>>> +       depends on ARCH_ARM || ARCH_ARM64
>>>> +       bool
>>>> +
>>>> +if TF_A
>>>> +
>>>> +config TF_A_ARCH_STRING
>>>> +        string
>>>> +        default "aarch32" if ARCH_ARM
>>>> +        default "aarch64" if ARCH_ARM64
>>>> +
>>>> +config TF_A_ARM_ARCH_MAJOR
>>>> +        int
>>>> +        default "7" if ARCH_ARM
>>>> +        default "8" if ARCH_ARM64
>>>
>>> Shouldn't this be configurable?
>>> aarch64 is always v8, but for aarch32 you can have either v7 or v8.
>>
>> Ah, indeed. Didn't know about cores like the Cortex-A32.
>> I will make this configurable.
>>
>>>> +
>>>> +config TF_A_VERSION
>>>> +       string
>>>> +       default "v2.2"
>>>> +       prompt "TF-A version"
>>>> +       help
>>>> +         Enter the TF-A version you want to build. Usally something like "v2.2"
>>>> +
>>>> +config TF_A_MD5
>>>> +       string
>>>> +       default "bb300e5a62c911e189c80d935d497a4b"
>>>> +       prompt "TF-A source md5"
>>>> +
>>>> +config TF_A_PLATFORM
>>>> +       string
>>>> +       prompt "TF-A target platform"
>>>> +       help
>>>> +         The TF-A target platform.
>>>> +
>>>> +if ARCH_ARM64
>>>> +config TF_A_ARM_ARCH_MINOR
>>>> +       int
>>>> +       default 0
>>>> +       prompt "TF-A target ARMv8.MINOR version"
>>>> +       help
>>>> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
>>>> +endif
>>>> +
>>>> +config TF_A_DTB
>>>> +       string
>>>> +       prompt "TF-A DTB file name"
>>>> +       help
>>>> +         Device Tree Binary file name
>>>> +
>>>> +config TF_A_ARTIFACTS
>>>> +       string
>>>> +       prompt "TF-A artifact file names"
>>>> +       default "bl2.bin"
>>>> +       help
>>>> +         A space-separated list of artifacts to copy to the image directory.
>>>> +         All file names are relative to the appropriate TF-A platform build
>>>> +         directory.
>>>> +
>>>> +comment "Payloads"
>>>> +
>>>> +choice
>>>> +       prompt "BL32 Payload"
>>>> +       default TF_A_BL32_NONE
>>>> +       help
>>>> +         payload for BL32 (Secure World OS)
>>>> +
>>>> +       config TF_A_BL32_NONE
>>>> +               prompt "None"
>>>> +               bool
>>>> +
>>>> +       config TF_A_BL32_SP_MIN
>>>> +               depends on ARCH_ARM
>>>> +               prompt "sp_min"
>>>> +               bool
>>>> +
>>>> +       config TF_A_BL32_TSP
>>>> +               depends on ARCH_ARM64
>>>> +               prompt "Test Secure Payload"
>>>> +               bool
>>>
>>> No way to specify other payloads?
>>
>> I don't use any others at the moment, so no.
>> If you want to use e.g. OP-TEE, you can amend the the tf-a.in.
>> A string prompt won't save you the effort of doing that, because you
>> would need to specify dependencies anyway.
>>
>>>
>>>> +
>>>> +endchoice
>>>> +
>>>> +if TF_A_BL32_TSP
>>>> +choice TF_A_BL32_TSP_RAM_LOCATION
>>>> +       prompt "TSP location"
>>>> +       default TF_A_BL32_TSP_RAM_LOCATION_TSRAM
>>>> +
>>>> +       config TF_A_BL32_TSP_RAM_LOCATION_TSRAM
>>>> +               prompt "Trusted SRAM"
>>>> +               bool
>>>> +
>>>> +       config TF_A_BL32_TSP_RAM_LOCATION_TDRAM
>>>> +               prompt "Trusted DRAM (if available)"
>>>> +               bool
>>>> +
>>>> +       config TF_A_BL32_TSP_RAM_LOCATION_DRAM
>>>> +               prompt "Secure DRAM region (configured by TrustZone controller)"
>>>> +               bool
>>>> +endchoice
>>>> +
>>>> +config TF_A_BL32_TSP_RAM_LOCATION_STRING
>>>> +        string
>>>> +        default "tsram" if TF_A_BL32_TSP_RAM_LOCATION_TSRAM
>>>> +        default "tdram" if TF_A_BL32_TSP_RAM_LOCATION_TDRAM
>>>> +        default "dram"  if TF_A_BL32_TSP_RAM_LOCATION_DRAM
>>>> +
>>>> +endif
>>>> +
>>>> +endif
>>>> diff --git a/rules/tf-a.make b/rules/tf-a.make
>>>> new file mode 100644
>>>> index 000000000000..9ee554476927
>>>> --- /dev/null
>>>> +++ b/rules/tf-a.make
>>>> @@ -0,0 +1,123 @@
>>>> +# -*-makefile-*-
>>>> +#
>>>> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
>>>> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> +#
>>>> +# See CREDITS for details about who has contributed to this project.
>>>> +#
>>>> +# For further information about the PTXdist project and license conditions
>>>> +# see the README file.
>>>> +#
>>>> +
>>>> +#
>>>> +# We provide this package
>>>> +#
>>>> +PACKAGES-$(PTXCONF_TF_A) += tf-a
>>>> +
>>>> +#
>>>> +# Paths and names
>>>> +#
>>>> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
>>>> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
>>>> +TF_A           := tf-a-$(TF_A_VERSION)
>>>> +TF_A_SUFFIX    := tar.gz
>>>> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
>>>> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
>>>> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
>>>> +TF_A_LICENSE    := BSD-3-Clause
>>>> +
>>>> +# ----------------------------------------------------------------------------
>>>> +# Prepare
>>>> +# ----------------------------------------------------------------------------
>>>> +
>>>> +TF_A_WRAPPER_BLACKLIST := \
>>>> +       TARGET_HARDEN_RELRO \
>>>> +       TARGET_HARDEN_BINDNOW \
>>>> +       TARGET_HARDEN_PIE \
>>>> +       TARGET_DEBUG \
>>>> +       TARGET_BUILD_ID
>>>> +
>>>> +TF_A_PATH      := PATH=$(CROSS_PATH)
>>>> +TF_A_MAKE_OPT  := \
>>>> +       V=$(PTXDIST_VERBOSE) \
>>>> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
>>>> +       HOSTCC=$(HOSTCC)
>>>> +TF_A_CONF_TOOL := NO
>>>> +TF_A_MAKE_ENV  := $(CROSS_ENV)
>>>
>>> Do you need $(CROSS_ENV) here? (not used in e.g. u-boot.make)
>>
>> Yes, u-boot's Kbuild is known to ptxdist, so we don't
>> need to specify it explicitly.
> 
> Uhm, here I didn't need to specify CROSS_ENV and things seemed to work
> just fine.

CROSS_ENV sets stuff like CC and LD. I took a look inside the v2.2 TF-A Makefile
and on line 808 there is a reference to $(CC) before it's defined, so in that case it
would come from the environment. It's probably unintended, but to account for
any other scripts that may reference LD, CC and the like without defining them
first, I guess it's better to have CROSS_ENV in. Do you agree?

> 
>>
>>>> +$(STATEDIR)/tf-a.prepare:
>>>> +       @$(call targetinfo)
>>>> +       @$(call touch)
>>>
>>> I think this does nothing and can be removed.
>>
>> Right. See tf-a.compile below
>>
>>>
>>>> +
>>>> +# ----------------------------------------------------------------------------
>>>> +# Compile
>>>> +# ----------------------------------------------------------------------------
>>>> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
>>>> +
>>>> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
>>>> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
>>>> +
>>>> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
>>>> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
> 
> I think this should not be hardcoded.
> 
> (Sorry, didn't spot this one before)

Will drop it.

> 
>>>> +ifdef PTXCONF_TF_A_BL32_TSP
>>>> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
>>>> +endif
>>>> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
>>>> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
>>>> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
>>>> +endif
>>>> +
>>>> +ifneq ($(PTXCONF_TF_A_DTB),)
>>>> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
>>>> +endif
>>>> +
>>>> +ifdef PTXCONF_TF_A_BL32_SP_MIN
>>>> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
>>>> +endif
>>>> +
>>>> +TF_A_MAKE_OPT += all
>>>> +
>>>> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
>>>> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
>>>> +
>>>> +$(STATEDIR)/tf-a.compile:
>>>> +       @$(call targetinfo)
>>>> +       @rm -rf $(TF_A_DIR)/build/
>>>> +       @$(call world/compile, TF_A)
>>>> +       @$(call touch)
>>>
>>> Can't you use the default compile rule here?
>>
>> Yes. I will move the deletion of the build directory to the prepare stage.
>> Seems like old versions of TF-A had problems when reconfiguring without a clean?
> 
> Not sure; I have not seen such problems myself.

I've sent a question to the colleague who first added the line.
Will keep it in for now.

> 
>>
>>>> +
>>>> +# ----------------------------------------------------------------------------
>>>> +# Install
>>>> +# ----------------------------------------------------------------------------
>>>> +
>>>> +$(STATEDIR)/tf-a.install:
>>>> +       @$(call targetinfo)
>>>> +
>>>> +ifeq ($(TF_A_ARTIFACTS),)
>>>> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
>>>> +else
>>>> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
>>>> +endif
>>>
>>> Shouldn't this (copying files to IMAGEDIR) happen in targetinstall
>>> rather than install? Again based on what is being done for other
>>> bootloaders.
>>
>> On some systems, the BootROM doesn't directly invoke the TF-A, but instead it is
>> a payload embedded into another bootloader. For this to work, we need to do it
>> in the install stage, so the bootloader rule can depend on this and have the
>> artifact available.
>>
>> That was the original line of thought, but apparently, how I did it is not
>> completely sound. I should instead have both an install and targetinstall:
>>
>> - install installs into sysroot for use by other non-image rules
>> - targetinstall installs into IMAGEDIR for use by image rules
>>
>> I will incorporate these changes along with your other suggestions in a v2.
> 
> Thank you,
> 
> Guillermo
> 
>>
>> Thanks!
>> Ahmad
>>
>>>
>>> Thanks,
>>>
>>> Guillermo
>>>
>>>> +
>>>> +       @$(call touch)
>>>> +
>>>> +# ----------------------------------------------------------------------------
>>>> +# Target-Install
>>>> +# ----------------------------------------------------------------------------
>>>> +
>>>> +$(STATEDIR)/tf-a.targetinstall:
>>>> +       @$(call targetinfo)
>>>> +       @$(call touch)
>>>> +
>>>> +# ----------------------------------------------------------------------------
>>>> +# Clean
>>>> +# ----------------------------------------------------------------------------
>>>> +
>>>> +$(STATEDIR)/tf-a.clean:
>>>> +       @$(call targetinfo)
>>>> +       @$(call clean_pkg, TF_A)
>>>> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
>>>> +
>>>> +# vim: syntax=make
>>>> --
>>>> 2.25.0
>>>>
>>>>
>>>> _______________________________________________
>>>> ptxdist mailing list
>>>> ptxdist@pengutronix.de
>>>
>>>
>>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 
> 

-- 
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] 14+ messages in thread

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

Hi Ahmad,

El mar., 11 feb. 2020 a las 16:22, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> Hi,
>
> On 2/11/20 9:37 AM, Guillermo Rodriguez Garcia wrote:
> > Hi Ahmad,
> >
> > Some other questions and comments, please see below.
> >
> > El lun., 10 feb. 2020 a las 17:16, Ahmad Fatoum
> > (<a.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>
> >> ---
> >>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
> >>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 236 insertions(+)
> >>  create mode 100644 platforms/tf-a.in
> >>  create mode 100644 rules/tf-a.make
> >>
> >> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
> >> new file mode 100644
> >> index 000000000000..5aa4ca473c35
> >> --- /dev/null
> >> +++ b/platforms/tf-a.in
> >> @@ -0,0 +1,113 @@
> >> +## SECTION=bootloader
> >> +
> >> +menuconfig TF_A
> >> +       select BOOTLOADER
> >> +       prompt "ARM Trusted Firmware-A"
> >> +       depends on ARCH_ARM || ARCH_ARM64
> >> +       bool
> >> +
> >> +if TF_A
> >> +
> >> +config TF_A_ARCH_STRING
> >> +        string
> >> +        default "aarch32" if ARCH_ARM
> >> +        default "aarch64" if ARCH_ARM64
> >> +
> >> +config TF_A_ARM_ARCH_MAJOR
> >> +        int
> >> +        default "7" if ARCH_ARM
> >> +        default "8" if ARCH_ARM64
> >
> > Shouldn't this be configurable?
> > aarch64 is always v8, but for aarch32 you can have either v7 or v8.
>
> Ah, indeed. Didn't know about cores like the Cortex-A32.
> I will make this configurable.
>
> >> +
> >> +config TF_A_VERSION
> >> +       string
> >> +       default "v2.2"
> >> +       prompt "TF-A version"
> >> +       help
> >> +         Enter the TF-A version you want to build. Usally something like "v2.2"
> >> +
> >> +config TF_A_MD5
> >> +       string
> >> +       default "bb300e5a62c911e189c80d935d497a4b"
> >> +       prompt "TF-A source md5"
> >> +
> >> +config TF_A_PLATFORM
> >> +       string
> >> +       prompt "TF-A target platform"
> >> +       help
> >> +         The TF-A target platform.
> >> +
> >> +if ARCH_ARM64
> >> +config TF_A_ARM_ARCH_MINOR
> >> +       int
> >> +       default 0
> >> +       prompt "TF-A target ARMv8.MINOR version"
> >> +       help
> >> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
> >> +endif
> >> +
> >> +config TF_A_DTB
> >> +       string
> >> +       prompt "TF-A DTB file name"
> >> +       help
> >> +         Device Tree Binary file name
> >> +
> >> +config TF_A_ARTIFACTS
> >> +       string
> >> +       prompt "TF-A artifact file names"
> >> +       default "bl2.bin"
> >> +       help
> >> +         A space-separated list of artifacts to copy to the image directory.
> >> +         All file names are relative to the appropriate TF-A platform build
> >> +         directory.
> >> +
> >> +comment "Payloads"
> >> +
> >> +choice
> >> +       prompt "BL32 Payload"
> >> +       default TF_A_BL32_NONE
> >> +       help
> >> +         payload for BL32 (Secure World OS)
> >> +
> >> +       config TF_A_BL32_NONE
> >> +               prompt "None"
> >> +               bool
> >> +
> >> +       config TF_A_BL32_SP_MIN
> >> +               depends on ARCH_ARM
> >> +               prompt "sp_min"
> >> +               bool
> >> +
> >> +       config TF_A_BL32_TSP
> >> +               depends on ARCH_ARM64
> >> +               prompt "Test Secure Payload"
> >> +               bool
> >
> > No way to specify other payloads?
>
> I don't use any others at the moment, so no.
> If you want to use e.g. OP-TEE, you can amend the the tf-a.in.
> A string prompt won't save you the effort of doing that, because you
> would need to specify dependencies anyway.
>
> >
> >> +
> >> +endchoice
> >> +
> >> +if TF_A_BL32_TSP
> >> +choice TF_A_BL32_TSP_RAM_LOCATION
> >> +       prompt "TSP location"
> >> +       default TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> >> +
> >> +       config TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> >> +               prompt "Trusted SRAM"
> >> +               bool
> >> +
> >> +       config TF_A_BL32_TSP_RAM_LOCATION_TDRAM
> >> +               prompt "Trusted DRAM (if available)"
> >> +               bool
> >> +
> >> +       config TF_A_BL32_TSP_RAM_LOCATION_DRAM
> >> +               prompt "Secure DRAM region (configured by TrustZone controller)"
> >> +               bool
> >> +endchoice
> >> +
> >> +config TF_A_BL32_TSP_RAM_LOCATION_STRING
> >> +        string
> >> +        default "tsram" if TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> >> +        default "tdram" if TF_A_BL32_TSP_RAM_LOCATION_TDRAM
> >> +        default "dram"  if TF_A_BL32_TSP_RAM_LOCATION_DRAM
> >> +
> >> +endif
> >> +
> >> +endif
> >> diff --git a/rules/tf-a.make b/rules/tf-a.make
> >> new file mode 100644
> >> index 000000000000..9ee554476927
> >> --- /dev/null
> >> +++ b/rules/tf-a.make
> >> @@ -0,0 +1,123 @@
> >> +# -*-makefile-*-
> >> +#
> >> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
> >> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> +#
> >> +# See CREDITS for details about who has contributed to this project.
> >> +#
> >> +# For further information about the PTXdist project and license conditions
> >> +# see the README file.
> >> +#
> >> +
> >> +#
> >> +# We provide this package
> >> +#
> >> +PACKAGES-$(PTXCONF_TF_A) += tf-a
> >> +
> >> +#
> >> +# Paths and names
> >> +#
> >> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
> >> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
> >> +TF_A           := tf-a-$(TF_A_VERSION)
> >> +TF_A_SUFFIX    := tar.gz
> >> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
> >> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
> >> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
> >> +TF_A_LICENSE    := BSD-3-Clause
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Prepare
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +TF_A_WRAPPER_BLACKLIST := \
> >> +       TARGET_HARDEN_RELRO \
> >> +       TARGET_HARDEN_BINDNOW \
> >> +       TARGET_HARDEN_PIE \
> >> +       TARGET_DEBUG \
> >> +       TARGET_BUILD_ID
> >> +
> >> +TF_A_PATH      := PATH=$(CROSS_PATH)
> >> +TF_A_MAKE_OPT  := \
> >> +       V=$(PTXDIST_VERBOSE) \
> >> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
> >> +       HOSTCC=$(HOSTCC)
> >> +TF_A_CONF_TOOL := NO
> >> +TF_A_MAKE_ENV  := $(CROSS_ENV)
> >
> > Do you need $(CROSS_ENV) here? (not used in e.g. u-boot.make)
>
> Yes, u-boot's Kbuild is known to ptxdist, so we don't
> need to specify it explicitly.

Uhm, here I didn't need to specify CROSS_ENV and things seemed to work
just fine.

>
> >> +$(STATEDIR)/tf-a.prepare:
> >> +       @$(call targetinfo)
> >> +       @$(call touch)
> >
> > I think this does nothing and can be removed.
>
> Right. See tf-a.compile below
>
> >
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Compile
> >> +# ----------------------------------------------------------------------------
> >> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
> >> +
> >> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
> >> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
> >> +
> >> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
> >> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1

I think this should not be hardcoded.

(Sorry, didn't spot this one before)

> >> +ifdef PTXCONF_TF_A_BL32_TSP
> >> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
> >> +endif
> >> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
> >> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
> >> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
> >> +endif
> >> +
> >> +ifneq ($(PTXCONF_TF_A_DTB),)
> >> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
> >> +endif
> >> +
> >> +ifdef PTXCONF_TF_A_BL32_SP_MIN
> >> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
> >> +endif
> >> +
> >> +TF_A_MAKE_OPT += all
> >> +
> >> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
> >> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
> >> +
> >> +$(STATEDIR)/tf-a.compile:
> >> +       @$(call targetinfo)
> >> +       @rm -rf $(TF_A_DIR)/build/
> >> +       @$(call world/compile, TF_A)
> >> +       @$(call touch)
> >
> > Can't you use the default compile rule here?
>
> Yes. I will move the deletion of the build directory to the prepare stage.
> Seems like old versions of TF-A had problems when reconfiguring without a clean?

Not sure; I have not seen such problems myself.

>
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Install
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +$(STATEDIR)/tf-a.install:
> >> +       @$(call targetinfo)
> >> +
> >> +ifeq ($(TF_A_ARTIFACTS),)
> >> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
> >> +else
> >> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
> >> +endif
> >
> > Shouldn't this (copying files to IMAGEDIR) happen in targetinstall
> > rather than install? Again based on what is being done for other
> > bootloaders.
>
> On some systems, the BootROM doesn't directly invoke the TF-A, but instead it is
> a payload embedded into another bootloader. For this to work, we need to do it
> in the install stage, so the bootloader rule can depend on this and have the
> artifact available.
>
> That was the original line of thought, but apparently, how I did it is not
> completely sound. I should instead have both an install and targetinstall:
>
> - install installs into sysroot for use by other non-image rules
> - targetinstall installs into IMAGEDIR for use by image rules
>
> I will incorporate these changes along with your other suggestions in a v2.

Thank you,

Guillermo

>
> Thanks!
> Ahmad
>
> >
> > Thanks,
> >
> > Guillermo
> >
> >> +
> >> +       @$(call touch)
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Target-Install
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +$(STATEDIR)/tf-a.targetinstall:
> >> +       @$(call targetinfo)
> >> +       @$(call touch)
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Clean
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +$(STATEDIR)/tf-a.clean:
> >> +       @$(call targetinfo)
> >> +       @$(call clean_pkg, TF_A)
> >> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
> >> +
> >> +# vim: syntax=make
> >> --
> >> 2.25.0
> >>
> >>
> >> _______________________________________________
> >> ptxdist mailing list
> >> ptxdist@pengutronix.de
> >
> >
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

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

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

Hi,

On 2/11/20 9:37 AM, Guillermo Rodriguez Garcia wrote:
> Hi Ahmad,
> 
> Some other questions and comments, please see below.
> 
> El lun., 10 feb. 2020 a las 17:16, Ahmad Fatoum
> (<a.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>
>> ---
>>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
>>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 236 insertions(+)
>>  create mode 100644 platforms/tf-a.in
>>  create mode 100644 rules/tf-a.make
>>
>> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
>> new file mode 100644
>> index 000000000000..5aa4ca473c35
>> --- /dev/null
>> +++ b/platforms/tf-a.in
>> @@ -0,0 +1,113 @@
>> +## SECTION=bootloader
>> +
>> +menuconfig TF_A
>> +       select BOOTLOADER
>> +       prompt "ARM Trusted Firmware-A"
>> +       depends on ARCH_ARM || ARCH_ARM64
>> +       bool
>> +
>> +if TF_A
>> +
>> +config TF_A_ARCH_STRING
>> +        string
>> +        default "aarch32" if ARCH_ARM
>> +        default "aarch64" if ARCH_ARM64
>> +
>> +config TF_A_ARM_ARCH_MAJOR
>> +        int
>> +        default "7" if ARCH_ARM
>> +        default "8" if ARCH_ARM64
> 
> Shouldn't this be configurable?
> aarch64 is always v8, but for aarch32 you can have either v7 or v8.

Ah, indeed. Didn't know about cores like the Cortex-A32.
I will make this configurable.

>> +
>> +config TF_A_VERSION
>> +       string
>> +       default "v2.2"
>> +       prompt "TF-A version"
>> +       help
>> +         Enter the TF-A version you want to build. Usally something like "v2.2"
>> +
>> +config TF_A_MD5
>> +       string
>> +       default "bb300e5a62c911e189c80d935d497a4b"
>> +       prompt "TF-A source md5"
>> +
>> +config TF_A_PLATFORM
>> +       string
>> +       prompt "TF-A target platform"
>> +       help
>> +         The TF-A target platform.
>> +
>> +if ARCH_ARM64
>> +config TF_A_ARM_ARCH_MINOR
>> +       int
>> +       default 0
>> +       prompt "TF-A target ARMv8.MINOR version"
>> +       help
>> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
>> +endif
>> +
>> +config TF_A_DTB
>> +       string
>> +       prompt "TF-A DTB file name"
>> +       help
>> +         Device Tree Binary file name
>> +
>> +config TF_A_ARTIFACTS
>> +       string
>> +       prompt "TF-A artifact file names"
>> +       default "bl2.bin"
>> +       help
>> +         A space-separated list of artifacts to copy to the image directory.
>> +         All file names are relative to the appropriate TF-A platform build
>> +         directory.
>> +
>> +comment "Payloads"
>> +
>> +choice
>> +       prompt "BL32 Payload"
>> +       default TF_A_BL32_NONE
>> +       help
>> +         payload for BL32 (Secure World OS)
>> +
>> +       config TF_A_BL32_NONE
>> +               prompt "None"
>> +               bool
>> +
>> +       config TF_A_BL32_SP_MIN
>> +               depends on ARCH_ARM
>> +               prompt "sp_min"
>> +               bool
>> +
>> +       config TF_A_BL32_TSP
>> +               depends on ARCH_ARM64
>> +               prompt "Test Secure Payload"
>> +               bool
> 
> No way to specify other payloads?

I don't use any others at the moment, so no.
If you want to use e.g. OP-TEE, you can amend the the tf-a.in.
A string prompt won't save you the effort of doing that, because you
would need to specify dependencies anyway.

> 
>> +
>> +endchoice
>> +
>> +if TF_A_BL32_TSP
>> +choice TF_A_BL32_TSP_RAM_LOCATION
>> +       prompt "TSP location"
>> +       default TF_A_BL32_TSP_RAM_LOCATION_TSRAM
>> +
>> +       config TF_A_BL32_TSP_RAM_LOCATION_TSRAM
>> +               prompt "Trusted SRAM"
>> +               bool
>> +
>> +       config TF_A_BL32_TSP_RAM_LOCATION_TDRAM
>> +               prompt "Trusted DRAM (if available)"
>> +               bool
>> +
>> +       config TF_A_BL32_TSP_RAM_LOCATION_DRAM
>> +               prompt "Secure DRAM region (configured by TrustZone controller)"
>> +               bool
>> +endchoice
>> +
>> +config TF_A_BL32_TSP_RAM_LOCATION_STRING
>> +        string
>> +        default "tsram" if TF_A_BL32_TSP_RAM_LOCATION_TSRAM
>> +        default "tdram" if TF_A_BL32_TSP_RAM_LOCATION_TDRAM
>> +        default "dram"  if TF_A_BL32_TSP_RAM_LOCATION_DRAM
>> +
>> +endif
>> +
>> +endif
>> diff --git a/rules/tf-a.make b/rules/tf-a.make
>> new file mode 100644
>> index 000000000000..9ee554476927
>> --- /dev/null
>> +++ b/rules/tf-a.make
>> @@ -0,0 +1,123 @@
>> +# -*-makefile-*-
>> +#
>> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
>> +#
>> +# See CREDITS for details about who has contributed to this project.
>> +#
>> +# For further information about the PTXdist project and license conditions
>> +# see the README file.
>> +#
>> +
>> +#
>> +# We provide this package
>> +#
>> +PACKAGES-$(PTXCONF_TF_A) += tf-a
>> +
>> +#
>> +# Paths and names
>> +#
>> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
>> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
>> +TF_A           := tf-a-$(TF_A_VERSION)
>> +TF_A_SUFFIX    := tar.gz
>> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
>> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
>> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
>> +TF_A_LICENSE    := BSD-3-Clause
>> +
>> +# ----------------------------------------------------------------------------
>> +# Prepare
>> +# ----------------------------------------------------------------------------
>> +
>> +TF_A_WRAPPER_BLACKLIST := \
>> +       TARGET_HARDEN_RELRO \
>> +       TARGET_HARDEN_BINDNOW \
>> +       TARGET_HARDEN_PIE \
>> +       TARGET_DEBUG \
>> +       TARGET_BUILD_ID
>> +
>> +TF_A_PATH      := PATH=$(CROSS_PATH)
>> +TF_A_MAKE_OPT  := \
>> +       V=$(PTXDIST_VERBOSE) \
>> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
>> +       HOSTCC=$(HOSTCC)
>> +TF_A_CONF_TOOL := NO
>> +TF_A_MAKE_ENV  := $(CROSS_ENV)
> 
> Do you need $(CROSS_ENV) here? (not used in e.g. u-boot.make)

Yes, u-boot's Kbuild is known to ptxdist, so we don't
need to specify it explicitly.

>> +$(STATEDIR)/tf-a.prepare:
>> +       @$(call targetinfo)
>> +       @$(call touch)
> 
> I think this does nothing and can be removed.

Right. See tf-a.compile below

> 
>> +
>> +# ----------------------------------------------------------------------------
>> +# Compile
>> +# ----------------------------------------------------------------------------
>> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
>> +
>> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
>> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
>> +
>> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
>> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
>> +ifdef PTXCONF_TF_A_BL32_TSP
>> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
>> +endif
>> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
>> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
>> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
>> +endif
>> +
>> +ifneq ($(PTXCONF_TF_A_DTB),)
>> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
>> +endif
>> +
>> +ifdef PTXCONF_TF_A_BL32_SP_MIN
>> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
>> +endif
>> +
>> +TF_A_MAKE_OPT += all
>> +
>> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
>> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
>> +
>> +$(STATEDIR)/tf-a.compile:
>> +       @$(call targetinfo)
>> +       @rm -rf $(TF_A_DIR)/build/
>> +       @$(call world/compile, TF_A)
>> +       @$(call touch)
> 
> Can't you use the default compile rule here?

Yes. I will move the deletion of the build directory to the prepare stage.
Seems like old versions of TF-A had problems when reconfiguring without a clean?

>> +
>> +# ----------------------------------------------------------------------------
>> +# Install
>> +# ----------------------------------------------------------------------------
>> +
>> +$(STATEDIR)/tf-a.install:
>> +       @$(call targetinfo)
>> +
>> +ifeq ($(TF_A_ARTIFACTS),)
>> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
>> +else
>> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
>> +endif
> 
> Shouldn't this (copying files to IMAGEDIR) happen in targetinstall
> rather than install? Again based on what is being done for other
> bootloaders.

On some systems, the BootROM doesn't directly invoke the TF-A, but instead it is
a payload embedded into another bootloader. For this to work, we need to do it
in the install stage, so the bootloader rule can depend on this and have the
artifact available.

That was the original line of thought, but apparently, how I did it is not
completely sound. I should instead have both an install and targetinstall:

- install installs into sysroot for use by other non-image rules
- targetinstall installs into IMAGEDIR for use by image rules

I will incorporate these changes along with your other suggestions in a v2.

Thanks!
Ahmad

> 
> Thanks,
> 
> Guillermo
> 
>> +
>> +       @$(call touch)
>> +
>> +# ----------------------------------------------------------------------------
>> +# Target-Install
>> +# ----------------------------------------------------------------------------
>> +
>> +$(STATEDIR)/tf-a.targetinstall:
>> +       @$(call targetinfo)
>> +       @$(call touch)
>> +
>> +# ----------------------------------------------------------------------------
>> +# Clean
>> +# ----------------------------------------------------------------------------
>> +
>> +$(STATEDIR)/tf-a.clean:
>> +       @$(call targetinfo)
>> +       @$(call clean_pkg, TF_A)
>> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
>> +
>> +# vim: syntax=make
>> --
>> 2.25.0
>>
>>
>> _______________________________________________
>> ptxdist mailing list
>> ptxdist@pengutronix.de
> 
> 
> 

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

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

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

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

Hi Ahmad,

Some other questions and comments, please see below.

El lun., 10 feb. 2020 a las 17:16, 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>
> ---
>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 236 insertions(+)
>  create mode 100644 platforms/tf-a.in
>  create mode 100644 rules/tf-a.make
>
> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
> new file mode 100644
> index 000000000000..5aa4ca473c35
> --- /dev/null
> +++ b/platforms/tf-a.in
> @@ -0,0 +1,113 @@
> +## SECTION=bootloader
> +
> +menuconfig TF_A
> +       select BOOTLOADER
> +       prompt "ARM Trusted Firmware-A"
> +       depends on ARCH_ARM || ARCH_ARM64
> +       bool
> +
> +if TF_A
> +
> +config TF_A_ARCH_STRING
> +        string
> +        default "aarch32" if ARCH_ARM
> +        default "aarch64" if ARCH_ARM64
> +
> +config TF_A_ARM_ARCH_MAJOR
> +        int
> +        default "7" if ARCH_ARM
> +        default "8" if ARCH_ARM64

Shouldn't this be configurable?
aarch64 is always v8, but for aarch32 you can have either v7 or v8.

> +
> +config TF_A_VERSION
> +       string
> +       default "v2.2"
> +       prompt "TF-A version"
> +       help
> +         Enter the TF-A version you want to build. Usally something like "v2.2"
> +
> +config TF_A_MD5
> +       string
> +       default "bb300e5a62c911e189c80d935d497a4b"
> +       prompt "TF-A source md5"
> +
> +config TF_A_PLATFORM
> +       string
> +       prompt "TF-A target platform"
> +       help
> +         The TF-A target platform.
> +
> +if ARCH_ARM64
> +config TF_A_ARM_ARCH_MINOR
> +       int
> +       default 0
> +       prompt "TF-A target ARMv8.MINOR version"
> +       help
> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
> +endif
> +
> +config TF_A_DTB
> +       string
> +       prompt "TF-A DTB file name"
> +       help
> +         Device Tree Binary file name
> +
> +config TF_A_ARTIFACTS
> +       string
> +       prompt "TF-A artifact file names"
> +       default "bl2.bin"
> +       help
> +         A space-separated list of artifacts to copy to the image directory.
> +         All file names are relative to the appropriate TF-A platform build
> +         directory.
> +
> +comment "Payloads"
> +
> +choice
> +       prompt "BL32 Payload"
> +       default TF_A_BL32_NONE
> +       help
> +         payload for BL32 (Secure World OS)
> +
> +       config TF_A_BL32_NONE
> +               prompt "None"
> +               bool
> +
> +       config TF_A_BL32_SP_MIN
> +               depends on ARCH_ARM
> +               prompt "sp_min"
> +               bool
> +
> +       config TF_A_BL32_TSP
> +               depends on ARCH_ARM64
> +               prompt "Test Secure Payload"
> +               bool

No way to specify other payloads?

> +
> +endchoice
> +
> +if TF_A_BL32_TSP
> +choice TF_A_BL32_TSP_RAM_LOCATION
> +       prompt "TSP location"
> +       default TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> +
> +       config TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> +               prompt "Trusted SRAM"
> +               bool
> +
> +       config TF_A_BL32_TSP_RAM_LOCATION_TDRAM
> +               prompt "Trusted DRAM (if available)"
> +               bool
> +
> +       config TF_A_BL32_TSP_RAM_LOCATION_DRAM
> +               prompt "Secure DRAM region (configured by TrustZone controller)"
> +               bool
> +endchoice
> +
> +config TF_A_BL32_TSP_RAM_LOCATION_STRING
> +        string
> +        default "tsram" if TF_A_BL32_TSP_RAM_LOCATION_TSRAM
> +        default "tdram" if TF_A_BL32_TSP_RAM_LOCATION_TDRAM
> +        default "dram"  if TF_A_BL32_TSP_RAM_LOCATION_DRAM
> +
> +endif
> +
> +endif
> diff --git a/rules/tf-a.make b/rules/tf-a.make
> new file mode 100644
> index 000000000000..9ee554476927
> --- /dev/null
> +++ b/rules/tf-a.make
> @@ -0,0 +1,123 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> +#
> +# See CREDITS for details about who has contributed to this project.
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +#
> +# We provide this package
> +#
> +PACKAGES-$(PTXCONF_TF_A) += tf-a
> +
> +#
> +# Paths and names
> +#
> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
> +TF_A           := tf-a-$(TF_A_VERSION)
> +TF_A_SUFFIX    := tar.gz
> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
> +TF_A_LICENSE    := BSD-3-Clause
> +
> +# ----------------------------------------------------------------------------
> +# Prepare
> +# ----------------------------------------------------------------------------
> +
> +TF_A_WRAPPER_BLACKLIST := \
> +       TARGET_HARDEN_RELRO \
> +       TARGET_HARDEN_BINDNOW \
> +       TARGET_HARDEN_PIE \
> +       TARGET_DEBUG \
> +       TARGET_BUILD_ID
> +
> +TF_A_PATH      := PATH=$(CROSS_PATH)
> +TF_A_MAKE_OPT  := \
> +       V=$(PTXDIST_VERBOSE) \
> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
> +       HOSTCC=$(HOSTCC)
> +TF_A_CONF_TOOL := NO
> +TF_A_MAKE_ENV  := $(CROSS_ENV)

Do you need $(CROSS_ENV) here? (not used in e.g. u-boot.make)

> +# TF_A_DEBUG=yes
> +
> +
> +$(STATEDIR)/tf-a.prepare:
> +       @$(call targetinfo)
> +       @$(call touch)

I think this does nothing and can be removed.

> +
> +# ----------------------------------------------------------------------------
> +# Compile
> +# ----------------------------------------------------------------------------
> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
> +
> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
> +
> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
> +ifdef PTXCONF_TF_A_BL32_TSP
> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
> +endif
> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
> +endif
> +
> +ifneq ($(PTXCONF_TF_A_DTB),)
> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
> +endif
> +
> +ifdef PTXCONF_TF_A_BL32_SP_MIN
> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
> +endif
> +
> +TF_A_MAKE_OPT += all
> +
> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
> +
> +$(STATEDIR)/tf-a.compile:
> +       @$(call targetinfo)
> +       @rm -rf $(TF_A_DIR)/build/
> +       @$(call world/compile, TF_A)
> +       @$(call touch)

Can't you use the default compile rule here?

> +
> +# ----------------------------------------------------------------------------
> +# Install
> +# ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/tf-a.install:
> +       @$(call targetinfo)
> +
> +ifeq ($(TF_A_ARTIFACTS),)
> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
> +else
> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
> +endif

Shouldn't this (copying files to IMAGEDIR) happen in targetinstall
rather than install? Again based on what is being done for other
bootloaders.

Thanks,

Guillermo

> +
> +       @$(call touch)
> +
> +# ----------------------------------------------------------------------------
> +# Target-Install
> +# ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/tf-a.targetinstall:
> +       @$(call targetinfo)
> +       @$(call touch)
> +
> +# ----------------------------------------------------------------------------
> +# Clean
> +# ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/tf-a.clean:
> +       @$(call targetinfo)
> +       @$(call clean_pkg, TF_A)
> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
> +
> +# vim: syntax=make
> --
> 2.25.0
>
>
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de



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

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

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

* Re: [ptxdist] [PATCH] tf-a: new package for ARM trusted firmware A
  2020-02-10 16:50     ` Guillermo Rodriguez Garcia
@ 2020-02-11  6:20       ` Rouven Czerwinski
  0 siblings, 0 replies; 14+ messages in thread
From: Rouven Czerwinski @ 2020-02-11  6:20 UTC (permalink / raw)
  To: ptxdist, Ahmad Fatoum; +Cc: Alejandro Vazquez

On Mon, 2020-02-10 at 17:50 +0100, Guillermo Rodriguez Garcia wrote:
> El lun., 10 feb. 2020 a las 17:33, Ahmad Fatoum
> (<a.fatoum@pengutronix.de>) escribió:
> > Hello Guillermo,
> > 
> > On 2/10/20 5:29 PM, Guillermo Rodriguez Garcia wrote:
> > > Hello,
> > > 
> > > El lun., 10 feb. 2020 a las 17:16, 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>
> > > > ---
> > > >  platforms/tf-a.in | 113
> > > > ++++++++++++++++++++++++++++++++++++++++++
> > > >  rules/tf-a.make   | 123
> > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 236 insertions(+)
> > > >  create mode 100644 platforms/tf-a.in
> > > >  create mode 100644 rules/tf-a.make
> > > > 
> > > > diff --git a/platforms/tf-a.in b/platforms/tf-a.in
> > > > new file mode 100644
> > > > index 000000000000..5aa4ca473c35
> > > > --- /dev/null
> > > > +++ b/platforms/tf-a.in
> > > > @@ -0,0 +1,113 @@
> > > > +## SECTION=bootloader
> > > > +
> > > > +menuconfig TF_A
> > > > +       select BOOTLOADER
> > > > +       prompt "ARM Trusted Firmware-A"
> > > > +       depends on ARCH_ARM || ARCH_ARM64
> > > > +       bool
> > > > +
> > > > +if TF_A
> > > > +
> > > > +config TF_A_ARCH_STRING
> > > > +        string
> > > > +        default "aarch32" if ARCH_ARM
> > > > +        default "aarch64" if ARCH_ARM64
> > > > +
> > > > +config TF_A_ARM_ARCH_MAJOR
> > > > +        int
> > > > +        default "7" if ARCH_ARM
> > > > +        default "8" if ARCH_ARM64
> > > > +
> > > > +config TF_A_VERSION
> > > > +       string
> > > > +       default "v2.2"
> > > > +       prompt "TF-A version"
> > > > +       help
> > > > +         Enter the TF-A version you want to build. Usally
> > > > something like "v2.2"
> > > > +
> > > > +config TF_A_MD5
> > > > +       string
> > > > +       default "bb300e5a62c911e189c80d935d497a4b"
> > > > +       prompt "TF-A source md5"
> > > > +
> > > > +config TF_A_PLATFORM
> > > > +       string
> > > > +       prompt "TF-A target platform"
> > > > +       help
> > > > +         The TF-A target platform.
> > > > +
> > > > +if ARCH_ARM64
> > > > +config TF_A_ARM_ARCH_MINOR
> > > > +       int
> > > > +       default 0
> > > > +       prompt "TF-A target ARMv8.MINOR version"
> > > > +       help
> > > > +         The minor version of the ARMv8 architecture targeted.
> > > > Defaults to 0.
> > > > +endif
> > > > +
> > > > +config TF_A_DTB
> > > > +       string
> > > > +       prompt "TF-A DTB file name"
> > > > +       help
> > > > +         Device Tree Binary file name
> > > > +
> > > > +config TF_A_ARTIFACTS
> > > > +       string
> > > > +       prompt "TF-A artifact file names"
> > > > +       default "bl2.bin"
> > > > +       help
> > > > +         A space-separated list of artifacts to copy to the
> > > > image directory.
> > > > +         All file names are relative to the appropriate TF-A
> > > > platform build
> > > > +         directory.
> > > > +
> > > > +comment "Payloads"
> > > > +
> > > > +choice
> > > > +       prompt "BL32 Payload"
> > > > +       default TF_A_BL32_NONE
> > > > +       help
> > > > +         payload for BL32 (Secure World OS)
> > > > +
> > > > +       config TF_A_BL32_NONE
> > > > +               prompt "None"
> > > > +               bool
> > > > +
> > > > +       config TF_A_BL32_SP_MIN
> > > > +               depends on ARCH_ARM
> > > > +               prompt "sp_min"
> > > > +               bool
> > > > +
> > > > +       config TF_A_BL32_TSP
> > > > +               depends on ARCH_ARM64
> > > > +               prompt "Test Secure Payload"
> > > > +               bool
> > > > +
> > > > +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
> > > 
> > > I am missing here a way to pass additional parameters to TF-A (as
> > > in
> > > TFA_ADDITIONAL_PARAMETERS in Alejandro's version) as there are
> > > many
> > > other options (some of which may be vendor specific) that are not
> > > explicitly
> > > defined in the .in file.
> > 
> > I am not too keen on sidestepping the Kconfig by allowing free form
> > command line parameters.. Do we do this for other rules?
> 
> In most cases I agree that there are better ways, but in some cases
> it's
> the best option. See for example the "extra CFLAGS" & friends in the
> toolchain configuration.
> 
> For TF-A you need a way to pass vendor-specific parameters, as these
> may
> be different for each platform.
> 
> The buildroot rule files also added an "additional parameters" thing
> for TF-A.
> See: 
> https://git.buildroot.org/buildroot/tree/boot/arm-trusted-firmware/Config.in?id=9dbc934217e170578d4cbfdf524bc1b3988d0b9e#n103
> 
> Thanks,
> 
> Guillermo

I agree here as well, since TF-A can introduce arbitrary new parameters
which would create a lot of churn to keep the KConfig flags updated.
We also introduced this for OP-TEE, where it works well so far.

Regards,
Rouven

> > I can add it to v2 though.
> > 
> > > > diff --git a/rules/tf-a.make b/rules/tf-a.make
> > > > new file mode 100644
> > > > index 000000000000..9ee554476927
> > > > --- /dev/null
> > > > +++ b/rules/tf-a.make
> > > > @@ -0,0 +1,123 @@
> > > > +# -*-makefile-*-
> > > > +#
> > > > +# Copyright (C) 2018 by Rouven Czerwinski <
> > > > r.czerwinski@pengutronix.de>
> > > > +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > > +#
> > > > +# See CREDITS for details about who has contributed to this
> > > > project.
> > > > +#
> > > > +# For further information about the PTXdist project and
> > > > license conditions
> > > > +# see the README file.
> > > > +#
> > > > +
> > > > +#
> > > > +# We provide this package
> > > > +#
> > > > +PACKAGES-$(PTXCONF_TF_A) += tf-a
> > > > +
> > > > +#
> > > > +# Paths and names
> > > > +#
> > > > +TF_A_VERSION   := $(call
> > > > remove_quotes,$(PTXCONF_TF_A_VERSION))
> > > > +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
> > > > +TF_A           := tf-a-$(TF_A_VERSION)
> > > > +TF_A_SUFFIX    := tar.gz
> > > > +TF_A_URL       := 
> > > > https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
> > > > +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
> > > > +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
> > > > +TF_A_LICENSE    := BSD-3-Clause
> > > > +
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +# Prepare
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +
> > > > +TF_A_WRAPPER_BLACKLIST := \
> > > > +       TARGET_HARDEN_RELRO \
> > > > +       TARGET_HARDEN_BINDNOW \
> > > > +       TARGET_HARDEN_PIE \
> > > > +       TARGET_DEBUG \
> > > > +       TARGET_BUILD_ID
> > > > +
> > > > +TF_A_PATH      := PATH=$(CROSS_PATH)
> > > > +TF_A_MAKE_OPT  := \
> > > > +       V=$(PTXDIST_VERBOSE) \
> > > > +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
> > > > +       HOSTCC=$(HOSTCC)
> > > > +TF_A_CONF_TOOL := NO
> > > > +TF_A_MAKE_ENV  := $(CROSS_ENV)
> > > > +# TF_A_DEBUG=yes
> > > > +
> > > > +
> > > > +$(STATEDIR)/tf-a.prepare:
> > > > +       @$(call targetinfo)
> > > > +       @$(call touch)
> > > > +
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +# Compile
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
> > > > +
> > > > +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
> > > > +TF_A_BUILD_OUTPUT_DIR := $(call
> > > > ptx/ifdef,TF_A_DEBUG,debug,release)
> > > > +
> > > > +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
> > > > +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
> > > > +ifdef PTXCONF_TF_A_BL32_TSP
> > > > +TF_A_MAKE_OPT +=
> > > > ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRIN
> > > > G)
> > > > +endif
> > > > +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
> > > > +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
> > > > +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
> > > > +endif
> > > > +
> > > > +ifneq ($(PTXCONF_TF_A_DTB),)
> > > > +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
> > > > +endif
> > > > +
> > > > +ifdef PTXCONF_TF_A_BL32_SP_MIN
> > > > +TF_A_MAKE_OPT += AARCH32_SP=sp_min
> > > > +endif
> > > > +
> > > > +TF_A_MAKE_OPT += all
> > > > +
> > > > +TF_A_ARTIFACTS = $(addprefix
> > > > $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_
> > > > DIR)/, \
> > > > +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
> > > > +
> > > > +$(STATEDIR)/tf-a.compile:
> > > > +       @$(call targetinfo)
> > > > +       @rm -rf $(TF_A_DIR)/build/
> > > > +       @$(call world/compile, TF_A)
> > > > +       @$(call touch)
> > > > +
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +# Install
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +
> > > > +$(STATEDIR)/tf-a.install:
> > > > +       @$(call targetinfo)
> > > > +
> > > > +ifeq ($(TF_A_ARTIFACTS),)
> > > > +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
> > > > +else
> > > > +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
> > > > +endif
> > > > +
> > > > +       @$(call touch)
> > > > +
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +# Target-Install
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +
> > > > +$(STATEDIR)/tf-a.targetinstall:
> > > > +       @$(call targetinfo)
> > > > +       @$(call touch)
> > > > +
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +# Clean
> > > > +# ------------------------------------------------------------
> > > > ----------------
> > > > +
> > > > +$(STATEDIR)/tf-a.clean:
> > > > +       @$(call targetinfo)
> > > > +       @$(call clean_pkg, TF_A)
> > > > +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
> > > 
> > > This should probably remove whatever was installed in
> > > targetinstall
> > > (TF_A_ARTIFACTS)
> > > instead of "bl1.bin" and "fip.bin".
> > 
> > Thanks. Will be fixed.
> > 
> > > Guillermo
> > > 
> > 
> > --
> > 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] 14+ messages in thread

* Re: [ptxdist] [PATCH] tf-a: new package for ARM trusted firmware A
  2020-02-10 16:33   ` Ahmad Fatoum
@ 2020-02-10 16:50     ` Guillermo Rodriguez Garcia
  2020-02-11  6:20       ` Rouven Czerwinski
  0 siblings, 1 reply; 14+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-10 16:50 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Alejandro Vazquez, ptxdist

El lun., 10 feb. 2020 a las 17:33, Ahmad Fatoum
(<a.fatoum@pengutronix.de>) escribió:
>
> Hello Guillermo,
>
> On 2/10/20 5:29 PM, Guillermo Rodriguez Garcia wrote:
> > Hello,
> >
> > El lun., 10 feb. 2020 a las 17:16, 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>
> >> ---
> >>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
> >>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 236 insertions(+)
> >>  create mode 100644 platforms/tf-a.in
> >>  create mode 100644 rules/tf-a.make
> >>
> >> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
> >> new file mode 100644
> >> index 000000000000..5aa4ca473c35
> >> --- /dev/null
> >> +++ b/platforms/tf-a.in
> >> @@ -0,0 +1,113 @@
> >> +## SECTION=bootloader
> >> +
> >> +menuconfig TF_A
> >> +       select BOOTLOADER
> >> +       prompt "ARM Trusted Firmware-A"
> >> +       depends on ARCH_ARM || ARCH_ARM64
> >> +       bool
> >> +
> >> +if TF_A
> >> +
> >> +config TF_A_ARCH_STRING
> >> +        string
> >> +        default "aarch32" if ARCH_ARM
> >> +        default "aarch64" if ARCH_ARM64
> >> +
> >> +config TF_A_ARM_ARCH_MAJOR
> >> +        int
> >> +        default "7" if ARCH_ARM
> >> +        default "8" if ARCH_ARM64
> >> +
> >> +config TF_A_VERSION
> >> +       string
> >> +       default "v2.2"
> >> +       prompt "TF-A version"
> >> +       help
> >> +         Enter the TF-A version you want to build. Usally something like "v2.2"
> >> +
> >> +config TF_A_MD5
> >> +       string
> >> +       default "bb300e5a62c911e189c80d935d497a4b"
> >> +       prompt "TF-A source md5"
> >> +
> >> +config TF_A_PLATFORM
> >> +       string
> >> +       prompt "TF-A target platform"
> >> +       help
> >> +         The TF-A target platform.
> >> +
> >> +if ARCH_ARM64
> >> +config TF_A_ARM_ARCH_MINOR
> >> +       int
> >> +       default 0
> >> +       prompt "TF-A target ARMv8.MINOR version"
> >> +       help
> >> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
> >> +endif
> >> +
> >> +config TF_A_DTB
> >> +       string
> >> +       prompt "TF-A DTB file name"
> >> +       help
> >> +         Device Tree Binary file name
> >> +
> >> +config TF_A_ARTIFACTS
> >> +       string
> >> +       prompt "TF-A artifact file names"
> >> +       default "bl2.bin"
> >> +       help
> >> +         A space-separated list of artifacts to copy to the image directory.
> >> +         All file names are relative to the appropriate TF-A platform build
> >> +         directory.
> >> +
> >> +comment "Payloads"
> >> +
> >> +choice
> >> +       prompt "BL32 Payload"
> >> +       default TF_A_BL32_NONE
> >> +       help
> >> +         payload for BL32 (Secure World OS)
> >> +
> >> +       config TF_A_BL32_NONE
> >> +               prompt "None"
> >> +               bool
> >> +
> >> +       config TF_A_BL32_SP_MIN
> >> +               depends on ARCH_ARM
> >> +               prompt "sp_min"
> >> +               bool
> >> +
> >> +       config TF_A_BL32_TSP
> >> +               depends on ARCH_ARM64
> >> +               prompt "Test Secure Payload"
> >> +               bool
> >> +
> >> +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
> >
> > I am missing here a way to pass additional parameters to TF-A (as in
> > TFA_ADDITIONAL_PARAMETERS in Alejandro's version) as there are many
> > other options (some of which may be vendor specific) that are not explicitly
> > defined in the .in file.
>
> I am not too keen on sidestepping the Kconfig by allowing free form
> command line parameters.. Do we do this for other rules?

In most cases I agree that there are better ways, but in some cases it's
the best option. See for example the "extra CFLAGS" & friends in the
toolchain configuration.

For TF-A you need a way to pass vendor-specific parameters, as these may
be different for each platform.

The buildroot rule files also added an "additional parameters" thing for TF-A.
See: https://git.buildroot.org/buildroot/tree/boot/arm-trusted-firmware/Config.in?id=9dbc934217e170578d4cbfdf524bc1b3988d0b9e#n103

Thanks,

Guillermo

>
> I can add it to v2 though.
>
> >> diff --git a/rules/tf-a.make b/rules/tf-a.make
> >> new file mode 100644
> >> index 000000000000..9ee554476927
> >> --- /dev/null
> >> +++ b/rules/tf-a.make
> >> @@ -0,0 +1,123 @@
> >> +# -*-makefile-*-
> >> +#
> >> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
> >> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> +#
> >> +# See CREDITS for details about who has contributed to this project.
> >> +#
> >> +# For further information about the PTXdist project and license conditions
> >> +# see the README file.
> >> +#
> >> +
> >> +#
> >> +# We provide this package
> >> +#
> >> +PACKAGES-$(PTXCONF_TF_A) += tf-a
> >> +
> >> +#
> >> +# Paths and names
> >> +#
> >> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
> >> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
> >> +TF_A           := tf-a-$(TF_A_VERSION)
> >> +TF_A_SUFFIX    := tar.gz
> >> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
> >> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
> >> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
> >> +TF_A_LICENSE    := BSD-3-Clause
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Prepare
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +TF_A_WRAPPER_BLACKLIST := \
> >> +       TARGET_HARDEN_RELRO \
> >> +       TARGET_HARDEN_BINDNOW \
> >> +       TARGET_HARDEN_PIE \
> >> +       TARGET_DEBUG \
> >> +       TARGET_BUILD_ID
> >> +
> >> +TF_A_PATH      := PATH=$(CROSS_PATH)
> >> +TF_A_MAKE_OPT  := \
> >> +       V=$(PTXDIST_VERBOSE) \
> >> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
> >> +       HOSTCC=$(HOSTCC)
> >> +TF_A_CONF_TOOL := NO
> >> +TF_A_MAKE_ENV  := $(CROSS_ENV)
> >> +# TF_A_DEBUG=yes
> >> +
> >> +
> >> +$(STATEDIR)/tf-a.prepare:
> >> +       @$(call targetinfo)
> >> +       @$(call touch)
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Compile
> >> +# ----------------------------------------------------------------------------
> >> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
> >> +
> >> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
> >> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
> >> +
> >> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
> >> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
> >> +ifdef PTXCONF_TF_A_BL32_TSP
> >> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
> >> +endif
> >> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
> >> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
> >> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
> >> +endif
> >> +
> >> +ifneq ($(PTXCONF_TF_A_DTB),)
> >> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
> >> +endif
> >> +
> >> +ifdef PTXCONF_TF_A_BL32_SP_MIN
> >> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
> >> +endif
> >> +
> >> +TF_A_MAKE_OPT += all
> >> +
> >> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
> >> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
> >> +
> >> +$(STATEDIR)/tf-a.compile:
> >> +       @$(call targetinfo)
> >> +       @rm -rf $(TF_A_DIR)/build/
> >> +       @$(call world/compile, TF_A)
> >> +       @$(call touch)
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Install
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +$(STATEDIR)/tf-a.install:
> >> +       @$(call targetinfo)
> >> +
> >> +ifeq ($(TF_A_ARTIFACTS),)
> >> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
> >> +else
> >> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
> >> +endif
> >> +
> >> +       @$(call touch)
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Target-Install
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +$(STATEDIR)/tf-a.targetinstall:
> >> +       @$(call targetinfo)
> >> +       @$(call touch)
> >> +
> >> +# ----------------------------------------------------------------------------
> >> +# Clean
> >> +# ----------------------------------------------------------------------------
> >> +
> >> +$(STATEDIR)/tf-a.clean:
> >> +       @$(call targetinfo)
> >> +       @$(call clean_pkg, TF_A)
> >> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
> >
> > This should probably remove whatever was installed in targetinstall
> > (TF_A_ARTIFACTS)
> > instead of "bl1.bin" and "fip.bin".
>
> Thanks. Will be fixed.
>
> >
> > Guillermo
> >
>
>
> --
> 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] 14+ messages in thread

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

Hello Guillermo,

On 2/10/20 5:29 PM, Guillermo Rodriguez Garcia wrote:
> Hello,
> 
> El lun., 10 feb. 2020 a las 17:16, 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>
>> ---
>>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
>>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 236 insertions(+)
>>  create mode 100644 platforms/tf-a.in
>>  create mode 100644 rules/tf-a.make
>>
>> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
>> new file mode 100644
>> index 000000000000..5aa4ca473c35
>> --- /dev/null
>> +++ b/platforms/tf-a.in
>> @@ -0,0 +1,113 @@
>> +## SECTION=bootloader
>> +
>> +menuconfig TF_A
>> +       select BOOTLOADER
>> +       prompt "ARM Trusted Firmware-A"
>> +       depends on ARCH_ARM || ARCH_ARM64
>> +       bool
>> +
>> +if TF_A
>> +
>> +config TF_A_ARCH_STRING
>> +        string
>> +        default "aarch32" if ARCH_ARM
>> +        default "aarch64" if ARCH_ARM64
>> +
>> +config TF_A_ARM_ARCH_MAJOR
>> +        int
>> +        default "7" if ARCH_ARM
>> +        default "8" if ARCH_ARM64
>> +
>> +config TF_A_VERSION
>> +       string
>> +       default "v2.2"
>> +       prompt "TF-A version"
>> +       help
>> +         Enter the TF-A version you want to build. Usally something like "v2.2"
>> +
>> +config TF_A_MD5
>> +       string
>> +       default "bb300e5a62c911e189c80d935d497a4b"
>> +       prompt "TF-A source md5"
>> +
>> +config TF_A_PLATFORM
>> +       string
>> +       prompt "TF-A target platform"
>> +       help
>> +         The TF-A target platform.
>> +
>> +if ARCH_ARM64
>> +config TF_A_ARM_ARCH_MINOR
>> +       int
>> +       default 0
>> +       prompt "TF-A target ARMv8.MINOR version"
>> +       help
>> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
>> +endif
>> +
>> +config TF_A_DTB
>> +       string
>> +       prompt "TF-A DTB file name"
>> +       help
>> +         Device Tree Binary file name
>> +
>> +config TF_A_ARTIFACTS
>> +       string
>> +       prompt "TF-A artifact file names"
>> +       default "bl2.bin"
>> +       help
>> +         A space-separated list of artifacts to copy to the image directory.
>> +         All file names are relative to the appropriate TF-A platform build
>> +         directory.
>> +
>> +comment "Payloads"
>> +
>> +choice
>> +       prompt "BL32 Payload"
>> +       default TF_A_BL32_NONE
>> +       help
>> +         payload for BL32 (Secure World OS)
>> +
>> +       config TF_A_BL32_NONE
>> +               prompt "None"
>> +               bool
>> +
>> +       config TF_A_BL32_SP_MIN
>> +               depends on ARCH_ARM
>> +               prompt "sp_min"
>> +               bool
>> +
>> +       config TF_A_BL32_TSP
>> +               depends on ARCH_ARM64
>> +               prompt "Test Secure Payload"
>> +               bool
>> +
>> +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
> 
> I am missing here a way to pass additional parameters to TF-A (as in
> TFA_ADDITIONAL_PARAMETERS in Alejandro's version) as there are many
> other options (some of which may be vendor specific) that are not explicitly
> defined in the .in file.

I am not too keen on sidestepping the Kconfig by allowing free form
command line parameters.. Do we do this for other rules?

I can add it to v2 though. 

>> diff --git a/rules/tf-a.make b/rules/tf-a.make
>> new file mode 100644
>> index 000000000000..9ee554476927
>> --- /dev/null
>> +++ b/rules/tf-a.make
>> @@ -0,0 +1,123 @@
>> +# -*-makefile-*-
>> +#
>> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
>> +#
>> +# See CREDITS for details about who has contributed to this project.
>> +#
>> +# For further information about the PTXdist project and license conditions
>> +# see the README file.
>> +#
>> +
>> +#
>> +# We provide this package
>> +#
>> +PACKAGES-$(PTXCONF_TF_A) += tf-a
>> +
>> +#
>> +# Paths and names
>> +#
>> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
>> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
>> +TF_A           := tf-a-$(TF_A_VERSION)
>> +TF_A_SUFFIX    := tar.gz
>> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
>> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
>> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
>> +TF_A_LICENSE    := BSD-3-Clause
>> +
>> +# ----------------------------------------------------------------------------
>> +# Prepare
>> +# ----------------------------------------------------------------------------
>> +
>> +TF_A_WRAPPER_BLACKLIST := \
>> +       TARGET_HARDEN_RELRO \
>> +       TARGET_HARDEN_BINDNOW \
>> +       TARGET_HARDEN_PIE \
>> +       TARGET_DEBUG \
>> +       TARGET_BUILD_ID
>> +
>> +TF_A_PATH      := PATH=$(CROSS_PATH)
>> +TF_A_MAKE_OPT  := \
>> +       V=$(PTXDIST_VERBOSE) \
>> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
>> +       HOSTCC=$(HOSTCC)
>> +TF_A_CONF_TOOL := NO
>> +TF_A_MAKE_ENV  := $(CROSS_ENV)
>> +# TF_A_DEBUG=yes
>> +
>> +
>> +$(STATEDIR)/tf-a.prepare:
>> +       @$(call targetinfo)
>> +       @$(call touch)
>> +
>> +# ----------------------------------------------------------------------------
>> +# Compile
>> +# ----------------------------------------------------------------------------
>> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
>> +
>> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
>> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
>> +
>> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
>> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
>> +ifdef PTXCONF_TF_A_BL32_TSP
>> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
>> +endif
>> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
>> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
>> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
>> +endif
>> +
>> +ifneq ($(PTXCONF_TF_A_DTB),)
>> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
>> +endif
>> +
>> +ifdef PTXCONF_TF_A_BL32_SP_MIN
>> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
>> +endif
>> +
>> +TF_A_MAKE_OPT += all
>> +
>> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
>> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
>> +
>> +$(STATEDIR)/tf-a.compile:
>> +       @$(call targetinfo)
>> +       @rm -rf $(TF_A_DIR)/build/
>> +       @$(call world/compile, TF_A)
>> +       @$(call touch)
>> +
>> +# ----------------------------------------------------------------------------
>> +# Install
>> +# ----------------------------------------------------------------------------
>> +
>> +$(STATEDIR)/tf-a.install:
>> +       @$(call targetinfo)
>> +
>> +ifeq ($(TF_A_ARTIFACTS),)
>> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
>> +else
>> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
>> +endif
>> +
>> +       @$(call touch)
>> +
>> +# ----------------------------------------------------------------------------
>> +# Target-Install
>> +# ----------------------------------------------------------------------------
>> +
>> +$(STATEDIR)/tf-a.targetinstall:
>> +       @$(call targetinfo)
>> +       @$(call touch)
>> +
>> +# ----------------------------------------------------------------------------
>> +# Clean
>> +# ----------------------------------------------------------------------------
>> +
>> +$(STATEDIR)/tf-a.clean:
>> +       @$(call targetinfo)
>> +       @$(call clean_pkg, TF_A)
>> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
> 
> This should probably remove whatever was installed in targetinstall
> (TF_A_ARTIFACTS)
> instead of "bl1.bin" and "fip.bin".

Thanks. Will be fixed.

> 
> Guillermo
> 


-- 
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] 14+ messages in thread

* Re: [ptxdist] [PATCH] tf-a: new package for ARM trusted firmware A
  2020-02-10 16:15 Ahmad Fatoum
@ 2020-02-10 16:29 ` Guillermo Rodriguez Garcia
  2020-02-10 16:33   ` Ahmad Fatoum
  2020-02-11  8:37 ` Guillermo Rodriguez Garcia
  1 sibling, 1 reply; 14+ messages in thread
From: Guillermo Rodriguez Garcia @ 2020-02-10 16:29 UTC (permalink / raw)
  To: ptxdist; +Cc: Alejandro Vazquez, Ahmad Fatoum

Hello,

El lun., 10 feb. 2020 a las 17:16, 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>
> ---
>  platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
>  rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 236 insertions(+)
>  create mode 100644 platforms/tf-a.in
>  create mode 100644 rules/tf-a.make
>
> diff --git a/platforms/tf-a.in b/platforms/tf-a.in
> new file mode 100644
> index 000000000000..5aa4ca473c35
> --- /dev/null
> +++ b/platforms/tf-a.in
> @@ -0,0 +1,113 @@
> +## SECTION=bootloader
> +
> +menuconfig TF_A
> +       select BOOTLOADER
> +       prompt "ARM Trusted Firmware-A"
> +       depends on ARCH_ARM || ARCH_ARM64
> +       bool
> +
> +if TF_A
> +
> +config TF_A_ARCH_STRING
> +        string
> +        default "aarch32" if ARCH_ARM
> +        default "aarch64" if ARCH_ARM64
> +
> +config TF_A_ARM_ARCH_MAJOR
> +        int
> +        default "7" if ARCH_ARM
> +        default "8" if ARCH_ARM64
> +
> +config TF_A_VERSION
> +       string
> +       default "v2.2"
> +       prompt "TF-A version"
> +       help
> +         Enter the TF-A version you want to build. Usally something like "v2.2"
> +
> +config TF_A_MD5
> +       string
> +       default "bb300e5a62c911e189c80d935d497a4b"
> +       prompt "TF-A source md5"
> +
> +config TF_A_PLATFORM
> +       string
> +       prompt "TF-A target platform"
> +       help
> +         The TF-A target platform.
> +
> +if ARCH_ARM64
> +config TF_A_ARM_ARCH_MINOR
> +       int
> +       default 0
> +       prompt "TF-A target ARMv8.MINOR version"
> +       help
> +         The minor version of the ARMv8 architecture targeted. Defaults to 0.
> +endif
> +
> +config TF_A_DTB
> +       string
> +       prompt "TF-A DTB file name"
> +       help
> +         Device Tree Binary file name
> +
> +config TF_A_ARTIFACTS
> +       string
> +       prompt "TF-A artifact file names"
> +       default "bl2.bin"
> +       help
> +         A space-separated list of artifacts to copy to the image directory.
> +         All file names are relative to the appropriate TF-A platform build
> +         directory.
> +
> +comment "Payloads"
> +
> +choice
> +       prompt "BL32 Payload"
> +       default TF_A_BL32_NONE
> +       help
> +         payload for BL32 (Secure World OS)
> +
> +       config TF_A_BL32_NONE
> +               prompt "None"
> +               bool
> +
> +       config TF_A_BL32_SP_MIN
> +               depends on ARCH_ARM
> +               prompt "sp_min"
> +               bool
> +
> +       config TF_A_BL32_TSP
> +               depends on ARCH_ARM64
> +               prompt "Test Secure Payload"
> +               bool
> +
> +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

I am missing here a way to pass additional parameters to TF-A (as in
TFA_ADDITIONAL_PARAMETERS in Alejandro's version) as there are many
other options (some of which may be vendor specific) that are not explicitly
defined in the .in file.

> diff --git a/rules/tf-a.make b/rules/tf-a.make
> new file mode 100644
> index 000000000000..9ee554476927
> --- /dev/null
> +++ b/rules/tf-a.make
> @@ -0,0 +1,123 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
> +#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
> +#
> +# See CREDITS for details about who has contributed to this project.
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +#
> +# We provide this package
> +#
> +PACKAGES-$(PTXCONF_TF_A) += tf-a
> +
> +#
> +# Paths and names
> +#
> +TF_A_VERSION   := $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
> +TF_A_MD5       := $(call remove_quotes,$(PTXCONF_TF_A_MD5))
> +TF_A           := tf-a-$(TF_A_VERSION)
> +TF_A_SUFFIX    := tar.gz
> +TF_A_URL       := https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
> +TF_A_SOURCE    := $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
> +TF_A_DIR       := $(BUILDDIR)/$(TF_A)
> +TF_A_LICENSE    := BSD-3-Clause
> +
> +# ----------------------------------------------------------------------------
> +# Prepare
> +# ----------------------------------------------------------------------------
> +
> +TF_A_WRAPPER_BLACKLIST := \
> +       TARGET_HARDEN_RELRO \
> +       TARGET_HARDEN_BINDNOW \
> +       TARGET_HARDEN_PIE \
> +       TARGET_DEBUG \
> +       TARGET_BUILD_ID
> +
> +TF_A_PATH      := PATH=$(CROSS_PATH)
> +TF_A_MAKE_OPT  := \
> +       V=$(PTXDIST_VERBOSE) \
> +       CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
> +       HOSTCC=$(HOSTCC)
> +TF_A_CONF_TOOL := NO
> +TF_A_MAKE_ENV  := $(CROSS_ENV)
> +# TF_A_DEBUG=yes
> +
> +
> +$(STATEDIR)/tf-a.prepare:
> +       @$(call targetinfo)
> +       @$(call touch)
> +
> +# ----------------------------------------------------------------------------
> +# Compile
> +# ----------------------------------------------------------------------------
> +TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
> +
> +TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
> +TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
> +
> +TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
> +TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
> +ifdef PTXCONF_TF_A_BL32_TSP
> +TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
> +endif
> +TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
> +ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
> +TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
> +endif
> +
> +ifneq ($(PTXCONF_TF_A_DTB),)
> +TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
> +endif
> +
> +ifdef PTXCONF_TF_A_BL32_SP_MIN
> +TF_A_MAKE_OPT += AARCH32_SP=sp_min
> +endif
> +
> +TF_A_MAKE_OPT += all
> +
> +TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
> +       $(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
> +
> +$(STATEDIR)/tf-a.compile:
> +       @$(call targetinfo)
> +       @rm -rf $(TF_A_DIR)/build/
> +       @$(call world/compile, TF_A)
> +       @$(call touch)
> +
> +# ----------------------------------------------------------------------------
> +# Install
> +# ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/tf-a.install:
> +       @$(call targetinfo)
> +
> +ifeq ($(TF_A_ARTIFACTS),)
> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
> +else
> +       @install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
> +endif
> +
> +       @$(call touch)
> +
> +# ----------------------------------------------------------------------------
> +# Target-Install
> +# ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/tf-a.targetinstall:
> +       @$(call targetinfo)
> +       @$(call touch)
> +
> +# ----------------------------------------------------------------------------
> +# Clean
> +# ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/tf-a.clean:
> +       @$(call targetinfo)
> +       @$(call clean_pkg, TF_A)
> +       @rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin

This should probably remove whatever was installed in targetinstall
(TF_A_ARTIFACTS)
instead of "bl1.bin" and "fip.bin".

Guillermo

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

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

* [ptxdist] [PATCH] tf-a: new package for ARM trusted firmware A
@ 2020-02-10 16:15 Ahmad Fatoum
  2020-02-10 16:29 ` Guillermo Rodriguez Garcia
  2020-02-11  8:37 ` Guillermo Rodriguez Garcia
  0 siblings, 2 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2020-02-10 16:15 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>
---
 platforms/tf-a.in | 113 ++++++++++++++++++++++++++++++++++++++++++
 rules/tf-a.make   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 236 insertions(+)
 create mode 100644 platforms/tf-a.in
 create mode 100644 rules/tf-a.make

diff --git a/platforms/tf-a.in b/platforms/tf-a.in
new file mode 100644
index 000000000000..5aa4ca473c35
--- /dev/null
+++ b/platforms/tf-a.in
@@ -0,0 +1,113 @@
+## SECTION=bootloader
+
+menuconfig TF_A
+	select BOOTLOADER
+	prompt "ARM Trusted Firmware-A"
+	depends on ARCH_ARM || ARCH_ARM64
+	bool
+
+if TF_A
+
+config TF_A_ARCH_STRING
+        string
+        default "aarch32" if ARCH_ARM
+        default "aarch64" if ARCH_ARM64
+
+config TF_A_ARM_ARCH_MAJOR
+        int
+        default "7" if ARCH_ARM
+        default "8" if ARCH_ARM64
+
+config TF_A_VERSION
+	string
+	default "v2.2"
+	prompt "TF-A version"
+	help
+	  Enter the TF-A version you want to build. Usally something like "v2.2"
+
+config TF_A_MD5
+	string
+	default "bb300e5a62c911e189c80d935d497a4b"
+	prompt "TF-A source md5"
+
+config TF_A_PLATFORM
+	string
+	prompt "TF-A target platform"
+	help
+	  The TF-A target platform.
+
+if ARCH_ARM64
+config TF_A_ARM_ARCH_MINOR
+	int
+	default 0
+	prompt "TF-A target ARMv8.MINOR version"
+	help
+	  The minor version of the ARMv8 architecture targeted. Defaults to 0.
+endif
+
+config TF_A_DTB
+	string
+	prompt "TF-A DTB file name"
+	help
+	  Device Tree Binary file name
+
+config TF_A_ARTIFACTS
+	string
+	prompt "TF-A artifact file names"
+	default "bl2.bin"
+	help
+	  A space-separated list of artifacts to copy to the image directory.
+	  All file names are relative to the appropriate TF-A platform build
+	  directory.
+
+comment "Payloads"
+
+choice
+	prompt "BL32 Payload"
+	default TF_A_BL32_NONE
+	help
+	  payload for BL32 (Secure World OS)
+
+	config TF_A_BL32_NONE
+		prompt "None"
+		bool
+
+	config TF_A_BL32_SP_MIN
+		depends on ARCH_ARM
+		prompt "sp_min"
+		bool
+
+	config TF_A_BL32_TSP
+		depends on ARCH_ARM64
+		prompt "Test Secure Payload"
+		bool
+
+endchoice
+
+if TF_A_BL32_TSP
+choice TF_A_BL32_TSP_RAM_LOCATION
+	prompt "TSP location"
+	default TF_A_BL32_TSP_RAM_LOCATION_TSRAM
+
+	config TF_A_BL32_TSP_RAM_LOCATION_TSRAM
+		prompt "Trusted SRAM"
+		bool
+
+	config TF_A_BL32_TSP_RAM_LOCATION_TDRAM
+		prompt "Trusted DRAM (if available)"
+		bool
+
+	config TF_A_BL32_TSP_RAM_LOCATION_DRAM
+		prompt "Secure DRAM region (configured by TrustZone controller)"
+		bool
+endchoice
+
+config TF_A_BL32_TSP_RAM_LOCATION_STRING
+        string
+        default "tsram" if TF_A_BL32_TSP_RAM_LOCATION_TSRAM
+        default "tdram" if TF_A_BL32_TSP_RAM_LOCATION_TDRAM
+        default "dram"  if TF_A_BL32_TSP_RAM_LOCATION_DRAM
+
+endif
+
+endif
diff --git a/rules/tf-a.make b/rules/tf-a.make
new file mode 100644
index 000000000000..9ee554476927
--- /dev/null
+++ b/rules/tf-a.make
@@ -0,0 +1,123 @@
+# -*-makefile-*-
+#
+# Copyright (C) 2018 by Rouven Czerwinski <r.czerwinski@pengutronix.de>
+#               2019 by Ahmad Fatoum <a.fatoum@pengutronix.de>
+#
+# See CREDITS for details about who has contributed to this project.
+#
+# For further information about the PTXdist project and license conditions
+# see the README file.
+#
+
+#
+# We provide this package
+#
+PACKAGES-$(PTXCONF_TF_A) += tf-a
+
+#
+# Paths and names
+#
+TF_A_VERSION	:= $(call remove_quotes,$(PTXCONF_TF_A_VERSION))
+TF_A_MD5	:= $(call remove_quotes,$(PTXCONF_TF_A_MD5))
+TF_A		:= tf-a-$(TF_A_VERSION)
+TF_A_SUFFIX	:= tar.gz
+TF_A_URL	:= https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/snapshot/$(TF_A_VERSION).$(TF_A_SUFFIX)
+TF_A_SOURCE	:= $(SRCDIR)/$(TF_A).$(TF_A_SUFFIX)
+TF_A_DIR	:= $(BUILDDIR)/$(TF_A)
+TF_A_LICENSE    := BSD-3-Clause
+
+# ----------------------------------------------------------------------------
+# Prepare
+# ----------------------------------------------------------------------------
+
+TF_A_WRAPPER_BLACKLIST := \
+	TARGET_HARDEN_RELRO \
+	TARGET_HARDEN_BINDNOW \
+	TARGET_HARDEN_PIE \
+	TARGET_DEBUG \
+	TARGET_BUILD_ID
+
+TF_A_PATH	:= PATH=$(CROSS_PATH)
+TF_A_MAKE_OPT	:= \
+	V=$(PTXDIST_VERBOSE) \
+	CROSS_COMPILE=$(BOOTLOADER_CROSS_COMPILE) \
+	HOSTCC=$(HOSTCC)
+TF_A_CONF_TOOL	:= NO
+TF_A_MAKE_ENV	:= $(CROSS_ENV)
+# TF_A_DEBUG=yes
+
+
+$(STATEDIR)/tf-a.prepare:
+	@$(call targetinfo)
+	@$(call touch)
+
+# ----------------------------------------------------------------------------
+# Compile
+# ----------------------------------------------------------------------------
+TF_A_MAKE_OPT += PLAT=$(PTXCONF_TF_A_PLATFORM)
+
+TF_A_MAKE_OPT += DEBUG=$(call ptx/ifdef,TF_A_DEBUG,1,0)
+TF_A_BUILD_OUTPUT_DIR := $(call ptx/ifdef,TF_A_DEBUG,debug,release)
+
+TF_A_MAKE_OPT += ARCH=$(PTXCONF_TF_A_ARCH_STRING)
+TF_A_MAKE_OPT += LOAD_IMAGE_V2=1
+ifdef PTXCONF_TF_A_BL32_TSP
+TF_A_MAKE_OPT += ARM_TSP_RAM_LOCATION=$(PTXCONF_TF_A_BL32_TSP_RAM_LOCATION_STRING)
+endif
+TF_A_MAKE_OPT += ARM_ARCH_MAJOR=$(PTXCONF_TF_A_ARM_ARCH_MAJOR)
+ifdef PTXCONF_TF_A_ARM_ARCH_MINOR
+TF_A_MAKE_OPT += ARM_ARCH_MINOR=$(PTXCONF_TF_A_ARM_ARCH_MINOR)
+endif
+
+ifneq ($(PTXCONF_TF_A_DTB),)
+TF_A_MAKE_OPT += DTB_FILE_NAME=$(PTXCONF_TF_A_DTB)
+endif
+
+ifdef PTXCONF_TF_A_BL32_SP_MIN
+TF_A_MAKE_OPT += AARCH32_SP=sp_min
+endif
+
+TF_A_MAKE_OPT += all
+
+TF_A_ARTIFACTS = $(addprefix $(TF_A_DIR)/build/$(PTXCONF_TF_A_PLATFORM)/$(TF_A_BUILD_OUTPUT_DIR)/, \
+	$(call remove_quotes,$(PTXCONF_TF_A_ARTIFACTS)))
+
+$(STATEDIR)/tf-a.compile:
+	@$(call targetinfo)
+	@rm -rf $(TF_A_DIR)/build/
+	@$(call world/compile, TF_A)
+	@$(call touch)
+
+# ----------------------------------------------------------------------------
+# Install
+# ----------------------------------------------------------------------------
+
+$(STATEDIR)/tf-a.install:
+	@$(call targetinfo)
+
+ifeq ($(TF_A_ARTIFACTS),)
+	$(warning TF_A_ARTIFACTS is empty. nothing to install.)
+else
+	@install -D -m644 $(TF_A_ARTIFACTS) $(IMAGEDIR)
+endif
+
+	@$(call touch)
+
+# ----------------------------------------------------------------------------
+# Target-Install
+# ----------------------------------------------------------------------------
+
+$(STATEDIR)/tf-a.targetinstall:
+	@$(call targetinfo)
+	@$(call touch)
+
+# ----------------------------------------------------------------------------
+# Clean
+# ----------------------------------------------------------------------------
+
+$(STATEDIR)/tf-a.clean:
+	@$(call targetinfo)
+	@$(call clean_pkg, TF_A)
+	@rm -f $(IMAGEDIR)/bl1.bin $(IMAGEDIR)/fip.bin
+
+# vim: syntax=make
-- 
2.25.0


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

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

end of thread, other threads:[~2020-02-12 16:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-12 16:36 [ptxdist] [PATCH] tf-a: new package for ARM trusted firmware A Ahmad Fatoum
2020-02-12 16:38 ` Ahmad Fatoum
  -- strict thread matches above, loose matches on Subject: below --
2020-02-10 16:15 Ahmad Fatoum
2020-02-10 16:29 ` Guillermo Rodriguez Garcia
2020-02-10 16:33   ` Ahmad Fatoum
2020-02-10 16:50     ` Guillermo Rodriguez Garcia
2020-02-11  6:20       ` Rouven Czerwinski
2020-02-11  8:37 ` Guillermo Rodriguez Garcia
2020-02-11 15:22   ` Ahmad Fatoum
2020-02-11 16:27     ` Guillermo Rodriguez Garcia
2020-02-11 16:53       ` Ahmad Fatoum
2020-02-12  9:07         ` Guillermo Rodriguez Garcia
2020-02-12  9:28           ` Ahmad Fatoum
2020-02-12  9:32             ` Guillermo Rodriguez Garcia

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