|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 04/11] optee: add OP-TEE mediator skeleton
Hi Volodymyr,
On 12/18/18 9:11 PM, Volodymyr Babchuk wrote:
> From: Volodymyr Babchuk <vlad.babchuk@xxxxxxxxx>
>
> Add very basic OP-TEE mediator. It can probe for OP-TEE presence,
> tell it about domain creation/destruction and forward all known
> 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. Also,
> problems can arise if Dom0 uses pages mapped from other domains.
> So, this patch should not be merged without next patches in the series.
The last sentence is not necessary in the commit message. You can add it
after "---". So it is stripped down when the patch is committed, yet we
know it should not be merged :).
>
> This code issues two non-preemptible calls to OP-TEE: to create and
> to destroy client context. They can't block in OP-TEE, but OP-TEE can
> wait on a splinlocks, so there is no maximal execution time
> guaranteed.
This paragraph is worrying. From the discussion we had on the previous
version and some discussions at Linaro Connect with OP-TEE folks, I
gathered this call should not take a long time.
In general spinlock should have no contention or wait should be limited.
In particular, in the context of OP-TEE I would expect something is
really wrong if you wait on a spinlock for a really long time as you
block the normal world. So maybe this is just a misunderstanding of the
commit message?
>
> Signed-off-by: Volodymyr Babchuk <vlad.babchuk@xxxxxxxxx>
> ---
>
> Changes from v2:
> - Fixed coding style
> - Introduced tee/Kconfig
> - Fixed error messages
>
> xen/arch/arm/Kconfig | 2 +
> xen/arch/arm/tee/Kconfig | 4 +
> xen/arch/arm/tee/Makefile | 1 +
> xen/arch/arm/tee/optee.c | 151 ++++++++++++++++++++++++++++
> xen/include/asm-arm/tee/optee_smc.h | 50 +++++++++
> 5 files changed, 208 insertions(+)
> create mode 100644 xen/arch/arm/tee/Kconfig
> create mode 100644 xen/arch/arm/tee/optee.c
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index e527b2f885..99e6f0ebb2 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -237,3 +237,5 @@ source "arch/arm/platforms/Kconfig"
> source "common/Kconfig"
>
> source "drivers/Kconfig"
> +
> +source "arch/arm/tee/Kconfig"
> diff --git a/xen/arch/arm/tee/Kconfig b/xen/arch/arm/tee/Kconfig
> new file mode 100644
> index 0000000000..5b829db2e9
> --- /dev/null
> +++ 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 c54d4796ff..982c879684 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 0000000000..73ad25ee0b
> --- /dev/null
> +++ b/xen/arch/arm/tee/optee.c
> @@ -0,0 +1,151 @@
> +/*
> + * xen/arch/arm/tee/optee.c
> + *
> + * OP-TEE mediator
> + *
> + * Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> + * 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>
> +
> +/* Client ID 0 is reserved for hypervisor itself */
> +#define OPTEE_CLIENT_ID(domain) (domain->domain_id + 1)
The second 'domain' should be between () to avoid macro-expansion issue.
> +
> +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 ( (uint32_t)resp.a0 != OPTEE_MSG_UID_0 ||
> + (uint32_t)resp.a1 != OPTEE_MSG_UID_1 ||
> + (uint32_t)resp.a2 != OPTEE_MSG_UID_2 ||
> + (uint32_t)resp.a3 != OPTEE_MSG_UID_3 )
> + return false;
> +
> + return true;
> +}
> +
> +static int optee_enable(struct domain *d)
> +{
> + struct arm_smccc_res resp;
> +
> + /*
> + * Inform OP-TEE about a new guest.
> + * This is a "Fast" call in terms of OP-TEE. This basically
> + * means that it can't be preempted, because there is no
> + * thread allocated for it in OP-TEE. It is close to atomic
> + * context in linux kernel: E.g. no blocking calls can be issued.
> + * Also, interrupts are disabled.
> + * Right now OP-TEE just frees allocated memory, so it should be
> + * really fast.
The last sentence looks wrong in this context.
> + */
> + arm_smccc_smc(OPTEE_SMC_VM_CREATED, OPTEE_CLIENT_ID(d), 0, 0, 0, 0, 0, 0,
> + &resp);
> + if ( resp.a0 != OPTEE_SMC_RETURN_OK )
> + {
> + gprintk(XENLOG_WARNING, "Unable to create OPTEE client: rc = 0x%X\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),
> + OPTEE_CLIENT_ID(current->domain),
> + &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. This is
> + * also a fast SMC call, like OPTEE_SMC_VM_CREATED, so
> + * it is also non-preemptible.
> + */
> + arm_smccc_smc(OPTEE_SMC_VM_DESTROYED, OPTEE_CLIENT_ID(d), 0, 0, 0, 0, 0,
> 0,
> + &resp);
> +}
> +
> +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 26d100e215..1c5a2479e9 100644
> --- a/xen/include/asm-arm/tee/optee_smc.h
> +++ b/xen/include/asm-arm/tee/optee_smc.h
> @@ -304,6 +304,56 @@ struct optee_smc_disable_shm_cache_result {
> #define OPTEE_SMC_ENABLE_SHM_CACHE \
> 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)
> *
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |