Reentrancy fixes by jenswi-linaro · Pull Request #24 · OP-TEE/optee_os

jenswi-linaro · 2014-08-25T08:53:14Z

Adds a mutex that waits in normal world if busy
Adds a new RPC service to wait in normal world
Imports bitstring.h from FreeBSD to aid mutex implementation
Adds a critical section in tee_ta_init_session
Unmaps TA before RPC exit and maps it again on return to handle
rescheduling of threads during RPC
Doesn't clear a1-a3 when returning busy
Bugfixes vector_std_smc_entry

pascal-brand38 · 2014-08-27T06:22:47Z

+1

jforissier · 2014-08-29T08:11:27Z

Nice code, easy to read :)

Could you explain briefly in the commit message what this patch fixes/improves at a higher level maybe? Here is my understanding.

Before, only one TEE invocation was possible at a given time. Other invocations would block in the Linux driver.
Now, several TEE invocations can run simultaneously in several threads/processes, provided they relate to different sessions. Simultaneous calls on the same session will still block (and probably don't make much sense anyways).

While you're at it, add a reference to the driver patch: 'This patch needs the associated driver update: "Allow parallel entries to secure world"'. Since we have 2 separate projects I think this can be useful.

jockebech · 2014-08-29T08:21:26Z

Is it needed to ifdef around this? I cannot see that it is defined anywhere else? Configurable from make?

64 is a reasonable limit, but in case we'd like to override it the ifndef will help. If we end up with mutexes in dynamic structures the number of needed mutexes will likely depend on how much memory is available. It could be nice to be able configure a larger number for some platforms.

jockebech · 2014-08-29T13:50:01Z

I'm happy with Jens motivations for our comments, so +1 from me.

jforissier · 2014-08-29T14:02:58Z

+1 from me, too.

jenswi-linaro · 2014-09-01T08:09:50Z

One more simple bugfix (but hard to find).

I'll squash and rebase once Pascal is happy with it.

pascal-brand38 · 2014-09-01T09:17:06Z

+1
Should you wait #31 to be merged before the rebase (I am awaiting for another +1 on pr-31)?

jenswi-linaro · 2014-09-01T11:04:09Z

I would have been nice to have this merged, but waiting another day is OK for me.

pascal-brand38 · 2014-09-01T11:06:20Z

So please go on. Squash on top of master, and I'll merge.

On 1 September 2014 13:04, Jens Wiklander [email protected] wrote:

I would have been nice to have this merged, but waiting another day is OK
for me.

—
Reply to this email directly or view it on GitHub
#24 (comment).

Before this patch: The normal world was only allowed to enter secure world with one thread at a time. After this patch: The normal world may try to enter secure world with as many threads as it likes, secure world will return busy when no more threads can be allowed. Secure world still only allows one active thread at a time, but during RPC another thread may enter and do some work. This is needed for cancellation to work. * Adds a mutex that waits in normal world if busy * Adds a new RPC service to wait in normal world * Imports bitstring.h from FreeBSD to aid mutex implementation * Adds a critical section in tee_ta_init_session * Unmaps TA before RPC exit and maps it again on return to handle rescheduling of threads during RPC * Doesn't clear a1-a3 when returning busy * Bugfixes vector_std_smc_entry This patch depends on the "Allow parallel entries to secure world" patch in optee_linuxdriver.

jenswi-linaro · 2014-09-01T11:17:56Z

Squashed on top of master

Tracing the log syscall is of very little value since it will generate some output to the console anyways. Worse, it pollutes the TA output in case of a panic or an abort. For example: o regression_4005.1 AE case 0 algo 0x40000710 line 2819 F/TC:?? 0 trace_syscall:132 syscall #27 (syscall_cryp_obj_alloc) F/TC:?? 0 trace_syscall:132 syscall #15 (syscall_cryp_state_alloc) F/TC:?? 0 trace_syscall:132 syscall #27 (syscall_cryp_obj_alloc) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #30 (syscall_cryp_obj_populate) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #29 (syscall_cryp_obj_reset) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #31 (syscall_cryp_obj_copy) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #28 (syscall_cryp_obj_close) F/TC:?? 0 trace_syscall:132 syscall #34 (syscall_authenc_init) F/TC:?? 0 trace_syscall:132 syscall #2 (syscall_panic) E/TC:?? 0 E/TC:?? 0 TA panicked with code 0xffff0006 F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: Status of TA cb3e5ba0-adf1-11e0-998b-0002a5d5c51b F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: arch: aarch64 F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: region 0: va 0x40004000 pa 0x100062d000 size 0x002000 flags rw-s (ldelf) F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: region 1: va 0x40006000 pa 0x100062f000 size 0x00d000 flags r-xs (ldelf) ... Therefore, skip the trace if the syscall number it TEE_SCN_LOG. Signed-off-by: Jerome Forissier <[email protected]>

Tracing the log syscall is of very little value since it will generate some output to the console anyways. Worse, it pollutes the TA output in case of a panic or an abort. For example: o regression_4005.1 AE case 0 algo 0x40000710 line 2819 F/TC:?? 0 trace_syscall:132 syscall #27 (syscall_cryp_obj_alloc) F/TC:?? 0 trace_syscall:132 syscall #15 (syscall_cryp_state_alloc) F/TC:?? 0 trace_syscall:132 syscall #27 (syscall_cryp_obj_alloc) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #30 (syscall_cryp_obj_populate) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #29 (syscall_cryp_obj_reset) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #31 (syscall_cryp_obj_copy) F/TC:?? 0 trace_syscall:132 syscall #24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall #28 (syscall_cryp_obj_close) F/TC:?? 0 trace_syscall:132 syscall #34 (syscall_authenc_init) F/TC:?? 0 trace_syscall:132 syscall #2 (syscall_panic) E/TC:?? 0 E/TC:?? 0 TA panicked with code 0xffff0006 F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: Status of TA cb3e5ba0-adf1-11e0-998b-0002a5d5c51b F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: arch: aarch64 F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: region 0: va 0x40004000 pa 0x100062d000 size 0x002000 flags rw-s (ldelf) F/TC:?? 0 trace_syscall:132 syscall #1 (syscall_log) E/LD: region 1: va 0x40006000 pa 0x100062f000 size 0x00d000 flags r-xs (ldelf) ... Therefore, skip the trace if the syscall number it TEE_SCN_LOG. Signed-off-by: Jerome Forissier <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Tracing the log syscall is of very little value since it will generate some output to the console anyways. Worse, it pollutes the TA output in case of a panic or an abort. For example: o regression_4005.1 AE case 0 algo 0x40000710 line 2819 F/TC:?? 0 trace_syscall:132 syscall OP-TEE#27 (syscall_cryp_obj_alloc) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#15 (syscall_cryp_state_alloc) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#27 (syscall_cryp_obj_alloc) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#30 (syscall_cryp_obj_populate) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#29 (syscall_cryp_obj_reset) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#31 (syscall_cryp_obj_copy) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#24 (syscall_cryp_obj_get_info) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#28 (syscall_cryp_obj_close) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#34 (syscall_authenc_init) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#2 (syscall_panic) E/TC:?? 0 E/TC:?? 0 TA panicked with code 0xffff0006 F/TC:?? 0 trace_syscall:132 syscall OP-TEE#1 (syscall_log) E/LD: Status of TA cb3e5ba0-adf1-11e0-998b-0002a5d5c51b F/TC:?? 0 trace_syscall:132 syscall OP-TEE#1 (syscall_log) E/LD: arch: aarch64 F/TC:?? 0 trace_syscall:132 syscall OP-TEE#1 (syscall_log) E/LD: region 0: va 0x40004000 pa 0x100062d000 size 0x002000 flags rw-s (ldelf) F/TC:?? 0 trace_syscall:132 syscall OP-TEE#1 (syscall_log) E/LD: region 1: va 0x40006000 pa 0x100062f000 size 0x00d000 flags r-xs (ldelf) ... Therefore, skip the trace if the syscall number it TEE_SCN_LOG. Signed-off-by: Jerome Forissier <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Consume upstream cpu on

jockebech reviewed Aug 29, 2014
View reviewed changes

jenswi-linaro force-pushed the reentrancy_fixes branch from 7e00f32 to f1c74b4 Compare September 1, 2014 11:14

ghost merged commit f1c74b4 into OP-TEE:master Sep 1, 2014

jenswi-linaro deleted the reentrancy_fixes branch September 26, 2014 09:59

airbak mentioned this pull request Oct 9, 2017

Issue: Load TA is blocked by libmpa #1862

Closed

angelina-zhao mentioned this pull request Jul 21, 2020

How to unwind a stack with mixed arm32 instruction and thumb instrustions? #3928

Closed

ldts mentioned this pull request Sep 22, 2020

Support for NXP SE050 device using the Plug and Trust library #4063

Closed

panda-balarka mentioned this pull request Oct 11, 2021

SUNXI_A64 - itr_add and itr_enable causes kernel panic #4910

Closed

CyanDevs pushed a commit to CyanDevs/optee_os that referenced this pull request Jan 23, 2023

Merge pull request OP-TEE#24 from jordanrh1/consume-upstream-cpu-on

23f7019

Consume upstream cpu on

LeiSen62 mentioned this pull request Jul 18, 2023

Panic in interrupt_main_handler #6183

Closed

Flynnsin mentioned this pull request Sep 30, 2025

RPMB read and write probability error (EMMC KLMxGxJETD-B041) #7548

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reentrancy fixes#24

Reentrancy fixes#24
1 commit merged intoOP-TEE:masterfrom
jenswi-linaro:reentrancy_fixes

jenswi-linaro commented Aug 25, 2014

Uh oh!

pascal-brand38 commented Aug 27, 2014

Uh oh!

jforissier commented Aug 29, 2014

Uh oh!

jockebech Aug 29, 2014

Uh oh!

jenswi-linaro Aug 29, 2014

Uh oh!

jockebech commented Aug 29, 2014

Uh oh!

jforissier commented Aug 29, 2014

Uh oh!

jenswi-linaro commented Sep 1, 2014

Uh oh!

pascal-brand38 commented Sep 1, 2014

Uh oh!

jenswi-linaro commented Sep 1, 2014

Uh oh!

pascal-brand38 commented Sep 1, 2014

Uh oh!

jenswi-linaro commented Sep 1, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jenswi-linaro commented Aug 25, 2014

Uh oh!

pascal-brand38 commented Aug 27, 2014

Uh oh!

jforissier commented Aug 29, 2014

Uh oh!

jockebech Aug 29, 2014

Choose a reason for hiding this comment

Uh oh!

jenswi-linaro Aug 29, 2014

Choose a reason for hiding this comment

Uh oh!

jockebech commented Aug 29, 2014

Uh oh!

jforissier commented Aug 29, 2014

Uh oh!

jenswi-linaro commented Sep 1, 2014

Uh oh!

pascal-brand38 commented Sep 1, 2014

Uh oh!

jenswi-linaro commented Sep 1, 2014

Uh oh!

pascal-brand38 commented Sep 1, 2014

Uh oh!

jenswi-linaro commented Sep 1, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants