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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 13/07/2023 08:02, Jens Wiklander wrote:
> > > +}
> > > +
> > >    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);
> > NIT: I would consider to check if the callback is NULL. This would avoid
> > providing dummy helper.
> Yes, that's an advantage, but we'd treat this callback differently
> from others. 
I haven't checked the rest of the callbacks. But I would argue that any 
callback that are expected to be optional, should have an if 
(...->ops->fn). This reducing the amount of dummy helper (4 lines) each 
times.
> I'd prefer to keep this as it is if you don't mind.
I am Ok with that. Once FFA is merged, I will try to remember to go 
through the callback and there the empty ones.
Cheers,

--
Julien Grall