Re: [XEN PATCH v9 02/24] xen/arm: add TEE teardown to arch_domain_teardown()

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jens,

> On 5 Jul 2023, at 11:34, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> Adds a progress state for tee_domain_teardown() to be called from
> arch_domain_teardown(). tee_domain_teardown() calls the new callback
> domain_teardown() in struct tee_mediator_ops.
> 
> An empty domain_teardown() callback is added to the OP-TEE mediator.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Co-developed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> 
> ---
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
> ---
> xen/arch/arm/domain.c              | 36 ++++++++++++++++++++++++++++++
> xen/arch/arm/include/asm/tee/tee.h |  7 ++++++
> xen/arch/arm/tee/optee.c           |  6 +++++
> xen/arch/arm/tee/tee.c             |  8 +++++++
> 4 files changed, 57 insertions(+)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 15d9709a97d2..18171decdc66 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -795,6 +795,42 @@ fail:
> 
> int arch_domain_teardown(struct domain *d)
> {
> +    int ret = 0;
> +
> +    BUG_ON(!d->is_dying);
> +
> +    /* See domain_teardown() for an explanation of all of this magic. */
> +    switch ( d->teardown.arch_val )
> +    {
> +#define PROGRESS(x)                             \
> +        d->teardown.arch_val = PROG_ ## x;      \
> +        fallthrough;                            \
> +    case PROG_ ## x
> +
> +        enum {
> +            PROG_none,
> +            PROG_tee,
> +            PROG_done,
> +        };
> +
> +    case PROG_none:
> +        BUILD_BUG_ON(PROG_none != 0);
> +
> +    PROGRESS(tee):
> +        ret = tee_domain_teardown(d);
> +        if ( ret )
> +            return ret;
> +        break;
> +
> +    PROGRESS(done):
> +        break;
> +
> +#undef PROGRESS
> +
> +    default:
> +        BUG();
> +    }
> +
>     return 0;
> }
> 
> diff --git a/xen/arch/arm/include/asm/tee/tee.h 
> b/xen/arch/arm/include/asm/tee/tee.h
> index f483986385c8..da324467e130 100644
> --- a/xen/arch/arm/include/asm/tee/tee.h
> +++ b/xen/arch/arm/include/asm/tee/tee.h
> @@ -34,6 +34,7 @@ struct tee_mediator_ops {
>      * guest and create own structures for the new domain.
>      */
>     int (*domain_init)(struct domain *d);
> +    int (*domain_teardown)(struct domain *d);
> 
>     /*
>      * Called during domain destruction to relinquish resources used
> @@ -62,6 +63,7 @@ struct tee_mediator_desc {
> 
> bool tee_handle_call(struct cpu_user_regs *regs);
> int tee_domain_init(struct domain *d, uint16_t tee_type);
> +int tee_domain_teardown(struct domain *d);
> int tee_relinquish_resources(struct domain *d);
> uint16_t tee_get_type(void);
> 
> @@ -93,6 +95,11 @@ static inline int tee_relinquish_resources(struct domain 
> *d)
>     return 0;
> }
> 
> +static inline int tee_domain_teardown(struct domain *d)
> +{
> +    return 0;
> +}
> +
> static inline uint16_t tee_get_type(void)
> {
>     return XEN_DOMCTL_CONFIG_TEE_NONE;
> diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
> index 301d205a36c5..c91bd7d5ac25 100644
> --- a/xen/arch/arm/tee/optee.c
> +++ b/xen/arch/arm/tee/optee.c
> @@ -268,6 +268,11 @@ static int optee_domain_init(struct domain *d)
>     return 0;
> }
> 
> +static int optee_domain_teardown(struct domain *d)
> +{
> +    return 0;
> +}
> +
> static uint64_t regpair_to_uint64(register_t reg0, register_t reg1)
> {
>     return ((uint64_t)reg0 << 32) | (uint32_t)reg1;
> @@ -1732,6 +1737,7 @@ static const struct tee_mediator_ops optee_ops =
> {
>     .probe = optee_probe,
>     .domain_init = optee_domain_init,
> +    .domain_teardown = optee_domain_teardown,
>     .relinquish_resources = optee_relinquish_resources,
>     .handle_call = optee_handle_call,
> };
> diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
> index 3964a8a5cddf..ddd17506a9ff 100644
> --- a/xen/arch/arm/tee/tee.c
> +++ b/xen/arch/arm/tee/tee.c
> @@ -52,6 +52,14 @@ int tee_domain_init(struct domain *d, uint16_t tee_type)
>     return cur_mediator->ops->domain_init(d);
> }
> 
> +int tee_domain_teardown(struct domain *d)
> +{
> +    if ( !cur_mediator )
> +        return 0;
> +
> +    return cur_mediator->ops->domain_teardown(d);
> +}
> +
> int tee_relinquish_resources(struct domain *d)
> {
>     if ( !cur_mediator )
> -- 
> 2.34.1
>