[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v8 02/22] xen/arm: tee: add a primitive FF-A mediator
Hi Andrew, > On 4 May 2023, at 15:14, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> wrote: > > Hi Andrew, > >> On 14 Apr 2023, at 10:58, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote: >> >> Hi, >> >> On Thu, Apr 13, 2023 at 3:27 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> >> wrote: >>> >>> On 13/04/2023 1:26 pm, Julien Grall wrote: >>>>> +static int ffa_domain_init(struct domain *d) >>>>> +{ >>>>> + struct ffa_ctx *ctx; >>>>> + >>>>> + if ( !ffa_version ) >>>>> + return -ENODEV; >>>>> + >>>>> + ctx = xzalloc(struct ffa_ctx); >>>>> + if ( !ctx ) >>>>> + return -ENOMEM; >>>>> + >>>>> + d->arch.tee = ctx; >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/* This function is supposed to undo what ffa_domain_init() has done */ >>>> >>>> I think there is a problem in the TEE framework. The callback >>>> .relinquish_resources() will not be called if domain_create() failed. >>>> So this will result to a memory leak. >>>> >>>> We also can't call .relinquish_resources() on early domain creation >>>> failure because relinquishing resources can take time and therefore >>>> needs to be preemptible. >>>> >>>> So I think we need to introduce a new callback domain_free() that will >>>> be called arch_domain_destroy(). Is this something you can look at? >>> >>> >>> Cleanup of an early domain creation failure, however you do it, is at >>> most "the same amount of time again". It cannot (absent of development >>> errors) take the same indefinite time periods of time that a full >>> domain_destroy() can. >>> >>> The error path in domain_create() explicitly does call domain_teardown() >>> so we can (eventually) purge these duplicate cleanup paths. There are >>> far too many easy errors to be made which occur from having split >>> cleanup, and we have had to issue XSAs in the past to address some of >>> them. (Hence the effort to try and specifically change things, and >>> remove the ability to introduce the errors in the first place.) >>> >>> >>> Right now, it is specifically awkward to do this nicely because >>> domain_teardown() doesn't call into a suitable arch hook. >>> >>> IMO the best option here is extend domain_teardown() with an >>> arch_domain_teardown() state/hook, and wire in the TEE cleanup path into >>> this too. >>> >>> Anything else is explicitly adding to technical debt that I (or someone >>> else) is going to have to revert further down the line. >>> >>> If you want, I am happy to prototype the arch_domain_teardown() bit of >>> the fix, but I will have to defer wiring in the TEE part to someone >>> capable of testing it. >> >> You're more than welcome to prototype the fix, I can test it and add >> it to the next version of the patch set when we're happy with the >> result. > > > Could you tell us if you are still happy to work on the prototype for > arch_domain_teardown and when you would be able to give a first prototype ? Could you answer to this question ? Cheers Bertrand
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |