[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

 


Rackspace

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