[Buildroot] [PATCH 1/4] boot/optee-os: support external TA SDK

[Buildroot] [PATCH 1/4] boot/optee-os: support external TA SDK

[Etienne Carriere]

It happens that Buildroot is used to generate the embedded filesystem
not not the boot images that which are built from another environment(s).
In such case OP-TEE SDK get built outside of Buildroot. This SDK is
needed by other OP-TEE packages in Buildroot to generate .ta files.

This change introduces BR2_TARGET_OPTEE_OS_SDK_PATH for configuration
to provide the path of an external OP-TEE SDK.

This configuration is mandated when neither BR2_TARGET_OPTEE_OS_CORE
nor BR2_TARGET_OPTEE_OS_SDK are enable while BR2_TARGET_OPTEE_OS=y.

When BR2_TARGET_OPTEE_OS_SDK_PATH is defined, the build stage of
optee-os copies the SDK file tree into optee-os build tree, for these
to be installed during target/staging install.

With this change BR2_TARGET_OPTEE_OS_SERVICES no more selects
BR2_TARGET_OPTEE_OS_CORE as .ta files can be build from external SDK.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 boot/optee-os/Config.in   | 13 ++++++++++++-
 boot/optee-os/optee-os.mk | 33 +++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/boot/optee-os/Config.in b/boot/optee-os/Config.in
index 4cb05798e5..7efc1f596e 100644
--- a/boot/optee-os/Config.in
+++ b/boot/optee-os/Config.in
@@ -72,7 +72,6 @@ config BR2_TARGET_OPTEE_OS_SDK
 config BR2_TARGET_OPTEE_OS_SERVICES
 	bool "Build service TAs and libs"
 	default y
-	select BR2_TARGET_OPTEE_OS_CORE
 	help
 	  This option installs the service trusted applications and
 	  trusted shared libraries built from OP-TEE OS source tree.
@@ -81,6 +80,18 @@ config BR2_TARGET_OPTEE_OS_SERVICES
 	  load these from this non-secure filesystem/directory into
 	  the secure world for execution.
 
+
+if !BR2_TARGET_OPTEE_OS_CORE && !BR2_TARGET_OPTEE_OS_SDK
+
+config BR2_TARGET_OPTEE_OS_SDK_PATH
+	string "External SDK path"
+	depends on !BR2_TARGET_OPTEE_OS_CORE && !BR2_TARGET_OPTEE_OS_SDK
+	help
+	  Path of the prebuilt OP-TEE SDK when one build the OP-TEE resources
+	  based on an external TA developement kit.
+
+endif
+
 config BR2_TARGET_OPTEE_OS_PLATFORM
 	string "Target platform (mandatory)"
 	help
diff --git a/boot/optee-os/optee-os.mk b/boot/optee-os/optee-os.mk
index bd415512c7..750516ad94 100644
--- a/boot/optee-os/optee-os.mk
+++ b/boot/optee-os/optee-os.mk
@@ -62,6 +62,8 @@ OPTEE_OS_LOCAL_SDK = $(OPTEE_OS_BUILDDIR_OUT)/export-ta_arm32
 OPTEE_OS_SDK = $(STAGING_DIR)/lib/optee/export-ta_arm32
 endif
 
+OPTEE_OS_EXTERNAL_SDK = $(call qstrip,$(BR2_TARGET_OPTEE_OS_SDK_PATH))
+
 ifeq ($(BR2_TARGET_OPTEE_OS_CORE),y)
 define OPTEE_OS_BUILD_CORE
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) O=$(OPTEE_OS_BUILDDIR_OUT) \
@@ -85,17 +87,38 @@ define OPTEE_OS_INSTALL_TARGET_CMDS
 endef
 endif # BR2_TARGET_OPTEE_OS_SERVICES
 
+# SDK staging install is common the SDK staging install, used when
+# either package builds SDK or package imports an external SDK.
+define OPTEE_OS_INSTALL_STAGING_SDK
+	mkdir -p $(OPTEE_OS_SDK)
+	cp -ardpf $(@D)/$(OPTEE_OS_LOCAL_SDK)/* $(OPTEE_OS_SDK)
+endef
+
 ifeq ($(BR2_TARGET_OPTEE_OS_SDK),y)
 define OPTEE_OS_BUILD_SDK
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) O=$(OPTEE_OS_BUILDDIR_OUT) \
 		 $(TARGET_CONFIGURE_OPTS) $(OPTEE_OS_MAKE_OPTS) ta_dev_kit
 endef
 define OPTEE_OS_INSTALL_STAGING_CMDS
-	mkdir -p $(OPTEE_OS_SDK)
-	cp -ardpf $(@D)/$(OPTEE_OS_LOCAL_SDK)/* $(OPTEE_OS_SDK)
+	$(OPTEE_OS_INSTALL_STAGING_SDK)
 endef
 endif # BR2_TARGET_OPTEE_OS_SDK
 
+ifneq ($(BR2_TARGET_OPTEE_OS_SDK_PATH),)
+ifeq (,$(wildcard $(OPTEE_OS_EXTERNAL_SDK)))
+$(error Invalid external SDK path $(OPTEE_OS_EXTERNAL_SDK))
+endif
+# SDK is not built from sources but imported from an external filetree
+# BR2_TARGET_OPTEE_OS_SDK_PATH mandates !BR2_TARGET_OPTEE_OS_SDK
+define OPTEE_OS_BUILD_SDK
+	mkdir -p $(@D)/$(OPTEE_OS_LOCAL_SDK)
+	cp -ardpf $(OPTEE_OS_EXTERNAL_SDK)/* $(@D)/$(OPTEE_OS_LOCAL_SDK)
+endef
+define OPTEE_OS_INSTALL_STAGING_CMDS
+	$(OPTEE_OS_INSTALL_STAGING_SDK)
+endef
+endif # BR2_TARGET_OPTEE_OS_SDK_PATH
+
 define OPTEE_OS_BUILD_CMDS
 	$(OPTEE_OS_BUILD_CORE)
 	$(OPTEE_OS_BUILD_SDK)
@@ -106,10 +129,12 @@ define OPTEE_OS_INSTALL_IMAGES_CMDS
 	$(OPTEE_OS_INSTALL_IMAGES_SERVICES)
 endef
 
-ifeq ($(BR2_TARGET_OPTEE_OS)$(BR_BUILDING),yy)
+ifeq ($(BR_BUILDING),y)
+ifneq (,$(filter y,$(BR2_TARGET_OPTEE_OS_CORE) $(BR2_TARGET_OPTEE_OS_SDK)))
 ifeq ($(call qstrip,$(BR2_TARGET_OPTEE_OS_PLATFORM)),)
 $(error No OP-TEE OS platform set. Check your BR2_TARGET_OPTEE_OS_PLATFORM setting)
 endif
-endif # BR2_TARGET_OPTEE_OS && BR2_BUILDING
+endif # BR2_TARGET_OPTEE_OS_CORE || BR2_TARGET_OPTEE_OS_SDK
+endif # BR2_BUILDING
 
 $(eval $(generic-package))
-- 
2.17.1

[Buildroot] [PATCH 2/4] package/optee-example: support external TA SDK

[Etienne Carriere]

OP-TEE Example package no more selects BR2_TARGET_OPTEE_OS_SDK as
OP-TEE OS can import an external SDK if not building it from source.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 package/optee-examples/Config.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/package/optee-examples/Config.in b/package/optee-examples/Config.in
index c0fecfdf50..6951c70721 100644
--- a/package/optee-examples/Config.in
+++ b/package/optee-examples/Config.in
@@ -3,7 +3,6 @@ config BR2_PACKAGE_OPTEE_EXAMPLES
 	depends on BR2_TARGET_OPTEE_OS
 	depends on BR2_TOOLCHAIN_HAS_THREADS # optee-client
 	select BR2_PACKAGE_OPTEE_CLIENT
-	select BR2_TARGET_OPTEE_OS_SDK
 	help
 	  Enable the OP-TEE examples package that brings examples of
 	  implementation of OP-TEE non-secure client applications and
-- 
2.17.1

[Buildroot] [PATCH 3/4] package/optee-test: support external TA SDK

[Etienne Carriere]

OP-TEE Test package no more selects BR2_TARGET_OPTEE_OS_SDK as
OP-TEE OS can import an external SDK if not building it from source.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 package/optee-test/Config.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/package/optee-test/Config.in b/package/optee-test/Config.in
index a40f1e9069..7564dcb3d2 100644
--- a/package/optee-test/Config.in
+++ b/package/optee-test/Config.in
@@ -3,7 +3,6 @@ config BR2_PACKAGE_OPTEE_TEST
 	depends on BR2_TARGET_OPTEE_OS
 	depends on BR2_TOOLCHAIN_HAS_THREADS # optee-client
 	select BR2_PACKAGE_OPTEE_CLIENT
-	select BR2_TARGET_OPTEE_OS_SDK
 	help
 	  This build option enables OP-TEE test package from the
 	  OP-TEE project. It helps platforms to verify the OP-TEE
-- 
2.17.1

[Buildroot] [PATCH 4/4] boot/optee-os: allow external SDK to build in-tree TAs

[Etienne Carriere]

When BR2_TARGET_OPTEE_OS_CORE is disabled, optee-os in-tree trusted
applications are not built along with with OP-TEE OS core. This change
allows one to build in-tree trusted application even when not building
OP-TEE OS core. In this case, the SDK must be provided either from
package build (BR2_TARGET_OPTEE_OS_SDK) or from an external path
(BR2_TARGET_OPTEE_OS_SDK_PATH).

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 boot/optee-os/optee-os.mk | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/boot/optee-os/optee-os.mk b/boot/optee-os/optee-os.mk
index 750516ad94..77493a720d 100644
--- a/boot/optee-os/optee-os.mk
+++ b/boot/optee-os/optee-os.mk
@@ -77,6 +77,22 @@ endef
 endif # BR2_TARGET_OPTEE_OS_CORE
 
 ifeq ($(BR2_TARGET_OPTEE_OS_SERVICES),y)
+ifneq ($(BR2_TARGET_OPTEE_OS_CORE),y)
+# If not building core, one can still build TAs from built SDK or external SDK
+ifeq ($(BR2_TARGET_OPTEE_OS_SDK),y)
+OPTEE_OS_TAS_SDK = $(OPTEE_OS_LOCAL_SDK)
+else
+OPTEE_OS_TAS_SDK = $(OPTEE_OS_EXTERNAL_SDK)
+endif
+define OPTEE_OS_BUILD_TAS_ONLY
+	$(foreach f,$(wildcard $(@D)/ta/*/Makefile), \
+		$(TARGET_CONFIGURE_OPTS) \
+		$(MAKE) CROSS_COMPILE=$(TARGET_CROSS) \
+			TA_DEV_KIT_DIR=$(OPTEE_OS_TAS_SDK) \
+			-C $(dir $f) all
+	)
+endef
+endif # !BR2_TARGET_OPTEE_OS_CORE
 define OPTEE_OS_INSTALL_TARGET_CMDS
 	$(if $(wildcard $(@D)/$(OPTEE_OS_BUILDDIR_OUT)/ta/*/*.ta),
 		$(INSTALL) -D -m 444 -t $(TARGET_DIR)/lib/optee_armtz \
@@ -122,6 +138,7 @@ endif # BR2_TARGET_OPTEE_OS_SDK_PATH
 define OPTEE_OS_BUILD_CMDS
 	$(OPTEE_OS_BUILD_CORE)
 	$(OPTEE_OS_BUILD_SDK)
+	$(OPTEE_OS_BUILD_TAS_ONLY)
 endef
 
 define OPTEE_OS_INSTALL_IMAGES_CMDS
-- 
2.17.1

[Buildroot] [PATCH 1/4] boot/optee-os: support external TA SDK

[Thomas Petazzoni]

Hello Etienne,

On Fri, 17 May 2019 10:45:53 +0200
Etienne Carriere <etienne.carriere@linaro.org> wrote:

> It happens that Buildroot is used to generate the embedded filesystem
> not not the boot images that which are built from another environment(s).

not not -> but not

that which -> which

> In such case OP-TEE SDK get built outside of Buildroot. This SDK is

get -> gets

> needed by other OP-TEE packages in Buildroot to generate .ta files.
> 
> This change introduces BR2_TARGET_OPTEE_OS_SDK_PATH for configuration
> to provide the path of an external OP-TEE SDK.
> 
> This configuration is mandated when neither BR2_TARGET_OPTEE_OS_CORE
> nor BR2_TARGET_OPTEE_OS_SDK are enable while BR2_TARGET_OPTEE_OS=y.

enable -> enabled

> 
> When BR2_TARGET_OPTEE_OS_SDK_PATH is defined, the build stage of
> optee-os copies the SDK file tree into optee-os build tree, for these
> to be installed during target/staging install.
> 
> With this change BR2_TARGET_OPTEE_OS_SERVICES no more selects
> BR2_TARGET_OPTEE_OS_CORE as .ta files can be build from external SDK.
> 
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>

To be honest, I am not convinced we want to support such a special use
case. It makes the whole OP-TEE packaging more complicated. I don't
think we need to support use cases where some random part of the system
gets built by Buildroot, and another part is built externally.

So unless you can convince me that the use case is really important, I
would be tempted to not accepted this patch series. I'm Cc'ing a bunch
of other folks so that they can share their opinion.

Best regards,

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

[Buildroot] [PATCH 1/4] boot/optee-os: support external TA SDK

[Yann E. MORIN]

Thomas, Etienne, All,

On 2019-06-01 15:30 +0200, Thomas Petazzoni spake thusly:
> On Fri, 17 May 2019 10:45:53 +0200
> Etienne Carriere <etienne.carriere@linaro.org> wrote:
> 
> > It happens that Buildroot is used to generate the embedded filesystem
> > not not the boot images that which are built from another environment(s).
> > In such case OP-TEE SDK get built outside of Buildroot. This SDK is
> > needed by other OP-TEE packages in Buildroot to generate .ta files.
> > 
> > This change introduces BR2_TARGET_OPTEE_OS_SDK_PATH for configuration
> > to provide the path of an external OP-TEE SDK.
> > 
> > This configuration is mandated when neither BR2_TARGET_OPTEE_OS_CORE
> > nor BR2_TARGET_OPTEE_OS_SDK are enable while BR2_TARGET_OPTEE_OS=y.
> > 
> > When BR2_TARGET_OPTEE_OS_SDK_PATH is defined, the build stage of
> > optee-os copies the SDK file tree into optee-os build tree, for these
> > to be installed during target/staging install.
> > 
> > With this change BR2_TARGET_OPTEE_OS_SERVICES no more selects
> > BR2_TARGET_OPTEE_OS_CORE as .ta files can be build from external SDK.
> > 
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> 
> To be honest, I am not convinced we want to support such a special use
> case. It makes the whole OP-TEE packaging more complicated. I don't
> think we need to support use cases where some random part of the system
> gets built by Buildroot, and another part is built externally.
> 
> So unless you can convince me that the use case is really important, I
> would be tempted to not accepted this patch series. I'm Cc'ing a bunch
> of other folks so that they can share their opinion.

Well, and even if the case is "important", and we decide to add support
for it, then why don't we allow for the same situation with other
packages, where parts of them are built out-side of Buildroot and then
shoe-horned at force into the Buildroot-built system?

If people really want to support such a corner case, then they are gfree
to implement their hacks in a br2-external tree.

Of course, the hack becomes pretty ugly once this is touching a package
already in Buildroot...

One in-between solution, would be to introduce a new package,
optee-os-custom-sdk, a very trivial package, which goal is to grab the
SDK files from "somewhere", potentially downloading etc... and storing
them where optee-os can use them from. Something like:

    boot/optee-os/optee-os.mk:
        OPTEE_OS_DEPENDENCIES += $(if $(BR2_TARGET_OPTEE_OS_CUSTOM_SDK),optee-os-custom-sdk)

    boot/optee-os-custom-sdk/config.in:
        config BR2_TARGET_OPTEE_OS_CUSTOM_SDK
            bool "optee-os-custom-sdk"
            depends on BR2_TARGET_OPTEE_OS
            help
              Blabla SDK files from outter space...

        config BR2_TARGET_OPTEE_OS_CUSTOM_SDK_URI
            bool "URI to SDK files"
            depends on BR2_TARGET_OPTEE_OS_CUSTOM_SDK_URI
            help
              Blaba where to find them (dir, tarball, git...)

And I'll lt your imagination come up with the content of
optee-os-custom-sdk.mk to handle that.

Note however how the Kconfig option BR2_TARGET_OPTEE_OS_CUSTOM_SDK
depends on BR2_TARGET_OPTEE_OS, but at build time the dependency is
inverted... This is not very nice, though...

I would be OKish if we were to introduce such a package, though.

Regards,
Yann E. MORIN.

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

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/4] boot/optee-os: support external TA SDK

[Thomas Petazzoni]

Hello Etienne,

On Sat, 1 Jun 2019 15:30:41 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> To be honest, I am not convinced we want to support such a special use
> case. It makes the whole OP-TEE packaging more complicated. I don't
> think we need to support use cases where some random part of the system
> gets built by Buildroot, and another part is built externally.
> 
> So unless you can convince me that the use case is really important, I
> would be tempted to not accepted this patch series. I'm Cc'ing a bunch
> of other folks so that they can share their opinion.

So, this series has been sitting in patchwork for a while, and I am
still not convinced we want to support something like this, and
generally the feedback from other folks at the Buildroot Developers
Meeting is that we should probably not support it.

So, I'll mark the series as Rejected. If you believe you have more
arguments on why this is necessary, what are the use cases, then we can
of course always revisit that decision, since nothing is ever set in
stone.

Thanks!

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

[Buildroot] [PATCH 1/4] boot/optee-os: support external TA SDK

[Etienne Carriere]

Hello Thomas and all,

I get your points.
Thanks for the feedback and sorry I have not answered earlier.


I considered your "optee-os-custom-sdk" package proposal but OP-TEE
does not provide a specific package for building these specific OP-TEE
applications (TAs).
I don't think I will insist on requesting such external SDK config.

Best Regards,
Etienne

On Sun, 27 Oct 2019 at 17:27, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Etienne,
>
> On Sat, 1 Jun 2019 15:30:41 +0200
> Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
>
> > To be honest, I am not convinced we want to support such a special use
> > case. It makes the whole OP-TEE packaging more complicated. I don't
> > think we need to support use cases where some random part of the system
> > gets built by Buildroot, and another part is built externally.
> >
> > So unless you can convince me that the use case is really important, I
> > would be tempted to not accepted this patch series. I'm Cc'ing a bunch
> > of other folks so that they can share their opinion.
>
> So, this series has been sitting in patchwork for a while, and I am
> still not convinced we want to support something like this, and
> generally the feedback from other folks at the Buildroot Developers
> Meeting is that we should probably not support it.
>
> So, I'll mark the series as Rejected. If you believe you have more
> arguments on why this is necessary, what are the use cases, then we can
> of course always revisit that decision, since nothing is ever set in
> stone.
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com