Re: [ptxdist] [PATCH v3] tf-a: new package for ARM trusted firmware A

[Michael Olbrich <m.olbrich@pengutronix.de>]

On Tue, Feb 25, 2020 at 10:20:52AM +0100, Philipp Zabel wrote:
> Hi,
> 
> On Wed, 2020-02-19 at 11:42 +0100, Ahmad Fatoum wrote:
> > Trusted Firmware-A (TF-A) is a reference implementation of secure world
> > software for Arm A-Profile architectures (Armv8-A and Armv7-A).
> > 
> > Cc: Alejandro Vazquez <avazquez.dev@gmail.com>
> > Signed-off-by: Rouven Czerwinski <rouven@czerwinskis.de>
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> > v2 -> v3:
> >   - sorted version and md5 Kconfig options to top (mol)
> >   - Added trailing spaces to align arrow after main prompt (mol)
> >   - Changed TF_A_VERSION help text to indicate that the version can be any
> >     git commitsh
> >   - Removed default value for TF_A_ARTIFACTS (mol)
> >   - integrated TF_A_VERSION into BUILD_STRING of resulting artifacts
> >     (Guillermo, mtr)
> >   - unconditional error if TF_A_ARTIFACTS is empty instead of warning later on
> >     (mol)
> >   - removed diplicate Prepare header (mol)
> >   - resorted make variable definitions
> >   - avoid unportable install --target-directory (Guillermo)
> >   - documented support for globbing in TF_A_ARTIFACTS
> >   - don't glob delete in IMAGEDIR, instead expand the glob in the build
> >     directory first
> > v1 -> v2:
> >   - Made TF_A_ARCH_MAJOR configurable to support 32 bit ARMv8 (Guillermo)
> >   - Replaces stm32mp-specific TF_A_DTB with TF_A_EXTRA_ARGS to contain
> >     all board/vendor specific options
> >   - removed reference to no longer existing CREDITS file
> >   - removed TF_A_MAKE_OPT contents that are set elsewhere
> >   - reduced uses of += in favor of directly appending to the string
> >   - delete old build directory in prepare instead of compile
> >   - use default compile stage (Guillermo)
> >   - install artifacts to sysroot /usr/lib/firmware in install stage
> >   - install artifacts to IMAGEDIR in targetinstall
> >   - fix clean stage to delete proper artifacts
> > ---
> >  platforms/tf-a.in | 140 ++++++++++++++++++++++++++++++++++++++++++++++
> >  rules/tf-a.make   | 118 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 258 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..75dad9d159f4
> > --- /dev/null
> > +++ b/platforms/tf-a.in
> > @@ -0,0 +1,140 @@
> [...]
> > +config TF_A_VERSION
> > +	string
> > +	default "v2.2"
> > +	prompt "TF-A version"
> > +	help
> > +	  Enter the TF-A git commitsh you want to build. Usally a tagged release
>                                    ^                       ^
> Some missing vowels here. Also, the git documentation uses spells it
> "commit-ish" [1], but in most places it just says "commit".
> 
> [1] https://git-scm.com/docs/gitglossary

"commit-ish" is correct here. Because any commit-ish can be used here (e.g.
tags), not just commits.

> [...]
> > diff --git a/rules/tf-a.make b/rules/tf-a.make
> > new file mode 100644
> > index 000000000000..bb7ba9e96822
> > --- /dev/null
> > +++ b/rules/tf-a.make
> > @@ -0,0 +1,118 @@
> [...]
> > +#
> > +# 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
> 
> What about the other licenses listed in docs/licenses.rst, such as NCSA,
> and Zlib? Does any of this code make it into the TF-A binary?
> 
> > +
> > +# ----------------------------------------------------------------------------
> > +# Prepare
> > +# ----------------------------------------------------------------------------
> > +
> > +TF_A_WRAPPER_BLACKLIST := \
> > +	TARGET_HARDEN_RELRO \
> > +	TARGET_HARDEN_BINDNOW \
> > +	TARGET_HARDEN_PIE \
> > +	TARGET_DEBUG \
> > +	TARGET_BUILD_ID
> > +
> > +# TF_A_DEBUG=1
> 
> Is this a leftover? I see no other packages with a commented out
> FOO_DEBUG=1 line.

I asked him to remove this in a previous version. So far he is resisting.
But I can do a fixup here if necessary :-).

Michael

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

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de