[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 04/13] optee: add OP-TEE mediator skeleton
Hi Julien,
On 03.09.18 20:38, Julien Grall wrote:
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? :).
I tried couple of time, but with no success :)
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.
Some time ago you said that I can't be sure that Dom0 is 1:1 mapped,
because of grant refs. So, actually, any access should be forbidden. I
can omit
REGISTER_TEE_MEDIATOR(optee, "OP-TEE", &optee_ops);
till the very last patch in the series.
But then it would not compile, because optee_ops is the static struct...
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
---
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@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>
+
+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_WARNIREGISTER_TEE_MEDIATOR(optee, "OP-TEE", &optee_ops);NG, "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
Something like
#define OPTEE_CLIENT_ID(domain) (domain->domain_id + 1)
?
+ &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.
It is the fast SMCCC call, so it can't be preempted. And it is really
fast, at lest in OP-TEE case. Basically, OP-TEE just removes some
entries from the list and frees some memory. And yes, it can't fail.
+}
+
+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,
--
Volodymyr Babchuk
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
