Re: [ptxdist] [PATCH] networkmanager: version bump 1.20.8-> 1.22.6

2020-02-12 Thread Uwe Kleine-König
On Wed, Feb 12, 2020 at 11:11:12AM +0100, Ulrich Ölmann wrote:
> Two new config options have been added in the meantime (see NetworkManager
> commits [1] & [2]) which are both set to their reasonable default values.
> 
> The license did not change, but its text was adjusted - citing from
> NetworkManagers's commit [3]:
> 
>   The update to the GPL text is purely cosmetic. However, shipping the exact
>   same file as GNU publishes may help distros that deduplicate the license 
> texts
>   or hardlink duplicates.
> 
> [1] 
> https://gitlab.freedesktop.org/NetworkManager/NetworkManager/commit/69f048bf0ca387d2dc4683cfdfe9d170bfceb52b
> [2] 
> https://gitlab.freedesktop.org/NetworkManager/NetworkManager/commit/d27fcd07541ae6f524115d5b0f36e14673135ca3
> [3] 
> https://gitlab.freedesktop.org/NetworkManager/NetworkManager/commit/e9f2ea6c22f36cb7986d2228763629ed44b9e76b
> 
> Signed-off-by: Ulrich Ölmann 
> ---
>  rules/networkmanager.make | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/rules/networkmanager.make b/rules/networkmanager.make
> index de0e755b3a5d..86542b6f74e1 100644
> --- a/rules/networkmanager.make
> +++ b/rules/networkmanager.make
> @@ -15,15 +15,15 @@ PACKAGES-$(PTXCONF_NETWORKMANAGER) += networkmanager
>  #
>  # Paths and names
>  #
> -NETWORKMANAGER_VERSION   := 1.20.8
> -NETWORKMANAGER_MD5   := c0ceb5ab14bfdfeee07536d94cc5c548
> +NETWORKMANAGER_VERSION   := 1.22.6
> +NETWORKMANAGER_MD5   := 0f4b493cc0c67f94a2ad3573363eb3b2
>  NETWORKMANAGER   := NetworkManager-$(NETWORKMANAGER_VERSION)
>  NETWORKMANAGER_SUFFIX:= tar.xz
> -NETWORKMANAGER_URL   := 
> https://ftp.gnome.org/pub/GNOME/sources/NetworkManager/1.20/$(NETWORKMANAGER).$(NETWORKMANAGER_SUFFIX)
> +NETWORKMANAGER_URL   := 
> https://ftp.gnome.org/pub/GNOME/sources/NetworkManager/1.22/$(NETWORKMANAGER).$(NETWORKMANAGER_SUFFIX)

I wonder if it would make sense to use some make magic to derive the
directory name (1.22) from NETWORKMANAGER_VERSION.

Hmm, this is more complicated than I thought, the following works:

EMPTY =
SPACE = $(EMPTY) $(EMPTY)
NETWORKMANAGER_MAJOR := $(subst $(SPACE),.,$(wordlist 1, 2, $(subst ., 
,$(NETWORKMANAGER_VERSION

or

NETWORKMANAGER_V1 = $(word 1, $(subst ., ,$(NETWORKMANAGER_VERSION)))
NETWORKMANAGER_V2 = $(word 2, $(subst ., ,$(NETWORKMANAGER_VERSION)))
NETWORKMANAGER_MAJOR = $(NETWORKMANAGER_V1).$(NETWORKMANAGER_V2)

Unless someone can come up with something easier, probably not worth the
effort?

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |

___
ptxdist mailing list
ptxdist@pengutronix.de


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

2020-02-12 Thread 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 
Signed-off-by: Rouven Czerwinski 
Signed-off-by: Ahmad Fatoum 
---
v1 -> v2:
 - Made TF_A_ARCH_MAJOR configurable to support 32 bit ARMv8 (Guillermo)
 - Replaces stm32mp-specific TF_A_DTB with TF_A_EXTRA_ARGS to contain
   all board/vendor specific options
 - removed reference to no longer existing CREDITS file
 - removed TF_A_MAKE_OPT contents that are set elsewhere
 - reduced uses of += in favor of directly appending to the string
 - delete old build directory in prepare instead of compile
 - use default compile stage (Guillermo)
 - install artifacts to sysroot /usr/lib/firmware in install stage
 - install artifacts to IMAGEDIR in targetinstall
 - fix clean stage to delete proper artifacts
---
 platforms/tf-a.in | 138 ++
 rules/tf-a.make   | 114 ++
 2 files changed, 252 insertions(+)
 create mode 100644 platforms/tf-a.in
 create mode 100644 rules/tf-a.make

diff --git a/platforms/tf-a.in b/platforms/tf-a.in
new file mode 100644
index ..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 ..9f895d32518d
--- 

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

2020-02-12 Thread Ahmad Fatoum
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 
> Signed-off-by: Rouven Czerwinski 
> Signed-off-by: Ahmad Fatoum 
> ---
> Cc: Guillermo Rodriguez Garcia 
> 
> 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- |

___
ptxdist mailing list
ptxdist@pengutronix.de


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

2020-02-12 Thread 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 
Signed-off-by: Rouven Czerwinski 
Signed-off-by: Ahmad Fatoum 
---
Cc: Guillermo Rodriguez Garcia 

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

[ptxdist] [PATCH] networkmanager: version bump 1.20.8-> 1.22.6

2020-02-12 Thread Ulrich Ölmann
Two new config options have been added in the meantime (see NetworkManager
commits [1] & [2]) which are both set to their reasonable default values.

The license did not change, but its text was adjusted - citing from
NetworkManagers's commit [3]:

  The update to the GPL text is purely cosmetic. However, shipping the exact
  same file as GNU publishes may help distros that deduplicate the license texts
  or hardlink duplicates.

[1] 
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/commit/69f048bf0ca387d2dc4683cfdfe9d170bfceb52b
[2] 
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/commit/d27fcd07541ae6f524115d5b0f36e14673135ca3
[3] 
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/commit/e9f2ea6c22f36cb7986d2228763629ed44b9e76b

Signed-off-by: Ulrich Ölmann 
---
 rules/networkmanager.make | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/rules/networkmanager.make b/rules/networkmanager.make
index de0e755b3a5d..86542b6f74e1 100644
--- a/rules/networkmanager.make
+++ b/rules/networkmanager.make
@@ -15,15 +15,15 @@ PACKAGES-$(PTXCONF_NETWORKMANAGER) += networkmanager
 #
 # Paths and names
 #
-NETWORKMANAGER_VERSION := 1.20.8
-NETWORKMANAGER_MD5 := c0ceb5ab14bfdfeee07536d94cc5c548
+NETWORKMANAGER_VERSION := 1.22.6
+NETWORKMANAGER_MD5 := 0f4b493cc0c67f94a2ad3573363eb3b2
 NETWORKMANAGER := NetworkManager-$(NETWORKMANAGER_VERSION)
 NETWORKMANAGER_SUFFIX  := tar.xz
-NETWORKMANAGER_URL := 
https://ftp.gnome.org/pub/GNOME/sources/NetworkManager/1.20/$(NETWORKMANAGER).$(NETWORKMANAGER_SUFFIX)
+NETWORKMANAGER_URL := 
https://ftp.gnome.org/pub/GNOME/sources/NetworkManager/1.22/$(NETWORKMANAGER).$(NETWORKMANAGER_SUFFIX)
 NETWORKMANAGER_SOURCE  := $(SRCDIR)/$(NETWORKMANAGER).$(NETWORKMANAGER_SUFFIX)
 NETWORKMANAGER_DIR := $(BUILDDIR)/$(NETWORKMANAGER)
 NETWORKMANAGER_LICENSE := GPL-2.0-or-later AND LGPL-2.0-or-later
-NETWORKMANAGER_LICENSE_FILES := 
file://COPYING;md5=cbbffd568227ada506640fe950a4823b
+NETWORKMANAGER_LICENSE_FILES := 
file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263
 
 # 
 # Prepare
@@ -67,9 +67,11 @@ NETWORKMANAGER_CONF_OPT = \
-Dnetconfig=false \
-Dnmcli=$(call ptx/truefalse,PTXCONF_NETWORKMANAGER_NMCLI) \
-Dnmtui=$(call ptx/truefalse,PTXCONF_NETWORKMANAGER_NMTUI) \
+   -Dnm_cloud_setup=false \
-Dofono=false \
-Dovs=false \
-Dpolkit=$(call ptx/truefalse,PTXCONF_NETWORKMANAGER_POLKIT) \
+   -Dconfig_auth_polkit_default=default \
-Dpolkit_agent=false \
-Dppp=$(call ptx/truefalse,PTXCONF_NETWORKMANAGER_PPP) \
-Dpppd=/usr/sbin/pppd \
-- 
2.25.0


___
ptxdist mailing list
ptxdist@pengutronix.de


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

2020-02-12 Thread Guillermo Rodriguez Garcia
El mié., 12 feb. 2020 a las 10:28, Ahmad Fatoum
() 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 

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

2020-02-12 Thread Ahmad Fatoum
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,
>>>
>>> 

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

2020-02-12 Thread Guillermo Rodriguez Garcia
Hi Ahmad,

El mar., 11 feb. 2020 a las 17:53, Ahmad Fatoum
() 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
> > () 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
> >>> () 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 
>  Signed-off-by: Rouven Czerwinski 
>  Signed-off-by: Ahmad Fatoum 
>  ---
>   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 ..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 
>