Re: [PATCH v2 06/13] optee: add domain contexts

[Julien Grall <julien.grall@arm.com>]

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.

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".

With that, most of the code in this patch will become unnecessary. So 
you potentially can fold it into something else.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel