Re: [Tee-dev] [PATCH v2 04/13] optee: add OP-TEE mediator skeleton

Hi Volodymyr,

On 03/09/18 17:54, Volodymyr Babchuk wrote:

Add very basic OP-TEE mediator. It can probe for OP-TEE presence, tell it about domain creation/destuction and forward all known

s/destuction/destruction/

calls.

This is all what is needed for Dom0 to work with OP-TEE as long as Dom0 shares 1:1 mapped pages with OP-TEE. Any attempt to call OP-TEE from DomU will fail and can lead to spectacular results.

Shall we expect fireworks? :).

Anyway, I think this is a call for forbidding DomU access until it is supported. This also has the benefits to allow merging Dom0 support for OP-TEE without the rest.

Signed-off-by: Volodymyr Babchuk volodymyr_babchuk@epam.com

xen/arch/arm/tee/Kconfig | 4 ++ xen/arch/arm/tee/Makefile | 1 + xen/arch/arm/tee/optee.c | 134 ++++++++++++++++++++++++++++++++++++ xen/include/asm-arm/tee/optee_smc.h | 50 ++++++++++++++ 4 files changed, 189 insertions(+) create mode 100644 xen/arch/arm/tee/optee.c

diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig index e69de29..5b829db 100644 --- a/xen/arch/arm/tee/Kconfig +++ b/xen/arch/arm/tee/Kconfig @@ -0,0 +1,4 @@ +config OPTEE

• bool "Enable OP-TEE mediator"
• default n
• depends on TEE

diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile index c54d479..982c879 100644 --- a/xen/arch/arm/tee/Makefile +++ b/xen/arch/arm/tee/Makefile @@ -1 +1,2 @@ obj-y += tee.o +obj-$(CONFIG_OPTEE) += optee.o diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c new file mode 100644 index 0000000..7bb84d9 --- /dev/null +++ b/xen/arch/arm/tee/optee.c @@ -0,0 +1,134 @@ +/*

• • xen/arch/arm/tee/optee.c
• • OP-TEE mediator
• • Volodymyr Babchuk volodymyr_babchuk@epam.com
• • Copyright (c) 2018 EPAM Systems.
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• */

+#include <xen/device_tree.h> +#include <xen/sched.h> +#include <asm/smccc.h> +#include <asm/tee/tee.h>

+#include <asm/tee/optee_msg.h> +#include <asm/tee/optee_smc.h>

+static bool optee_probe(void) +{

• struct dt_device_node *node;
• struct arm_smccc_res resp;
• /* Check for entry in dtb */
• node = dt_find_compatible_node(NULL, NULL, "linaro,optee-tz");
• if ( !node )

•    return false;
  

• /* Check UID */
• arm_smccc_smc(ARM_SMCCC_CALL_UID_FID(TRUSTED_OS_END), &resp);
• if ( resp.a0 != OPTEE_MSG_UID_0 ||

•     resp.a1 != OPTEE_MSG_UID_1 ||
  

•     resp.a2 != OPTEE_MSG_UID_2 ||
  

•     resp.a3 != OPTEE_MSG_UID_3 )
  

I would be extra cautious with the sign-extension here. It would be better to follow the spec regarding UUID by casting resp.a0 & co to 32-bit.

•    return false;
  

• return true;

+}

+static int optee_enable(struct domain *d) +{

• struct arm_smccc_res resp;
• arm_smccc_smc(OPTEE_SMC_VM_CREATED, d->domain_id + 1, 0, 0, 0, 0, 0, 0,

•              &resp);
  

• if ( resp.a0 != OPTEE_SMC_RETURN_OK ) {

•    gprintk(XENLOG_WARNING, "OP-TEE don't want to support domain: %d\n",
  

This message is slightly odd. It would be better to say:

"Unable to create OP-TEE client: rc=%d\n".

•            (uint32_t)resp.a0);
  

•    return -ENODEV;
  

• }
• return 0;

+}

+static void forward_call(struct cpu_user_regs *regs) +{

• struct arm_smccc_res resp;
• arm_smccc_smc(get_user_reg(regs, 0),

•              get_user_reg(regs, 1),
  

•              get_user_reg(regs, 2),
  

•              get_user_reg(regs, 3),
  

•              get_user_reg(regs, 4),
  

•              get_user_reg(regs, 5),
  

•              get_user_reg(regs, 6),
  

•              /* client id 0 is reserved for hypervisor itself */
  

•              current->domain->domain_id + 1,
  

I would prefer if the client ID is encoded in a macro. So this could be re-used.

•              &resp);
  

• set_user_reg(regs, 0, resp.a0);
• set_user_reg(regs, 1, resp.a1);
• set_user_reg(regs, 2, resp.a2);
• set_user_reg(regs, 3, resp.a3);
• set_user_reg(regs, 4, 0);
• set_user_reg(regs, 5, 0);
• set_user_reg(regs, 6, 0);
• set_user_reg(regs, 7, 0);

+}

+static void optee_domain_destroy(struct domain *d) +{

• struct arm_smccc_res resp;
• /* At this time all domain VCPUs should be stopped */
• /* Inform OP-TEE that domain is shutting down */
• arm_smccc_smc(OPTEE_SMC_VM_DESTROYED, d->domain_id + 1, 0, 0, 0, 0, 0, 0,

•              &resp);
  

So this SMC should never fail or even get preempted? I was kind of expecting that it may time some time to destroy a domain.

+}

+static bool optee_handle_call(struct cpu_user_regs *regs) +{

• switch ( get_user_reg(regs, 0) )
• {
• case OPTEE_SMC_CALLS_COUNT:
• case OPTEE_SMC_CALLS_UID:
• case OPTEE_SMC_CALLS_REVISION:
• case OPTEE_SMC_CALL_GET_OS_UUID:
• case OPTEE_SMC_FUNCID_GET_OS_REVISION:
• case OPTEE_SMC_ENABLE_SHM_CACHE:
• case OPTEE_SMC_DISABLE_SHM_CACHE:
• case OPTEE_SMC_GET_SHM_CONFIG:
• case OPTEE_SMC_EXCHANGE_CAPABILITIES:
• case OPTEE_SMC_CALL_WITH_ARG:
• case OPTEE_SMC_CALL_RETURN_FROM_RPC:

•    forward_call(regs);
  

•    return true;
  

• default:

•    return false;
  

• }

+}

+static const struct tee_mediator_ops optee_ops = +{

• .probe = optee_probe,
• .enable = optee_enable,
• .domain_destroy = optee_domain_destroy,
• .handle_call = optee_handle_call,

+};

+REGISTER_TEE_MEDIATOR(optee, "OP-TEE", &optee_ops);

+/*

• • Local variables:
• • mode: C
• • c-file-style: "BSD"
• • c-basic-offset: 4
• • indent-tabs-mode: nil
• • End:
• */

diff --git a/xen/include/asm-arm/tee/optee_smc.h b/xen/include/asm-arm/tee/optee_smc.h index 26d100e..1c5a247 100644 --- a/xen/include/asm-arm/tee/optee_smc.h +++ b/xen/include/asm-arm/tee/optee_smc.h @@ -305,6 +305,56 @@ struct optee_smc_disable_shm_cache_result { OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_ENABLE_SHM_CACHE) /*

• • Inform OP-TEE about a new virtual machine
• • Hypervisor issues this call during virtual machine (guest) creation.
• • OP-TEE records VM_ID of new virtual machine and makes self ready
• • to receive requests from it.
• • Call requests usage:
• • a0 SMC Function ID, OPTEE_SMC_VM_CREATED
• • a1 VM_ID of newly created virtual machine
• • a2-6 Not used
• • a7 Hypervisor Client ID register. Must be 0, because only hypervisor

• •  can issue this call
    

• • Normal return register usage:
• • a0 OPTEE_SMC_RETURN_OK
• • a1-7 Preserved
• • Error return:
• • a0 OPTEE_SMC_RETURN_ENOTAVAIL OP-TEE has no resources for

• • 			another VM
    

• • a1-7 Preserved
• */

+#define OPTEE_SMC_FUNCID_VM_CREATED 13 +#define OPTEE_SMC_VM_CREATED \

• OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_VM_CREATED)

+/*

• • Inform OP-TEE about shutdown of a virtual machine
• • Hypervisor issues this call during virtual machine (guest) destruction.
• • OP-TEE will clean up all resources associated with this VM.
• • Call requests usage:
• • a0 SMC Function ID, OPTEE_SMC_VM_DESTROYED
• • a1 VM_ID of virtual machine being shutted down
• • a2-6 Not used
• • a7 Hypervisor Client ID register. Must be 0, because only hypervisor

• •  can issue this call
    

• • Normal return register usage:
• • a0 OPTEE_SMC_RETURN_OK
• • a1-7 Preserved
• */

+#define OPTEE_SMC_FUNCID_VM_DESTROYED 14 +#define OPTEE_SMC_VM_DESTROYED \

• OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_VM_DESTROYED)

+/*

• Resume from RPC (for example after processing a foreign interrupt)
• Call register usage:

Cheers,