libteec: implement OCALLs #171

HernanGatta · 2019-09-23T08:51:37Z

Overview

This PR implements OCALLs in the TEE Client API.

Related PRs: OP-TEE, Linux, OP-TEE Examples.

Session Open

A new function is added, TEEC_OpenSessionEx, whose signature is identical to TEEC_OpenSession, but takes two additional parameters. The first is an array of typed settings, while the second provides the number of settings in the array.

One such setting is the OCALL setting, which allows the developer to specify a callback for whenever an OCALL arrives, as well as an arbitrary context pointer.

The rationale behind using session settings is that it is conceivable that in the future, additional features are added that require configuration. Instead of creating new, independent functions to deal with those new features, a new setting type can be added and the existing API set remains the same. The Open Enclave SDK implements a similar pattern for its oe_create_enclave function.

When the OCALL setting is specified in TEEC_OpenSessionEx, it checks that the underlying context reports that OP-TEE supports OCALLs. If so, it opens a session in the usual way via TEEC_OpenSession, then stores the OCALL configuration in the session object.

Sample usage:

TEEC_SessionSettingOcall ocall_setting = { my_ocall_handler, &my_context };
TEEC_SessionSetting settings[] = {
        { .type = TEEC_SESSION_SETTING_OCALL, .u.ocall = &ocall_setting }
};

res = TEEC_OpenSessionEx(&ctx, &sess, &uuid, TEEC_LOGIN_PUBLIC, NULL,
                         NULL, &err_origin, settings, 1);

OCALL Handling

When OCALL support is configured, calls to TEEC_InvokeFunction are rerouted via the ECALL IOCTL instead of the INVOKE IOCTL. Around the IOCTL invocation, a loop exists to catch early returns, indicating that an OCALL request arrived. When that happens, the Client API takes care of dealing with shared memory allocation and free requests, as well as of parameter processing before and after dispatching an OCALL to the OCALL handler.

Signed-off-by: hegatta@microsoft.com

etienne-lms

As a general comment, looks a fine way to open a session bound to a remote service provider.
I can't tell if it is the best architecture for customized RPC support.`

For the patch series i felt a bit disoriented by ocall, ecall, Ex session references.
What about renaming TEEC_OpenSessionEx() into TEEC_OpenSessionWithRPC()?
Same, teec_ecall() could be renamed into teec_invoke_command_with_rpc().

As for the patch series, could you make a series like:

Preliminary patch for moving the legacy into new teec_invoke_command().
Patch for TEEC_OpenSessionEx() and OCALL related.
Patch implementing teec_ecall() / teec_invoke_ca_rpc_command().
Last patch for TEE capability TEE_OPTEE_CAP_OCALL.
Your commit "Detect OCALL support." nicely fits. Maybe rename into TEE_OPTEE_CAP_CA_RPC.

I noted some minor style issues, let's see that later.

etienne-lms · 2020-03-19T08:55:04Z

public/tee_client_api.h

+ * @param p The paramType.
+ * @param i The i-th parameter to set the type for.
+ */
+#define TEEC_PARAM_TYPE_SET(t, i) (((uint32_t)(t) & 0xF) << ((i)*4))


prefer something like:

#define TEEC_PARAM_TYPE_SET(t, i) (((uint32_t)(t) & GENMASK(3, 0)) << \ ((i) * 4))

Mm... I'd have to steal GENMASK from the kernel, unless it's in some user-space header I'm not familiar with. I don't know if that'd be clearer.

Perhaps I can take 0xF out and give it a name that describes why that number?

etienne-lms · 2020-03-19T08:56:23Z

libteec/include/linux/tee.h

-#define TEE_IOCTL_SHM_DMA_BUF	0x2	/* dma-buf handle on shared memory */
+#define TEE_IOCTL_SHM_MAPPED	(1 << 0)/* memory mapped in normal world */
+#define TEE_IOCTL_SHM_DMA_BUF	(1 << 1)/* dma-buf handle on shared memory */
+#define TEE_IOCTL_SHM_OCALL	(1 << 2)/* memory used for an OCALL */


for an output call */

I'd prefer keeping OCALL since that's what we call it.

HernanGatta · 2020-03-19T17:58:07Z

@etienne-lms, thank you for the review.

Regarding the renaming of TEEC_OpenSessionEx to TEEC_OpenSessionWithRpc, I would argue that there is nothing in the signature of the new function that is RPC/OCALL-specific. The settings mechanism is generic and could be used for other session settings in the future, so I'd be reluctant to rename the function to something that isn't also as generic.

The Ex suffix comes from my time working on Windows, where C APIs that are overloaded over time pick up the Extended suffix. I wasn't sure what else to call the new function, so I tacked Ex at the end; I'm all for renaming the function to something else.

Perhaps TEEC_OpenSessionWithSettings?

With respect to replacing "OCALL" with "RPC", I'm not opposed. However, @jenswi-linaro had expressed a liking for the name "OCALL" in the issue we discussed this feature in last year. Additionally, OCALL, or "out call", is the term used by the Open Enclave SDK and Intel's SGX for the same feature, so I figured we could reuse it.

Having said that, I'll go with whatever name you all coalesce on that fits within the readily established nomenclature.

Lastly, as to your request for a cleaner patch set, I'll gladly readjust the commits to fit what you requested, though I'll wait until we've decided on the right names.

etienne-lms · 2020-03-19T19:57:28Z

TEEC_OpenSessionWithSettings is fine also.
As for OCalls/ECalls, keep these naming if they reference to some standard. Just make sure each is described in some header file.
As for the capability, TEE_OPTEE_CAP_CA_OCALL to route OCall to CA provided handlers, no?

Regarding, the TEEC_OpenSessionExtended(), @jenswi-linaro mentioned registering OCall handlers from the context. Do you think each session should provide the OCall handler? Or one could register OCall settings to the TEEC_Context created, applicable to all sessions created in the context.

HernanGatta · 2020-03-19T20:42:11Z

As for the capability, TEE_OPTEE_CAP_CA_OCALL to route OCall to CA provided handlers, no?

Yes.

I'm of two minds about adding this capability. On the one hand, it's possible that the version of OP-TEE that the client library is talking to does not support OCALLs. In this case, we fail right away if the CA expects OCALL support but OP-TEE does not provide it. On the other hand, we could do away with the capability, and just open the session. If the CA expects OCALL support, this means that the TA probably makes use of OCALLs. If OP-TEE was compiled without OCALL support, the TA will fail to make OCALLs, and that failure will presumably be reported back to the CA.

So we have the choice between checking for OCALL support and failing right away before even opening the session, or not checking for OCALL support and letting the TA fail to send any later.

I chose to be explicit and have OP-TEE report support, or lack thereof, for the feature. However, if we wish to not pile on another capability, it's possible to do without it.

What do you think?

Regarding, the TEEC_OpenSessionExtended(), @jenswi-linaro mentioned registering OCall handlers from the context. Do you think each session should provide the OCall handler? Or one could register OCall settings to the TEEC_Context created, applicable to all sessions created in the context.

I've considered this, and I'm open to changing my mind.

The way I conceptualize OCALLs is as a response to a function invocation. That is, one calls TEEC_InvokeFunction and one might get an OCALL in return. Since the former is called on a session and not a context, it seemed logical to me to attach OCALLs to sessions. An OCALL is a response to a function invocation in the context of an established session.

Additionally, a developer presumably controls both the CA and the TA. As such, when opening a session to a TA that is known to request OCALLs, it would make sense that the conduit for delivery is the session; OCALLs at the context level may be conceptualized as coming from any TA to which a session is opened via that context, regardless of whether all or only some of the TAs request OCALLs.

Lastly, OCALLs must still be attached to sessions, at least technically. This is because on session close, all outstanding OCALLs must be cancelled. If one prematurely closes a session, say due to an error, we can't wait until the context is released to cancel pending OCALLs because the secure thread is stuck on an RPC.

The flip-side to these arguments is an observation that Jens made whereby a TA may call another TA which itself sends an OCALL. Therefore, the CA might receive an OCALL from a TA other than the one the session was opened against. This is the reason I added the TA UUID parameter to the OCALL handler callback, which wasn't there in the first try at this feature. In light of this, it could make sense to have a catch-all OCALL handler in the CA for all possible TAs. Hence, if the handler must already support handling OCALLs from multiple TAs, it may as well handle OCALLs from all TAs opened against a context, at which point the handler may also as well be attached to the context.

The counterargument to the previous argument is that a developer might still want to know which session an OCALL came from and decide what to do accordingly, especially if multiple sessions are opened on the same context against the same multi-instance TA: the handler will probably want to know which session the OCALL came from to distinguish between TA instances. This could be resolved by adding a session parameter to the OCALL handler callback (which I think I should add anyway). The problem with this approach is that the OCALL handler callback loses the ability to have a per-session callback context (the context parameter in the callback), it being replaced instead by a per-context, all-sessions context.

Thoughts?

Lastly, Jens also mentioned that one is not allowed to call back into the TA from the OCALL handler, which is true, and I've added no way to detect this. I'll go ahead and implement a check to prevent re-entry.

jforissier · 2020-03-20T09:18:41Z

Adding my 2 (euro) cents to the discussion...

Regarding the handler: considering all the arguments above, I have a slight preference for attaching the callback to the context rather than the session.
I'm fine with name TEEC_OpenSessionEx() (brings back some memories! 😉) although I have no strong opinion. TBH WithSettings does not convey much more information than Ex. GP add a 2 when they evolve their APIs (I'm thinking Internal Core), so at least there would be no conflicts in both cases.
I am also in favor of having a capability flag and failing early if OP-TEE does not support the feature.

jenswi-linaro

Some initial comments

jenswi-linaro · 2020-03-20T10:43:45Z

libteec/include/linux/tee.h

-#define TEE_IOCTL_SHM_DMA_BUF	0x2	/* dma-buf handle on shared memory */
+#define TEE_IOCTL_SHM_MAPPED	(1 << 0)/* memory mapped in normal world */
+#define TEE_IOCTL_SHM_DMA_BUF	(1 << 1)/* dma-buf handle on shared memory */
+#define TEE_IOCTL_SHM_OCALL	(1 << 2)/* memory used for an OCALL */


I'd prefer keeping OCALL since that's what we call it.

jenswi-linaro · 2020-03-20T10:50:16Z

libteec/include/linux/tee.h

@@ -63,6 +64,7 @@
 * OP-TEE specific capabilities
 */
 #define TEE_OPTEE_CAP_TZ	(1 << 0)
+#define TEE_OPTEE_CAP_OCALL	(1 << 1)/* Supports calls from TA to CA */


I think this should be a generic capability instead, TEE_GEN_CAP_OCALL. I suppose only OP-TEE will support it in the nearest future, but I see no reason why other TEEs shouldn't be able to reuse infrastructure.

I've turned it into a generic one, and is now handled as such in the library.

jenswi-linaro · 2020-03-20T11:15:10Z

libteec/src/tee_client_api.c

+		}
+	}
+
+	res = TEEC_OpenSession(ctx, session, destination, connection_method,


This means that OCALLs can't be done while the TA is initializing. It might not be a big problem, but it will be a bit annoying when documenting and perhaps lead to some special cases too.

This has been addressed by attaching the OCALL handler to the context, as you've requested.

jenswi-linaro · 2020-03-23T08:45:54Z

So we have the choice between checking for OCALL support and failing right away before even opening the session, or not checking for OCALL support and letting the TA fail to send any later.

I chose to be explicit and have OP-TEE report support, or lack thereof, for the feature. However, if we wish to not pile on another capability, it's possible to do without it.

I prefer failing early, it's a bit more tidy in my mind.

Registering OCALL handler in the context or the session?
I'm leaning towards context since I consider this a global thing. It will have to be able to serve all TAs in a family of TAs regardless of to which TA the session in opened since a TA may call another TA. The OCALL handler will need to know which session it's serving. It would be handy to have a session specific pointer too. We could add TEEC_SetSessionSpecificPointer() and TEEC_GetSessionSpecificPointer(). I think we should avoid the name context here since we already have a TEEC_Context.

HernanGatta · 2020-04-27T18:04:46Z

Design question:

Currently, the PR has the TEE Client API pass shared memory to the CA that was allocated on request of the TA via the memref element of TEEC_Parameter.

While this reflects the underlying fact that the shared memory in question can be used across multiple OCALLs (i.e., it's not a tmpref whose lifetime is tied to the current OCALL), it does put the burden of dealing with offsets into the parent shared memory buffer on the OCALL handler, and thus on the CA developer. It also exposes an implementation detail, namely that the current OCALL implementation puts all buffers passed from the TA to the CA into the same shared memory element. As a result, what starts out in the TA as a buffer-size pair into TEE_InvokeCACommand winds up on the CA side as a buffer-size-offset triplet.

This is not how function invocations on TAs work: regardless of whether the underlying shared memory can persist across multiple invocations or is temporary, the TA function invocation handler gets a buffer and a size; no offset.

The changes I'm working on would instead have the TEE Client API pass buffers to the CA's OCALL handler as tmprefs, removing the burden of dealing with the offset from the OCALL handler.

I'm torn: on the one hand, using temporary reference semantics makes it easier to write OCALL handlers because offsets are not involved, and don't have to be, since libteec can perform the pointer arithmetic on behalf of the developer. On the other, it's semantically incorrect.

What do you all think?

P.S.: The Travis CI failure is about style complaints that I don't think are true issues.

HernanGatta · 2020-04-29T08:17:59Z

I prefer failing early, it's a bit more tidy in my mind.

Checked during context initialization.

Registering OCALL handler in the context or the session?
I'm leaning towards context since I consider this a global thing. It will have to be able to serve all TAs in a family of TAs regardless of to which TA the session in opened since a TA may call another TA. The OCALL handler will need to know which session it's serving. It would be handy to have a session specific pointer too. We could add TEEC_SetSessionSpecificPointer() and TEEC_GetSessionSpecificPointer(). I think we should avoid the name context here since we already have a TEEC_Context.

I've added the handler and a context-specific data pointer as a setting on TEEC_Context as well as a session-specific pointer also as a setting on TEEC_Session.

jforissier · 2020-04-29T08:37:36Z

P.S.: The Travis CI failure is about style complaints that I don't think are true issues.

I think there is only one warning type that we want to ignore here ("do not add new typedefs"), but all the others can be eliminated:

"Missing blank line" is caused by SLIST_ENTRY(teec_shm_record) not being recognized as a type. Can be fixed by adding SLIST_ENTRY\(.*\) to typedefs.checkpatch
"Alignment should match open parenthesis": align as expected
"spaces preferred around that '*'": add the new types to typedefs.checkpatch: TEEC_ContextSetting, TEEC_ContextSettingOcall, TEEC_SessionSetting and TEEC_SessionSettingData.

HernanGatta · 2020-04-29T08:42:32Z

"Missing blank line" is caused by SLIST_ENTRY(teec_shm_record) not being recognized as a type. Can be fixed by adding SLIST_ENTRY\(.*\) to typedefs.checkpatch

"spaces preferred around that '*'": add the new types to typedefs.checkpatch: TEEC_ContextSetting, TEEC_ContextSettingOcall, TEEC_SessionSetting and TEEC_SessionSettingData.

Ah, thanks for the pointers! Will do.

"Alignment should match open parenthesis": align as expected

I'll happily do so, but the style won't match the rest of the file if I do. Parameters are indented with three tabs in the file, for example:

static void teec_post_process_operation(TEEC_Operation *operation,
			struct tee_ioctl_param *params,
			TEEC_SharedMemory *shms)
{
	...

Preference?

libteec/include/linux/tee.h

etienne-lms · 2020-06-01T07:49:21Z

This P-R needs to be rebased.

HernanGatta · 2020-06-10T01:00:41Z

This P-R needs to be rebased.

Done.

On a separate note, I noticed warnings still about typedefs from checkpatch in Travis CI. It's curious because each commit that adds a new typedef also adds the typedefs in question to typedefs.checkpatch.

Any ideas?

jforissier · 2020-06-10T08:16:15Z

On a separate note, I noticed warnings still about typedefs from checkpatch in Travis CI. It's curious because each commit that adds a new typedef also adds the typedefs in question to typedefs.checkpatch.

Any ideas?

The purpose of typedefs.checkpatch is not to prevent this particular warning but to avoid later errors that would occur if the new type was not known to checkpatch. In other words, it allows correct parsing of the code later on. So this is good: the tool raised our attention, we chose to ignore the warning, and we don't expect any new warning when using the new types.

jforissier · 2020-06-10T09:19:51Z

One minor thing re. commit "checkpatch: reverse sort typedefs.checkpatch", and although I have reviewed it already... I suggest the following additional change:

diff --git a/typedefs.checkpatch b/typedefs.checkpatch
index 9b7906d..729ae36 100644
--- a/typedefs.checkpatch
+++ b/typedefs.checkpatch
@@ -69,9 +69,9 @@ CK_FLAGS
 CK_DESTROYMUTEX
 CK_DATE_PTR
 CK_DATE
-CK_CREATEMUTEX
 CK_C_INITIALIZE_ARGS_PTR
 CK_C_INITIALIZE_ARGS
+CK_CREATEMUTEX
 CK_CHAR_PTR
 CK_CHAR
 CK_CCM_PARAMS_PTR

Indeed, sort -r (as recommended in the comment) depends on the current locale and I'd rather we use the C locale, which follows traditional byte order. For example, consider this:

$ printf "CK_CREATEMUTEX\nCK_C_INITIALIZE_ARGS_PTR\nCK_CHAR_PTR" | LC_ALL=C sort -r
CK_C_INITIALIZE_ARGS_PTR
CK_CREATEMUTEX
CK_CHAR_PTR
$ printf "CK_CREATEMUTEX\nCK_C_INITIALIZE_ARGS_PTR\nCK_CHAR_PTR" | LC_ALL=en_US.UTF-8 sort -r
CK_CREATEMUTEX
CK_C_INITIALIZE_ARGS_PTR
CK_CHAR_PTR

Note, this "C" ordering happens to be what I obtained in vim using the :sort! command applied to all lines but the inital comments (although, according to :help sort, there is no guarantee).

In any case there does not seem to be any impact on checkpatch -- the important thing is to have foo<something> come before foo (in reverse sort) [1] which I guess is true for all locales (?).

[1] As I said OP-TEE/optee_os#2685 (comment) and this is still true with checkpatch.pl from kernel v5.5

libteec/CMakeLists.txt

HernanGatta · 2020-06-18T20:48:18Z

I suggest the following additional change: [...]

Done.

The purpose of typedefs.checkpatch is not to prevent this particular warning but to avoid later errors [...]

Thank you for taking the time to explain this!

jforissier

In the description for "libteec: implement OCALL support during function invocation":

s/TEEC_InvokeFunction/TEEC_InvokeCommand()/
It could be worth mentioning explicitly that the OCALL handler is executed in the context of the thread that called TEEC_InvokeCommand() (did I get this right? I think I did.)

Plus one minor thing below. With that:

Reviewed-by: Jerome Forissier <jerome@forissier.org>

public/tee_client_api.h

etienne-lms · 2020-06-19T09:32:05Z

public/tee_client_api.h

+ * OCALLs.
+ *
+ * @param handler  Pointer to the function to execute to handle an OCALL.
+ * @param data     Arbitrary pointer to pass to the OCALL handler function.


This refer to TEEC_OcallHandler() argument ctxData?

Yes. I've added comments indicating this.

etienne-lms · 2020-06-19T09:35:59Z

public/tee_client_api_extensions.h

+ *                     to. If name is set to NULL, the default TEE is connected
+ *                     to. NULL is the only supported value in this version of
+ *                     the API implementation.
+ * @param context      The context structure which is to be initialized.


s/@param context/@param ctx/

I actually made a mistake in the header declaration. Everywhere else the parameter is named context, so I've changed ctx to that.

etienne-lms · 2020-06-19T09:38:42Z

public/tee_client_api_extensions.h

@@ -50,6 +50,64 @@ TEEC_Result TEEC_RegisterSharedMemoryFileDescriptor(TEEC_Context *context,
 						    TEEC_SharedMemory *sharedMem,
 						    int fd);

+/**
+ * TEEC_InitializeContext2() - Initializes a context holding connection
+ * information on the specific TEE, designated by the name string.


Maybe state that this function reflects standard TEEC_InitializeContext() with 2 extra arguments to associate context data to the TEE context.

And same for TEEC_OpenSession2() that implements standard TEEC_OpenSession() with extra session data.

Edited as requested.

etienne-lms · 2020-06-19T09:40:49Z

libteec/src/tee_client_api.c

+	uint32_t n;
+	TEEC_Result res;
+
+	if ((!settings && numSettings) || (settings && !numSettings))


I think (settings && !numSettings) should be a valid case. If numSettings is 0, no setting is registered with the TEE context and that's fine.

etienne-lms · 2020-06-19T13:07:01Z

libteec/src/tee_client_api.c

+	if ((!settings && numSettings) || (settings && !numSettings))
+		return TEEC_ERROR_BAD_PARAMETERS;
+
+	if (settings) {


minor: can remove if (settings)

etienne-lms · 2020-06-19T13:12:39Z

libteec/src/tee_client_api.c

@@ -51,12 +52,35 @@
 /* How many device sequence numbers will be tried before giving up */
 #define TEEC_MAX_DEV_SEQ	10

+/* No SHM flags */
+#define TEE_IOCTL_SHM_NONE	0


Should this be in libteec/include/linux/tee.h around line 46?

Moved it there.

HernanGatta · 2020-10-19T04:43:15Z

In the description for "libteec: implement OCALL support during function invocation":

s/TEEC_InvokeFunction/TEEC_InvokeCommand()/

It could be worth mentioning explicitly that the OCALL handler is executed in the context of the thread that called TEEC_InvokeCommand() (did I get this right? I think I did.)

Plus one minor thing below. With that:

Reviewed-by: Jerome Forissier <jerome@forissier.org>

Thank you @jforissier and @etienne-lms for your reviews, and my sincere apologies for the massive delay.

I believe I've addressed your comments. As to Jérôme's comments on the commit description, I've edited it as requested (and you are indeed correct on the thread context comment.)

I've foregone adding the Reviewed-by tag since it's been a while and you might have more comments.

jforissier · 2020-10-19T11:25:52Z

Thank you @jforissier and @etienne-lms for your reviews, and my sincere apologies for the massive delay.

Hi @HernanGatta, it's good to hear from you again! 👍 😄

To be honest, I was afraid that you would have moved on to something else, which would have been unfortunate given the amount of work already done on this feature (especially on your side!). That being said, don't worry about the delay. We are not on a tight schedule, and contributions and reviews are always made on a best effort basis. It should be pretty clear to everyone, hopefully.

I believe I've addressed your comments. As to Jérôme's comments on the commit description, I've edited it as requested (and you are indeed correct on the thread context comment.)

I've foregone adding the Reviewed-by tag since it's been a while and you might have more comments.

Thanks. I've double checked, it all looks good and well documented, so:

Reviewed-by: Jerome Forissier <jerome@forissier.org>

HernanGatta · 2020-10-29T08:03:50Z

To be honest, I was afraid that you would have moved on to something else, which would have been unfortunate given the amount of work already done on this feature (especially on your side!). That being said, don't worry about the delay. We are not on a tight schedule, and contributions and reviews are always made on a best effort basis. It should be pretty clear to everyone, hopefully.

Thank you for your note. There's been a bit of a switcheroo regarding work and priorities but I'm not letting this go :)

Thanks. I've double checked, it all looks good and well documented, so:

Reviewed-by: Jerome Forissier <jerome@forissier.org>

Applied, thanks!

Rearrange entries in typedefs.checkpatch in reverse alphabetical order. Signed-off-by: Hernan Gatta <hegatta@microsoft.com> Reviewed-by: Jerome Forissier <jerome@forissier.org>

A new flag is required to mark shared memory registered for OCALL usage. Signed-off-by: Hernan Gatta <hegatta@microsoft.com>

Enable detection of whether the TEE was built with OCALL support during context initialization. Signed-off-by: Hernan Gatta <hegatta@microsoft.com>

Add a new function, TEEC_InitializeContext2, that allows a developer to pass configuration parameters in the form of distinct settings. One of the new settings is for configuring OCALLs, where the caller sets a callback handler for when an OCALL arrives from a TA. Having multiple settings in this manner helps reduce the number of auxiliary functions necessary. Were new functionality to be added in the future, no new functions would need to be introduced. Instead, one would only require a new setting. Signed-off-by: Hernan Gatta <hegatta@microsoft.com>

Add a new function, TEEC_OpenSession2, that allows a developer to pass configuration parameters in the form of distinct settings. One of the new settings is for attaching an arbitrary pointer to the session. This is useful when an OCALL arrives from the TA and the handler requires contextual information to proceed that changes depending on which session the OCALL arrived through. Having multiple settings in this manner helps reduce the number of auxiliary functions necessary. Were new functionality to be added in the future, no new functions would need to be introduced. Instead, one would only require a new setting. Signed-off-by: Hernan Gatta <hegatta@microsoft.com>

OCALLs allow a TA to invoke functions on their CA with parameters, if desired, both during session open and command invoke. The flow begins when a CA calls TEEC_InvokeCommand. If the TEE context was initialized with the OCALL setting, libteec includes an additional parameter, the OCALL parameter, in the function invocation IOCTL. The presence of the OCALL parameter lets the kernel driver know that an OCALL may result from the invocation. If an OCALL does arrive from the TA, the OCALL parameter includes information about the OCALL, including the ID of the function that libteec must handle. These are: allocate shared memory, free shared memory, and invoke a function on the CA. If either of the first two functions arrives at libteec, the library handles these on behalf of the CA, allocating and freeing shared memory as necessary. When the third function arrives, libteec processes the OCALL's parameters. These will have temporarily replaced the parameters of the original function invocation. Additionally, the 'func' element of the IOCTL parameters will have been modified to carry the command ID that the TA requests that the CA execute on its behalf. The library passes this ID along with the parameters and arbitrary data pointers configured via the settings API to the CA-provided OCALL handler. After the handler is finished processing the request, libteec performs minor post-processing on the parameters and calls back into the driver to let it know that the OCALL has been handled. It is possible for a TA to invoke multiple OCALLs in the same originating function invocation. Additionally, the same mechanism detailed above applies to when a CA calls TEEC_OpenSession or TEEC_OpenSession2. That is, the TA may invoke one or more functions in sequence on its CA during session open. Lastly, the CA-provided OCALL handler is executed in the context of the thread that originally called TEEC_OpenSession, TEEC_OpenSession2, or TEEC_InvokeCommand. That is, when a TA invokes a function on its CA, these functions call into the CA-provided OCALL handler. Signed-off-by: Hernan Gatta <hegatta@microsoft.com> Reviewed-by: Jerome Forissier <jerome@forissier.org>

The addition of OCALL functionality breaks binary compatibility with existing applications by modifying the layout of certain structures. Section 3.1 of the GlobalPlatform TEE Client API specification does not mandate binary compatibility. As such, increase the version number for the library from 1.0.0 to 2.0.0. Signed-off-by: Hernan Gatta <hegatta@microsoft.com>

HernanGatta · 2021-04-08T01:04:25Z

Latest push is a rebase only.

github-actions · 2021-12-24T01:29:59Z

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

This was referenced Sep 23, 2019

Generic RPC Support OP-TEE/optee_os#3292

Closed

ocall: add an example to show how to use OCALLs linaro-swg/optee_examples#64

Closed

[RFC] core: add a new pta to support any ree service for TA OP-TEE/optee_os#3270

Closed

HernanGatta force-pushed the optee-generic-rpc-support branch from dd1e278 to 2bafe5f Compare September 23, 2019 09:12

HernanGatta force-pushed the optee-generic-rpc-support branch 2 times, most recently from 781c07a to 0a7174b Compare March 18, 2020 03:39

This was referenced Mar 18, 2020

[RFC] core: implement OCALLs OP-TEE/optee_os#3673

Open

driver: tee: optee: implement OCALLs linaro-swg/linux#72

Open

HernanGatta changed the title ~~Generic RPC Support~~ libteec: implement OCALLs Mar 19, 2020

HernanGatta mentioned this pull request Mar 19, 2020

Custom RPC from user TA OP-TEE/optee_os#3303

Closed

etienne-lms reviewed Mar 19, 2020

View reviewed changes

jenswi-linaro reviewed Mar 20, 2020

View reviewed changes

HernanGatta force-pushed the optee-generic-rpc-support branch 6 times, most recently from bf071e5 to 38cf78b Compare April 22, 2020 22:33

HernanGatta force-pushed the optee-generic-rpc-support branch from 38cf78b to e275327 Compare April 29, 2020 07:33

HernanGatta commented Apr 29, 2020

View reviewed changes

libteec/include/linux/tee.h Outdated Show resolved Hide resolved

HernanGatta commented Apr 29, 2020

View reviewed changes

libteec/include/linux/tee.h Outdated Show resolved Hide resolved

HernanGatta force-pushed the optee-generic-rpc-support branch from 1b78c51 to 9713137 Compare June 10, 2020 00:54

jforissier reviewed Jun 15, 2020

View reviewed changes

libteec/CMakeLists.txt Outdated Show resolved Hide resolved

HernanGatta force-pushed the optee-generic-rpc-support branch from 9713137 to 547aaf8 Compare June 18, 2020 20:47

jforissier reviewed Jun 19, 2020

View reviewed changes

public/tee_client_api.h Outdated Show resolved Hide resolved

etienne-lms reviewed Jun 19, 2020

View reviewed changes

HernanGatta force-pushed the optee-generic-rpc-support branch 2 times, most recently from 5ffb563 to cfd36b0 Compare October 19, 2020 04:39

HernanGatta force-pushed the optee-generic-rpc-support branch from cfd36b0 to 8472211 Compare October 19, 2020 05:01

HernanGatta force-pushed the optee-generic-rpc-support branch from 8472211 to d3f1555 Compare October 29, 2020 07:56

HernanGatta added 7 commits April 5, 2021 22:15

checkpatch: reverse sort typedefs.checkpatch

d535712

Rearrange entries in typedefs.checkpatch in reverse alphabetical order. Signed-off-by: Hernan Gatta <hegatta@microsoft.com> Reviewed-by: Jerome Forissier <jerome@forissier.org>

libteec: allow flags in teec_shm_register

77fb26f

A new flag is required to mark shared memory registered for OCALL usage. Signed-off-by: Hernan Gatta <hegatta@microsoft.com>

libteec: detect OCALL support

d4923bb

Enable detection of whether the TEE was built with OCALL support during context initialization. Signed-off-by: Hernan Gatta <hegatta@microsoft.com>

HernanGatta force-pushed the optee-generic-rpc-support branch from d3f1555 to 65e343a Compare April 8, 2021 00:58

jenswi-linaro mentioned this pull request May 28, 2021

Simplified OCALL OP-TEE/optee_os#4650

Open

github-actions bot added the Stale label Dec 24, 2021

github-actions bot closed this Dec 29, 2021

libteec: implement OCALLs #171

libteec: implement OCALLs #171

Conversation

HernanGatta commented Sep 23, 2019 • edited

Overview

Session Open

OCALL Handling

etienne-lms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HernanGatta Apr 29, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HernanGatta commented Mar 19, 2020

etienne-lms commented Mar 19, 2020

HernanGatta commented Mar 19, 2020 • edited

jforissier commented Mar 20, 2020

jenswi-linaro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenswi-linaro commented Mar 23, 2020 • edited

HernanGatta commented Apr 27, 2020 • edited

HernanGatta commented Apr 29, 2020

jforissier commented Apr 29, 2020

HernanGatta commented Apr 29, 2020 • edited

etienne-lms commented Jun 1, 2020

HernanGatta commented Jun 10, 2020

jforissier commented Jun 10, 2020

jforissier commented Jun 10, 2020

HernanGatta commented Jun 18, 2020 • edited

jforissier left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HernanGatta Oct 19, 2020 • edited

Choose a reason for hiding this comment

HernanGatta commented Oct 19, 2020 • edited

jforissier commented Oct 19, 2020

HernanGatta commented Oct 29, 2020

HernanGatta commented Apr 8, 2021

github-actions bot commented Dec 24, 2021

HernanGatta commented Sep 23, 2019 •

edited

HernanGatta Apr 29, 2020 •

edited

HernanGatta commented Mar 19, 2020 •

edited

jenswi-linaro commented Mar 23, 2020 •

edited

HernanGatta commented Apr 27, 2020 •

edited

HernanGatta commented Apr 29, 2020 •

edited

HernanGatta commented Jun 18, 2020 •

edited

jforissier left a comment •

edited

HernanGatta Oct 19, 2020 •

edited

HernanGatta commented Oct 19, 2020 •

edited