[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 03/10] xen/arm: optee: add OP-TEE mediator skeleton
Hi Volodymyr,
On 3/15/19 7:00 PM, Volodymyr Babchuk wrote:
+ */
+
+#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>
+
+#define OPTEE_ENABLED ((void*)0x1)
Please don't do that. Introduce a dummy structure instead and fill it
up when needed.
It will be removed in the patch #5. But probably yes, I can
create empty optee_domain structure in this patch.
I know it will be removed in patch #5. This is exactly why I don't want
to see such things at the first place.
+
+/* Client ID 0 is reserved for hypervisor itself */
s/for/for the/
+#define OPTEE_CLIENT_ID(domain) ((domain)->domain_id + 1)
+
+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_domain_init(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.
This does not really make sense to describe Linux here. Can't you just
make the wording OS agnostic?
It was just as an example. But okay, I'll remove mention of Linux.
Xen developer may not know the Linux internals.
+ * Also, interrupts are disabled.
+ *
+ * a7 should be 0, so we can't skip last 6 parameters of arm_smccc_smc()
+ */
+ 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",
gprintk will print the current vCPU and not the domain created. This
is not very useful to know the current domain. So it would be better
to use:
printk(XENLOG_G_WARNING, "%pd: Unable to create OPTEE client: rc ...", d);
Good point, thank you.
+ (uint32_t)resp.a0);
+
+ return -ENODEV;
+ }
+
+ d->arch.tee = OPTEE_ENABLED;
+
+ 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 int optee_relinquish_resources(struct domain *d)
+{
+ return 0;
+}
+
+static void optee_domain_destroy(struct domain *d)
+{
+ struct arm_smccc_res resp;
+
+ if ( !d->arch.tee )
+ return;
+
+ /*
+ * 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.
+ * At this time all domain VCPUs should be stopped. OP-TEE
+ * relies on this.
+ *
+ * a7 should be 0, so we can't skip last 6 parameters od arm_smccc_smc()
NIT: s/od/of/
+ */
+ arm_smccc_smc(OPTEE_SMC_VM_DESTROYED, OPTEE_CLIENT_ID(d), 0, 0, 0, 0, 0, 0,
+ &resp);
Your split between domain_destroy and relinquish_resources looks
wrong. If you relinquish resources before telling OP-TEE then you are
at risk that OP-TEE will use those resources.
Instead you should first tell OP-TEE the domain is shutting down, then
release the resources.
Both this calls are supposed to be called after all guest's VCPUs are
stopped, so OP-TEE will be not able to use any resources allocated to
the domain, because OP-TEE is schedule by the Normal World. I assume,
this is true for any other TEE.
I don't know enough OP-TEE in general to be able to say how it works.
However, you can only be sure the buffers will not be used after
VM_DESTROYED is called. Before, this may be the case, but this a pretty
fragile assumption. If it does not hold, then you may end up to corrupt
the hypervisor (or another guest).
It is not a position I would like to be in. The more that it should not
require too much work to re-order the code.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
