core: fix crash during syscall ftrace

Syscall ftrace collects data during a syscall. get_fbuf() checks if
thread_get_id_may_fail() != -1 to see if a function is called under
normal thread execution.

core: fix crash during syscall ftrace

Syscall ftrace collects data during a syscall. get_fbuf() checks if
thread_get_id_may_fail() != -1 to see if a function is called under
normal thread execution. This can lead to an inconsistent state if a
native interrupt occur while ftrace_enter() or ftrace_return() is
recording data in the ftrace buffer. So fix this by using
thread_is_in_normal_mode() to exclude ftrace during interrupt
processing.

Reported-by: Jerome Forissier <jerome.forissier@linaro.org>
Closes: https://github.com/OP-TEE/optee_os/issues/7216
Fixes: 099918f6744c ("ftrace: Add support for syscall function tracer")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a)

show more ...

core: fix data abort during ftrace

With commit c10e3fa93d24 ("core: fix race in handling TA panic") the
resources of a panicked TAs are released as early as possible, including
the user space mapped

core: fix data abort during ftrace

With commit c10e3fa93d24 ("core: fix race in handling TA panic") the
resources of a panicked TAs are released as early as possible, including
the user space mapped ftrace buffer. However, the pointer to the ftrace
buffer is stored in the ts_session for quick and easy access. The ftrace
buffer is always retrieved with get_fbuf() that already have a few other
checks to see if the buffer is currently available. So add a check to
see that the TA hasn't panicked also.

Fixes: c10e3fa93d24 ("core: fix race in handling TA panic")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a)

show more ...

ftrace: change implementation to use binary circular buffer

The current implementation of function tracing (CFG_FTRACE_SUPPORT)
produces human-readable text into the output buffer that is passed to

ftrace: change implementation to use binary circular buffer

The current implementation of function tracing (CFG_FTRACE_SUPPORT)
produces human-readable text into the output buffer that is passed to
tee-supplicant and ultimately saved to the Linux filesystem. Two main
issues with that:

1. The string formatting code is somewhat complex. It introduces
significant overhead in the execution time of the instrumented
functions.
2. The various policies about how to handle a buffer full condition
(CFG_FTRACE_BUF_WHEN_FULL) are not very convenient. In particular,
"shift" is typically the most desirable option because it always
keeps the most recent entries, but it is very inefficient to the
point of not being usable in practice.

This commit addresses the above concerns by making the ftrace buffer
circular one, each entry being 64-bit value. The formatting code is
offloaded to a new Python script: scripts/ftrace_format.py. The
output is unchanged except for an added field showing the current
depth in the call stack.

Typical usage (captured on QEMUv8):

build$ mkdir -p ../tmp
build$ chmod a+w ../tmp
build$ make CFG_FTRACE_SUPPORT=y CFG_FTRACE_BUF_SIZE=15000 \
CFG_TA_MCOUNT=y CFG_ULIBS_MCOUNT=y CFG_SYSCALL_FTRACE=y \
QEMU_VIRTFS_AUTOMOUNT=y run
$ xtest regression_1004
...
$ cp /tmp/ftrace-cb3e5ba0-adf1-11e0-998b-0002a5d5c51b.out /mnt/host/tmp
build$ cd ..
optee$ optee_os/scripts/ftrace_format.py \
tmp/ftrace-cb3e5ba0-adf1-11e0-998b-0002a5d5c51b.out |
optee_os/scripts/symbolize.py \
-d optee_os/out/arm/core \
-d out-br/build/optee_test_ext-1.0/ta/*/out | less
TEE load address @ 0x5ab04000
Function graph for TA: cb3e5ba0-adf1-11e0-998b-0002a5d5c51b @ 80085000
| 1 | __ta_entry() {
| 2 | __utee_entry() {
43.840 us | 3 | ta_header_get_session()
7.216 us | 3 | tahead_get_trace_level()
14.480 us | 3 | trace_set_level()
| 3 | malloc_add_pool() {
| 4 | raw_malloc_add_pool() {
46.032 us | 5 | bpool()
| 5 | raw_realloc() {
166.256 us | 6 | bget()
23.056 us | 6 | raw_malloc_return_hook()
267.952 us | 5 | }
398.720 us | 4 | }
426.992 us | 3 | }
| 3 | TEE_GetPropertyAsU32() {
23.600 us | 4 | is_propset_pseudo_handle()
| 4 | __utee_check_instring_annotation() {
26.416 us | 5 | strlen()
| 5 | check_access() {
| 6 | TEE_CheckMemoryAccessRights() {
| 7 | _utee_check_access_rights() {
| 8 | syscall_check_access_rights() {
| 9 | ts_get_current_session() {
4.304 us | 10 | ts_get_current_session_may_fail()
10.976 us | 9 | }
| 9 | to_user_ta_ctx() {
2.496 us | 10 | is_user_ta_ctx()
8.096 us | 9 | }
| 9 | vm_check_access_rights() {
| 10 | vm_buf_is_inside_um_private() {
| 11 | core_is_buffer_inside() {
...

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Sumit Garg <sumit.garg@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>

show more ...

libutils: ftrace: Add definitions for separating architectural headers

Add definitions for separating architectural headers. In kernel mode,
risc-v may include riscv.h to have the timer related func

libutils: ftrace: Add definitions for separating architectural headers

Add definitions for separating architectural headers. In kernel mode,
risc-v may include riscv.h to have the timer related functions. In TA
libraries, risc-v may include riscv_user_sysreg.h to have those
functions.

Signed-off-by: Alvin Chang <alvinga@andestech.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Marouene Boubakri <marouene.boubakri@nxp.com>

show more ...

ftrace: Refactor ftrace buffer dump implementation

Current implementation does a lot of tricky bits with ftrace buffer
pointer. It also leads to false positive -Warray-bounds warnings with
GCC 11.2

ftrace: Refactor ftrace buffer dump implementation

Current implementation does a lot of tricky bits with ftrace buffer
pointer. It also leads to false positive -Warray-bounds warnings with
GCC 11.2 toolchain as well. So refactor it to use array indexes instead.
Also, move hardcoded ftrace line sizes to macros instead for better
understanding.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a)

show more ...

Rename to barrier_read_counter_timer()

Renames barrier_read_cntpct() to barrier_read_counter_timer() to use a
neutral name for the counter.

With SPMC at S-EL2 OP-TEE will be virtualized and must us

Rename to barrier_read_counter_timer()

Renames barrier_read_cntpct() to barrier_read_counter_timer() to use a
neutral name for the counter.

With SPMC at S-EL2 OP-TEE will be virtualized and must use CNTVCT
instead of CNTPCT while the old physical OP-TEE must continue to use
CNTPCT.

Reviewed-by: Jerome Forissier <jerome@forissier.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

Use barrier_read_cntpct() to read CNTPCT

Arm ARM quite clearly mentions [1] [2] that such reads must be preceded
by an ISB to forbid re-ordering.

[1] https://developer.arm.com/documentation/ddi0487

Use barrier_read_cntpct() to read CNTPCT

Arm ARM quite clearly mentions [1] [2] that such reads must be preceded
by an ISB to forbid re-ordering.

[1] https://developer.arm.com/documentation/ddi0487/fc/ page D13-2863
"Synchronization requirements for AArch64 System registers" and page
G8-6146 "Ordering of reads of System registers".
[2] https://developer.arm.com/documentation/ddi0406/cd/ page B3-1441
"Ordering of reads of system control registers"

Reviewed-by: Jerome Forissier <jerome@forissier.org>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Reported-by: Olivier Deprez <Olivier.Deprez@arm.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: add generic struct ts_session

As a step in making room for Secure Partitions (SPs) running at S-EL0
add a Trusted Service (TS) abstraction. Both TAs and SPs is a TS.

Adds the generic struct t

core: add generic struct ts_session

As a step in making room for Secure Partitions (SPs) running at S-EL0
add a Trusted Service (TS) abstraction. Both TAs and SPs is a TS.

Adds the generic struct ts_session. All future sessions structs
(currently only struct tee_ta_session exists) should add this struct to
allow generic session operations.

With this struct comes new functions replacing previous struct
tee_ta_session oriented functions. The following functions are replaced
as:
tee_ta_get_current_session() -> ts_get_current_session()
tee_ta_push_current_session() -> ts_push_current_session()
tee_ta_pop_current_session() -> ts_pop_current_session()
tee_ta_get_calling_session() -> ts_get_calling_session()

ts_get_current_session() is changed compared to its predecessor to
panic() in case of failure to return a valid pointer.

A new function ts_get_current_session_may_fail() is added to handle an
eventual case where a return NULL session may be handled.

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: make thread ID a short int

Changes thread_get_id() and thread_get_id_may_fail() to return 'short
int' instead of 'int'. That is, 16 bits instead of 32 on all supported
architectures which is m

core: make thread ID a short int

Changes thread_get_id() and thread_get_id_may_fail() to return 'short
int' instead of 'int'. That is, 16 bits instead of 32 on all supported
architectures which is more than enough since the largest thread ID
value is (CFG_NUM_THREADS - 1). Note, struct wait_queue_elem::handle
is already a short int.

trace_ext_get_thread_id() is not changed (still returns an int) because
it is part of the TA API and modifying it would needlessly introduce
incompatibilities.

Signed-off-by: Jerome Forissier <jerome@forissier.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

libutee, ldelf: add leading underscore to syscall wrappers

libutee defines assembler wrapper functions for each OP-TEE system call.
These wrappers have a utee_ prefix. This commit adds a leading
und

libutee, ldelf: add leading underscore to syscall wrappers

libutee defines assembler wrapper functions for each OP-TEE system call.
These wrappers have a utee_ prefix. This commit adds a leading
underscore so that the names cannot clash with user-defined symbols.
Doing so is common practice for "system" libraries, as defined by the C
standard in a set of requirements that can be summarized as follows
(excerpt from the GNU libc documentation [1]):

[R]eserved names include all external identifiers (global functions
and variables) that begin with an underscore (‘_’) and all identifiers
regardless of use that begin with either two underscores or an
underscore followed by a capital letter are reserved names. This is so
that the library and header files can define functions, variables, and
macros for internal purposes without risk of conflict with names in
user programs.

The utee_*() wrappers are internal to OP-TEE and are not supposed to be
called directly by TAs so this should not have any user-visible impact.

Link: [1] https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html
Signed-off-by: Jerome Forissier <jerome@forissier.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>

show more ...

ftrace: introduce CFG_FTRACE_BUF_WHEN_FULL

Function tracing can become extremely slow in case a big buffer size is
used (say, CFG_FTRACE_BUF_SIZE=6000000 instead of the default 2048
bytes). This is

ftrace: introduce CFG_FTRACE_BUF_WHEN_FULL

Function tracing can become extremely slow in case a big buffer size is
used (say, CFG_FTRACE_BUF_SIZE=6000000 instead of the default 2048
bytes). This is because of the "shifting" algorithm used when the buffer
is full, which copies almost the full buffer before inserting a new
line.

In order to mitigate this problem, this patch introduces two new
methods to handle the buffer full condition:

1. Discard existing data and write new lines to the beginning of the
buffer.
2. Stop adding new lines.

The method can be selected at build time with CFG_FTRACE_BUF_WHEN_FULL.
Supported values are "shift", "wrap" and "stop".

Signed-off-by: Jerome Forissier <jerome@forissier.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

ftrace: Add support for syscall function tracer

This patch adds support for syscall tracing in TEE core. It complements
existing ftrace support for user TAs via adding trace for syscalls that
are in

ftrace: Add support for syscall function tracer

This patch adds support for syscall tracing in TEE core. It complements
existing ftrace support for user TAs via adding trace for syscalls that
are invoked by user TAs into the TEE core.

And after this patch ftrace will cover both TA and TEE core code. So lets
rename config option from CFG_TA_FTRACE_SUPPORT to CFG_FTRACE_SUPPORT.

It is optional to enable syscall trace via CFG_SYSCALL_FTRACE=y config
option in addition to CFG_FTRACE_SUPPORT=y config option.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Jerome Forissier <jerome@forissier.org>

show more ...

ftrace: core: prepare support for syscall ftrace

Enable compilation of ftrace library code for TEE core.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Jerome Forissier <jerome@fori

ftrace: core: prepare support for syscall ftrace

Enable compilation of ftrace library code for TEE core.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Jerome Forissier <jerome@forissier.org>

show more ...

ftrace: move ftrace code from libutee to libutils

Since TEE core and TA can share most of ftrace library code, so move
ftrace code from libutee to libutils library which is shared among TEE
core and

ftrace: move ftrace code from libutee to libutils

Since TEE core and TA can share most of ftrace library code, so move
ftrace code from libutee to libutils library which is shared among TEE
core and TA.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Jerome Forissier <jerome@forissier.org>

show more ...