Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add processing of code coverage for optee core #4192

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
14 changes: 13 additions & 1 deletion core/arch/arm/kernel/boot.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: BSD-2-Clause
/*
* Copyright (c) 2015-2020, Linaro Limited
* Copyright 2020 NXP
*/

#include <arm.h>
Expand Down Expand Up @@ -202,15 +203,23 @@ static void secondary_init_cntfrq(void)
}
#endif

#ifdef CFG_CORE_SANITIZE_KADDRESS
/*
* Gcov needs the constructor to run at init. There is a constructor
* function for each function to gather coverage data.
* These functions are created automatically by the compiler.
* Each constructor function will call __gcov_int() for their function.
*/
#if defined(CFG_CORE_SANITIZE_KADDRESS) || defined(CFG_CORE_GCOV_SUPPORT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could remove this #if, add __aybe_unused attribute to the function
and in init_asan() below:

static void init_asan(void)
{
	if (IS_ENABLED(CFG_CORE_GCOV_SUPPORT))
		init_run_constructors();
}

static void init_run_constructors(void)
{
const vaddr_t *ctor;

for (ctor = &__ctor_list; ctor < &__ctor_end; ctor++)
((void (*)(void))(*ctor))();
}
#endif /* CFG_CORE_SANITIZE_KADDRESS || CFG_CORE_GCOV_SUPPORT */

#ifdef CFG_CORE_SANITIZE_KADDRESS
static void init_asan(void)
{

Expand Down Expand Up @@ -260,6 +269,9 @@ static void init_asan(void)
#else /*CFG_CORE_SANITIZE_KADDRESS*/
static void init_asan(void)
{
#ifdef CFG_CORE_GCOV_SUPPORT
init_run_constructors();
#endif
}
#endif /*CFG_CORE_SANITIZE_KADDRESS*/

Expand Down
12 changes: 12 additions & 0 deletions core/arch/arm/plat-imx/conf.mk
Original file line number Diff line number Diff line change
Expand Up @@ -425,5 +425,17 @@ CFG_IMX_CAAM ?= y
endif
endif

ifeq ($(CFG_CORE_GCOV_SUPPORT), y)
ifneq ($(CFG_WITH_LPAE), y)
# Default:
# TEE_RAM_VA_SIZE = CORE_MMU_PGDIR_SIZE
# = BIT(CORE_MMU_PGDIR_SHIFT)
# with LPAE disabled: #define CORE_MMU_PGDIR_SHIFT 20
# = BIT(20)
# = 0x80000 (512 Ko)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be 0x100000 (1MB)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In effect, my bad

CFG_TEE_RAM_VA_SIZE := 0x200000 # 2MB
endif
endif

# Cryptographic configuration
include core/arch/arm/plat-imx/crypto_conf.mk
12 changes: 12 additions & 0 deletions core/core.mk
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ endif
cppflags$(sm) += -Ildelf/include
cppflags$(sm) += -Ilib/libutee/include

ifeq ($(CFG_CORE_GCOV_SUPPORT),y)
cppflags$(sm) += --coverage
cflags$(sm) += --coverage
aflags$(sm) += --coverage
endif

ifeq ($(filter y, $(CFG_CORE_DYN_SHM) $(CFG_CORE_RESERVED_SHM)),)
$(error error: No shared memory configured)
endif
Expand Down Expand Up @@ -119,6 +125,12 @@ include mk/lib.mk
base-prefix := $(sm)-
endif

ifeq ($(CFG_GCOV_SUPPORT),y)
libname = gcov
libdir = lib/libgcov
include mk/lib.mk
endif

ifeq ($(firstword $(subst /, ,$(CFG_CRYPTOLIB_DIR))),core)
# If a library can be compiled for both core and user space a base-prefix
# is needed in order to avoid conflicts in the output. However, if the
Expand Down
9 changes: 9 additions & 0 deletions core/include/optee_rpc_cmd.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: BSD-2-Clause */
/*
* Copyright (c) 2016-2017, Linaro Limited
* Copyright 2020 NXP
*/

#ifndef __OPTEE_RPC_CMD_H
Expand Down Expand Up @@ -139,6 +140,14 @@
*/
#define OPTEE_RPC_CMD_FTRACE 11

/*
* Send code coverage data to normal world
*
* [in] memref[1] filepath
* [in] memref[2] code coverage data
*/
#define OPTEE_RPC_CMD_GCOV 12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent with 2 tabs to align with above OPTEE_RPC_CMD_FTRACE.


/*
* Register timestamp buffer in the linux kernel optee driver
*
Expand Down
154 changes: 154 additions & 0 deletions core/pta/gcov.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/* SPDX-License-Identifier: BSD-2-Clause */
/*
* Copyright 2020 NXP
*/

#include <gcov.h>
#include <kernel/pseudo_ta.h>
#include <kernel/tee_ta_manager.h>
#include <pta_gcov.h>

/*
* Name of the TA
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this inline comment

#define TA_NAME "gcov.pta"

/*
* Proxy function which calls gcov_get_version()
*
* @ptypes The ptypes
* @params The parameters
* [out] value[0].a version of gcov
* [none]
* [none]
* [none]
*/
static TEE_Result pta_gcov_get_version(uint32_t ptypes, TEE_Param params[4])
{
uint32_t exp_ptypes = TEE_PARAM_TYPES(TEE_PARAM_TYPE_VALUE_OUTPUT,
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE);

if (exp_ptypes != ptypes) {
EMSG("Wrong param_types, exp %x, got %x", exp_ptypes, ptypes);
return TEE_ERROR_BAD_PARAMETERS;
}

return gcov_get_version(&params[0].value.a);
}

/*
* Proxy function which calls gcov_reset_coverage_data()
*
* @ptypes The ptypes
* @params The parameters
* [none]
* [none]
* [none]
* [none]
*/
static TEE_Result pta_gcov_core_reset(uint32_t ptypes,
__unused TEE_Param params[4])
{
uint32_t exp_ptypes = TEE_PARAM_TYPES(TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE);

if (exp_ptypes != ptypes) {
EMSG("Wrong param_types, exp %x, got %x", exp_ptypes, ptypes);
return TEE_ERROR_BAD_PARAMETERS;
}

return gcov_reset_coverage_data();
}

/*
* Proxy function which calls gcov_dump_coverage_data()
*
* @ptypes The ptypes
* @params The parameters
* [in] memref[0] filepath
* [in] memref[1] code coverage data
* [none]
* [none]
*/
static TEE_Result pta_gcov_dump(uint32_t ptypes, TEE_Param params[4])
{
uint32_t exp_ptypes = TEE_PARAM_TYPES(TEE_PARAM_TYPE_MEMREF_INPUT,
TEE_PARAM_TYPE_MEMREF_INPUT,
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE);

if (exp_ptypes != ptypes) {
EMSG("Wrong param_types, exp %x, got %x", exp_ptypes, ptypes);
return TEE_ERROR_BAD_PARAMETERS;
}

/* Dump the data */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this inline comment

return gcov_dump_coverage_data(params[0].memref.buffer,
params[1].memref.buffer,
params[1].memref.size);
}

/*
* Proxy function which calls gcov_dump_coverage_data()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/gcov_dump_coverage_data/gcov_dump_all_coverage_data/

*
* @ptypes The ptypes
* @params The parameters
* [in] memref[0] description
* [none]
* [none]
* [none]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer have this ABI description in lib/libutee/include/pta_gcov.h.

*/
static TEE_Result pta_gcov_core_dump_all(uint32_t ptypes, TEE_Param params[4])
{
uint32_t exp_ptypes = TEE_PARAM_TYPES(TEE_PARAM_TYPE_MEMREF_INPUT,
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE,
TEE_PARAM_TYPE_NONE);

if (exp_ptypes != ptypes) {
EMSG("Wrong param_types, exp %x, got %x", exp_ptypes, ptypes);
return TEE_ERROR_BAD_PARAMETERS;
}

/* Dump the data */
return gcov_dump_all_coverage_data(params[0].memref.buffer);
}

/*
* @Handler function upon invocation of a command of the TA
*
* @psess The Session
* @cmd The command to execute
* @ptypes The types of the parameters
* @params The parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argument description not needed, implicit since PTA format (pseudo_ta_register() below)

*/
static TEE_Result invoke_command(void *psess __unused, uint32_t cmd,
uint32_t ptypes, TEE_Param params[4])
{
switch (cmd) {
case PTA_CMD_GCOV_GET_VERSION:
return pta_gcov_get_version(ptypes, params);
case PTA_CMD_GCOV_CORE_RESET:
return pta_gcov_core_reset(ptypes, params);
case PTA_CMD_GCOV_DUMP:
return pta_gcov_dump(ptypes, params);
case PTA_CMD_GCOV_CORE_DUMP_ALL:
return pta_gcov_core_dump_all(ptypes, params);
default:
EMSG("Command %d not supported", cmd);
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: return TEE_ERROR_BAD_PARAMETERS; straight here?

}

return TEE_ERROR_BAD_PARAMETERS;
}

/*
* Register the PTA gcov with appropriate handler functions
*/
pseudo_ta_register(.uuid = PTA_GCOV_UUID, .name = TA_NAME,
.flags = PTA_DEFAULT_FLAGS,
.invoke_command_entry_point = invoke_command);
1 change: 1 addition & 0 deletions core/pta/sub.mk
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ srcs-$(CFG_WITH_STATS) += stats.c
srcs-$(CFG_SYSTEM_PTA) += system.c

subdirs-y += bcm
srcs-$(CFG_GCOV_SUPPORT) += gcov.c
1 change: 1 addition & 0 deletions core/tee/sub.mk
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,4 @@ endif #CFG_WITH_USER_TA,y

srcs-y += uuid.c
srcs-y += tee_ta_enc_manager.c
srcs-$(CFG_GCOV_SUPPORT) += tee_gcov_writer.c
81 changes: 81 additions & 0 deletions core/tee/tee_gcov_writer.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/* SPDX-License-Identifier: BSD-2-Clause */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// SPDX-...

/*
* Copyright 2020 NXP
*/

#include <string.h>
#include <trace.h>

#include <initcall.h>
#include <kernel/thread.h>
#include <mm/mobj.h>
#include <mm/tee_mmu_types.h>
#include <optee_rpc_cmd.h>

#include <gcov.h>

/*
* Write the coverage data to filesystem in the file filepath
*
* @filepath Path of the file to write
* @cov_data Coverage data to write
* @cov_data_size Coverage data size
*/
static TEE_Result tee_gcov_writer(const char *filepath, char *cov_data,
uint32_t cov_data_size)
{
TEE_Result res = TEE_SUCCESS;
struct thread_param params[2] = {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/{}/{ }/

struct mobj *mobj = NULL;
char *buf = NULL;
size_t pl_sz = 0;
uint32_t filepath_size = 0;

if (!filepath || !cov_data) {
EMSG("Wrong parameters");
return TEE_ERROR_BAD_PARAMETERS;
}

/* Compute size counting null terminator */
filepath_size = strlen(filepath) + 1;

pl_sz = ROUNDUP(filepath_size + cov_data_size, SMALL_PAGE_SIZE);

mobj = thread_rpc_alloc_payload(pl_sz);
if (!mobj) {
EMSG("Gcov thread_rpc_alloc_payload failed");
return TEE_ERROR_OUT_OF_MEMORY;
}

buf = mobj_get_va(mobj, 0);
if (!buf) {
EMSG("Can't get pointer on buffer");
res = TEE_ERROR_OUT_OF_MEMORY;
goto out;
}

memcpy(buf, filepath, filepath_size);
memcpy(buf + filepath_size, cov_data, cov_data_size);

params[0] = THREAD_PARAM_MEMREF(IN, mobj, 0, filepath_size);
params[1] = THREAD_PARAM_MEMREF(IN, mobj, filepath_size, cov_data_size);

res = thread_rpc_cmd(OPTEE_RPC_CMD_GCOV, 2, params);
if (res)
EMSG("Gcov thread_rpc_cmd res: %#" PRIx32, res);

out:
thread_rpc_free_payload(mobj);

return res;
}

/*
* Register tee_gcov_writer() to gcov library
*/
static TEE_Result register_gcov_write_late(void)
{
return register_gcov_dump_writer(tee_gcov_writer);
}

service_init_late(register_gcov_write_late);