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

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 09/05/2018 02:38 PM, Volodymyr Babchuk wrote:
> 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).
Resources shared with other domain have a counter. The function 
domain_relinquish_resources will not block because resource have not 
been unshared for that domain.
Instead, reference on the resource will be dropped. The domain structure 
will get released when all the resource was released (see put_domain()).
The preemption is only here because it can take time to release 
everything (any Xen atomic action should be under few ms).
> 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.
Usually public interface are designed to be robust and future-proof 
because it is a pain to update them. It does not need to be fully 
implemented in the callee side. If OP-TEE folks thinks a fast call is ok 
and it would be easy to upgrade, then I can deal with it.
But please write that in the commit message (and potentially in the 
code). So we keep track what was the reason behind that choice.
Cheers,

--
Julien Grall

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