Re: [PATCH v2 04/13] optee: add OP-TEE mediator skeleton

[Volodymyr Babchuk <volodymyr_babchuk@epam.com>]

Hi,

On 05.09.18 16:16, Julien Grall wrote:
> Hi,
> 
> On 09/05/2018 01:17 PM, Volodymyr Babchuk wrote:
>> On 04.09.18 22:48, Julien Grall wrote:
>>>
>>>
>>> On 09/03/2018 06:55 PM, Volodymyr Babchuk wrote:
>>>> Hi Julien,
>>>
>>> Hi Volodymyr,
>>>
>>>> On 03.09.18 20:38, Julien Grall wrote:
>>>>> Hi Volodymyr,
>>>>>
>>>>> On 03/09/18 17:54, Volodymyr Babchuk wrote:
>>>>>> Add very basic OP-TEE mediator. It can probe for OP-TEE presence,
>>>>>> tell it about domain creation/destuction and forward all known
>>>>>
>>>>> s/destuction/destruction/
>>>>>
>>>>>> calls.
>>>>>>
>>>>>> This is all what is needed for Dom0 to work with OP-TEE as long
>>>>>> as Dom0 shares 1:1 mapped pages with OP-TEE. Any attempt to call
>>>>>> OP-TEE from DomU will fail and can lead to spectacular results.
>>>>>
>>>>> Shall we expect fireworks? :).
>>>> I tried couple of time, but with no success :)
>>>>
>>>>> Anyway, I think this is a call for forbidding DomU access until it 
>>>>> is supported. This also has the benefits to allow merging Dom0 
>>>>> support for OP-TEE without the rest.
>>>> Some time ago you said that I can't be sure that Dom0 is 1:1 mapped, 
>>>> because of grant refs. So, actually, any access should be forbidden. 
>>>> I can omit
>>>
>>> Oh right. I that case, make it clear in the commit message because 
>>> there are nothing in Dom0 preventing to share page that are not 
>>> direct mapped.
>>
>>> This will make easier for the commiter (either Stefano or I) to know 
>>> this can't go without the rest of the series.
>>>
>> Ah, sure. Had to indicate this explicitly. Will do this in the next 
>> version of the series.
>>
>> [...]
>>>>>
>>>>>> +                  &resp);
>>>>>> +
>>>>>> +    set_user_reg(regs, 0, resp.a0);
>>>>>> +    set_user_reg(regs, 1, resp.a1);
>>>>>> +    set_user_reg(regs, 2, resp.a2);
>>>>>> +    set_user_reg(regs, 3, resp.a3);
>>>>>> +    set_user_reg(regs, 4, 0);
>>>>>> +    set_user_reg(regs, 5, 0);
>>>>>> +    set_user_reg(regs, 6, 0);
>>>>>> +    set_user_reg(regs, 7, 0);
>>>>>> +}
>>>>>> +
>>>>>> +static void optee_domain_destroy(struct domain *d)
>>>>>> +{
>>>>>> +    struct arm_smccc_res resp;
>>>>>> +
>>>>>> +    /* At this time all domain VCPUs should be stopped */
>>>>>> +
>>>>>> +    /* Inform OP-TEE that domain is shutting down */
>>>>>> +    arm_smccc_smc(OPTEE_SMC_VM_DESTROYED, d->domain_id + 1, 0, 0, 
>>>>>> 0, 0, 0, 0,
>>>>>> +                  &resp);
>>>>>
>>>>> So this SMC should never fail or even get preempted? I was kind of 
>>>>> expecting that it may time some time to destroy a domain.
>>>>
>>>> It is the fast SMCCC call, so it can't be preempted. And it is 
>>>> really fast, at lest in OP-TEE case.
>>>
>>> Without number, I can't really know what fast means here. Do you have 
>>> a rough idea?
>> "Fast" used there in a sense, defined in SMCCC:
>>
>> "
>> Fast Calls execute atomic operations.
>>
>> The call appears to be atomic from the perspective of the calling PE, 
>> and returns when the requested
>> operation has completed."
>>
>> And "Standard Call" (will be introduced in the series) is the 
>> "Yielding Call". Probably I should use term from SMCCC, but for some 
>> reason I stick to term used it OP-TEE.
> 
> I don't mind which term you used as long as you clearly define them 
> within your series.
> 
>>
>> I can do some measurements on how "fast" this particular call is. But 
>> problem is that it is really atomic from OP-TEE perspective.
> 
> My concern is usually such operation can take time. In the context of 
> Xen, the domain destruction path is preemptible which allow a domain to 
> be rescheduled.
> 
> While today, SMC_VM_DESTROYED is a fast call because there are not much. 
> I am a bit concern that in the future this may grow and therefore turn 
> into a long operation (i.e few ms).
> 
> So I am a bit concerned that this interface is not future-proof.

I think, this is broader issue. I.e. the same is true for any calls to 
firmware.

Domain destruction path in XEN is preemptible because it deals with a 
lots of resources and some resources can be blocked right now, because 
they can be shared with other domains (at least, it is my understanding).

Current model of virtualization in OP-TEE is much simpler and domain 
destruction is truly atomic thing. It is really just couple of calls to 
free().

I can't tell what will be in the future, but I see some options how to 
overcome newly discovered issues, if they will ever rise. But I can tell 
that right now there is no much sense in introducing yielding calls for 
domain destruction/creation. This would require significant changes in 
OP-TEE core without any visible benefit.


-- 
Volodymyr Babchuk

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