* [Buildroot] [PATCH v2 2/5] optee-client: new package
2018-11-23 16:33 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: new package Etienne Carriere
@ 2018-11-23 16:33 ` Etienne Carriere
2018-12-10 21:57 ` Thomas Petazzoni
2018-11-23 16:33 ` [Buildroot] [PATCH v2 3/5] optee-benchmark: " Etienne Carriere
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Etienne Carriere @ 2018-11-23 16:33 UTC (permalink / raw)
To: buildroot
OP-TEE client API library and supplicant daemon from the
OP-TEE project.
The package is added to the Security menu of BR configuration.
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
- Add option BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION to ensure
OP-TEE client version is synced with OP-TEE OS version when
the later if enabled.
- Remove useless OPTEE_CLIENT_INSTALL_IMAGE=YES.
---
package/Config.in | 1 +
package/optee-client/Config.in | 73 ++++++++++++++++++++++++++++++++++
package/optee-client/S30optee | 26 ++++++++++++
package/optee-client/optee-client.hash | 4 ++
package/optee-client/optee-client.mk | 30 ++++++++++++++
5 files changed, 134 insertions(+)
create mode 100644 package/optee-client/Config.in
create mode 100644 package/optee-client/S30optee
create mode 100644 package/optee-client/optee-client.hash
create mode 100644 package/optee-client/optee-client.mk
diff --git a/package/Config.in b/package/Config.in
index b60e770..8c3b1bf 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2047,6 +2047,7 @@ endmenu
menu "Security"
source "package/checkpolicy/Config.in"
+ source "package/optee-client/Config.in"
source "package/paxtest/Config.in"
source "package/policycoreutils/Config.in"
source "package/refpolicy/Config.in"
diff --git a/package/optee-client/Config.in b/package/optee-client/Config.in
new file mode 100644
index 0000000..cff452b
--- /dev/null
+++ b/package/optee-client/Config.in
@@ -0,0 +1,73 @@
+config BR2_PACKAGE_OPTEE_CLIENT
+ bool "Embed OP-TEE client"
+ help
+ Enable the OP-TEE client package that brings non-secure
+ client application resources for OP-TEE support. OP-TEE
+ client is a component delivered by the OP-TEE project.
+
+ https://github.com/OP-TEE/optee_client
+
+ The client API library allows application to invoke
+ trusted applications hosted in the OP-TEE OS secure world.
+ The supplicant provides services hosted by the non-secure
+ world and invoked by the secure world.
+
+if BR2_PACKAGE_OPTEE_CLIENT
+
+choice
+ prompt "OP-TEE client version"
+ default BR2_PACKAGE_OPTEE_CLIENT_LATEST
+ help
+ Select the version of OP-TEE client you want to use
+
+config BR2_PACKAGE_OPTEE_CLIENT_LATEST
+ bool "sync with latest registered release tag"
+ help
+ Sync on latest release tag. This currently fetches the
+ latest registered release tag from the OP-TEE official
+ Git repository.
+
+config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
+ bool "sync with a specific Git"
+ help
+ Sync with a specific OP-TEE Git repository.
+
+endchoice
+
+config BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION
+ bool "use same version ref for OP-TEE components"
+ depends on BR2_PACKAGE_OPTEE_CLIENT_LATEST
+ default true
+ help
+ When enabled, OP-TEE client version must match the version
+ set for the other OP-TEE components.
+
+config BR2_PACKAGE_OPTEE_CLIENT_VERSION
+ string
+ default BR2_TARGET_OPTEE_OS_VERSION \
+ if BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION && \
+ BR2_TARGET_OPTEE_OS
+ default "3.3.0" if BR2_PACKAGE_OPTEE_CLIENT_LATEST
+ default BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION \
+ if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
+ help
+ Reference in the target Git repository to sync with.
+
+if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
+
+config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL
+ string "Git repository site"
+ help
+ Specific location of the reference source tree Git
+ repository.
+
+config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION
+ string "target reference to pull in the Git repository"
+ help
+ Package version reference to sync with. As source file
+ reference is a Git repository, the version reference can
+ be any Git reference as a tag or a sha1.
+
+endif
+
+endif #BR2_PACKAGE_OPTEE_CLIENT
diff --git a/package/optee-client/S30optee b/package/optee-client/S30optee
new file mode 100644
index 0000000..c893243
--- /dev/null
+++ b/package/optee-client/S30optee
@@ -0,0 +1,26 @@
+#!/bin/sh
+#
+# /etc/init.d/optee
+#
+# Start/stop tee-supplicant (OP-TEE normal world daemon)
+#
+case "$1" in
+ start)
+ if [ -e /usr/sbin/tee-supplicant -a -e /dev/teepriv0 ]; then
+ echo "Starting tee-supplicant..."
+ /usr/sbin/tee-supplicant &
+ exit 0
+ else
+ echo "tee-supplicant or TEE device not found"
+ exit 1
+ fi
+
+ ;;
+ stop)
+ killall tee-supplicant
+ ;;
+ status)
+ cat /dev/teepriv0 2>&1 | grep -q "Device or resource busy" || not="not "
+ echo "tee-supplicant is ${not}active"
+ ;;
+esac
diff --git a/package/optee-client/optee-client.hash b/package/optee-client/optee-client.hash
new file mode 100644
index 0000000..ed7bf4e
--- /dev/null
+++ b/package/optee-client/optee-client.hash
@@ -0,0 +1,4 @@
+# From https://github.com/OP-TEE/optee_client/archive/3.3.0.tar.gz
+sha256 63af1567fdcdbe28b45be274266a89aa81bef3d0fd8ec5a6eb680046a92e1177 optee-client-3.3.0.tar.gz
+# Locally computed
+sha256 fda8385993f112d7ca61b88b54ba5b4cbeec7e43a0f9b317d5186703c1985e8f LICENSE
diff --git a/package/optee-client/optee-client.mk b/package/optee-client/optee-client.mk
new file mode 100644
index 0000000..ccc5d12
--- /dev/null
+++ b/package/optee-client/optee-client.mk
@@ -0,0 +1,30 @@
+################################################################################
+#
+# optee-client
+#
+################################################################################
+
+OPTEE_CLIENT_VERSION = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_VERSION))
+OPTEE_CLIENT_LICENSE = BSD-3-Clause
+OPTEE_CLIENT_LICENSE_FILES = LICENSE
+
+ifeq ($(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT),y)
+OPTEE_CLIENT_SITE = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL))
+OPTEE_CLIENT_SITE_METHOD = git
+BR_NO_CHECK_HASH_FOR += $(OPTEE_CLIENT_SOURCE)
+else
+OPTEE_CLIENT_SITE = $(call github,OP-TEE,optee_client,$(OPTEE_CLIENT_VERSION))
+endif
+
+define OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT
+ $(INSTALL) -m 0755 -D $(OPTEE_CLIENT_PKGDIR)/S30optee \
+ $(TARGET_DIR)/etc/init.d/S30optee
+endef
+
+define OPTEE_CLIENT_INSTALL_INIT_SYSV
+ $(OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT)
+endef
+
+OPTEE_CLIENT_INSTALL_STAGING = YES
+
+$(eval $(cmake-package))
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Buildroot] [PATCH v2 2/5] optee-client: new package
2018-11-23 16:33 ` [Buildroot] [PATCH v2 2/5] optee-client: " Etienne Carriere
@ 2018-12-10 21:57 ` Thomas Petazzoni
2018-12-12 9:27 ` Etienne Carriere
2018-12-12 15:49 ` Etienne Carriere
0 siblings, 2 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2018-12-10 21:57 UTC (permalink / raw)
To: buildroot
Hello Etienne,
On Fri, 23 Nov 2018 17:33:34 +0100, Etienne Carriere wrote:
> diff --git a/package/optee-client/Config.in b/package/optee-client/Config.in
> new file mode 100644
> index 0000000..cff452b
> --- /dev/null
> +++ b/package/optee-client/Config.in
> @@ -0,0 +1,73 @@
> +config BR2_PACKAGE_OPTEE_CLIENT
> + bool "Embed OP-TEE client"
Just:
bool "optee-client"
> + help
> + Enable the OP-TEE client package that brings non-secure
> + client application resources for OP-TEE support. OP-TEE
> + client is a component delivered by the OP-TEE project.
> +
> + https://github.com/OP-TEE/optee_client
Please move this at the very end of the Config.in help text, i.e...
> +
> + The client API library allows application to invoke
> + trusted applications hosted in the OP-TEE OS secure world.
> + The supplicant provides services hosted by the non-secure
> + world and invoked by the secure world.
... here.
> +
> +if BR2_PACKAGE_OPTEE_CLIENT
> +
> +choice
> + prompt "OP-TEE client version"
prompt "version"
> + default BR2_PACKAGE_OPTEE_CLIENT_LATEST
> + help
> + Select the version of OP-TEE client you want to use
> +
> +config BR2_PACKAGE_OPTEE_CLIENT_LATEST
> + bool "sync with latest registered release tag"
bool "3.3.0"
> + help
> + Sync on latest release tag. This currently fetches the
Don't say "latest", because it won't always be the latest.
> + latest registered release tag from the OP-TEE official
> + Git repository.
> +
> +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> + bool "sync with a specific Git"
bool "Custom Git repository"
> + help
> + Sync with a specific OP-TEE Git repository.
Is there actually a need to specify a custom version for this client
library ? For the OS part, which is platform-specific, I understand,
but for optee-client, is this really needed ?
> +endchoice
> +
> +config BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION
> + bool "use same version ref for OP-TEE components"
I don't understand why you have this here. If you really want to do
that, what about adding a third choice above:
bool "use same version as optee-os"
> + depends on BR2_PACKAGE_OPTEE_CLIENT_LATEST
> + default true
default true doesn't mean anything, "default y" does. And it should
depend on BR2_TARGET_OPTEE_OS being selected.
But how can this make sense ? If the version for optee-os is a Git
commit hash, how can optee-client use the same version, given that they
are stored in two separate Git repositories, and that therefore it's
impossible/unlikely that optee-os/optee-client will have the same Git
commit hash. Or maybe this is only intended to work with Git tags? In
this case, it should be clearly explained.
> + help
> + When enabled, OP-TEE client version must match the version
> + set for the other OP-TEE components.
> +
> +config BR2_PACKAGE_OPTEE_CLIENT_VERSION
> + string
> + default BR2_TARGET_OPTEE_OS_VERSION \
> + if BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION && \
> + BR2_TARGET_OPTEE_OS
The dependency on BR2_TARGET_OPTEE_OS should not come here, but be on
the BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION option.
> + default "3.3.0" if BR2_PACKAGE_OPTEE_CLIENT_LATEST
> + default BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION \
> + if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> + help
> + Reference in the target Git repository to sync with.
> +
> +if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> +
> +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL
> + string "Git repository site"
string "URL of custom repository"
> + help
> + Specific location of the reference source tree Git
> + repository.
> +
> +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION
> + string "target reference to pull in the Git repository"
string "Custom repository version"
> + help
> + Package version reference to sync with. As source file
Don't use "sync", you don't sync with Git.
> + reference is a Git repository, the version reference can
> + be any Git reference as a tag or a sha1.
> +
> +endif
> +
> +endif #BR2_PACKAGE_OPTEE_CLIENT
> diff --git a/package/optee-client/S30optee b/package/optee-client/S30optee
> new file mode 100644
> index 0000000..c893243
> --- /dev/null
> +++ b/package/optee-client/S30optee
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +#
> +# /etc/init.d/optee
Drop this comment, it is useless, and in fact wrong: the file will not
have this name in a Buildroot filesystem.
> +#
> +# Start/stop tee-supplicant (OP-TEE normal world daemon)
> +#
> +case "$1" in
> + start)
> + if [ -e /usr/sbin/tee-supplicant -a -e /dev/teepriv0 ]; then
Drop this test, just start tee-supplicatn.
> + echo "Starting tee-supplicant..."
> + /usr/sbin/tee-supplicant &
Please use start-stop-daemon. See
https://patchwork.ozlabs.org/patch/994013/ for the "right" way of
writing an init script.
> + exit 0
> + else
> + echo "tee-supplicant or TEE device not found"
> + exit 1
> + fi
> +
> + ;;
> + stop)
> + killall tee-supplicant
Please use start-stop-daemon.
> + ;;
> + status)
> + cat /dev/teepriv0 2>&1 | grep -q "Device or resource busy" || not="not "
> + echo "tee-supplicant is ${not}active"
We don't provide a "status" target in other init scripts.
> + ;;
> +esac
> diff --git a/package/optee-client/optee-client.hash b/package/optee-client/optee-client.hash
> new file mode 100644
> index 0000000..ed7bf4e
> --- /dev/null
> +++ b/package/optee-client/optee-client.hash
> @@ -0,0 +1,4 @@
> +# From https://github.com/OP-TEE/optee_client/archive/3.3.0.tar.gz
> +sha256 63af1567fdcdbe28b45be274266a89aa81bef3d0fd8ec5a6eb680046a92e1177 optee-client-3.3.0.tar.gz
> +# Locally computed
> +sha256 fda8385993f112d7ca61b88b54ba5b4cbeec7e43a0f9b317d5186703c1985e8f LICENSE
Move the license hash in package/optee-client/3.3.0/optee-client.hash,
as it is specific to this version.
> diff --git a/package/optee-client/optee-client.mk b/package/optee-client/optee-client.mk
> new file mode 100644
> index 0000000..ccc5d12
> --- /dev/null
> +++ b/package/optee-client/optee-client.mk
> @@ -0,0 +1,30 @@
> +################################################################################
> +#
> +# optee-client
> +#
> +################################################################################
> +
> +OPTEE_CLIENT_VERSION = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_VERSION))
> +OPTEE_CLIENT_LICENSE = BSD-3-Clause
The license text contains a BSD-2-Clause license.
> +OPTEE_CLIENT_LICENSE_FILES = LICENSE
> +
> +ifeq ($(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT),y)
> +OPTEE_CLIENT_SITE = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL))
> +OPTEE_CLIENT_SITE_METHOD = git
> +BR_NO_CHECK_HASH_FOR += $(OPTEE_CLIENT_SOURCE)
> +else
> +OPTEE_CLIENT_SITE = $(call github,OP-TEE,optee_client,$(OPTEE_CLIENT_VERSION))
> +endif
> +
> +define OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT
> + $(INSTALL) -m 0755 -D $(OPTEE_CLIENT_PKGDIR)/S30optee \
> + $(TARGET_DIR)/etc/init.d/S30optee
> +endef
> +
> +define OPTEE_CLIENT_INSTALL_INIT_SYSV
> + $(OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT)
Please do the $(INSTALL) right here, there is no reason to have an
indirection through the OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT
variable.
> +OPTEE_CLIENT_INSTALL_STAGING = YES
Please move this a bit above in the .mk file. We generally have such
statements before the build/installation commands.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread* [Buildroot] [PATCH v2 2/5] optee-client: new package
2018-12-10 21:57 ` Thomas Petazzoni
@ 2018-12-12 9:27 ` Etienne Carriere
2018-12-12 15:49 ` Etienne Carriere
1 sibling, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2018-12-12 9:27 UTC (permalink / raw)
To: buildroot
Thanks Thomas for the feedback.
I will address your comments in a V3, I see no point to argue.
Regards,
etienne
On Mon, 10 Dec 2018 at 22:57, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Etienne,
>
> On Fri, 23 Nov 2018 17:33:34 +0100, Etienne Carriere wrote:
>
> > diff --git a/package/optee-client/Config.in b/package/optee-client/Config.in
> > new file mode 100644
> > index 0000000..cff452b
> > --- /dev/null
> > +++ b/package/optee-client/Config.in
> > @@ -0,0 +1,73 @@
> > +config BR2_PACKAGE_OPTEE_CLIENT
> > + bool "Embed OP-TEE client"
>
> Just:
>
> bool "optee-client"
>
> > + help
> > + Enable the OP-TEE client package that brings non-secure
> > + client application resources for OP-TEE support. OP-TEE
> > + client is a component delivered by the OP-TEE project.
> > +
> > + https://github.com/OP-TEE/optee_client
>
> Please move this at the very end of the Config.in help text, i.e...
>
> > +
> > + The client API library allows application to invoke
> > + trusted applications hosted in the OP-TEE OS secure world.
> > + The supplicant provides services hosted by the non-secure
> > + world and invoked by the secure world.
>
> ... here.
>
> > +
> > +if BR2_PACKAGE_OPTEE_CLIENT
> > +
> > +choice
> > + prompt "OP-TEE client version"
>
> prompt "version"
>
> > + default BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + help
> > + Select the version of OP-TEE client you want to use
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + bool "sync with latest registered release tag"
>
> bool "3.3.0"
>
> > + help
> > + Sync on latest release tag. This currently fetches the
>
> Don't say "latest", because it won't always be the latest.
>
> > + latest registered release tag from the OP-TEE official
> > + Git repository.
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> > + bool "sync with a specific Git"
>
> bool "Custom Git repository"
>
> > + help
> > + Sync with a specific OP-TEE Git repository.
>
> Is there actually a need to specify a custom version for this client
> library ? For the OS part, which is platform-specific, I understand,
> but for optee-client, is this really needed ?
>
> > +endchoice
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION
> > + bool "use same version ref for OP-TEE components"
>
> I don't understand why you have this here. If you really want to do
> that, what about adding a third choice above:
>
> bool "use same version as optee-os"
>
> > + depends on BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + default true
>
> default true doesn't mean anything, "default y" does. And it should
> depend on BR2_TARGET_OPTEE_OS being selected.
>
> But how can this make sense ? If the version for optee-os is a Git
> commit hash, how can optee-client use the same version, given that they
> are stored in two separate Git repositories, and that therefore it's
> impossible/unlikely that optee-os/optee-client will have the same Git
> commit hash. Or maybe this is only intended to work with Git tags? In
> this case, it should be clearly explained.
>
> > + help
> > + When enabled, OP-TEE client version must match the version
> > + set for the other OP-TEE components.
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_VERSION
> > + string
> > + default BR2_TARGET_OPTEE_OS_VERSION \
> > + if BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION && \
> > + BR2_TARGET_OPTEE_OS
>
> The dependency on BR2_TARGET_OPTEE_OS should not come here, but be on
> the BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION option.
>
> > + default "3.3.0" if BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + default BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION \
> > + if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> > + help
> > + Reference in the target Git repository to sync with.
> > +
> > +if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL
> > + string "Git repository site"
>
> string "URL of custom repository"
>
> > + help
> > + Specific location of the reference source tree Git
> > + repository.
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION
> > + string "target reference to pull in the Git repository"
>
> string "Custom repository version"
>
> > + help
> > + Package version reference to sync with. As source file
>
> Don't use "sync", you don't sync with Git.
>
> > + reference is a Git repository, the version reference can
> > + be any Git reference as a tag or a sha1.
> > +
> > +endif
> > +
> > +endif #BR2_PACKAGE_OPTEE_CLIENT
> > diff --git a/package/optee-client/S30optee b/package/optee-client/S30optee
> > new file mode 100644
> > index 0000000..c893243
> > --- /dev/null
> > +++ b/package/optee-client/S30optee
> > @@ -0,0 +1,26 @@
> > +#!/bin/sh
> > +#
> > +# /etc/init.d/optee
>
> Drop this comment, it is useless, and in fact wrong: the file will not
> have this name in a Buildroot filesystem.
>
> > +#
> > +# Start/stop tee-supplicant (OP-TEE normal world daemon)
> > +#
> > +case "$1" in
> > + start)
> > + if [ -e /usr/sbin/tee-supplicant -a -e /dev/teepriv0 ]; then
>
> Drop this test, just start tee-supplicatn.
>
> > + echo "Starting tee-supplicant..."
> > + /usr/sbin/tee-supplicant &
>
> Please use start-stop-daemon. See
> https://patchwork.ozlabs.org/patch/994013/ for the "right" way of
> writing an init script.
>
> > + exit 0
> > + else
> > + echo "tee-supplicant or TEE device not found"
> > + exit 1
> > + fi
> > +
> > + ;;
> > + stop)
> > + killall tee-supplicant
>
> Please use start-stop-daemon.
>
> > + ;;
> > + status)
> > + cat /dev/teepriv0 2>&1 | grep -q "Device or resource busy" || not="not "
> > + echo "tee-supplicant is ${not}active"
>
> We don't provide a "status" target in other init scripts.
>
> > + ;;
> > +esac
> > diff --git a/package/optee-client/optee-client.hash b/package/optee-client/optee-client.hash
> > new file mode 100644
> > index 0000000..ed7bf4e
> > --- /dev/null
> > +++ b/package/optee-client/optee-client.hash
> > @@ -0,0 +1,4 @@
> > +# From https://github.com/OP-TEE/optee_client/archive/3.3.0.tar.gz
> > +sha256 63af1567fdcdbe28b45be274266a89aa81bef3d0fd8ec5a6eb680046a92e1177 optee-client-3.3.0.tar.gz
> > +# Locally computed
> > +sha256 fda8385993f112d7ca61b88b54ba5b4cbeec7e43a0f9b317d5186703c1985e8f LICENSE
>
> Move the license hash in package/optee-client/3.3.0/optee-client.hash,
> as it is specific to this version.
>
> > diff --git a/package/optee-client/optee-client.mk b/package/optee-client/optee-client.mk
> > new file mode 100644
> > index 0000000..ccc5d12
> > --- /dev/null
> > +++ b/package/optee-client/optee-client.mk
> > @@ -0,0 +1,30 @@
> > +################################################################################
> > +#
> > +# optee-client
> > +#
> > +################################################################################
> > +
> > +OPTEE_CLIENT_VERSION = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_VERSION))
> > +OPTEE_CLIENT_LICENSE = BSD-3-Clause
>
> The license text contains a BSD-2-Clause license.
>
> > +OPTEE_CLIENT_LICENSE_FILES = LICENSE
> > +
> > +ifeq ($(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT),y)
> > +OPTEE_CLIENT_SITE = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL))
> > +OPTEE_CLIENT_SITE_METHOD = git
> > +BR_NO_CHECK_HASH_FOR += $(OPTEE_CLIENT_SOURCE)
> > +else
> > +OPTEE_CLIENT_SITE = $(call github,OP-TEE,optee_client,$(OPTEE_CLIENT_VERSION))
> > +endif
> > +
> > +define OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT
> > + $(INSTALL) -m 0755 -D $(OPTEE_CLIENT_PKGDIR)/S30optee \
> > + $(TARGET_DIR)/etc/init.d/S30optee
> > +endef
> > +
> > +define OPTEE_CLIENT_INSTALL_INIT_SYSV
> > + $(OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT)
>
> Please do the $(INSTALL) right here, there is no reason to have an
> indirection through the OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT
> variable.
>
> > +OPTEE_CLIENT_INSTALL_STAGING = YES
>
> Please move this a bit above in the .mk file. We generally have such
> statements before the build/installation commands.
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread* [Buildroot] [PATCH v2 2/5] optee-client: new package
2018-12-10 21:57 ` Thomas Petazzoni
2018-12-12 9:27 ` Etienne Carriere
@ 2018-12-12 15:49 ` Etienne Carriere
1 sibling, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2018-12-12 15:49 UTC (permalink / raw)
To: buildroot
Hi Thomas,
Just a word below specifically on the OP-TEE client version selection.
On Mon, 10 Dec 2018 at 22:57, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Etienne,
>
> On Fri, 23 Nov 2018 17:33:34 +0100, Etienne Carriere wrote:
>
> > diff --git a/package/optee-client/Config.in b/package/optee-client/Config.in
> > new file mode 100644
> > index 0000000..cff452b
> > --- /dev/null
> > +++ b/package/optee-client/Config.in
> > @@ -0,0 +1,73 @@
> > +config BR2_PACKAGE_OPTEE_CLIENT
> > + bool "Embed OP-TEE client"
>
> Just:
>
> bool "optee-client"
>
> > + help
> > + Enable the OP-TEE client package that brings non-secure
> > + client application resources for OP-TEE support. OP-TEE
> > + client is a component delivered by the OP-TEE project.
> > +
> > + https://github.com/OP-TEE/optee_client
>
> Please move this at the very end of the Config.in help text, i.e...
>
> > +
> > + The client API library allows application to invoke
> > + trusted applications hosted in the OP-TEE OS secure world.
> > + The supplicant provides services hosted by the non-secure
> > + world and invoked by the secure world.
>
> ... here.
>
> > +
> > +if BR2_PACKAGE_OPTEE_CLIENT
> > +
> > +choice
> > + prompt "OP-TEE client version"
>
> prompt "version"
>
> > + default BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + help
> > + Select the version of OP-TEE client you want to use
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + bool "sync with latest registered release tag"
>
> bool "3.3.0"
>
> > + help
> > + Sync on latest release tag. This currently fetches the
>
> Don't say "latest", because it won't always be the latest.
>
> > + latest registered release tag from the OP-TEE official
> > + Git repository.
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> > + bool "sync with a specific Git"
>
> bool "Custom Git repository"
>
> > + help
> > + Sync with a specific OP-TEE Git repository.
>
> Is there actually a need to specify a custom version for this client
> library ? For the OS part, which is platform-specific, I understand,
> but for optee-client, is this really needed ?
Yes. This allows on to use an older specific OP-TEE version and due to
backward compatibility issues, one should specify the target version
for each OP-TEE components if one is not the "latest" registered in
BR.
>
> > +endchoice
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION
> > + bool "use same version ref for OP-TEE components"
>
> I don't understand why you have this here. If you really want to do
> that, what about adding a third choice above:
>
> bool "use same version as optee-os"
>
> > + depends on BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + default true
>
> default true doesn't mean anything, "default y" does. And it should
> depend on BR2_TARGET_OPTEE_OS being selected.
>
> But how can this make sense ? If the version for optee-os is a Git
> commit hash, how can optee-client use the same version, given that they
> are stored in two separate Git repositories, and that therefore it's
> impossible/unlikely that optee-os/optee-client will have the same Git
> commit hash. Or maybe this is only intended to work with Git tags? In
> this case, it should be clearly explained.
Right. This attempts for synchro of the OP-TEE sources craps.
I will remove the all and leave to the users to be consistent in their
OP-TEE configuration when selecting specific package version.
>
> > + help
> > + When enabled, OP-TEE client version must match the version
> > + set for the other OP-TEE components.
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_VERSION
> > + string
> > + default BR2_TARGET_OPTEE_OS_VERSION \
> > + if BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION && \
> > + BR2_TARGET_OPTEE_OS
>
> The dependency on BR2_TARGET_OPTEE_OS should not come here, but be on
> the BR2_PACKAGE_OPTEE_CLIENT_SYNCED_VERSION option.
>
> > + default "3.3.0" if BR2_PACKAGE_OPTEE_CLIENT_LATEST
> > + default BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION \
> > + if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> > + help
> > + Reference in the target Git repository to sync with.
> > +
> > +if BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL
> > + string "Git repository site"
>
> string "URL of custom repository"
>
> > + help
> > + Specific location of the reference source tree Git
> > + repository.
> > +
> > +config BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_VERSION
> > + string "target reference to pull in the Git repository"
>
> string "Custom repository version"
>
> > + help
> > + Package version reference to sync with. As source file
>
> Don't use "sync", you don't sync with Git.
>
> > + reference is a Git repository, the version reference can
> > + be any Git reference as a tag or a sha1.
> > +
> > +endif
> > +
> > +endif #BR2_PACKAGE_OPTEE_CLIENT
> > diff --git a/package/optee-client/S30optee b/package/optee-client/S30optee
> > new file mode 100644
> > index 0000000..c893243
> > --- /dev/null
> > +++ b/package/optee-client/S30optee
> > @@ -0,0 +1,26 @@
> > +#!/bin/sh
> > +#
> > +# /etc/init.d/optee
>
> Drop this comment, it is useless, and in fact wrong: the file will not
> have this name in a Buildroot filesystem.
>
> > +#
> > +# Start/stop tee-supplicant (OP-TEE normal world daemon)
> > +#
> > +case "$1" in
> > + start)
> > + if [ -e /usr/sbin/tee-supplicant -a -e /dev/teepriv0 ]; then
>
> Drop this test, just start tee-supplicatn.
>
> > + echo "Starting tee-supplicant..."
> > + /usr/sbin/tee-supplicant &
>
> Please use start-stop-daemon. See
> https://patchwork.ozlabs.org/patch/994013/ for the "right" way of
> writing an init script.
>
> > + exit 0
> > + else
> > + echo "tee-supplicant or TEE device not found"
> > + exit 1
> > + fi
> > +
> > + ;;
> > + stop)
> > + killall tee-supplicant
>
> Please use start-stop-daemon.
>
> > + ;;
> > + status)
> > + cat /dev/teepriv0 2>&1 | grep -q "Device or resource busy" || not="not "
> > + echo "tee-supplicant is ${not}active"
>
> We don't provide a "status" target in other init scripts.
>
> > + ;;
> > +esac
> > diff --git a/package/optee-client/optee-client.hash b/package/optee-client/optee-client.hash
> > new file mode 100644
> > index 0000000..ed7bf4e
> > --- /dev/null
> > +++ b/package/optee-client/optee-client.hash
> > @@ -0,0 +1,4 @@
> > +# From https://github.com/OP-TEE/optee_client/archive/3.3.0.tar.gz
> > +sha256 63af1567fdcdbe28b45be274266a89aa81bef3d0fd8ec5a6eb680046a92e1177 optee-client-3.3.0.tar.gz
> > +# Locally computed
> > +sha256 fda8385993f112d7ca61b88b54ba5b4cbeec7e43a0f9b317d5186703c1985e8f LICENSE
>
> Move the license hash in package/optee-client/3.3.0/optee-client.hash,
> as it is specific to this version.
>
> > diff --git a/package/optee-client/optee-client.mk b/package/optee-client/optee-client.mk
> > new file mode 100644
> > index 0000000..ccc5d12
> > --- /dev/null
> > +++ b/package/optee-client/optee-client.mk
> > @@ -0,0 +1,30 @@
> > +################################################################################
> > +#
> > +# optee-client
> > +#
> > +################################################################################
> > +
> > +OPTEE_CLIENT_VERSION = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_VERSION))
> > +OPTEE_CLIENT_LICENSE = BSD-3-Clause
>
> The license text contains a BSD-2-Clause license.
>
> > +OPTEE_CLIENT_LICENSE_FILES = LICENSE
> > +
> > +ifeq ($(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_GIT),y)
> > +OPTEE_CLIENT_SITE = $(call qstrip,$(BR2_PACKAGE_OPTEE_CLIENT_CUSTOM_REPO_URL))
> > +OPTEE_CLIENT_SITE_METHOD = git
> > +BR_NO_CHECK_HASH_FOR += $(OPTEE_CLIENT_SOURCE)
> > +else
> > +OPTEE_CLIENT_SITE = $(call github,OP-TEE,optee_client,$(OPTEE_CLIENT_VERSION))
> > +endif
> > +
> > +define OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT
> > + $(INSTALL) -m 0755 -D $(OPTEE_CLIENT_PKGDIR)/S30optee \
> > + $(TARGET_DIR)/etc/init.d/S30optee
> > +endef
> > +
> > +define OPTEE_CLIENT_INSTALL_INIT_SYSV
> > + $(OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT)
>
> Please do the $(INSTALL) right here, there is no reason to have an
> indirection through the OPTEE_CLIENT_INSTALL_SUPPLICANT_SCRIPT
> variable.
>
> > +OPTEE_CLIENT_INSTALL_STAGING = YES
>
> Please move this a bit above in the .mk file. We generally have such
> statements before the build/installation commands.
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Buildroot] [PATCH v2 3/5] optee-benchmark: new package
2018-11-23 16:33 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: new package Etienne Carriere
2018-11-23 16:33 ` [Buildroot] [PATCH v2 2/5] optee-client: " Etienne Carriere
@ 2018-11-23 16:33 ` Etienne Carriere
2018-12-10 21:59 ` Thomas Petazzoni
2018-11-23 16:33 ` [Buildroot] [PATCH v2 4/5] optee-examples: " Etienne Carriere
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Etienne Carriere @ 2018-11-23 16:33 UTC (permalink / raw)
To: buildroot
OP-TEE performance benchmark tools for the OP-TEE project.
This packages generates embedded Linux based OS materials used
to retrieve execution timing information on invocation of the
OP-TEE secure services.
It is added next to the OP-TEE client package in BR configuration.
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
- Add dependency on OP-TEE client.
- Add option BR2_PACKAGE_OPTEE_BENCHMARK_SYNCED_VERSION to ensure
OP-TEE benchmark version is synced with OP-TEE client version.
- Remove useless OPTEE_BENCHMARK_INSTALL_STAGING and
OPTEE_BENCHMARK_INSTALL_IMAGES.
- Remove unused BR2_PACKAGE_OPTEE_BENCHMARK_GIT_REFERENCE.
- Remove useless _INSTALL_STAGING/_INSTALL_IMAGES=YES.
---
package/Config.in | 1 +
package/optee-benchmark/Config.in | 69 ++++++++++++++++++++++++++++
package/optee-benchmark/optee-benchmark.hash | 2 +
package/optee-benchmark/optee-benchmark.mk | 22 +++++++++
4 files changed, 94 insertions(+)
create mode 100644 package/optee-benchmark/Config.in
create mode 100644 package/optee-benchmark/optee-benchmark.hash
create mode 100644 package/optee-benchmark/optee-benchmark.mk
diff --git a/package/Config.in b/package/Config.in
index 8c3b1bf..38200af 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2047,6 +2047,7 @@ endmenu
menu "Security"
source "package/checkpolicy/Config.in"
+ source "package/optee-benchmark/Config.in"
source "package/optee-client/Config.in"
source "package/paxtest/Config.in"
source "package/policycoreutils/Config.in"
diff --git a/package/optee-benchmark/Config.in b/package/optee-benchmark/Config.in
new file mode 100644
index 0000000..2d56a7e
--- /dev/null
+++ b/package/optee-benchmark/Config.in
@@ -0,0 +1,69 @@
+config BR2_PACKAGE_OPTEE_BENCHMARK
+ bool "Embed OP-TEE benchmark support"
+ select BR2_PACKAGE_OPTEE_CLIENT
+ select BR2_PACKAGE_LIBYAML
+ help
+ Enable the OP-TEE benchmark package that brings facilities
+ for profiling traversal and execution timings when
+ invoking OP-TEE. OP-TEE benchmark is a component delivered
+ by the OP-TEE project.
+
+ http://github.com/linaro-swg/optee_benchmark
+
+if BR2_PACKAGE_OPTEE_BENCHMARK
+
+choice
+ prompt "OP-TEE Benchmark version"
+ default BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
+ help
+ Select the version of OP-TEE benchmark you want to use
+
+config BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
+ bool "sync with latest release tag"
+ help
+ Sync on latest release tag. This currently fetches the
+ latest registered release tag from the OP-TEE official
+ Git repository.
+
+config BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_GIT
+ bool "sync with a specific Git"
+ help
+ Sync with a specific OP-TEE Git repository.
+
+endchoice
+
+config BR2_PACKAGE_OPTEE_BENCHMARK_SYNCED_VERSION
+ bool "use same version ref for OP-TEE components"
+ depends on BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
+ default true
+ help
+ When enabled package version must match the version set for
+ OP-TEE client.
+
+config BR2_PACKAGE_OPTEE_BENCHMARK_VERSION
+ string
+ default BR2_PACKAGE_OPTEE_CLIENT_VERSION \
+ if BR2_PACKAGE_OPTEE_BENCHMARK_SYNCED_VERSION
+ default "3.3.0" if BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
+ default BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_REPO_VERSION \
+ if BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_GIT
+ help
+ Reference in the target Git repository to sync with.
+
+if BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_GIT
+
+config BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_REPO_URL
+ string "Git repository site"
+ help
+ Specific location of the reference source tree Git repository.
+
+config BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_REPO_VERSION
+ string "target reference to pull in the Git repository"
+ help
+ Package version reference to sync with. As source file
+ reference is a Git repository, the version reference can be
+ any Git reference as a tag or a sha1.
+
+endif
+
+endif #BR2_PACKAGE_OPTEE_BENCHMARK
diff --git a/package/optee-benchmark/optee-benchmark.hash b/package/optee-benchmark/optee-benchmark.hash
new file mode 100644
index 0000000..d93c26c
--- /dev/null
+++ b/package/optee-benchmark/optee-benchmark.hash
@@ -0,0 +1,2 @@
+# From https://github.com/linaro-swg/optee_benchmark/archive/3.3.0.tar.gz
+sha256 bfba3749ac8b37628550696f0625452ae8aef060eff5b3b1c4283a5dad8a3383 optee-benchmark-3.3.0.tar.gz
diff --git a/package/optee-benchmark/optee-benchmark.mk b/package/optee-benchmark/optee-benchmark.mk
new file mode 100644
index 0000000..8eef0f6
--- /dev/null
+++ b/package/optee-benchmark/optee-benchmark.mk
@@ -0,0 +1,22 @@
+################################################################################
+#
+# optee-benchmarch
+#
+################################################################################
+
+OPTEE_BENCHMARK_VERSION = $(call qstrip,$(BR2_PACKAGE_OPTEE_BENCHMARK_VERSION))
+OPTEE_BENCHMARK_LICENSE = BSD-2-Clause
+
+OPTEE_BENCHMARK_DEPENDENCIES = optee-client libyaml
+
+ifeq ($(BR2_PACKAGE_OPTEE_BENCHMARK_LATEST),y)
+OPTEE_BENCHMARK_SITE = $(call github,linaro-swg,optee_benchmark,$(OPTEE_BENCHMARK_VERSION))
+endif
+
+ifeq ($(BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_GIT),y)
+OPTEE_BENCHMARK_SITE = $(call qstrip,$(BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_REPO_URL))
+OPTEE_BENCHMARK_SITE_METHOD = git
+BR_NO_CHECK_HASH_FOR += $(OPTEE_BENCHMARK_SOURCE)
+endif
+
+$(eval $(cmake-package))
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Buildroot] [PATCH v2 3/5] optee-benchmark: new package
2018-11-23 16:33 ` [Buildroot] [PATCH v2 3/5] optee-benchmark: " Etienne Carriere
@ 2018-12-10 21:59 ` Thomas Petazzoni
2018-12-12 9:30 ` Etienne Carriere
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2018-12-10 21:59 UTC (permalink / raw)
To: buildroot
Hello Etienne,
On Fri, 23 Nov 2018 17:33:35 +0100, Etienne Carriere wrote:
> OP-TEE performance benchmark tools for the OP-TEE project.
>
> This packages generates embedded Linux based OS materials used
> to retrieve execution timing information on invocation of the
> OP-TEE secure services.
>
> It is added next to the OP-TEE client package in BR configuration.
>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Thanks. I have pretty much the same comments as for PATCH 2/5 on
optee-client, so if you could apply the same logic to this PATCH 3/5,
it would be nice. A few other things though.
> package/Config.in | 1 +
> package/optee-benchmark/Config.in | 69 ++++++++++++++++++++++++++++
> package/optee-benchmark/optee-benchmark.hash | 2 +
> package/optee-benchmark/optee-benchmark.mk | 22 +++++++++
> 4 files changed, 94 insertions(+)
Please add an entry in the DEVELOPERS file (it should be done in each
patch for the package being added by that patch).
> diff --git a/package/optee-benchmark/Config.in b/package/optee-benchmark/Config.in
> new file mode 100644
> index 0000000..2d56a7e
> --- /dev/null
> +++ b/package/optee-benchmark/Config.in
> @@ -0,0 +1,69 @@
> +config BR2_PACKAGE_OPTEE_BENCHMARK
> + bool "Embed OP-TEE benchmark support"
> + select BR2_PACKAGE_OPTEE_CLIENT
> + select BR2_PACKAGE_LIBYAML
> + help
> + Enable the OP-TEE benchmark package that brings facilities
> + for profiling traversal and execution timings when
> + invoking OP-TEE. OP-TEE benchmark is a component delivered
> + by the OP-TEE project.
> +
> + http://github.com/linaro-swg/optee_benchmark
> +
> +if BR2_PACKAGE_OPTEE_BENCHMARK
> +
> +choice
> + prompt "OP-TEE Benchmark version"
> + default BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
> + help
> + Select the version of OP-TEE benchmark you want to use
> +
> +config BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
> + bool "sync with latest release tag"
> + help
> + Sync on latest release tag. This currently fetches the
> + latest registered release tag from the OP-TEE official
> + Git repository.
> +
> +config BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_GIT
> + bool "sync with a specific Git"
> + help
> + Sync with a specific OP-TEE Git repository.
Do we really need all this version customization stuff for
optee-benchmark ? I doubt it is needed. Buildroot generally doesn't
provide a version selection, except for highly HW-specific packages
(kernel, bootloaders, firmware, etc.).
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Buildroot] [PATCH v2 3/5] optee-benchmark: new package
2018-12-10 21:59 ` Thomas Petazzoni
@ 2018-12-12 9:30 ` Etienne Carriere
2018-12-13 8:27 ` Etienne Carriere
0 siblings, 1 reply; 14+ messages in thread
From: Etienne Carriere @ 2018-12-12 9:30 UTC (permalink / raw)
To: buildroot
On Mon, 10 Dec 2018 at 22:59, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Etienne,
>
> On Fri, 23 Nov 2018 17:33:35 +0100, Etienne Carriere wrote:
> > OP-TEE performance benchmark tools for the OP-TEE project.
> >
> > This packages generates embedded Linux based OS materials used
> > to retrieve execution timing information on invocation of the
> > OP-TEE secure services.
> >
> > It is added next to the OP-TEE client package in BR configuration.
> >
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
>
> Thanks. I have pretty much the same comments as for PATCH 2/5 on
> optee-client, so if you could apply the same logic to this PATCH 3/5,
Indeed I will, thanks.
> it would be nice. A few other things though.
>
> > package/Config.in | 1 +
> > package/optee-benchmark/Config.in | 69 ++++++++++++++++++++++++++++
> > package/optee-benchmark/optee-benchmark.hash | 2 +
> > package/optee-benchmark/optee-benchmark.mk | 22 +++++++++
> > 4 files changed, 94 insertions(+)
>
> Please add an entry in the DEVELOPERS file (it should be done in each
> patch for the package being added by that patch).
>
> > diff --git a/package/optee-benchmark/Config.in b/package/optee-benchmark/Config.in
> > new file mode 100644
> > index 0000000..2d56a7e
> > --- /dev/null
> > +++ b/package/optee-benchmark/Config.in
> > @@ -0,0 +1,69 @@
> > +config BR2_PACKAGE_OPTEE_BENCHMARK
> > + bool "Embed OP-TEE benchmark support"
> > + select BR2_PACKAGE_OPTEE_CLIENT
> > + select BR2_PACKAGE_LIBYAML
> > + help
> > + Enable the OP-TEE benchmark package that brings facilities
> > + for profiling traversal and execution timings when
> > + invoking OP-TEE. OP-TEE benchmark is a component delivered
> > + by the OP-TEE project.
> > +
> > + http://github.com/linaro-swg/optee_benchmark
> > +
> > +if BR2_PACKAGE_OPTEE_BENCHMARK
> > +
> > +choice
> > + prompt "OP-TEE Benchmark version"
> > + default BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
> > + help
> > + Select the version of OP-TEE benchmark you want to use
> > +
> > +config BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
> > + bool "sync with latest release tag"
> > + help
> > + Sync on latest release tag. This currently fetches the
> > + latest registered release tag from the OP-TEE official
> > + Git repository.
> > +
> > +config BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_GIT
> > + bool "sync with a specific Git"
> > + help
> > + Sync with a specific OP-TEE Git repository.
>
> Do we really need all this version customization stuff for
> optee-benchmark ? I doubt it is needed. Buildroot generally doesn't
> provide a version selection, except for highly HW-specific packages
> (kernel, bootloaders, firmware, etc.).
Ok fine, let's default supporta single version and only from the
official Git repo.
regards,
etienne
>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Buildroot] [PATCH v2 3/5] optee-benchmark: new package
2018-12-12 9:30 ` Etienne Carriere
@ 2018-12-13 8:27 ` Etienne Carriere
0 siblings, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2018-12-13 8:27 UTC (permalink / raw)
To: buildroot
On Wed, 12 Dec 2018 at 10:30, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> On Mon, 10 Dec 2018 at 22:59, Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > Hello Etienne,
> >
> > On Fri, 23 Nov 2018 17:33:35 +0100, Etienne Carriere wrote:
> > > OP-TEE performance benchmark tools for the OP-TEE project.
> > >
> > > This packages generates embedded Linux based OS materials used
> > > to retrieve execution timing information on invocation of the
> > > OP-TEE secure services.
> > >
> > > It is added next to the OP-TEE client package in BR configuration.
> > >
> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> >
> > Thanks. I have pretty much the same comments as for PATCH 2/5 on
> > optee-client, so if you could apply the same logic to this PATCH 3/5,
>
> Indeed I will, thanks.
>
> > it would be nice. A few other things though.
> >
> > > package/Config.in | 1 +
> > > package/optee-benchmark/Config.in | 69 ++++++++++++++++++++++++++++
> > > package/optee-benchmark/optee-benchmark.hash | 2 +
> > > package/optee-benchmark/optee-benchmark.mk | 22 +++++++++
> > > 4 files changed, 94 insertions(+)
> >
> > Please add an entry in the DEVELOPERS file (it should be done in each
> > patch for the package being added by that patch).
> >
> > > diff --git a/package/optee-benchmark/Config.in b/package/optee-benchmark/Config.in
> > > new file mode 100644
> > > index 0000000..2d56a7e
> > > --- /dev/null
> > > +++ b/package/optee-benchmark/Config.in
> > > @@ -0,0 +1,69 @@
> > > +config BR2_PACKAGE_OPTEE_BENCHMARK
> > > + bool "Embed OP-TEE benchmark support"
> > > + select BR2_PACKAGE_OPTEE_CLIENT
> > > + select BR2_PACKAGE_LIBYAML
> > > + help
> > > + Enable the OP-TEE benchmark package that brings facilities
> > > + for profiling traversal and execution timings when
> > > + invoking OP-TEE. OP-TEE benchmark is a component delivered
> > > + by the OP-TEE project.
> > > +
> > > + http://github.com/linaro-swg/optee_benchmark
> > > +
> > > +if BR2_PACKAGE_OPTEE_BENCHMARK
> > > +
> > > +choice
> > > + prompt "OP-TEE Benchmark version"
> > > + default BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
> > > + help
> > > + Select the version of OP-TEE benchmark you want to use
> > > +
> > > +config BR2_PACKAGE_OPTEE_BENCHMARK_LATEST
> > > + bool "sync with latest release tag"
> > > + help
> > > + Sync on latest release tag. This currently fetches the
> > > + latest registered release tag from the OP-TEE official
> > > + Git repository.
> > > +
> > > +config BR2_PACKAGE_OPTEE_BENCHMARK_CUSTOM_GIT
> > > + bool "sync with a specific Git"
> > > + help
> > > + Sync with a specific OP-TEE Git repository.
> >
> > Do we really need all this version customization stuff for
> > optee-benchmark ? I doubt it is needed. Buildroot generally doesn't
> > provide a version selection, except for highly HW-specific packages
> > (kernel, bootloaders, firmware, etc.).
>
> Ok fine, let's default supporta single version and only from the
> official Git repo.
Hello Thomas,
Sorry, I must step back on this.
I fear the OP-TEE components (OS, benchmark, client, test and
examples) are not mature enough to be fully compatible across
unaligned versions.
Maybe in some near future but not for the moment.
If a BR board/platform config defines a specific OP-TEE OS version,
then the other OP-TEE parts should used this same versioning ID.
I thus plan to keep the configs BR2_PACKAGE_OPTEE_*_CUSTOM_GIT with
xxx_CUSTOM_REPO_URL and xxx_REPO_VERSION.
Do you think it is acceptable?
regards,
etienne
>
> regards,
> etienne
>
> >
> > Thanks,
> >
> > Thomas
> > --
> > Thomas Petazzoni, CTO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Buildroot] [PATCH v2 4/5] optee-examples: new package
2018-11-23 16:33 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: new package Etienne Carriere
2018-11-23 16:33 ` [Buildroot] [PATCH v2 2/5] optee-client: " Etienne Carriere
2018-11-23 16:33 ` [Buildroot] [PATCH v2 3/5] optee-benchmark: " Etienne Carriere
@ 2018-11-23 16:33 ` Etienne Carriere
2018-11-23 16:33 ` [Buildroot] [PATCH v2 5/5] optee-test: " Etienne Carriere
2018-12-10 21:46 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: " Thomas Petazzoni
4 siblings, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2018-11-23 16:33 UTC (permalink / raw)
To: buildroot
This package generates embedded Linux based OS userland client
applications and OP-TEE OS trusted applications all embedded in
the file system. These applications shows how to use the APIs
OP-TEE OS is based on, both in the non secure and secure worlds.
Package is added next to the OP-TEE client package in the BR
package configuration.
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
- Replace BR2_arm with BR2_ARM_CPU_ARMV7 as OP-TEE supports only
BR2_ARM_CPU_ARMV7 architectures among the 32bit Arm machines.
- Select OP-TEE client and add dependency on OP-TEE OS.
- Add option BR2_PACKAGE_OPTEE_EXAMPLES_SYNCED_VERSION to ensure
OP-TEE examples version is synced with OP-TEE OS version.
- Do not force output build directory, rely on native path: out/.
- Replace if/endif with depends on in Config.in.
- Remove useless OPTEE_EXAMPLES_INSTALL_STAGING=YES.
- Add package official URL in Config.in package description.
---
package/Config.in | 1 +
package/optee-examples/Config.in | 68 ++++++++++++++++++++++++++++++
package/optee-examples/optee-examples.hash | 4 ++
package/optee-examples/optee-examples.mk | 47 +++++++++++++++++++++
4 files changed, 120 insertions(+)
create mode 100644 package/optee-examples/Config.in
create mode 100644 package/optee-examples/optee-examples.hash
create mode 100644 package/optee-examples/optee-examples.mk
diff --git a/package/Config.in b/package/Config.in
index 38200af..35870d0 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2049,6 +2049,7 @@ menu "Security"
source "package/checkpolicy/Config.in"
source "package/optee-benchmark/Config.in"
source "package/optee-client/Config.in"
+ source "package/optee-examples/Config.in"
source "package/paxtest/Config.in"
source "package/policycoreutils/Config.in"
source "package/refpolicy/Config.in"
diff --git a/package/optee-examples/Config.in b/package/optee-examples/Config.in
new file mode 100644
index 0000000..de16246
--- /dev/null
+++ b/package/optee-examples/Config.in
@@ -0,0 +1,68 @@
+config BR2_PACKAGE_OPTEE_EXAMPLES
+ bool "Embed OP-TEE examples"
+ depends on BR2_aarch64 || BR2_ARM_CPU_ARMV7A
+ depends on BR2_TARGET_OPTEE_OS
+ select BR2_PACKAGE_OPTEE_CLIENT
+ help
+ Enable the OP-TEE examples package that brings examples of
+ implementation of OP-TEE non-secure client applications and
+ secure trusted applications. OP-TEE examples is a
+ component delivered by the OP-TEE project.
+
+ https://github.com/linaro-swg/optee_examples
+
+if BR2_PACKAGE_OPTEE_EXAMPLES
+
+choice
+ prompt "OP-TEE exmaples version"
+ default BR2_PACKAGE_OPTEE_EXAMPLES_LATEST
+ help
+ Select the version of OP-TEE exmaples you want to use
+
+config BR2_PACKAGE_OPTEE_EXAMPLES_LATEST
+ bool "sync with latest release tag"
+ help
+ Sync on latest release tag. This currently fetches the
+ latest registered release tag from the OP-TEE official
+ Git repository.
+
+config BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_GIT
+ bool "sync with a specific Git"
+ help
+ Sync with a specific OP-TEE Git repository.
+
+endchoice
+
+config BR2_PACKAGE_OPTEE_EXAMPLES_SYNCED_VERSION
+ bool "use same version ref for OP-TEE components"
+ depends on BR2_PACKAGE_OPTEE_EXAMPLES_LATEST
+ help
+ When enabled package version must match the version set for
+ OP-TEE OS and client components.
+
+config BR2_PACKAGE_OPTEE_EXAMPLES_VERSION
+ string
+ default BR2_TARGET_OPTEE_OS_VERSION \
+ if BR2_PACKAGE_OPTEE_EXAMPLES_SYNCED_VERSION
+ default "3.3.0" if BR2_PACKAGE_OPTEE_EXAMPLES_LATEST
+ default BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_REPO_VERSION \
+ if BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_GIT
+ help
+ Reference in the target Git repository to sync with.
+
+config BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_REPO_URL
+ string "Git repository site"
+ depends on BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_GIT
+ help
+ Specific location of the reference source tree Git
+ repository.
+
+config BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_REPO_VERSION
+ string "target reference to pull in the Git repository"
+ depends on BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_GIT
+ help
+ Package version reference to sync with. As source file
+ reference is a Git repository, the version reference can
+ be any Git reference as a tag or a sha1.
+
+endif #BR2_PACKAGE_OPTEE_EXAMPLES
diff --git a/package/optee-examples/optee-examples.hash b/package/optee-examples/optee-examples.hash
new file mode 100644
index 0000000..77b7466
--- /dev/null
+++ b/package/optee-examples/optee-examples.hash
@@ -0,0 +1,4 @@
+# From https://github.com/linaro-swg/optee_examples/archive/3.3.0.tar.gz
+sha256 504642edd1510562dcc213637d8869190dd581986daf938ed3e85088830e0ef9 optee-examples-3.3.0.tar.gz
+# Locally computed
+sha256 6f1ef8449cb82ae79d2155605f7985bdf0f08e7ab5007de9b4362e8bf28733b9 LICENSE
diff --git a/package/optee-examples/optee-examples.mk b/package/optee-examples/optee-examples.mk
new file mode 100644
index 0000000..08b25b2
--- /dev/null
+++ b/package/optee-examples/optee-examples.mk
@@ -0,0 +1,47 @@
+################################################################################
+#
+# optee-examples
+#
+################################################################################
+
+OPTEE_EXAMPLES_VERSION = $(call qstrip,$(BR2_PACKAGE_OPTEE_EXAMPLES_VERSION))
+OPTEE_EXAMPLES_LICENSE = BSD-2-Clause
+OPTEE_EXAMPLES_LICENSE_FILES = LICENSE
+
+ifeq ($(BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_GIT),y)
+OPTEE_EXAMPLES_SITE = $(call qstrip,$(BR2_PACKAGE_OPTEE_EXAMPLES_CUSTOM_REPO_URL))
+OPTEE_EXAMPLES_SITE_METHOD = git
+BR_NO_CHECK_HASH_FOR += $(OPTEE_EXAMPLES_SOURCE)
+else
+OPTEE_EXAMPLES_SITE = $(call github,linaro-swg,optee_examples,$(OPTEE_EXAMPLES_VERSION))
+endif
+
+OPTEE_EXAMPLES_DEPENDENCIES = optee-client optee-os
+
+ifeq ($(BR2_aarch64),y)
+OPTEE_EXAMPLES_SDK = $(STAGING_DIR)/lib/optee/export-ta_arm64
+endif
+ifeq ($(BR2_arm),y)
+OPTEE_EXAMPLES_SDK = $(STAGING_DIR)/lib/optee/export-ta_arm32
+endif
+
+define OPTEE_EXAMPLES_BUILD_TAS
+ @$(foreach f,$(wildcard $(@D)/*/ta/Makefile), \
+ $(TARGET_CONFIGURE_OPTS) \
+ $(MAKE) CROSS_COMPILE=$(TARGET_CROSS) \
+ TA_DEV_KIT_DIR=$(OPTEE_EXAMPLES_SDK) \
+ -C $(dir $f) all &&) true
+endef
+
+define OPTEE_EXAMPLES_INSTALL_TAS
+ @$(foreach f,$(wildcard $(@D)/*/ta/out/*.ta), \
+ mkdir -p $(TARGET_DIR)/lib/optee_armtz && \
+ $(INSTALL) -v -p --mode=444 \
+ --target-directory=$(TARGET_DIR)/lib/optee_armtz $f \
+ &&) true
+endef
+
+OPTEE_EXAMPLES_POST_BUILD_HOOKS += OPTEE_EXAMPLES_BUILD_TAS
+OPTEE_EXAMPLES_POST_INSTALL_TARGET_HOOKS += OPTEE_EXAMPLES_INSTALL_TAS
+
+$(eval $(cmake-package))
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Buildroot] [PATCH v2 5/5] optee-test: new package
2018-11-23 16:33 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: new package Etienne Carriere
` (2 preceding siblings ...)
2018-11-23 16:33 ` [Buildroot] [PATCH v2 4/5] optee-examples: " Etienne Carriere
@ 2018-11-23 16:33 ` Etienne Carriere
2018-12-10 21:46 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: " Thomas Petazzoni
4 siblings, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2018-11-23 16:33 UTC (permalink / raw)
To: buildroot
OP-TEE test package provide test materials as part of the OP-TEE
project helping platforms to verify their OP-TEE components
against a set of regression and performance tests.
Package is added in the BR package configuration next to the
OP-TEE client package.
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v1 -> v2:
- Replace BR2_arm with BR2_ARM_CPU_ARMV7 as OP-TEE supports only
BR2_ARM_CPU_ARMV7 architectures among the 32bit Arm machines.
- Add missing dependency on BR2_TARGET_OPTEE_OS and select
BR2_PACKAGE_OPTEE_CLIENT when enabled.
- Add option BR2_PACKAGE_OPTEE_TEST_SYNCED_VERSION to ensure
OP-TEE test version is synced with OP-TEE OS version.
- Fix official repo URL in Config.in package description.
- Remove useless OPTEE_TEST_INSTALL_STAGING=YES.
- Do not force output build directory and rely on native one: out/.
---
package/Config.in | 1 +
.../optee-test/3.3.0/0001-cmake-rely-on-C.patch | 32 +++++++++
package/optee-test/Config.in | 75 ++++++++++++++++++++++
package/optee-test/optee-test.hash | 4 ++
package/optee-test/optee-test.mk | 48 ++++++++++++++
5 files changed, 160 insertions(+)
create mode 100644 package/optee-test/3.3.0/0001-cmake-rely-on-C.patch
create mode 100644 package/optee-test/Config.in
create mode 100644 package/optee-test/optee-test.hash
create mode 100644 package/optee-test/optee-test.mk
diff --git a/package/Config.in b/package/Config.in
index 35870d0..ff53a75 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2050,6 +2050,7 @@ menu "Security"
source "package/optee-benchmark/Config.in"
source "package/optee-client/Config.in"
source "package/optee-examples/Config.in"
+ source "package/optee-test/Config.in"
source "package/paxtest/Config.in"
source "package/policycoreutils/Config.in"
source "package/refpolicy/Config.in"
diff --git a/package/optee-test/3.3.0/0001-cmake-rely-on-C.patch b/package/optee-test/3.3.0/0001-cmake-rely-on-C.patch
new file mode 100644
index 0000000..ea7b966
--- /dev/null
+++ b/package/optee-test/3.3.0/0001-cmake-rely-on-C.patch
@@ -0,0 +1,32 @@
+cmake: component rely on C support
+
+Without specifing optee_client source expects only C source file
+support cmake may attempt to look for resources as g++. When
+building with environments that do not provide such tools as when
+building from native buildroot ofr a qemu target, optee_client
+fails to build. This change ensure a minimal C support allows to
+build optee_client with cmake.
+
+Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
+Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 0290205..a3fd269 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -1,4 +1,5 @@
+ cmake_minimum_required (VERSION 3.2)
++project (optee_test C)
+
+ # Default cross compile settings
+ set (CMAKE_TOOLCHAIN_FILE CMakeToolchain.txt)
+diff --git a/ta/CMakeLists.txt b/ta/CMakeLists.txt
+index 22d7727..795237e 100644
+--- a/ta/CMakeLists.txt
++++ b/ta/CMakeLists.txt
+@@ -1,4 +1,4 @@
+-project (xtest-ta-headers)
++project (xtest-ta-headers C)
+
+ add_library(${PROJECT_NAME} INTERFACE)
+
diff --git a/package/optee-test/Config.in b/package/optee-test/Config.in
new file mode 100644
index 0000000..545db03
--- /dev/null
+++ b/package/optee-test/Config.in
@@ -0,0 +1,75 @@
+config BR2_PACKAGE_OPTEE_TEST
+ bool "optee_test"
+ depends on BR2_aarch64 || BR2_ARM_CPU_ARMV7A
+ depends on BR2_TARGET_OPTEE_OS
+ select BR2_PACKAGE_OPTEE_CLIENT
+ help
+ This build option enables OP-TEE test package from the
+ OP-TEE project. It helps platforms to verify the OP-TEE
+ installation against a set of regression and performance
+ tests.
+
+ The package generates userspace test applications and
+ data files for the Linux userland. It also generates
+ OP-TEE trusted applications embedded in the target
+ directory /lib/optee-armtz. These are loaded into the
+ secure world at runtime.
+
+ http://github.com/OP-TEE/optee_test
+
+if BR2_PACKAGE_OPTEE_TEST
+
+choice
+ prompt "OP-TEE test version"
+ default BR2_PACKAGE_OPTEE_TEST_LATEST
+ help
+ Select the version of OP-TEE test you want to use
+
+config BR2_PACKAGE_OPTEE_TEST_LATEST
+ bool "sync with latest release tag"
+ help
+ This fetches the latest registered release tag from
+ the OP-TEE test official Git repository.
+
+config BR2_PACKAGE_OPTEE_TEST_CUSTOM_GIT
+ bool "sync with a specific Git"
+ help
+ Sync with a specific OP-TEE Git repository.
+
+endchoice
+
+config BR2_PACKAGE_OPTEE_TEST_SYNCED_VERSION
+ bool "use same version ref for OP-TEE components"
+ depends on BR2_PACKAGE_OPTEE_TEST_LATEST
+ help
+ When enabled, OP-TEE examples version must match the version
+ set for the other OP-TEE components.
+
+config BR2_PACKAGE_OPTEE_TEST_VERSION
+ string
+ default BR2_TARGET_OPTEE_OS_VERSION \
+ if BR2_PACKAGE_OPTEE_TEST_SYNCED_VERSION
+ default "3.3.0" if BR2_PACKAGE_OPTEE_TEST_LATEST
+ default BR2_PACKAGE_OPTEE_TEST_CUSTOM_REPO_VERSION \
+ if BR2_PACKAGE_OPTEE_TEST_CUSTOM_GIT
+ help
+ Reference in the target Git repository to sync with.
+
+if BR2_PACKAGE_OPTEE_TEST_CUSTOM_GIT
+
+config BR2_PACKAGE_OPTEE_TEST_CUSTOM_REPO_URL
+ string "Git repository site"
+ help
+ Specific location of the reference source tree Git
+ repository.
+
+config BR2_PACKAGE_OPTEE_TEST_CUSTOM_REPO_VERSION
+ string "target reference to pull in the Git repository"
+ help
+ Package version reference to sync with. As source file
+ reference is a Git repository, the version reference can
+ be any Git reference as a tag or a sha1.
+
+endif
+
+endif #BR2_PACKAGE_OPTEE_TEST
diff --git a/package/optee-test/optee-test.hash b/package/optee-test/optee-test.hash
new file mode 100644
index 0000000..0da2212
--- /dev/null
+++ b/package/optee-test/optee-test.hash
@@ -0,0 +1,4 @@
+# From https://github.com/OP-TEE/optee_test/archive/3.3.0.tar.gz
+sha256 9651d5db0d28856e45d6bc25ce603bfcf641435bd3264d95b449f093665c8521 optee-test-3.3.0.tar.gz
+# Locally computed
+sha256 6e6810981f0ddab9e0d44399d0700a15d9f760a3c2843cc866659c2074139ae7 LICENSE.md
diff --git a/package/optee-test/optee-test.mk b/package/optee-test/optee-test.mk
new file mode 100644
index 0000000..8040ee5
--- /dev/null
+++ b/package/optee-test/optee-test.mk
@@ -0,0 +1,48 @@
+################################################################################
+#
+# optee-test
+#
+################################################################################
+
+OPTEE_TEST_VERSION = $(call qstrip,$(BR2_PACKAGE_OPTEE_TEST_VERSION))
+OPTEE_TEST_LICENSE = GPL-2.0, BSD-2-Clause,
+OPTEE_TEST_LICENSE_FILES = LICENSE.md
+
+ifeq ($(BR2_PACKAGE_OPTEE_TEST_CUSTOM_GIT),y)
+OPTEE_TEST_SITE = $(call qstrip,$(BR2_PACKAGE_OPTEE_TEST_CUSTOM_REPO_URL))
+OPTEE_TEST_SITE_METHOD = git
+BR_NO_CHECK_HASH_FOR += $(OPTEE_TEST_SOURCE)
+else
+OPTEE_TEST_SITE = $(call github,OP-TEE,optee_test,$(OPTEE_TEST_VERSION))
+endif
+
+OPTEE_TEST_DEPENDENCIES = optee-client optee-os
+
+ifeq ($(BR2_aarch64),y)
+OPTEE_TEST_SDK = $(STAGING_DIR)/lib/optee/export-ta_arm64
+endif
+ifeq ($(BR2_arm),y)
+OPTEE_TEST_SDK = $(STAGING_DIR)/lib/optee/export-ta_arm32
+endif
+OPTEE_TEST_CONF_OPTS = -DOPTEE_TEST_SDK=$(OPTEE_TEST_SDK)
+
+define OPTEE_TEST_BUILD_TAS
+ @$(foreach f,$(wildcard $(@D)/ta/*/Makefile), \
+ $(TARGET_CONFIGURE_OPTS) \
+ $(MAKE) CROSS_COMPILE=$(TARGET_CROSS) \
+ TA_DEV_KIT_DIR=$(OPTEE_TEST_SDK) \
+ -C $(dir $f) all &&) true
+endef
+
+define OPTEE_TEST_INSTALL_TAS
+ @$(foreach f,$(wildcard $(@D)/ta/*/out/*.ta), \
+ mkdir -p $(TARGET_DIR)/lib/optee_armtz && \
+ $(INSTALL) -v -p --mode=444 \
+ --target-directory=$(TARGET_DIR)/lib/optee_armtz $f \
+ &&) true
+endef
+
+OPTEE_TEST_POST_BUILD_HOOKS += OPTEE_TEST_BUILD_TAS
+OPTEE_TEST_POST_INSTALL_TARGET_HOOKS += OPTEE_TEST_INSTALL_TAS
+
+$(eval $(cmake-package))
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Buildroot] [PATCH v2 1/5] boot/optee-os: new package
2018-11-23 16:33 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: new package Etienne Carriere
` (3 preceding siblings ...)
2018-11-23 16:33 ` [Buildroot] [PATCH v2 5/5] optee-test: " Etienne Carriere
@ 2018-12-10 21:46 ` Thomas Petazzoni
2018-12-12 9:24 ` Etienne Carriere
4 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2018-12-10 21:46 UTC (permalink / raw)
To: buildroot
Hello Etienne,
Thanks for this second iteration, and thanks for submitting OPTEE to
Buildroot, this would be a very useful addition. I now took the time to
look into it, and I have a few questions.
On Fri, 23 Nov 2018 17:33:33 +0100, Etienne Carriere wrote:
> diff --git a/boot/Config.in b/boot/Config.in
> index 8e0c8e5..cd14731 100644
> --- a/boot/Config.in
> +++ b/boot/Config.in
> @@ -13,6 +13,7 @@ source "boot/gummiboot/Config.in"
> source "boot/lpc32xxcdl/Config.in"
> source "boot/mv-ddr-marvell/Config.in"
> source "boot/mxs-bootlets/Config.in"
> +source "boot/optee-os/Config.in"
> source "boot/riscv-pk/Config.in"
> source "boot/s500-bootloader/Config.in"
> source "boot/syslinux/Config.in"
> diff --git a/boot/optee-os/Config.in b/boot/optee-os/Config.in
> new file mode 100644
> index 0000000..7a598c6
> --- /dev/null
> +++ b/boot/optee-os/Config.in
> @@ -0,0 +1,100 @@
> +config BR2_TARGET_OPTEE_OS
> + bool "optee_os"
> + depends on BR2_aarch64 || BR2_ARM_CPU_ARMV7A
Shouldn't this be:
depends on BR2_ARM_CPU_ARMV8A || BR2_ARM_CPU_ARMV7A
indeed, with depends on BR2_aarch64 || BR2_ARM_CPU_ARMV7A, you don't
allow using OPTEE on a Cortex-A53/57/72 that would be used in 32-bit
mode. Is this wanted ?
> + help
> + OP-TEE OS provides the secure world boot image and the trust
> + application development kit of the OP-TEE project. OP-TEE OS
> + also provides generic trusted application one can embedded
> + into its system.
> +
> + http://github.com/OP-TEE/optee_os
> +
> +if BR2_TARGET_OPTEE_OS
> +
> +choice
> + prompt "OP-TEE OS version"
> + default BR2_TARGET_OPTEE_OS_LATEST
> + help
> + Select the version of OP-TEE OS you want to use
> +
> +config BR2_TARGET_OPTEE_OS_LATEST
> + bool "sync with latest registered release tag"
Please use:
bool "3.3.0"
so that it is similar with what we do in boot/uboot/Config.in for
example.
> + help
> + This fetches the latest registered release tag from
Don't say "latest", because it's not true: it's fetching one specific
version.
> + the OP-TEE OS official Git repository.
> +
> +config BR2_TARGET_OPTEE_OS_CUSTOM_GIT
> + bool "sync on custom OP-TEE OS Git repository"
Just:
bool "Custom Git repository"
> + help
> + Sync with a specific OP-TEE Git repository.
"Sync" is not really correct here. Actually, I think a help text is not
really needed. See what boot/uboot/Config.in is doing, and follow that
example.
> +endchoice
> +
> +config BR2_TARGET_OPTEE_OS_VERSION
> + string
> + default "3.3.0" if BR2_TARGET_OPTEE_OS_LATEST
> + default BR2_TARGET_OPTEE_OS_CUSTOM_REPO_VERSION \
> + if BR2_TARGET_OPTEE_OS_CUSTOM_GIT
Please put this option after the REPO_URL/REPO_VERSION options.
Put a:
if BR2_TARGET_OPTEE_OS_CUSTOM_GIT
here.
> +config BR2_TARGET_OPTEE_OS_CUSTOM_REPO_URL
> + string "sourcetree-site"
string "URL of custom repository"
> + depends on BR2_TARGET_OPTEE_OS_CUSTOM_GIT
Drop this.
> + help
> + Specific location of the reference source tree Git
> + repository.
> +
> +config BR2_TARGET_OPTEE_OS_CUSTOM_REPO_VERSION
> + string "git reference to pull"
string "Custom repository version"
> + depends on BR2_TARGET_OPTEE_OS_CUSTOM_GIT
And that
> + help
> + Reference in the target git repository to sync with.
Finish with an
endif
here.
> +# Building core, TA libraries/devkit and/or generic TA services
This comment is not really needed.
> +config BR2_TARGET_OPTEE_OS_CORE
> + bool "Build core"
> + default y
> + help
> + This option will build and install the OP-TEE core
> + boot images.
> +
> +config BR2_TARGET_OPTEE_OS_SDK
> + bool "Build TA devkit"
> + default y
> + help
> + This option will build and install the OP-TEE development
> + kit for building OP-TEE trusted application images. It is
> + installed in the staging filetree in /lib/optee directory.
Indentation of the last line is odd.
filetree -> directory
> +config BR2_TARGET_OPTEE_OS_SERVICES
> + bool "Build service TAs"
> + depends on BR2_TARGET_OPTEE_OS_SDK
> + default y
> + help
> + This option install the generic trusted applications built
> + from OP-TEE OS source tree. These are installed in the target
> + /lib/optee_armtz directory. At runtime OP-TEE OS can load
> + trusted applications from a non secure filesystem into the
> + secure world for execution.
> +
> +# Building TA libraries and/or core images require target platform info
This comment is also not very useful.
> diff --git a/boot/optee-os/optee-os.hash b/boot/optee-os/optee-os.hash
> new file mode 100644
> index 0000000..02828a3
> --- /dev/null
> +++ b/boot/optee-os/optee-os.hash
> @@ -0,0 +1,4 @@
> +# From https://github.com/OP-TEE/optee_os/archive/3.3.0.tar.gz
> +sha256 7b62e9fe650e197473eb2f4dc35c09d1e6395eb48dc1c16cc139d401b359ac6f optee-os-3.3.0.tar.gz
> +# Locally computed
> +sha256 fda8385993f112d7ca61b88b54ba5b4cbeec7e43a0f9b317d5186703c1985e8f LICENSE
Please put the license hash in boot/optee-os/3.3.0/optee-os.hash, so
that it applies only to the 3.3.0 version and not to custom versions.
> diff --git a/boot/optee-os/optee-os.mk b/boot/optee-os/optee-os.mk
> new file mode 100644
> index 0000000..14ad143
> --- /dev/null
> +++ b/boot/optee-os/optee-os.mk
> @@ -0,0 +1,101 @@
> +################################################################################
> +#
> +# optee-os
> +#
> +################################################################################
> +
> +OPTEE_OS_VERSION = $(call qstrip,$(BR2_TARGET_OPTEE_OS_VERSION))
> +OPTEE_OS_LICENSE = BSD-2-Clause
> +OPTEE_OS_LICENSE_FILES = LICENSE
Move the OPTEE_OS_INSTALL_STAGING = YES and OPTEE_OS_INSTALL_IMAGES =
YES here.
> +ifeq ($(BR2_TARGET_OPTEE_OS_CUSTOM_GIT),y)
> +OPTEE_OS_SITE = $(call qstrip,$(BR2_TARGET_OPTEE_OS_CUSTOM_REPO_URL))
> +OPTEE_OS_SITE_METHOD = git
> +BR_NO_CHECK_HASH_FOR += $(OPTEE_OS_SOURCE)
> +else
> +OPTEE_OS_SITE = $(call github,OP-TEE,optee_os,$(OPTEE_OS_VERSION))
> +endif
> +
> +OPTEE_OS_DEPENDENCIES = openssl host-python-pycrypto
Are you sure these are needed? I could build for arm32 without them. If
you really need openssl for the target, then the Config.in should
select BR2_PACKAGE_OPENSSL.
> +# On 64bit targets, OP-TEE OS can be built in 32bit mode, or
> +# can be built in 64bit mode and support 32bit and 64bit
> +# trusted applications. Since buildroot currently references
> +# a single cross compiler, build exclusively in 32bit
> +# or 64bit mode.
> +OPTEE_OS_MAKE_OPTS = CROSS_COMPILE="$(TARGET_CROSS)"
> +OPTEE_OS_MAKE_OPTS += CROSS_COMPILE_core="$(TARGET_CROSS)"
OPTEE_OS_MAKE_OPTS = \
CROSS_COMPILE="$(TARGET_CROSS)" \
CROSS_COMPILE_core="$(TARGET_CROSS)"
> +ifeq ($(BR2_aarch64),y)
> +OPTEE_OS_MAKE_OPTS += CROSS_COMPILE_ta_arm64="$(TARGET_CROSS)"
> +endif
> +ifeq ($(BR2_arm),y)
> +OPTEE_OS_MAKE_OPTS += CROSS_COMPILE_ta_arm32="$(TARGET_CROSS)"
> +endif
> +
> +# Get mandatory PLAFORM and optional PLATFORM_FLAVOR
> +OPTEE_OS_MAKE_OPTS += PLATFORM=$(call qstrip,$(BR2_TARGET_OPTEE_OS_PLATFORM))
> +ifneq ($(BR2_TARGET_OPTEE_OS_PLATFORM_FLAVOR),)
> +OPTEE_OS_MAKE_OPTS += PLATFORM_FLAVOR=$(call qstrip,$(BR2_TARGET_OPTEE_OS_PLATFORM_FLAVOR))
> +endif
> +OPTEE_OS_MAKE_OPTS += $(call qstrip,$(BR2_TARGET_OPTEE_OS_ADDITIONAL_VARIABLES))
> +
> +# Requests OP-TEE OS to build from subdirectory out/ of its synced sourcetree root path
> +# otherwise the output directory path depends on the target platform name.
> +OPTEE_OS_BUILDDIR_OUT = out
> +
> +ifeq ($(BR2_aarch64),y)
> +OPTEE_OS_LOCAL_SDK = $(OPTEE_OS_BUILDDIR_OUT)/export-ta_arm64
> +endif
> +ifeq ($(BR2_arm),y)
> +OPTEE_OS_LOCAL_SDK = $(OPTEE_OS_BUILDDIR_OUT)/export-ta_arm32
> +endif
> +
> +ifeq ($(BR2_TARGET_OPTEE_OS_CORE),y)
> +define OPTEE_OS_BUILD_CORE
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) O=$(OPTEE_OS_BUILDDIR_OUT) \
> + $(TARGET_CONFIGURE_OPTS) $(OPTEE_OS_MAKE_OPTS) all
> +endef
> +define OPTEE_OS_INSTALL_CORE
This should be:
define OPTEE_OS_INSTALL_IMAGES_CMDS
> + mkdir -p $(BINARIES_DIR)
> + cp -dpf $(@D)/$(OPTEE_OS_BUILDDIR_OUT)/core/tee.bin $(BINARIES_DIR)
> + cp -dpf $(@D)/$(OPTEE_OS_BUILDDIR_OUT)/core/tee-*_v2.bin $(BINARIES_DIR)
> +endef
> +endif
> +
> +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_SDK
This should be:
define OPTEE_OS_INSTALL_STAGING_CMDS
> + mkdir -p $(STAGING_DIR)/lib/optee
> + cp -ardpf $(@D)/$(OPTEE_OS_LOCAL_SDK) $(STAGING_DIR)/lib/optee
> +endef
> +endif
> +
> +ifeq ($(BR2_TARGET_OPTEE_OS_SERVICES),y)
> +# Core build already generates the TA services binaries. Install them.
Is it the "core" that builds the TA services binaries? According to
your Config.in dependencies, you can install the TA services binaries
without building the Core, so it's not very consistent.
Also, in my testing, building the zynq7k-zc702 platform, it never
installed anything:
>>> optee-os 3.3.0 Installing to target
mkdir -p /home/thomas/projets/buildroot/output/target/lib/optee_armtz
true
> +define OPTEE_OS_INSTALL_SERVICES
This should be:
define OPTEE_OS_INSTALL_TARGET_CMDS
> + mkdir -p $(TARGET_DIR)/lib/optee_armtz
> + $(foreach f,$(wildcard $(@D)/ta/*/$(OPTEE_OS_BUILDDIR_OUT)/*.ta), \
> + $(INSTALL) -v -p --mode=444 \
> + --target-directory=$(TARGET_DIR)/lib/optee_armtz \
> + $f &&) true
This seems more complicated that it needs to be. You could simplify this
entire block this way:
$(INSTALL) -D -m 444 -t $(TARGET_DIR)/lib/optee_armtz $(@D)/ta/*/$(OPTEE_OS_BUILDDIR_OUT)/*.ta
or if you really want to use a loop:
$(foreach f,$(wildcard $(@D)/ta/*/$(OPTEE_OS_BUILDDIR_OUT)/*.ta), \
$(INSTALL) -D -m 444 $(f) $(TARGET_DIR)/lib/optee_armtz/$(notdir $(f))
)
> +define OPTEE_OS_BUILD_CMDS
> + $(OPTEE_OS_BUILD_CORE)
> + $(OPTEE_OS_BUILD_SDK)
> +endef
> +
> +define OPTEE_OS_INSTALL_IMAGES_CMDS
> + $(OPTEE_OS_INSTALL_CORE)
> + $(OPTEE_OS_INSTALL_SDK)
> + $(OPTEE_OS_INSTALL_SERVICES)
So, what is wrong here is to install everything within
INSTALL_IMAGES_CMDS. That's why above, I suggest to use
INSTALL_IMAGES_CMDS to install the core, INSTALL_STAGING_CMDS to
install the SDK and INSTALL_TARGET_CMDS to install the services.
> +endef
> +
> +OPTEE_OS_INSTALL_STAGING = YES
> +OPTEE_OS_INSTALL_IMAGES = YES
As explained, this should move earlier in the file.
> +$(eval $(generic-package))
So, with the changes described above, I could build for
PLATFORM=zynq7k-zc702 (with the issue that no services are installed).
However, on ARM64 with PLATFORM=marvell-armada7k8k, it fails to build
entirely. It tries to pass ARM32 gcc flags to an ARM64 compiler.
Defconfig:
BR2_aarch64=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
# BR2_TARGET_ROOTFS_TAR is not set
BR2_TARGET_OPTEE_OS=y
BR2_TARGET_OPTEE_OS_PLATFORM="marvell-armada7k8k"
Log:
CC out/ta_arm32-lib/libmbedtls/mbedtls/library/aesni.o
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
CC out/ta_arm32-lib/libmbedtls/mbedtls/library/arc4.o
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
CC out/ta_arm32-lib/libmbedtls/mbedtls/library/asn1parse.o
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/aes.o] Error 1
make[2]: *** Waiting for unfinished jobs....
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/arc4.o] Error 1
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/aesni.o] Error 1
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/asn1parse.o] Error 1
Could you have a look at solving this issue and taking into account the
above comments for a v3 ?
Last, but not least, we would really need to have a test case for this
in the support/testing/ infrastructure. At least one test for an ARM32
platform and one test for an ARM64 platform. The minimal test would be
to just do a build. A better test would use PLATFORM=vexpress-qemu_virt
and PLATFORM=vexpress-qemu_armv8a and do some runtime testing.
Best regards,
Thomas Petazzoni
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread* [Buildroot] [PATCH v2 1/5] boot/optee-os: new package
2018-12-10 21:46 ` [Buildroot] [PATCH v2 1/5] boot/optee-os: " Thomas Petazzoni
@ 2018-12-12 9:24 ` Etienne Carriere
0 siblings, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2018-12-12 9:24 UTC (permalink / raw)
To: buildroot
On Mon, 10 Dec 2018 at 22:46, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Etienne,
>
> Thanks for this second iteration, and thanks for submitting OPTEE to
> Buildroot, this would be a very useful addition. I now took the time to
> look into it, and I have a few questions.
>
Hello Thomas,
Thanks for spending time on this.
To sump up, I agree with most comments and will push a v3 to fix the issues.
Thanks for all the suggestions and directives on BR files style and ways.
Please find some answers below, the main issue I think is the 32b/64b
dual support.
And also a short note on test configs.
regards,
etienne
> On Fri, 23 Nov 2018 17:33:33 +0100, Etienne Carriere wrote:
>
> > diff --git a/boot/Config.in b/boot/Config.in
> > index 8e0c8e5..cd14731 100644
> > --- a/boot/Config.in
> > +++ b/boot/Config.in
> > @@ -13,6 +13,7 @@ source "boot/gummiboot/Config.in"
> > source "boot/lpc32xxcdl/Config.in"
> > source "boot/mv-ddr-marvell/Config.in"
> > source "boot/mxs-bootlets/Config.in"
> > +source "boot/optee-os/Config.in"
> > source "boot/riscv-pk/Config.in"
> > source "boot/s500-bootloader/Config.in"
> > source "boot/syslinux/Config.in"
> > diff --git a/boot/optee-os/Config.in b/boot/optee-os/Config.in
> > new file mode 100644
> > index 0000000..7a598c6
> > --- /dev/null
> > +++ b/boot/optee-os/Config.in
> > @@ -0,0 +1,100 @@
> > +config BR2_TARGET_OPTEE_OS
> > + bool "optee_os"
> > + depends on BR2_aarch64 || BR2_ARM_CPU_ARMV7A
>
> Shouldn't this be:
>
> depends on BR2_ARM_CPU_ARMV8A || BR2_ARM_CPU_ARMV7A
>
> indeed, with depends on BR2_aarch64 || BR2_ARM_CPU_ARMV7A, you don't
> allow using OPTEE on a Cortex-A53/57/72 that would be used in 32-bit
> mode. Is this wanted ?
Oh, thanks! you're absolutely right. Any Armv8-A should be able to run OP-TEE.
>
> > + help
> > + OP-TEE OS provides the secure world boot image and the trust
> > + application development kit of the OP-TEE project. OP-TEE OS
> > + also provides generic trusted application one can embedded
> > + into its system.
> > +
> > + http://github.com/OP-TEE/optee_os
> > +
> > +if BR2_TARGET_OPTEE_OS
> > +
> > +choice
> > + prompt "OP-TEE OS version"
> > + default BR2_TARGET_OPTEE_OS_LATEST
> > + help
> > + Select the version of OP-TEE OS you want to use
> > +
> > +config BR2_TARGET_OPTEE_OS_LATEST
> > + bool "sync with latest registered release tag"
>
> Please use:
>
> bool "3.3.0"
>
> so that it is similar with what we do in boot/uboot/Config.in for
> example.
Ok, i'll check that.
>
> > + help
> > + This fetches the latest registered release tag from
>
> Don't say "latest", because it's not true: it's fetching one specific
> version.
Agree, this is why I stated "latest *registered* release tag".
I will change to "This fetches the target release tag from..."
>
> > + the OP-TEE OS official Git repository.
> > +
> > +config BR2_TARGET_OPTEE_OS_CUSTOM_GIT
> > + bool "sync on custom OP-TEE OS Git repository"
>
> Just:
>
> bool "Custom Git repository"
>
> > + help
> > + Sync with a specific OP-TEE Git repository.
>
> "Sync" is not really correct here. Actually, I think a help text is not
> really needed. See what boot/uboot/Config.in is doing, and follow that
> example.
Ok, thanks. simpler is better.
>
> > +endchoice
> > +
> > +config BR2_TARGET_OPTEE_OS_VERSION
> > + string
> > + default "3.3.0" if BR2_TARGET_OPTEE_OS_LATEST
> > + default BR2_TARGET_OPTEE_OS_CUSTOM_REPO_VERSION \
> > + if BR2_TARGET_OPTEE_OS_CUSTOM_GIT
>
> Please put this option after the REPO_URL/REPO_VERSION options.
>
>
> Put a:
>
> if BR2_TARGET_OPTEE_OS_CUSTOM_GIT
>
> here.
>
> > +config BR2_TARGET_OPTEE_OS_CUSTOM_REPO_URL
> > + string "sourcetree-site"
>
> string "URL of custom repository"
>
> > + depends on BR2_TARGET_OPTEE_OS_CUSTOM_GIT
>
> Drop this.
Ok, so you prefer this way:
#if BR2_FOO_xxx
config BR2_FOO_yyy
(...)
#endif
rather than:
config BR2_FOO_yyy
depends on BR2_FOO_xxx
(...)
I will do. Is there any reason why BR prefers the former vs the later?
>
> > + help
> > + Specific location of the reference source tree Git
> > + repository.
> > +
> > +config BR2_TARGET_OPTEE_OS_CUSTOM_REPO_VERSION
> > + string "git reference to pull"
>
> string "Custom repository version"
>
> > + depends on BR2_TARGET_OPTEE_OS_CUSTOM_GIT
>
> And that
>
> > + help
> > + Reference in the target git repository to sync with.
>
> Finish with an
>
> endif
>
> here.
>
> > +# Building core, TA libraries/devkit and/or generic TA services
>
> This comment is not really needed.
>
> > +config BR2_TARGET_OPTEE_OS_CORE
> > + bool "Build core"
> > + default y
> > + help
> > + This option will build and install the OP-TEE core
> > + boot images.
> > +
> > +config BR2_TARGET_OPTEE_OS_SDK
> > + bool "Build TA devkit"
> > + default y
> > + help
> > + This option will build and install the OP-TEE development
> > + kit for building OP-TEE trusted application images. It is
> > + installed in the staging filetree in /lib/optee directory.
>
> Indentation of the last line is odd.
>
> filetree -> directory
>
> > +config BR2_TARGET_OPTEE_OS_SERVICES
> > + bool "Build service TAs"
> > + depends on BR2_TARGET_OPTEE_OS_SDK
> > + default y
> > + help
> > + This option install the generic trusted applications built
> > + from OP-TEE OS source tree. These are installed in the target
> > + /lib/optee_armtz directory. At runtime OP-TEE OS can load
> > + trusted applications from a non secure filesystem into the
> > + secure world for execution.
> > +
> > +# Building TA libraries and/or core images require target platform info
>
> This comment is also not very useful.
>
> > diff --git a/boot/optee-os/optee-os.hash b/boot/optee-os/optee-os.hash
> > new file mode 100644
> > index 0000000..02828a3
> > --- /dev/null
> > +++ b/boot/optee-os/optee-os.hash
> > @@ -0,0 +1,4 @@
> > +# From https://github.com/OP-TEE/optee_os/archive/3.3.0.tar.gz
> > +sha256 7b62e9fe650e197473eb2f4dc35c09d1e6395eb48dc1c16cc139d401b359ac6f optee-os-3.3.0.tar.gz
> > +# Locally computed
> > +sha256 fda8385993f112d7ca61b88b54ba5b4cbeec7e43a0f9b317d5186703c1985e8f LICENSE
>
> Please put the license hash in boot/optee-os/3.3.0/optee-os.hash, so
> that it applies only to the 3.3.0 version and not to custom versions.
Ok, I get your point. Thanks.
>
> > diff --git a/boot/optee-os/optee-os.mk b/boot/optee-os/optee-os.mk
> > new file mode 100644
> > index 0000000..14ad143
> > --- /dev/null
> > +++ b/boot/optee-os/optee-os.mk
> > @@ -0,0 +1,101 @@
> > +################################################################################
> > +#
> > +# optee-os
> > +#
> > +################################################################################
> > +
> > +OPTEE_OS_VERSION = $(call qstrip,$(BR2_TARGET_OPTEE_OS_VERSION))
> > +OPTEE_OS_LICENSE = BSD-2-Clause
> > +OPTEE_OS_LICENSE_FILES = LICENSE
>
> Move the OPTEE_OS_INSTALL_STAGING = YES and OPTEE_OS_INSTALL_IMAGES =
> YES here.
>
> > +ifeq ($(BR2_TARGET_OPTEE_OS_CUSTOM_GIT),y)
> > +OPTEE_OS_SITE = $(call qstrip,$(BR2_TARGET_OPTEE_OS_CUSTOM_REPO_URL))
> > +OPTEE_OS_SITE_METHOD = git
> > +BR_NO_CHECK_HASH_FOR += $(OPTEE_OS_SOURCE)
> > +else
> > +OPTEE_OS_SITE = $(call github,OP-TEE,optee_os,$(OPTEE_OS_VERSION))
> > +endif
> > +
> > +OPTEE_OS_DEPENDENCIES = openssl host-python-pycrypto
>
> Are you sure these are needed? I could build for arm32 without them. If
> you really need openssl for the target, then the Config.in should
> select BR2_PACKAGE_OPENSSL.
I see, my mistake. I had to set "OPTEE_OS_DEPENDENCIES = host-openssl"
but I forgot the "host-" prefix.
I will fix. thanks.
>
> > +# On 64bit targets, OP-TEE OS can be built in 32bit mode, or
> > +# can be built in 64bit mode and support 32bit and 64bit
> > +# trusted applications. Since buildroot currently references
> > +# a single cross compiler, build exclusively in 32bit
> > +# or 64bit mode.
> > +OPTEE_OS_MAKE_OPTS = CROSS_COMPILE="$(TARGET_CROSS)"
> > +OPTEE_OS_MAKE_OPTS += CROSS_COMPILE_core="$(TARGET_CROSS)"
>
> OPTEE_OS_MAKE_OPTS = \
> CROSS_COMPILE="$(TARGET_CROSS)" \
> CROSS_COMPILE_core="$(TARGET_CROSS)"
>
> > +ifeq ($(BR2_aarch64),y)
> > +OPTEE_OS_MAKE_OPTS += CROSS_COMPILE_ta_arm64="$(TARGET_CROSS)"
> > +endif
> > +ifeq ($(BR2_arm),y)
> > +OPTEE_OS_MAKE_OPTS += CROSS_COMPILE_ta_arm32="$(TARGET_CROSS)"
> > +endif
> > +
> > +# Get mandatory PLAFORM and optional PLATFORM_FLAVOR
> > +OPTEE_OS_MAKE_OPTS += PLATFORM=$(call qstrip,$(BR2_TARGET_OPTEE_OS_PLATFORM))
> > +ifneq ($(BR2_TARGET_OPTEE_OS_PLATFORM_FLAVOR),)
> > +OPTEE_OS_MAKE_OPTS += PLATFORM_FLAVOR=$(call qstrip,$(BR2_TARGET_OPTEE_OS_PLATFORM_FLAVOR))
> > +endif
> > +OPTEE_OS_MAKE_OPTS += $(call qstrip,$(BR2_TARGET_OPTEE_OS_ADDITIONAL_VARIABLES))
> > +
> > +# Requests OP-TEE OS to build from subdirectory out/ of its synced sourcetree root path
> > +# otherwise the output directory path depends on the target platform name.
> > +OPTEE_OS_BUILDDIR_OUT = out
> > +
> > +ifeq ($(BR2_aarch64),y)
> > +OPTEE_OS_LOCAL_SDK = $(OPTEE_OS_BUILDDIR_OUT)/export-ta_arm64
> > +endif
> > +ifeq ($(BR2_arm),y)
> > +OPTEE_OS_LOCAL_SDK = $(OPTEE_OS_BUILDDIR_OUT)/export-ta_arm32
> > +endif
> > +
> > +ifeq ($(BR2_TARGET_OPTEE_OS_CORE),y)
> > +define OPTEE_OS_BUILD_CORE
> > + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) O=$(OPTEE_OS_BUILDDIR_OUT) \
> > + $(TARGET_CONFIGURE_OPTS) $(OPTEE_OS_MAKE_OPTS) all
> > +endef
> > +define OPTEE_OS_INSTALL_CORE
>
> This should be:
>
> define OPTEE_OS_INSTALL_IMAGES_CMDS
>
> > + mkdir -p $(BINARIES_DIR)
> > + cp -dpf $(@D)/$(OPTEE_OS_BUILDDIR_OUT)/core/tee.bin $(BINARIES_DIR)
> > + cp -dpf $(@D)/$(OPTEE_OS_BUILDDIR_OUT)/core/tee-*_v2.bin $(BINARIES_DIR)
> > +endef
> > +endif
> > +
> > +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_SDK
>
> This should be:
>
> define OPTEE_OS_INSTALL_STAGING_CMDS
Ok, i will split install to target FS and install to staging.
>
> > + mkdir -p $(STAGING_DIR)/lib/optee
> > + cp -ardpf $(@D)/$(OPTEE_OS_LOCAL_SDK) $(STAGING_DIR)/lib/optee
> > +endef
> > +endif
> > +
> > +ifeq ($(BR2_TARGET_OPTEE_OS_SERVICES),y)
> > +# Core build already generates the TA services binaries. Install them.
>
> Is it the "core" that builds the TA services binaries? According to
> your Config.in dependencies, you can install the TA services binaries
> without building the Core, so it's not very consistent.
>
> Also, in my testing, building the zynq7k-zc702 platform, it never
> installed anything:
>
> >>> optee-os 3.3.0 Installing to target
> mkdir -p /home/thomas/projets/buildroot/output/target/lib/optee_armtz
> true
I will check that!
Since optee tag 3.3.0 at least 1 TA shall be build (from ta/avb/) and
hence installed.
>
> > +define OPTEE_OS_INSTALL_SERVICES
>
> This should be:
>
> define OPTEE_OS_INSTALL_TARGET_CMDS
>
> > + mkdir -p $(TARGET_DIR)/lib/optee_armtz
> > + $(foreach f,$(wildcard $(@D)/ta/*/$(OPTEE_OS_BUILDDIR_OUT)/*.ta), \
> > + $(INSTALL) -v -p --mode=444 \
> > + --target-directory=$(TARGET_DIR)/lib/optee_armtz \
> > + $f &&) true
>
> This seems more complicated that it needs to be. You could simplify this
> entire block this way:
>
> $(INSTALL) -D -m 444 -t $(TARGET_DIR)/lib/optee_armtz $(@D)/ta/*/$(OPTEE_OS_BUILDDIR_OUT)/*.ta
>
> or if you really want to use a loop:
>
> $(foreach f,$(wildcard $(@D)/ta/*/$(OPTEE_OS_BUILDDIR_OUT)/*.ta), \
> $(INSTALL) -D -m 444 $(f) $(TARGET_DIR)/lib/optee_armtz/$(notdir $(f))
> )
>
> > +define OPTEE_OS_BUILD_CMDS
> > + $(OPTEE_OS_BUILD_CORE)
> > + $(OPTEE_OS_BUILD_SDK)
> > +endef
> > +
> > +define OPTEE_OS_INSTALL_IMAGES_CMDS
> > + $(OPTEE_OS_INSTALL_CORE)
> > + $(OPTEE_OS_INSTALL_SDK)
> > + $(OPTEE_OS_INSTALL_SERVICES)
>
> So, what is wrong here is to install everything within
> INSTALL_IMAGES_CMDS. That's why above, I suggest to use
> INSTALL_IMAGES_CMDS to install the core, INSTALL_STAGING_CMDS to
> install the SDK and INSTALL_TARGET_CMDS to install the services.
Caught :)
>
> > +endef
> > +
> > +OPTEE_OS_INSTALL_STAGING = YES
> > +OPTEE_OS_INSTALL_IMAGES = YES
>
> As explained, this should move earlier in the file.
>
> > +$(eval $(generic-package))
>
> So, with the changes described above, I could build for
> PLATFORM=zynq7k-zc702 (with the issue that no services are installed).
>
> However, on ARM64 with PLATFORM=marvell-armada7k8k, it fails to build
> entirely. It tries to pass ARM32 gcc flags to an ARM64 compiler.
>
> Defconfig:
>
> BR2_aarch64=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y
> BR2_INIT_NONE=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> # BR2_TARGET_ROOTFS_TAR is not set
> BR2_TARGET_OPTEE_OS=y
> BR2_TARGET_OPTEE_OS_PLATFORM="marvell-armada7k8k"
>
> Log:
>
> CC out/ta_arm32-lib/libmbedtls/mbedtls/library/aesni.o
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
> CC out/ta_arm32-lib/libmbedtls/mbedtls/library/arc4.o
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
> CC out/ta_arm32-lib/libmbedtls/mbedtls/library/asn1parse.o
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
> make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/aes.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
> make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/arc4.o] Error 1
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb?
> make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/aesni.o] Error 1
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mthumb-interwork?
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mno-unaligned-access?; did you mean ?-Wno-aligned-new??
> aarch64-linux-gnu-gcc: error: unrecognized command line option ?-mfloat-abi=hard?
> make[2]: *** [mk/compile.mk:146: out/ta_arm32-lib/libmbedtls/mbedtls/library/asn1parse.o] Error 1
>
> Could you have a look at solving this issue and taking into account the
> above comments for a v3 ?
Ok, I see the issue here. Indeed I didn't address it.
Optee_os as a 64bit system is able to run 32bit and 64bit TAs
(userland applications).
64bit TA support (libs and service TAs) can be built using the same
toolchain as the OS.
But the 32bit TAs and TA libs need a toolchain for Aarch32.
As BR supports a single toolchain, I guess the BR should build 64bit
TA support only on 64bit archi.
I'll see how to do that. Maybe this needs some hacking in the optee_os
source tree.
I will see how get this solved.
An alternate way would be to select a 2nd toolchain. From BR? outside BR?
I fear this is a bit complex and needs some discussions.
>
> Last, but not least, we would really need to have a test case for this
> in the support/testing/ infrastructure. At least one test for an ARM32
> platform and one test for an ARM64 platform. The minimal test would be
> to just do a build. A better test would use PLATFORM=vexpress-qemu_virt
> and PLATFORM=vexpress-qemu_armv8a and do some runtime testing.
I agree. I have prepared something for that but due to dependency
issues I am waiting for this OP-TEEs to land before proposing a board.
1st step: introduce OP-TEE components (this series)
2nd step: change BR arm-trusted-firmware integration to allow it to boot OP-TEE.
3rd step: introduce a Qemu Arm board for the setup.
(an overview of this can be seens from
https://github.com/etienne-lms/buildroot/pull/3)
The vexpress-qemu_virt is working fine but that graphics are down.
To test the vexpress-qemu_armv8a, we need a compliant boot scheme.
I must spend a bit of time to setup a scheme using BR resources.
A way would be to have ATF to boot OP-TEE and either Linux or a U-boot
that boots Linux.
The OP-TEE project proposes a scheme using UEFI as Linux kernel. It
could be build from BR too.
> Best regards,
>
> Thomas Petazzoni
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread