[1/4] boot/optee-os: support external TA SDK
diff mbox series

Message ID 20190517084556.12242-1-etienne.carriere@linaro.org
State New
Headers show
Series
  • [1/4] boot/optee-os: support external TA SDK
Related show

Commit Message

Etienne Carriere May 17, 2019, 8:45 a.m. UTC
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(-)

Comments

Thomas Petazzoni June 1, 2019, 1:30 p.m. UTC | #1
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
Yann E. MORIN June 1, 2019, 1:58 p.m. UTC | #2
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

Patch
diff mbox series

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))