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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel