[RFC] hello_world: add demo code for using custom ree service by divneil · Pull Request #63 · linaro-swg/optee_examples

divneil · 2019-09-12T09:48:45Z

Signed-off-by: Divneil Rai Wadhawan divneil.r.wadhawan@intel.com

divneil · 2019-09-12T09:52:13Z

This one is just POC. I did this just to test dynamic library/message queue calls are working okay for my use-case.
optee_os PR: OP-TEE/optee_os#3270
optee_client PR: OP-TEE/optee_client#170
Ticket: OP-TEE/optee_os#3266

jockebech · 2019-09-12T09:54:15Z

 		return TEE_ERROR_BAD_PARAMETERS;

-	IMSG("Got value: %u from NW", params[0].value.a);
+	EMSG("Got value: %u from NW", params[0].value.a);


I think it still should be en I[informal] message and not an E[rror] message here and my other places in this function. Error should only be used when it's indeed an error.

I will fix the optee-examples fully before you can review in more detail. It was just POC intended for giving a view how to use the TEE -> REE service.

jockebech · 2019-09-12T09:55:33Z

+	TEE_UUID ree_uuid = TA_HELLO_WORLD_REE_UUID;
+	TEE_Result result = TEE_SUCCESS;
+	TEE_REESessionHandle ree_sess = NULL;
+	uint32_t ret_origin = 0, paramTypes;


Please put paramTypes on its own line (and initialize the variable).

jockebech · 2019-09-12T09:56:12Z

+	result = TEE_OpenREESession(&ree_uuid, 0, 0, NULL,
+					&ree_sess, &ret_origin);
+	if (result != TEE_SUCCESS) {
+		DMSG("Failed to open up REE Session\n");


This is indeed an error, so here you should use EMSG.

jockebech · 2019-09-12T09:56:34Z

+	result = TEE_InvokeREECommand(ree_sess, 0, HELLO_WORLD_MSG,
+					paramTypes, ree_params, &ret_origin);
+	if (result != TEE_SUCCESS) {
+		DMSG("Failed to invoke REE command\n");


s/DMSG/EMSG/

jockebech · 2019-09-12T09:57:49Z

+	TEE_REESessionHandle ree_sess = NULL;
+	uint32_t ret_origin = 0, paramTypes;
+	char msg[] = "Hello! from TEE";
+	TEE_Param ree_params[4];


Nothing wrong as such, but I think the general convention seems to be to just simply call this "params"

jockebech · 2019-09-12T10:02:52Z

+	void *service;
+	REEC_UUID ree_uuid = TA_HELLO_WORLD_REE_UUID;
+
+#if 0


I will refactor the code, so, as to clearly demonstrate the functionality. I think you may have already figured out, that there are 2 methods to avail REE custom service.
o msgq
o dynlib

So, for my testing purposes, I was enabling either 1 of them.

On a side note, I need help in putting the generated dynlib into filesystem tarball which is generated in the end. I didn't spend enough time in the build system, as a quick hack (from out-br folder) was available to copy dynlib to the running board.

What's dynlib? Is msgq Posix Message Queues?

dynlib -> dynamic library (.so).
msgq -> Yes Posix Message Queue.

jockebech · 2019-09-12T10:03:51Z

 	TEEC_CloseSession(&sess);

 	TEEC_FinalizeContext(&ctx);
+#if 0


Ditto? If this is something we don't want to use by default, then I think it's better to add a compile time flag.

I will create separate flows for message queues and dynamic lib, so, both will be enabled by default.

jockebech · 2019-09-12T10:07:48Z

Another suggestion here would be to create a new folder dynlib (or some other clever name) and put all the code there as a brand new host and TA example. The plain and simple existing hello_world is quite handy for new comers, since that shows the least amount of code needed to to a call to the TEE.

Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com>

divneil · 2019-09-13T09:54:03Z

Another suggestion here would be to create a new folder dynlib (or some other clever name) and

I thought of creating a new folder ta2ca which will extend hello_world. To copy the dynamic library to file system, can you please share which folder I need to use. Considering the process I need to follow to share the code, it will take some time.

jockebech · 2019-09-13T10:08:33Z

To copy the dynamic library to file system, can you please share which folder I need to use

I'm not too picky about that, try to come up with a name of a folder that has something to do with the example host/ta that you're trying to implement. You can see the rationale behind the ones we already have here: https://optee.readthedocs.io/en/latest/building/gits/optee_examples/optee_examples.html#example-applications

But the concept is the same in all for them.

./aes
./aes/ta
./aes/host
./hello_world
./hello_world/ta
./hello_world/host
....

If you need to add other code/libraries to fulfill the implementation of your example, then put it under "your" example folder. I've said a lot, did I manage to answer your question? :)

Btw, when you're code has been merged, I'd kindly ask you to also submit a documentation update for things like this.

divneil · 2019-09-13T11:18:12Z

I'm not too picky about that, try to come up with a name of a folder that has something to do with

Honestly, I am unable to come up with a small catchy name. I will try with ta2ca (Trusted Application to Client Application), but feel free to suggest any other name.

Hmm, I will rephrase the question. Earlier, I was pretty abstract.
Once, we build the distro, we get rootfs.cpio.gz. I am unable to make .so go into this cpio from host folder.

So, if I need push a patch in let's say build folder (or any folder other than optee_os/optee_client/optee_examples) to fix this, I will need an approval from my organization, so, wanted that process to start in parallel. So, can you please help with the git I need to target.

Btw, when you're code has been merged, I'd kindly ask you to also submit a documentation update for things like this.

Definitely.

- Supporting both message receiving: msgq and dll - Reverted changes done in hello_world Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com>

o one artifact remained in hello_world. Deleted it. Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com>

etienne-lms

some minor comment

etienne-lms · 2019-09-23T15:26:01Z

+ *
+ *   - Dynamic Library     : The library needs to be developed with a specific
+ *                           function signature and compiled as .so and placed
+ *                           in a PATH (/lib64) where tee-supplicant can find


suggest to replace /lib64 with i.e. /lib64 or even /lib.

etienne-lms · 2019-09-23T15:26:52Z

+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <pthread.h>


minor: sort in alpha ordering

etienne-lms · 2019-09-23T15:36:17Z

@@ -0,0 +1,13 @@
+CFG_TEE_TA_LOG_LEVEL ?= 4


minor: maybe 2 or 3 as default trace level.

etienne-lms · 2019-09-23T15:39:58Z

+	ree_session_handle ree_sess = NULL;
+	TEE_Result result = TEE_SUCCESS;
+	TEE_Param ree_params[4] = {0};
+	uint32_t param_types;


prefer always initialize local variables where defined, here add = 0.

etienne-lms · 2019-09-23T15:42:16Z

+	 * to be careful while reading the string data and not read
+	 * more than the buffer allocated. This is just for reference.
+	 */
+	EMSG("REE filled buffer: %s\n", (char *)ree_params[2].memref.buffer);


As this is not an effective error, prefer MSG().

Regarding comment above, I think this print should also implement required checks: buffer size still <= 64 and contains a terminal \0.

etienne-lms · 2019-09-23T15:50:27Z

+ */
+
+#ifndef __TA_HELLO_REE_H__
+#define __TA_HELLO_REE_H__


__HELLO_REE_TA_H__ as per filename.

divneil changed the title ~~hello_world: add demo code for using custom ree service~~ [RFC] hello_world: add demo code for using custom ree service Sep 12, 2019

jockebech reviewed Sep 12, 2019

View reviewed changes

hello_world: add demo code for using custom ree service

588d8f0

Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com>

divneil mentioned this pull request Sep 13, 2019

[RFC] tee-supplicant: add support for accessing custom REE service OP-TEE/optee_client#170

Closed

divneil-intel added 2 commits September 20, 2019 01:09

hello_ree: created a new fork from hello_world

563943f

- Supporting both message receiving: msgq and dll - Reverted changes done in hello_world Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com>

hello_ree: cleaned up the hello_world file

89ec043

o one artifact remained in hello_world. Deleted it. Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@intel.com>

etienne-lms reviewed Sep 23, 2019

View reviewed changes

divneil closed this Oct 23, 2019

Uh oh!

Conversation

divneil commented Sep 12, 2019

Uh oh!

divneil commented Sep 12, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jockebech commented Sep 12, 2019

Uh oh!

divneil commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jockebech commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

divneil commented Sep 13, 2019

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

divneil commented Sep 13, 2019 •

edited

Loading

jockebech commented Sep 13, 2019 •

edited

Loading