[Buildroot] [PATCH v2 1/8] boot/arm-trusted-firmware: support 32bit Arm targets

[Buildroot] [PATCH v2 1/8] boot/arm-trusted-firmware: support 32bit Arm targets

[Etienne Carriere]

This change allows one to build the TF-A (Trusted Firmware A) for
32bit architectures Armv7 and Armv8 platforms. TF-A supports these
configuration since release v1.5.

Change dependency on BR2_aarch64 into a dependency on either
BR2_ARM_CPU_ARMV8A/BR2_ARM_CPU_ARMV7A to better define
the target architectures supported by the package.

Set package configuration directive ARM_ARCH_MAJOR according to
BR2_ARM_CPU_ARMV8A/BR2_ARM_CPU_ARMV7A. Set package configuration
directive ARCH based on BR2_aarch64/BR2_arm.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
  - Set ARM_ARCH_MAJOR and ARCH according to Buildroot directives
    BR2_ARM_CPU_ARMV8A/BR2_ARM_CPU_ARMV7A and BR2_aarch64/BR2_arm.

---
 boot/arm-trusted-firmware/Config.in               |  3 ++-
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
index 823a351..428a4ce 100644
--- a/boot/arm-trusted-firmware/Config.in
+++ b/boot/arm-trusted-firmware/Config.in
@@ -1,6 +1,7 @@
 config BR2_TARGET_ARM_TRUSTED_FIRMWARE
 	bool "ARM Trusted Firmware (ATF)"
-	depends on BR2_aarch64 && BR2_TARGET_UBOOT
+	depends on (BR2_ARM_CPU_ARMV8A || BR2_ARM_CPU_ARMV7A) && \
+		   BR2_TARGET_UBOOT
 	help
 	  Enable this option if you want to build the ATF for your ARM
 	  based embedded device.
diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
index 8ca3864..fb80bd1 100644
--- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
+++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
@@ -35,6 +35,18 @@ ARM_TRUSTED_FIRMWARE_MAKE_OPTS += \
 	$(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES)) \
 	PLAT=$(ARM_TRUSTED_FIRMWARE_PLATFORM)
 
+ifeq ($(BR2_ARM_CPU_ARMV7A),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ARM_ARCH_MAJOR=7
+else ifeq ($(BR2_ARM_CPU_ARMV8A),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ARM_ARCH_MAJOR=8
+endif
+
+ifeq ($(BR2_arm),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ARCH=aarch32
+else ifeq ($(BR2_aarch64),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ARCH=aarch64
+endif
+
 ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33),y)
 ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL33=$(BINARIES_DIR)/u-boot.bin
 ARM_TRUSTED_FIRMWARE_DEPENDENCIES += uboot
-- 
1.9.1

[Buildroot] [PATCH v2 2/8] boot/arm-trusted-firmware: in-tree and OP-TEE BL32

[Etienne Carriere]

This change allows one to build trusted firmware (TF-A) with OP-TEE
as BL32 secure payload.

When BR2_TARGET_ARM_TRUSTED_FIRMWARE_INTREE_BL32 is enabled TF-A
builds a BL32 stage according the TF-A configuration directive.
If these specify no BL3 stage then TF-A will build without BL32
support. This is the default configuration and reflects TF-A legacy
integration in BR.

When BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32 is enabled
TF-A builds with support for the OP-TEE OS as BL32.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
 - No change

---
 boot/arm-trusted-firmware/Config.in               | 30 +++++++++++++++++++++++
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 13 ++++++++++
 2 files changed, 43 insertions(+)

diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
index 428a4ce..a1a0c54 100644
--- a/boot/arm-trusted-firmware/Config.in
+++ b/boot/arm-trusted-firmware/Config.in
@@ -91,6 +91,36 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT
 	  bl31.bin.  This is used for example by the Xilinx version of
 	  U-Boot SPL to load ATF on the ZynqMP SoC.
 
+choice
+	prompt "Select BL32 stage"
+	default BR2_TARGET_ARM_TRUSTED_FIRMWARE_INTREE_BL32
+	help
+	  Select BL32 stage for the trusted firmware
+
+config BR2_TARGET_ARM_TRUSTED_FIRMWARE_INTREE_BL32
+	bool "Intree or no BL32 stage"
+	help
+	  This option shall be set if the BL32 image is built from
+	  trusted firmware sources (i.e sp_min, tsp) or when no BL32
+	  is expected.
+
+	  When the BL32 stage shall be built from ATF source tree,
+	  the target BL32 payload shall be defined from configuration
+	  BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES, either
+	  using directive SPD=<bl32_id> (Aarch64 platforms,
+	  i.e SPD=tspd) or AARCH32_SP=<bl32_id> (Aarch32 and Armv7
+	  platforms, i.e "AARCH32_SP=sp_min"). If no SPD or AARCH32_SP
+	  directive is specified, ATF will build without BL32 support.
+
+config BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32
+	bool "OP-TEE OS as BL32"
+	depends on BR2_TARGET_OPTEE_OS
+	help
+	  This option allows to embed OP-TEE OS as the BL32 part of
+	  the ARM Trusted Firmware boot sequence.
+
+endchoice
+
 config BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33
 	bool "Use U-Boot as BL33"
 	depends on BR2_TARGET_UBOOT
diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
index fb80bd1..0ea4c0e 100644
--- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
+++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
@@ -47,6 +47,19 @@ else ifeq ($(BR2_aarch64),y)
 ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ARCH=aarch64
 endif
 
+ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32),y)
+ARM_TRUSTED_FIRMWARE_DEPENDENCIES += optee-os
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL32=$(BINARIES_DIR)/tee-header_v2.bin
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL32_EXTRA1=$(BINARIES_DIR)/tee-pager_v2.bin
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL32_EXTRA2=$(BINARIES_DIR)/tee-pageable_v2.bin
+ifeq ($(BR2_aarch64),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += SPD=opteed
+endif
+ifeq ($(BR2_arm),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += AARCH32_SP=optee
+endif
+endif # BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32
+
 ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33),y)
 ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL33=$(BINARIES_DIR)/u-boot.bin
 ARM_TRUSTED_FIRMWARE_DEPENDENCIES += uboot
-- 
1.9.1

[Buildroot] [PATCH v2 2/8] boot/arm-trusted-firmware: in-tree and OP-TEE BL32

[Thomas Petazzoni]

Hello Etienne,

On Tue, 19 Mar 2019 00:21:03 +0100
Etienne Carriere <etienne.carriere@linaro.org> wrote:

> This change allows one to build trusted firmware (TF-A) with OP-TEE
> as BL32 secure payload.
> 
> When BR2_TARGET_ARM_TRUSTED_FIRMWARE_INTREE_BL32 is enabled TF-A
> builds a BL32 stage according the TF-A configuration directive.
> If these specify no BL3 stage then TF-A will build without BL32
> support. This is the default configuration and reflects TF-A legacy
> integration in BR.
> 
> When BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32 is enabled
> TF-A builds with support for the OP-TEE OS as BL32.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

I have applied, after doing a few changes. See below.

> +choice
> +	prompt "Select BL32 stage"

Simplified to just "BL32"

> +config BR2_TARGET_ARM_TRUSTED_FIRMWARE_INTREE_BL32

Changed to:

BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL32_DEFAULT

I found the "in-tree" confusing, because in most cases, it's actually
"no BL32". What this option does is essentially nothing, i.e keeps the default.

> +	bool "Intree or no BL32 stage"

Changed to "Default"

> +	help
> +	  This option shall be set if the BL32 image is built from
> +	  trusted firmware sources (i.e sp_min, tsp) or when no BL32
> +	  is expected.
> +
> +	  When the BL32 stage shall be built from ATF source tree,
> +	  the target BL32 payload shall be defined from configuration
> +	  BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES, either
> +	  using directive SPD=<bl32_id> (Aarch64 platforms,
> +	  i.e SPD=tspd) or AARCH32_SP=<bl32_id> (Aarch32 and Armv7
> +	  platforms, i.e "AARCH32_SP=sp_min"). If no SPD or AARCH32_SP
> +	  directive is specified, ATF will build without BL32 support.

I've reworded this to simplify a bit the explanation.

> +
> +config BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32

Renamed to BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL32_OPTEE

> +ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32),y)
> +ARM_TRUSTED_FIRMWARE_DEPENDENCIES += optee-os
> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL32=$(BINARIES_DIR)/tee-header_v2.bin
> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL32_EXTRA1=$(BINARIES_DIR)/tee-pager_v2.bin
> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += BL32_EXTRA2=$(BINARIES_DIR)/tee-pageable_v2.bin

I grouped these definitions in a single assignment to
ARM_TRUSTED_FIRMWARE_MAKE_OPTS.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 3/8] boot/arm-trusted-firmware: support debug mode

[Etienne Carriere]

When the trusted firmware is built with debug support (DEBUG defined),
the generated images are located at a specific path. The non debug
images are located in generated directory build/<platform>/release/
while the debug images are located in generated directory
build/<platform>/debug/.

This change introduces boolean BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG
to define whether the release or debug configuration is used to build
trusted firmware. Note that enabling trusted firmware debug support, i.e
BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES="... DEBUG=1 ..."
without enabling BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG will fail since
Buildroot will get generated files from the wrong path.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
 - No change

---
 boot/arm-trusted-firmware/Config.in               | 7 +++++++
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
index a1a0c54..9a152b9 100644
--- a/boot/arm-trusted-firmware/Config.in
+++ b/boot/arm-trusted-firmware/Config.in
@@ -136,4 +136,11 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES
 	  Additional parameters for the ATF build
 	  E.G. 'DEBUG=1 LOG_LEVEL=20'
 
+config BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG
+	bool "Debug mode of the trusted firmware"
+	default n
+	help
+	  Enable this directive if trusted firmware is built in debug
+	  mode.
+
 endif
diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
index 0ea4c0e..68be00d 100644
--- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
+++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
@@ -28,7 +28,13 @@ endif
 ARM_TRUSTED_FIRMWARE_INSTALL_IMAGES = YES
 
 ARM_TRUSTED_FIRMWARE_PLATFORM = $(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM))
+
+ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += DEBUG=1
+ARM_TRUSTED_FIRMWARE_IMG_DIR = $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/debug
+else
 ARM_TRUSTED_FIRMWARE_IMG_DIR = $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/release
+endif
 
 ARM_TRUSTED_FIRMWARE_MAKE_OPTS += \
 	CROSS_COMPILE="$(TARGET_CROSS)" \
-- 
1.9.1

[Buildroot] [PATCH v2 3/8] boot/arm-trusted-firmware: support debug mode

[Thomas Petazzoni]

Hello Etienne,

On Tue, 19 Mar 2019 00:21:04 +0100
Etienne Carriere <etienne.carriere@linaro.org> wrote:

> When the trusted firmware is built with debug support (DEBUG defined),
> the generated images are located at a specific path. The non debug
> images are located in generated directory build/<platform>/release/
> while the debug images are located in generated directory
> build/<platform>/debug/.
> 
> This change introduces boolean BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG
> to define whether the release or debug configuration is used to build
> trusted firmware. Note that enabling trusted firmware debug support, i.e
> BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES="... DEBUG=1 ..."
> without enabling BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG will fail since
> Buildroot will get generated files from the wrong path.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

I've applied with a few minor tweaks.

> +config BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG
> +	bool "Debug mode of the trusted firmware"

"of the trusted firmware" is not really useful: options are already
indented under the package they apply to, so there's no need to repeat
the package name.

> +	default n

"default n" is the default, so it's not needed.

> +	help
> +	  Enable this directive if trusted firmware is built in debug
> +	  mode.

The wording is strange: you don't want to enable this option (not
directive) if trusted firmware is built in debug mode. You want to
enable it to build ATF in debug mode.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 4/8] boot/arm-trusted-firmware: support alternate image files

[Etienne Carriere]

Some platform may generate specific boot image files instead of
the generic files *.bin  when building TF-A package. This change
introduces new configuration directive for the arm-trusted-firmware
boot package.

BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES is boolean.
When disabled, install boot image files are .../*.bin.
When enabled, BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES shall
lists the names of the generated boot image files.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
 - No change

---
 boot/arm-trusted-firmware/Config.in               | 20 ++++++++++++++++++++
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 14 +++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
index 9a152b9..bb88760 100644
--- a/boot/arm-trusted-firmware/Config.in
+++ b/boot/arm-trusted-firmware/Config.in
@@ -121,6 +121,26 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32
 
 endchoice
 
+config BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES
+	bool "Use custom binary boot image filenames"
+	help
+	  If not set, build generates the boot image files with the
+	  generic filename that is images with .bin extension. These
+	  are copied to the output images/ directory.
+
+	  If enabled, use BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGES
+	  to list the generated file names.
+
+config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES
+	string "Binary boot image filenames"
+	depends on BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES
+	help
+	  This is used when TF-A platform generates boot image files
+	  with custom file names. When so, this should list the path
+	  of the generated files location relatively to the package
+	  build root directory. These files are copied to the output
+	  images/ directory.
+
 config BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33
 	bool "Use U-Boot as BL33"
 	depends on BR2_TARGET_UBOOT
diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
index 68be00d..45c2311 100644
--- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
+++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
@@ -135,6 +135,18 @@ define ARM_TRUSTED_FIRMWARE_BL31_UBOOT_INSTALL_ELF
 endef
 endif
 
+ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES),y)
+define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
+	$(foreach f,$(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES)), \
+		cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/$(f) $(BINARIES_DIR)/ \
+	)
+endef
+else
+define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
+	cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/*.bin $(BINARIES_DIR)/
+endef
+endif
+
 define ARM_TRUSTED_FIRMWARE_BUILD_CMDS
 	$(ARM_TRUSTED_FIRMWARE_BUILD_FIPTOOL)
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(ARM_TRUSTED_FIRMWARE_MAKE_OPTS) \
@@ -143,7 +155,7 @@ define ARM_TRUSTED_FIRMWARE_BUILD_CMDS
 endef
 
 define ARM_TRUSTED_FIRMWARE_INSTALL_IMAGES_CMDS
-	cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/*.bin $(BINARIES_DIR)/
+	$(ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL)
 	$(ARM_TRUSTED_FIRMWARE_BL31_UBOOT_INSTALL)
 	$(ARM_TRUSTED_FIRMWARE_BL31_UBOOT_INSTALL_ELF)
 endef
-- 
1.9.1

[Buildroot] [PATCH v2 4/8] boot/arm-trusted-firmware: support alternate image files

[Thomas Petazzoni]

Hello Etienne,

On Tue, 19 Mar 2019 00:21:05 +0100
Etienne Carriere <etienne.carriere@linaro.org> wrote:

> Some platform may generate specific boot image files instead of
> the generic files *.bin  when building TF-A package. This change
> introduces new configuration directive for the arm-trusted-firmware
> boot package.
> 
> BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES is boolean.
> When disabled, install boot image files are .../*.bin.
> When enabled, BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES shall
> lists the names of the generated boot image files.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

I think this is more complicated than it needs to be. See an alternate
proposal below.

> +config BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES
> +	bool "Use custom binary boot image filenames"
> +	help
> +	  If not set, build generates the boot image files with the
> +	  generic filename that is images with .bin extension. These
> +	  are copied to the output images/ directory.
> +
> +	  If enabled, use BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGES
> +	  to list the generated file names.
> +
> +config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES
> +	string "Binary boot image filenames"
> +	depends on BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES
> +	help
> +	  This is used when TF-A platform generates boot image files
> +	  with custom file names. When so, this should list the path
> +	  of the generated files location relatively to the package
> +	  build root directory. These files are copied to the output
> +	  images/ directory.

Just one new option:

config BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGE_NAMES
	string "Image names"
	default "*.bin"
	help
	  ...
> +ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES),y)
> +define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
> +	$(foreach f,$(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES)), \
> +		cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/$(f) $(BINARIES_DIR)/ \
> +	)
> +endef
> +else
> +define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
> +	cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/*.bin $(BINARIES_DIR)/
> +endef
> +endif

And then just:

ARM_TRUSTED_FIRMWARE_IMAGE_NAMES = $(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGE_NAMES))

define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
	$(foreach f,$(wildcard $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/$(ARM_TRUSTED_FIRMWARE_IMAGE_NAMES)), \
		$(INSTALL) -D -m 0644 $(f) $(BINARIES_DIR)/$(notdir $(f))
	)
endef

(totally untested of course)

Could you try something like this ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 4/8] boot/arm-trusted-firmware: support alternate image files

[Etienne Carriere]

Hi Thomas,

On Wed, 20 Mar 2019 at 23:03, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Etienne,
>
> On Tue, 19 Mar 2019 00:21:05 +0100
> Etienne Carriere <etienne.carriere@linaro.org> wrote:
>
> > Some platform may generate specific boot image files instead of
> > the generic files *.bin  when building TF-A package. This change
> > introduces new configuration directive for the arm-trusted-firmware
> > boot package.
> >
> > BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES is boolean.
> > When disabled, install boot image files are .../*.bin.
> > When enabled, BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES shall
> > lists the names of the generated boot image files.
> >
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
>
> I think this is more complicated than it needs to be. See an alternate
> proposal below.
>
> > +config BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES
> > +     bool "Use custom binary boot image filenames"
> > +     help
> > +       If not set, build generates the boot image files with the
> > +       generic filename that is images with .bin extension. These
> > +       are copied to the output images/ directory.
> > +
> > +       If enabled, use BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGES
> > +       to list the generated file names.
> > +
> > +config BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES
> > +     string "Binary boot image filenames"
> > +     depends on BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES
> > +     help
> > +       This is used when TF-A platform generates boot image files
> > +       with custom file names. When so, this should list the path
> > +       of the generated files location relatively to the package
> > +       build root directory. These files are copied to the output
> > +       images/ directory.
>
> Just one new option:
>
> config BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGE_NAMES
>         string "Image names"
>         default "*.bin"
>         help
>           ...
> > +ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_IMAGE_NAMES),y)
> > +define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
> > +     $(foreach f,$(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL_IMAGE_NAMES)), \
> > +             cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/$(f) $(BINARIES_DIR)/ \
> > +     )
> > +endef
> > +else
> > +define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
> > +     cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/*.bin $(BINARIES_DIR)/
> > +endef
> > +endif
>
> And then just:
>
> ARM_TRUSTED_FIRMWARE_IMAGE_NAMES = $(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_IMAGE_NAMES))
>
> define ARM_TRUSTED_FIRMWARE_BL_IMAGES_INSTALL
>         $(foreach f,$(wildcard $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/$(ARM_TRUSTED_FIRMWARE_IMAGE_NAMES)), \
>                 $(INSTALL) -D -m 0644 $(f) $(BINARIES_DIR)/$(notdir $(f))
>         )
> endef
>
> (totally untested of course)
>
> Could you try something like this ?

Sure, I'll test and send a update for the patch.
Thanks,
etienne

>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

[Buildroot] [PATCH v2 5/8] package/optee-test: fix dependency in TAs build

[Etienne Carriere]

Fix TA build command to build TA libraries before TAs.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
  - New commit not in v1 series.

---
 package/optee-test/optee-test.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/optee-test/optee-test.mk b/package/optee-test/optee-test.mk
index 73accf3..abed09d 100644
--- a/package/optee-test/optee-test.mk
+++ b/package/optee-test/optee-test.mk
@@ -16,7 +16,7 @@ OPTEE_TEST_CONF_OPTS = -DOPTEE_TEST_SDK=$(OPTEE_OS_SDK)
 # Trusted Application are not built from CMake due to ta_dev_kit
 # dependencies. We must build and install them on target.
 define OPTEE_TEST_BUILD_TAS
-	$(foreach f,$(wildcard $(@D)/ta/*/Makefile), \
+	$(foreach f,$(wildcard $(@D)/ta/*_lib/Makefile) $(wildcard $(@D)/ta/*/Makefile), \
 		$(TARGET_CONFIGURE_OPTS) \
 		$(MAKE) CROSS_COMPILE=$(TARGET_CROSS) \
 			TA_DEV_KIT_DIR=$(OPTEE_OS_SDK) \
-- 
1.9.1

[Buildroot] [PATCH v2 5/8] package/optee-test: fix dependency in TAs build

[Thomas Petazzoni]

On Tue, 19 Mar 2019 00:21:06 +0100
Etienne Carriere <etienne.carriere@linaro.org> wrote:

> Fix TA build command to build TA libraries before TAs.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes v1 -> v2:
>   - New commit not in v1 series.

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 6/8] configs/qemu_armv7a_tz_virt: Armv7-A emulation with TrustZone services

[Etienne Carriere]

This change introduces a Qemu board for an Armv7-A target executing
with OP-TEE secure world services.

The target Linux based normal world embeds the standard minimal
filesystem with OP-TEE non-secure components embedded files from
OP-TEE test, examples and benchmark packages.

The Linux custom configuration is dumped from the vexpress_defconfig
with few added fragments: OP-TEE driver and 9p for virtual filesystem
to ease file manipulation and exchanges through Qemu virtfs support.

U-Boot qemu_arm defconfig is modified with a configuration fragment
to set boot command, enable semihosting and remove U-Boot persistent
environment storage support.

The standard way for booting OP-TEE with a non-secure world companion
use the Arm Trusted Firmware-A as bootloader. OP-TEE OS provides the
BL32 image and U-boot the BL33 image. The boot scenario is:

  TF-A bootloader (BL1/BL2) => OP-TEE (BL32) => U-boot (BL33).
  | Executes as secure         | Secure         | Execs as Non-secure
  | Loads BL32/BL33 in RAM     | Jumps to BL33  | Always booted after
  | Jumps to BL32 once done    | as Non-secure  | secure world inits

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
  - Bump kernel to 4.19.
  - Use kernel defconfig and add optee and 9p as config fragments.
  - Remove ARM_ARCH_MAJOR from platform configuration directive
    BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES.

---
 board/qemu/armv7a-tz-virt/linux.fragment |   8 ++
 board/qemu/armv7a-tz-virt/readme.txt     | 145 +++++++++++++++++++++++++++++++
 board/qemu/armv7a-tz-virt/u-boot.config  |   7 ++
 configs/qemu_armv7a_tz_virt_defconfig    |  48 ++++++++++
 4 files changed, 208 insertions(+)
 create mode 100644 board/qemu/armv7a-tz-virt/linux.fragment
 create mode 100644 board/qemu/armv7a-tz-virt/readme.txt
 create mode 100644 board/qemu/armv7a-tz-virt/u-boot.config
 create mode 100644 configs/qemu_armv7a_tz_virt_defconfig

diff --git a/board/qemu/armv7a-tz-virt/linux.fragment b/board/qemu/armv7a-tz-virt/linux.fragment
new file mode 100644
index 0000000..8fca146
--- /dev/null
+++ b/board/qemu/armv7a-tz-virt/linux.fragment
@@ -0,0 +1,8 @@
+### Enable OP-TEE
+CONFIG_TEE=y
+CONFIG_OPTEE=y
+### Enable 9P VFS
+CONFIG_NET_9P=y
+CONFIG_NET_9P_VIRTIO=y
+CONFIG_9P_FS=y
+CONFIG_9P_FS_POSIX_ACL=y
diff --git a/board/qemu/armv7a-tz-virt/readme.txt b/board/qemu/armv7a-tz-virt/readme.txt
new file mode 100644
index 0000000..c5b54dc
--- /dev/null
+++ b/board/qemu/armv7a-tz-virt/readme.txt
@@ -0,0 +1,145 @@
+Board qemu_armv7a_tz_virt builds a Qemu Armv7-A target system with
+OP-TEE running in the TrustZone secure wolrd setup and a Linux based
+OS running in the non-secure wolrd. The board also builds the Qemu
+host to run the Arm target emulation.
+
+  make qemu_armv7a_tz_virt_defconfig
+  make
+
+BIOS used in the Qemu host is the Arm Trusted Firmware-A (TF-A). TF-A
+uses Qemu semihosting file access to access boot image files. The
+Qemu platform is quite specific for that in TF-A and one needs to
+rename some image files and run the emulation from the image directory
+for TF-A to boot the secure and non-secure world.
+
+I.e:
+  ln -s ./u-boot.bin output/images/bl33.bin
+  ln -s ./tee-header_v2.bin output/images/bl32.bin
+  ln -s ./tee-pager_v2.bin output/images/bl32_extra1.bin
+  ln -s ./tee-pageable_v2.bin output/images/bl32_extra2.bin
+
+Run the emulation from the output/images directory with:
+
+  cd output/images && ../host/bin/qemu-system-arm \
+	-machine virt -machine secure=on -cpu cortex-a15 \
+	-smp 1 -s -m 1024 -d unimp \
+	-serial stdio \
+	-netdev user,id=vmnic -device virtio-net-device,netdev=vmnic \
+	-semihosting-config enable,target=native \
+	-bios bl1.bin
+
+The boot stage traces (if any) followed by the login prompt will appear
+in the terminal that started Qemu.
+
+If you want to emulate more cores use "-smp {1|2|3|4}" to select the
+number of cores.
+
+Note "-netdev user,id=vmnic -device virtio-net-device,netdev=vmnic"
+brings virtfs support for file sharing with the hosted Linux OS. Board
+Linux configuration file for armv7a-tz-virt enables the requiredresources.
+BR2_PACKAGE_HOST_QEMU_VIRTFS=y build Qemu with required resources.
+
+Tested with QEMU 2.12.0
+
+-- Boot Details --
+
+TF-A is used as Qemu BIOS. Its BL1 image boots and load its BL2 image. In turn, this
+image loads the OP-TEE secure world (Armv7-A BL32 stage) and the U-boot as non-secure
+bootloader (BL33 stage).
+
+The Qemu natively host and loads in RAM the Qemu Arm target device tree. OP-TEE reads
+and modifes its content according to OP-TEE configuration.
+
+Enable TF-A traces from LOG_LEVEL (I.e LOG_LEVEL=40) from
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES.
+
+-- OP-TEE Traces --
+
+Secure boot stages and/or secure runtime services may use a serial link for
+their traces.
+
+The Arm Trusted Firmware outputs its traces on the Qemu standard (first)
+serial  interface.
+
+The OP-TEE OS uses the Qemu second serial interface.
+
+To get the OP-TEE OS traces one shall append a second -serial argument after
+-serial stdio in the Qemu command line. I.e the following enables 2 serial
+consoles over telnet connections:
+
+  cd output/images && ../host/bin/qemu-system-arm \
+	-machine virt -machine secure=on -cpu cortex-a15 \
+	-smp 1 -s -m 1024 -d unimp \
+	-serial telnet:127.0.0.1:1235,server \
+	-serial telnet:127.0.0.1:1236,server \
+	-netdev user,id=vmnic -device virtio-net-device,netdev=vmnic \
+	-semihosting-config enable,target=native \
+	-bios bl1.bin
+
+Qemu is now waiting for the telnet connection. From another shell, open a
+telnet connection on the port for the U-boot and Linux consoles:
+  telnet 127.0.0.1 1235
+
+and again for the secure console
+  telnet 127.0.0.1 1236
+
+-- Using gdb --
+
+One can debug the OP-TEE secure world using GDB through the Qemu host.
+To do so, one can simply run the qemu-system-arm emulation then
+run a GDB client and connect the Qemu internal GDB server.
+
+The example below assumes we run Qemu and the GDB client from the same
+host computer. We use option -S of qemu-system-arm to make Qemu
+waiting for the GDB continue instruction before booting the images.
+
+From a first shell:
+  cd output/images && ../host/bin/qemu-system-arm \
+	-machine virt -machine secure=on -cpu cortex-a15 \
+	-smp 1 -s -m 1024 -d unimp \
+	-serial stdio \
+	-netdev user,id=vmnic -device virtio-net-device,netdev=vmnic \
+	-semihosting-config enable,target=native \
+	-bios bl1.bin \
+	-S
+
+From a second shell:
+  ./output/host/bin/arm-linux-gnueabihf-gd
+  GNU gdb (GNU Toolchain for the A-profile Architecture 8.2-2018-08 (arm-rel-8.23)) 8.1.1.20180704-git
+  Copyright (C) 2018 Free Software Foundation, Inc.
+  ...
+  For help, type "help".
+  Type "apropos word" to search for commands related to "word".
+  (gdb) 
+
+From this GDB console, connect target, load OP-TEE core symbols, set a
+breakpoint to its entry (__text_start) and start emulation:
+
+  (gdb) target remote 127.0.0.1:1234
+  (gdb) symbol-file ../build/optee_os-<reference>/out/arm/core/tee.elf
+  (gdb) hbreak __text_start
+  Hardware assisted breakpoint 1 at 0xe100000: file core/arch/arm/kernel/generic_entry_a32.S, line 246.
+  (gdb) cont
+  Continuing.
+  
+  Thread 1 hit Breakpoint 1, _start () at core/arch/arm/kernel/generic_entry_a32.S:246
+  246		bootargs_entry
+  (gdb) 
+
+
+Emulation has started, TF-A has loaded OP-TEE and U-boot images in memory and
+has booted OP-TEE. Emulation stopped at OP-TEE core entry.
+
+
+Note: Qemu hosts a GDB service listening to TCP port 1234, as set through
+qemu-system-arm commandline option -s.
+
+
+Note: GDB server used above (from image/host/bin) was built from Buildroot
+using the following extra configuration directives:
+
+    BR2_ENABLE_DEBUG=y
+    BR2_PACKAGE_GDB=y
+    BR2_PACKAGE_HOST_GDB=y
+    BR2_TOOLCHAIN_BUILDROOT_CXX=y
+    BR2_TOOLCHAIN_BUILDROOT_GLIBC=y
diff --git a/board/qemu/armv7a-tz-virt/u-boot.config b/board/qemu/armv7a-tz-virt/u-boot.config
new file mode 100644
index 0000000..fd74bf1
--- /dev/null
+++ b/board/qemu/armv7a-tz-virt/u-boot.config
@@ -0,0 +1,7 @@
+CONFIG_SYS_TEXT_BASE=0x60000000
+CONFIG_BOOTCOMMAND="fdt addr ${fdt_addr} && fdt resize 1000 && smhload zImage ${kernel_addr_r} && smhload rootfs.cpio.gz ${ramdisk_addr_r} ramdisk_addr_end &&  setenv bootargs console=ttyAMA0,115200 earlyprintk=serial,ttyAMA0,115200 && fdt chosen ${ramdisk_addr_r} ${ramdisk_addr_end} && bootz ${kernel_addr_r} - ${fdt_addr}"
+CONFIG_SEMIHOSTING=y
+# Drop flash accesses
+CONFIG_ENV_IS_IN_FLASH=n
+CONFIG_MTD=n
+CONFIG_MTD_NOR_FLASH=n
diff --git a/configs/qemu_armv7a_tz_virt_defconfig b/configs/qemu_armv7a_tz_virt_defconfig
new file mode 100644
index 0000000..0c78270
--- /dev/null
+++ b/configs/qemu_armv7a_tz_virt_defconfig
@@ -0,0 +1,48 @@
+# Architecture
+BR2_arm=y
+BR2_cortex_a15=y
+BR2_ARM_ENABLE_NEON=y
+BR2_ARM_ENABLE_VFP=y
+BR2_ARM_FPU_VFPV3D16=y
+# System
+BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
+# Filesystems (support several boot config)
+BR2_TARGET_ROOTFS_CPIO=y
+BR2_TARGET_ROOTFS_CPIO_GZIP=y
+BR2_TARGET_ROOTFS_EXT2=y
+# Linux 4.16 series
+BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_4_19=y
+BR2_LINUX_KERNEL=y
+BR2_LINUX_KERNEL_CUSTOM_VERSION=y
+BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.19.16"
+BR2_LINUX_KERNEL_DEFCONFIG="vexpress"
+BR2_LINUX_KERNEL_CONFIG_FRAGMENT_FILES="board/qemu/armv7a-tz-virt/linux.fragment"
+BR2_LINUX_KERNEL_DTS_SUPPORT=y
+BR2_LINUX_KERNEL_INTREE_DTS_NAME="vexpress-v2p-ca15_a7"
+# TF-A for booting OP-TEE secure and uboot/linux non secure
+BR2_TARGET_ARM_TRUSTED_FIRMWARE=y
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://github.com/ARM-software/arm-trusted-firmware.git"
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="v2.0"
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="qemu"
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_OPTEE_AS_BL32=y
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33=y
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES="BL32_RAM_LOCATION=tdram LOG_LEVEL=30"
+#BR2_TARGET_ARM_TRUSTED_FIRMWARE_DEBUG=y
+# OP-TEE components
+BR2_TARGET_OPTEE_OS=y
+BR2_TARGET_OPTEE_OS_PLATFORM="vexpress-qemu_virt"
+BR2_TARGET_OPTEE_OS_ADDITIONAL_VARIABLES="CFG_TEE_CORE_LOG_LEVEL=2"
+BR2_PACKAGE_OPTEE_CLIENT=y
+BR2_PACKAGE_OPTEE_TEST=y
+BR2_PACKAGE_OPTEE_EXAMPLES=y
+BR2_PACKAGE_OPTEE_BENCHMARK=y
+# U-boot for booting the dear Linux kernel
+BR2_TARGET_UBOOT=y
+BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
+BR2_TARGET_UBOOT_BOARD_DEFCONFIG="qemu_arm"
+BR2_TARGET_UBOOT_CONFIG_FRAGMENT_FILES="board/qemu/armv7a-tz-virt/u-boot.config"
+# Qemu emulator for the Arm target
+BR2_PACKAGE_HOST_QEMU=y
+BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE=y
+BR2_PACKAGE_HOST_QEMU_VIRTFS=y
-- 
1.9.1

[Buildroot] [PATCH v2 6/8] configs/qemu_armv7a_tz_virt: Armv7-A emulation with TrustZone services

[Arnout Vandecappelle]

 Hi Etienne,

 Just a few points I noticed, not a complete review...

On 19/03/2019 00:21, Etienne Carriere wrote:
> +  ln -s ./u-boot.bin output/images/bl33.bin
> +  ln -s ./tee-header_v2.bin output/images/bl32.bin
> +  ln -s ./tee-pager_v2.bin output/images/bl32_extra1.bin
> +  ln -s ./tee-pageable_v2.bin output/images/bl32_extra2.bin

 This should be done in a post-build script.

 Also, don't use ./, i.e.

ln -s u-boot.bin output/images/bl33.bin

[snip]
> +Note "-netdev user,id=vmnic -device virtio-net-device,netdev=vmnic"
> +brings virtfs support for file sharing with the hosted Linux OS.

 I thought virtfs was enabled with -virtfs. The netdev option creates a virtio
net device and IIUC you use p9 to get an fs over that.

 But I don't think we typically enable or document that kind of feature in our
qemu configs. Just enable the minimum that is needed to be able to get a console
and a network connection.

 In fact, I think this defconfig should be as close as possible to the vexpress
defconfig. But I'm not sure if the a9 supports trustzone, so perhaps it's better
to switch the vexpress config to a15.

[snip]
> +# Qemu emulator for the Arm target
> +BR2_PACKAGE_HOST_QEMU=y
> +BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE=y
> +BR2_PACKAGE_HOST_QEMU_VIRTFS=y

 Why is this needed? AFAICS there is nothing special about the Buildroot-built
host-qemu, so the system-installed qemu should work just as well, no?



 Regards,
 Arnout

[Buildroot] [PATCH v2 6/8] configs/qemu_armv7a_tz_virt: Armv7-A emulation with TrustZone services

[Etienne Carriere]

Hello Arnout,


On Tue, 19 Mar 2019 at 23:00, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>  Hi Etienne,
>
>  Just a few points I noticed, not a complete review...
>
> On 19/03/2019 00:21, Etienne Carriere wrote:
> > +  ln -s ./u-boot.bin output/images/bl33.bin
> > +  ln -s ./tee-header_v2.bin output/images/bl32.bin
> > +  ln -s ./tee-pager_v2.bin output/images/bl32_extra1.bin
> > +  ln -s ./tee-pageable_v2.bin output/images/bl32_extra2.bin
>
>  This should be done in a post-build script.

This file renaming is very specific to the Qemu platform due to Qemu
semihosting. If done at installation time, it should be part of
optee-os.mk or arm-trusted-firmware.mk. But such a change would be
relevant only for the Qemu based boards. It is not generic at all.

I though it would be simpler to state this file renaming in the board
readme file, rather than a hack in boot/arm-trusted-firmware package
for the Qemu case.

>
>  Also, don't use ./, i.e.
>
> ln -s u-boot.bin output/images/bl33.bin

Ok, I'll fix.
Thanks

>
> [snip]
> > +Note "-netdev user,id=vmnic -device virtio-net-device,netdev=vmnic"
> > +brings virtfs support for file sharing with the hosted Linux OS.
>
>  I thought virtfs was enabled with -virtfs. The netdev option creates a virtio
> net device and IIUC you use p9 to get an fs over that.
>
>  But I don't think we typically enable or document that kind of feature in our
> qemu configs. Just enable the minimum that is needed to be able to get a console
> and a network connection.

Hmm, looking back at this... this setup and description is incomplete.
Actually OP-TEE can be used in networking support. When so, argument:
     -netdev user,id=vmnic -device virtio-net-device,netdev=vmnic
enable Qemu SLiRP user networking. This is used by the OP-TEE tests.

For virtfs, there are a few extra args to provide, no listed in the
proposed readme.txt.

I will fix the readme and remove info about virtfs.


>
>  In fact, I think this defconfig should be as close as possible to the vexpress
> defconfig. But I'm not sure if the a9 supports trustzone, so perhaps it's better
> to switch the vexpress config to a15.

c-a9 supports trustzone. But c-a15 is more handy to cover various
armv7-a cores flavors.
c-15 supports c-a9 features plus other features found in the armv7-a
family (virtualization, 40-bit addressing, ...)
The Qemu platforms upstreamed in TF-A and OP-TEE are based on c-a15.

That said, whether the proposed qemu_armv7a_tz_virt should replace
existing vexpress, I really cannot tell. Enabling arm-trustzone in
Qemu (-machine secure=on) may drop features that the current vexpress
supports (graphics?).

>
> [snip]
> > +# Qemu emulator for the Arm target
> > +BR2_PACKAGE_HOST_QEMU=y
> > +BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE=y
> > +BR2_PACKAGE_HOST_QEMU_VIRTFS=y
>
>  Why is this needed? AFAICS there is nothing special about the Buildroot-built
> host-qemu, so the system-installed qemu should work just as well, no?

By system-installed you mean the user OS, as my ubuntu, for example,
where I run Buildroot builds and tests?
I think it is a shame I need to install a specific version of Qemu on
my host when Buildroot can produce the up-to-date Qemu tools.

Regards,
etienne

>
>
>
>  Regards,
>  Arnout

[Buildroot] [PATCH v2 6/8] configs/qemu_armv7a_tz_virt: Armv7-A emulation with TrustZone services

[Arnout Vandecappelle]

On 20/03/2019 09:49, Etienne Carriere wrote:
> Hello Arnout,
> 
> 
> On Tue, 19 Mar 2019 at 23:00, Arnout Vandecappelle <arnout@mind.be> wrote:
>>
>>  Hi Etienne,
>>
>>  Just a few points I noticed, not a complete review...
>>
>> On 19/03/2019 00:21, Etienne Carriere wrote:
>>> +  ln -s ./u-boot.bin output/images/bl33.bin
>>> +  ln -s ./tee-header_v2.bin output/images/bl32.bin
>>> +  ln -s ./tee-pager_v2.bin output/images/bl32_extra1.bin
>>> +  ln -s ./tee-pageable_v2.bin output/images/bl32_extra2.bin
>>
>>  This should be done in a post-build script.
> 
> This file renaming is very specific to the Qemu platform due to Qemu
> semihosting. If done at installation time, it should be part of
> optee-os.mk or arm-trusted-firmware.mk. But such a change would be
> relevant only for the Qemu based boards. It is not generic at all.

 ... which is why I say it should be part of a post-build script that is
specific for this defconfig. I'm talking about BR2_ROOTFS_POST_BUILD_SCRIPT.


> I though it would be simpler to state this file renaming in the board
> readme file, rather than a hack in boot/arm-trusted-firmware package
> for the Qemu case.

[snip]
>>  In fact, I think this defconfig should be as close as possible to the vexpress
>> defconfig. But I'm not sure if the a9 supports trustzone, so perhaps it's better
>> to switch the vexpress config to a15.
> 
> c-a9 supports trustzone. But c-a15 is more handy to cover various
> armv7-a cores flavors.
> c-15 supports c-a9 features plus other features found in the armv7-a
> family (virtualization, 40-bit addressing, ...)

 ... which is exactly why a9 is interesting, because code built for a9 will run
on most other cortexes while code built for a15 will possibly not run on a9.

> The Qemu platforms upstreamed in TF-A and OP-TEE are based on c-a15.

 That, however, is a good reason to use a15 for this defconfig.


> That said, whether the proposed qemu_armv7a_tz_virt should replace
> existing vexpress, I really cannot tell.

 I never said to replace it! I said it should be as close as possible. IOW, any
difference between the tz defconfig and the vexpress defconfig should have a
good reason. And that reason should be explained in the commit message.

 This reminds me: the defconfig should be named qemu_arm_vexpress_tz_defconfig
so it is clear that it's a variant of qemu_arm_vexpress_defconfig.

 I would also tend to merge the readme with the existing
board/qemu/arm-vexpress/readme.txt because their content is a bit similar. But
there are also reasons to split them (e.g., the script that scrapes the qemu
launch command from the readme wouldn't work anymore).

> Enabling arm-trustzone in
> Qemu (-machine secure=on) may drop features that the current vexpress
> supports (graphics?).
> 
>>
>> [snip]
>>> +# Qemu emulator for the Arm target
>>> +BR2_PACKAGE_HOST_QEMU=y
>>> +BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE=y
>>> +BR2_PACKAGE_HOST_QEMU_VIRTFS=y
>>
>>  Why is this needed? AFAICS there is nothing special about the Buildroot-built
>> host-qemu, so the system-installed qemu should work just as well, no?
> 
> By system-installed you mean the user OS, as my ubuntu, for example,
> where I run Buildroot builds and tests?

 Indeed, that's what I mean.

> I think it is a shame I need to install a specific version of Qemu on
> my host when Buildroot can produce the up-to-date Qemu tools.

 Do you mean that trustzone emulation is a recent addition to qemu and is not
yet available in the version shipped with e.g. Ubuntu 18.04? That would indeed
be a valid reason to build qemu. But is should be explained in the commit log.

 Otherwise, we should be consistent with all our qemu defconfigs. I agree that
there is something to be said for building qemu as part of the defconfig. It
would increase the build time, but probably not that much more compared to
building the toolchain. But if we do this, it should be done for all qemu
defconfigs, and the readme's should be updated.

 Regards,
 Arnout

[Buildroot] [PATCH v2 6/8] configs/qemu_armv7a_tz_virt: Armv7-A emulation with TrustZone services

[Etienne Carriere]

On Wed, 20 Mar 2019 at 10:20, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 20/03/2019 09:49, Etienne Carriere wrote:
> > Hello Arnout,
> >
> >
> > On Tue, 19 Mar 2019 at 23:00, Arnout Vandecappelle <arnout@mind.be> wrote:
> >>
> >>  Hi Etienne,
> >>
> >>  Just a few points I noticed, not a complete review...
> >>
> >> On 19/03/2019 00:21, Etienne Carriere wrote:
> >>> +  ln -s ./u-boot.bin output/images/bl33.bin
> >>> +  ln -s ./tee-header_v2.bin output/images/bl32.bin
> >>> +  ln -s ./tee-pager_v2.bin output/images/bl32_extra1.bin
> >>> +  ln -s ./tee-pageable_v2.bin output/images/bl32_extra2.bin
> >>
> >>  This should be done in a post-build script.
> >
> > This file renaming is very specific to the Qemu platform due to Qemu
> > semihosting. If done at installation time, it should be part of
> > optee-os.mk or arm-trusted-firmware.mk. But such a change would be
> > relevant only for the Qemu based boards. It is not generic at all.
>
>  ... which is why I say it should be part of a post-build script that is
> specific for this defconfig. I'm talking about BR2_ROOTFS_POST_BUILD_SCRIPT.

Ok, great, I didn't saw there is such support. I'll add a script.


>
>
> > I though it would be simpler to state this file renaming in the board
> > readme file, rather than a hack in boot/arm-trusted-firmware package
> > for the Qemu case.
>
> [snip]
> >>  In fact, I think this defconfig should be as close as possible to the vexpress
> >> defconfig. But I'm not sure if the a9 supports trustzone, so perhaps it's better
> >> to switch the vexpress config to a15.
> >
> > c-a9 supports trustzone. But c-a15 is more handy to cover various
> > armv7-a cores flavors.
> > c-15 supports c-a9 features plus other features found in the armv7-a
> > family (virtualization, 40-bit addressing, ...)
>
>  ... which is exactly why a9 is interesting, because code built for a9 will run
> on most other cortexes while code built for a15 will possibly not run on a9.
>
> > The Qemu platforms upstreamed in TF-A and OP-TEE are based on c-a15.
>
>  That, however, is a good reason to use a15 for this defconfig.
>
>
> > That said, whether the proposed qemu_armv7a_tz_virt should replace
> > existing vexpress, I really cannot tell.
>
>  I never said to replace it! I said it should be as close as possible. IOW, any
> difference between the tz defconfig and the vexpress defconfig should have a
> good reason. And that reason should be explained in the commit message.
>
>  This reminds me: the defconfig should be named qemu_arm_vexpress_tz_defconfig
> so it is clear that it's a variant of qemu_arm_vexpress_defconfig.

Ok, i'll change.

>
>  I would also tend to merge the readme with the existing
> board/qemu/arm-vexpress/readme.txt because their content is a bit similar. But
> there are also reasons to split them (e.g., the script that scrapes the qemu
> launch command from the readme wouldn't work anymore).
>
> > Enabling arm-trustzone in
> > Qemu (-machine secure=on) may drop features that the current vexpress
> > supports (graphics?).
> >
> >>
> >> [snip]
> >>> +# Qemu emulator for the Arm target
> >>> +BR2_PACKAGE_HOST_QEMU=y
> >>> +BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE=y
> >>> +BR2_PACKAGE_HOST_QEMU_VIRTFS=y
> >>
> >>  Why is this needed? AFAICS there is nothing special about the Buildroot-built
> >> host-qemu, so the system-installed qemu should work just as well, no?
> >
> > By system-installed you mean the user OS, as my ubuntu, for example,
> > where I run Buildroot builds and tests?
>
>  Indeed, that's what I mean.
>
> > I think it is a shame I need to install a specific version of Qemu on
> > my host when Buildroot can produce the up-to-date Qemu tools.
>
>  Do you mean that trustzone emulation is a recent addition to qemu and is not
> yet available in the version shipped with e.g. Ubuntu 18.04? That would indeed
> be a valid reason to build qemu. But is should be explained in the commit log.

Qemu supports Arm secure world since 2.6.0, and there were fixes merged since.
Ubuntu 14.04 provides qemu 2.0.0.
Ubuntu-16.04 native qemu is 2.5.0.
Ubuntu 18.04 provides Qemu 2.11.1. I've not tested this later.
Buildroot is synced with 3.1.0, latest Qemu revision.

>
>  Otherwise, we should be consistent with all our qemu defconfigs. I agree that
> there is something to be said for building qemu as part of the defconfig. It
> would increase the build time, but probably not that much more compared to
> building the toolchain. But if we do this, it should be done for all qemu
> defconfigs, and the readme's should be updated.
>
>  Regards,
>  Arnout

[Buildroot] [PATCH v2 7/8] testing: test can use the locally generated qemu host tool

[Etienne Carriere]

This change adds argument local to emulator boot method. When local is
True, emulator.py runs the qemu host tool locally generated by the
test. Otherwise the test uses the qemu host as found from PATH.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
 - No commit not in v1 series.

---
 support/testing/infra/emulator.py | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
index 802e89d..5fbf213 100644
--- a/support/testing/infra/emulator.py
+++ b/support/testing/infra/emulator.py
@@ -1,5 +1,7 @@
 import pexpect
 
+import os
+
 import infra
 
 
@@ -8,6 +10,7 @@ class Emulator(object):
     def __init__(self, builddir, downloaddir, logtofile, timeout_multiplier):
         self.qemu = None
         self.downloaddir = downloaddir
+        self.builddir = builddir
         self.logfile = infra.open_log_file(builddir, "run", logtofile)
         # We use elastic runners on the cloud to runs our tests. Those runners
         # can take a long time to run the emulator. Use a timeout multiplier
@@ -30,13 +33,22 @@ class Emulator(object):
     #
     # options: array of command line options to pass to Qemu
     #
-    def boot(self, arch, kernel=None, kernel_cmdline=None, options=None):
+    # local: if True, the locally built qemu host tool is used instead of a
+    # qemu host tool found from the PATH.
+    #
+    def boot(self, arch, kernel=None, kernel_cmdline=None, options=None,
+             local=None):
         if arch in ["armv7", "armv5"]:
             qemu_arch = "arm"
         else:
             qemu_arch = arch
 
-        qemu_cmd = ["qemu-system-{}".format(qemu_arch),
+        if local:
+            basedir = os.path.join(self.builddir, "host/bin/")
+        else:
+            basedir = ""
+
+        qemu_cmd = [basedir + "qemu-system-{}".format(qemu_arch),
                     "-serial", "stdio",
                     "-display", "none"]
 
-- 
1.9.1

[Buildroot] [PATCH v2 7/8] testing: test can use the locally generated qemu host tool

[Etienne Carriere]

Dear all,

I made this to allow test_optee to use the qemu config from the platform.
But I wonder if it would be more suitable to have an generic option to
script run-tests to build the emulator within the test config.

Your thoughts?

Best regards,
etienne

On Tue, 19 Mar 2019 at 00:21, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change adds argument local to emulator boot method. When local is
> True, emulator.py runs the qemu host tool locally generated by the
> test. Otherwise the test uses the qemu host as found from PATH.
>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes v1 -> v2:
>  - No commit not in v1 series.
>
> ---
>  support/testing/infra/emulator.py | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
> index 802e89d..5fbf213 100644
> --- a/support/testing/infra/emulator.py
> +++ b/support/testing/infra/emulator.py
> @@ -1,5 +1,7 @@
>  import pexpect
>
> +import os
> +
>  import infra
>
>
> @@ -8,6 +10,7 @@ class Emulator(object):
>      def __init__(self, builddir, downloaddir, logtofile, timeout_multiplier):
>          self.qemu = None
>          self.downloaddir = downloaddir
> +        self.builddir = builddir
>          self.logfile = infra.open_log_file(builddir, "run", logtofile)
>          # We use elastic runners on the cloud to runs our tests. Those runners
>          # can take a long time to run the emulator. Use a timeout multiplier
> @@ -30,13 +33,22 @@ class Emulator(object):
>      #
>      # options: array of command line options to pass to Qemu
>      #
> -    def boot(self, arch, kernel=None, kernel_cmdline=None, options=None):
> +    # local: if True, the locally built qemu host tool is used instead of a
> +    # qemu host tool found from the PATH.
> +    #
> +    def boot(self, arch, kernel=None, kernel_cmdline=None, options=None,
> +             local=None):
>          if arch in ["armv7", "armv5"]:
>              qemu_arch = "arm"
>          else:
>              qemu_arch = arch
>
> -        qemu_cmd = ["qemu-system-{}".format(qemu_arch),
> +        if local:
> +            basedir = os.path.join(self.builddir, "host/bin/")
> +        else:
> +            basedir = ""
> +
> +        qemu_cmd = [basedir + "qemu-system-{}".format(qemu_arch),
>                      "-serial", "stdio",
>                      "-display", "none"]
>
> --
> 1.9.1
>

[Buildroot] [PATCH v2 7/8] testing: test can use the locally generated qemu host tool

[Arnout Vandecappelle]

On 19/03/2019 08:05, Etienne Carriere wrote:
> I made this to allow test_optee to use the qemu config from the platform.
> But I wonder if it would be more suitable to have an generic option to
> script run-tests to build the emulator within the test config.

 That would indeed be a better idea IMO.

> -        qemu_cmd = ["qemu-system-{}".format(qemu_arch),
> +        if local:
> +            basedir = os.path.join(self.builddir, "host/bin/")
> +        else:
> +            basedir = ""
> +
> +        qemu_cmd = [basedir + "qemu-system-{}".format(qemu_arch),
>                      "-serial", "stdio",
>                      "-display", "none"]

 Instead of constructing it like this, I think it would be simpler to always add
output/host/bin to PATH. Unfortunately, however, pexpect only respects the PATH
setting of the env argument since version 4.2.0 [1], and we can't rely on the
system-installed pexpect to have that version. So you have to mess about with
os.environ (which you'd want to restore to the original after running qemu).

 Regards,
 Arnout

[1] https://github.com/pexpect/pexpect/pull/348

[Buildroot] [PATCH v2 7/8] testing: test can use the locally generated qemu host tool

[Etienne Carriere]

On Tue, 19 Mar 2019 at 23:03, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 19/03/2019 08:05, Etienne Carriere wrote:
> > I made this to allow test_optee to use the qemu config from the platform.
> > But I wonder if it would be more suitable to have an generic option to
> > script run-tests to build the emulator within the test config.
>
>  That would indeed be a better idea IMO.
>
> > -        qemu_cmd = ["qemu-system-{}".format(qemu_arch),
> > +        if local:
> > +            basedir = os.path.join(self.builddir, "host/bin/")
> > +        else:
> > +            basedir = ""
> > +
> > +        qemu_cmd = [basedir + "qemu-system-{}".format(qemu_arch),
> >                      "-serial", "stdio",
> >                      "-display", "none"]
>
>  Instead of constructing it like this, I think it would be simpler to always add
> output/host/bin to PATH. Unfortunately, however, pexpect only respects the PATH
> setting of the env argument since version 4.2.0 [1], and we can't rely on the
> system-installed pexpect to have that version. So you have to mess about with
> os.environ (which you'd want to restore to the original after running qemu).

Playing with the PATH is not very handy and could make the test to use
other thing in output/host/bin than only qemu.
Please see proposal in v3 (here https://patchwork.ozlabs.org/patch/1061031/).

Regards,
etienne

>
>  Regards,
>  Arnout
>
> [1] https://github.com/pexpect/pexpect/pull/348

[Buildroot] [PATCH v2 8/8] support/testing: test_optee.py: test optee boot and testsuite

[Etienne Carriere]

Run a Qemu emulation over qemu_armv7a_tz_virt_defconfig and
run the embedded OP-TEE regression test suite (xtest).

Tool xtest dumps traces that contain '# ' (hash + space) which
corrupts infra/emulator.py sequence which use such traces to
find shell prompt when command is completed. To overcome the issue
the xtest traces are shown only if the test failed.

One can run the test from something like:

  $> ./support/testing/run-tests \
       -o output/optee-runtest -d output/dwl \
       tests.package.test_optee

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
  - Add argument local=True to test emulator to use the qemu host
    built from test configuration.
  - Fix typo in trace "Silent test takes a while, be patient..."

---
 support/testing/tests/package/test_optee.py | 40 +++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 support/testing/tests/package/test_optee.py

diff --git a/support/testing/tests/package/test_optee.py b/support/testing/tests/package/test_optee.py
new file mode 100644
index 0000000..fdf391a
--- /dev/null
+++ b/support/testing/tests/package/test_optee.py
@@ -0,0 +1,40 @@
+import os
+
+import infra.basetest
+
+
+class TestOptee(infra.basetest.BRTest):
+
+    with open(os.path.join(os.getcwd(), 'configs/qemu_armv7a_tz_virt_defconfig'), 'r') as config_file:
+        config = "".join(line for line in config_file if line[:1]!='#') + \
+                """
+                BR2_TOOLCHAIN_EXTERNAL=y
+                """
+
+    def test_run(self):
+        qemu_options = ['-machine', 'virt,secure=on']
+        qemu_options.extend(['-cpu', 'cortex-a15'])
+        qemu_options.extend(['-m', '1024'])
+        qemu_options.extend(['-semihosting-config', 'enable,target=native'])
+        qemu_options.extend(['-bios', 'bl1.bin'])
+
+        # Prepare env for Qemu/armv7a to find the boot images
+        os.chdir(os.path.join(self.builddir, 'images'))
+        if not os.path.exists('bl33.bin'):
+            os.symlink('u-boot.bin', 'bl33.bin')
+        if not os.path.exists('bl32.bin'):
+            os.symlink('tee-header_v2.bin', 'bl32.bin')
+        if not os.path.exists('bl32_extra1.bin'):
+            os.symlink('tee-pager_v2.bin', 'bl32_extra1.bin')
+        if not os.path.exists('bl32_extra2.bin'):
+            os.symlink('tee-pageable_v2.bin', 'bl32_extra2.bin')
+
+        self.emulator.boot(arch='armv7', options=qemu_options, local=True)
+        self.emulator.login()
+
+        # Trick test trace since it prints "# " and corrupts emulator run method
+        # Print test trace only if test fails.
+        cmd = 'echo "Silent test takes a while, be patient..."; ' + \
+              'xtest -t regression > /tmp/xtest.log || (cat /tmp/xtest.log && false)'
+        output, exit_code = self.emulator.run(cmd, timeout=240)
+        self.assertEqual(exit_code, 0)
-- 
1.9.1

[Buildroot] [PATCH v2 1/8] boot/arm-trusted-firmware: support 32bit Arm targets

[Thomas Petazzoni]

On Tue, 19 Mar 2019 00:21:02 +0100
Etienne Carriere <etienne.carriere@linaro.org> wrote:

> This change allows one to build the TF-A (Trusted Firmware A) for
> 32bit architectures Armv7 and Armv8 platforms. TF-A supports these
> configuration since release v1.5.
> 
> Change dependency on BR2_aarch64 into a dependency on either
> BR2_ARM_CPU_ARMV8A/BR2_ARM_CPU_ARMV7A to better define
> the target architectures supported by the package.
> 
> Set package configuration directive ARM_ARCH_MAJOR according to
> BR2_ARM_CPU_ARMV8A/BR2_ARM_CPU_ARMV7A. Set package configuration
> directive ARCH based on BR2_aarch64/BR2_arm.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes v1 -> v2:
>   - Set ARM_ARCH_MAJOR and ARCH according to Buildroot directives
>     BR2_ARM_CPU_ARMV8A/BR2_ARM_CPU_ARMV7A and BR2_aarch64/BR2_arm.

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com