[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


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Wed, 24 May 2023 15:23:20 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=XqxQ53YTx85Spgy8Kdqm1nKEC8oNCRb3nyrUPJpb73U=; b=DZoVXSO4f64ytiMKm47CUIaLCL21v4QwVqeml8WGo7PC4zWq8cKi7F8Jza26K/cdsJSe1bU2BuoAO7+n1yC+52vwMleJLNaL81r0kKgua6ZOZsf9G+Fvba+hfZ+IbZh2ZlyqZIRMY9TCD58pRI3LKOm/DhhRnT7Yk7Mu/wx9q3tr2UTSByrouv1D55Ietr2iMgLIE6/YYaz6yQhGx787nywVpj1fHr++SF+Lu/59l7EShLa3hVedHl26cGiqjFBJiXWt8JT9puXTV/6h2SUcaAMqcflALkYmQtltPMND/1PctGAUPcoN3VeIkw2VSmYvAlzEkdwAYvLN9HNxNQmcTA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hV+1iMMDCBDB1+rE21Ti1172zPl+mNV5+6ZcAQmp+qC6n/TOc6fk+eOp5+QnKGT58OiLz9FbycgGA0U93T8P5h2AEifrgk+EmBsJyEe7qZ/tdsbKa76b4UeqWjHBi966DX++ZCuUzR408IBrTP5tzJs+rIDkRd2PvqCmT/wxLX0CWRxWL/OLGWEcU8JEujgYkt3n3AwpTyL7RwHt3wk+yQbi9XxpVs/gFs1ustzywaJzTJgB9qEQ5840keCdx2rtP6o2hZ6llR+qkD6XIPUQeOiVrBvq3ebXQpqXE90VQVtpKJ0dmbgo9yXN5CbFng/OTDh/5Bizq3/FaOt8dxzuGQ==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Julien Grall <julien@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Marc Bonnici <Marc.Bonnici@xxxxxxx>, Achin Gupta <Achin.Gupta@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Jens Wiklander <jens.wiklander@xxxxxxxxxx>
  • Delivery-date: Wed, 24 May 2023 15:23:45 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHZbdfHy/Y46nL9n0ej3opYe7XQ5q8pKpqAgAAQ9oCAAUdZgIAftheAgB+SnAA=
  • Thread-topic: [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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.