On 09/05/2018 03:23 PM, Volodymyr Babchuk wrote:
Hi,
Hi,
On 05.09.18 17:10, Julien Grall wrote:
Hi,
On 09/03/2018 05:54 PM, Volodymyr Babchuk wrote:
OP-TEE meditor needs to store per-domain context, as will be seen
s/meditor/mediator/
in the next patches. At this moment it stores only reference to domain.
This allows us to filter out calls from domains that are not allowed to work with OP-TEE.
Signed-off-by: Volodymyr Babchuk volodymyr_babchuk@epam.com
xen/arch/arm/tee/optee.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c index 48bff5d..c895a99 100644 --- a/xen/arch/arm/tee/optee.c +++ b/xen/arch/arm/tee/optee.c @@ -19,6 +19,14 @@ #include <asm/tee/optee_msg.h> #include <asm/tee/optee_smc.h> +struct domain_ctx {
I would prefer if the structure has optee in its name such as optee_domain.
It is supposed to be internal for this file, so I chose this name. But I can rename it, no problem.
Even if it is internal, the structure will appear in debug symbols. If you use tools like pahole it will be quite difficult to know what the structure is for without a grep.
We also tend to abbreviate context in Xen using the ctxt.
+ struct list_head list; + struct domain *domain; +};
+static LIST_HEAD(domain_ctx_list); +static DEFINE_SPINLOCK(domain_ctx_list_lock);
static bool optee_probe(void) { struct dt_device_node *node; @@ -41,18 +49,49 @@ static bool optee_probe(void) return true; } +static struct domain_ctx *find_domain_ctx(struct domain* d) +{ + struct domain_ctx *ctx;
+ spin_lock(&domain_ctx_list_lock);
+ list_for_each_entry( ctx, &domain_ctx_list, list ) + { + if ( ctx->domain == d ) + { + spin_unlock(&domain_ctx_list_lock); + return ctx; + } + }
+ spin_unlock(&domain_ctx_list_lock); + return NULL; +}
Why do you need to store the context in a list? As you have a context per domain it would be better to store that directly in struct domain by introduce a field "void * tee".
Yes, this was my first intention. But I was unsure on if it is appropriate to add driver-specific fields to struct arch_domain. If you sat that it is okay (under #ifdef CONFIG_TEE), then I'll do in this way.
I would definitely prefer the "driver-specific" fields. This is likely going to be useful for other TEE as well.
Cheers,