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

[Julien Grall <julien.grall@arm.com>]

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.

Cheers,

-- 
Julien Grall

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