Hi Volodymyr,
On 09/11/2018 08:30 PM, Volodymyr Babchuk wrote:
On 11.09.18 14:53, Julien Grall wrote:
On 10/09/18 18:44, Volodymyr Babchuk wrote:
On 10.09.18 16:01, Julien Grall wrote:
On 03/09/18 17:54, Volodymyr Babchuk wrote:
OP-TEE usually uses the same idea with command buffers (see previous commit) to issue RPC requests. Problem is that initially it has no buffer, where it can write request. So the first RPC request it makes is special: it requests NW to allocate shared buffer for other RPC requests. Usually this buffer is allocated only once for every OP-TEE thread and it remains allocated all the time until shutdown.
Mediator needs to pin this buffer(s) to make sure that domain can't transfer it to someone else. Also it should be mapped into XEN address space, because mediator needs to check responses from guests.
Can you explain why you always need to keep the shared buffer mapped in Xen? Why not using access_guest_memory_by_ipa every time you want to get information from the guest?
Sorry, I just didn't know about this mechanism. But for performance reasons, I'd like to keep this buffers always mapped. You see, RPC returns are very frequent (for every IRQ, actually). So I think, it will be costly to map/unmap this buffer every time.
This is a bit misleading... This copy will *only* happen for IRQ during an RPC. What are the chances for that? Fairly limited. If this is happening too often, then the map/unmap here will be your least concern.
Now, this copy will happen for every IRQ when CPU is in S-EL1/S-EL0 mode. Chances are quite high, I must say. Look: OP-TEE or (TA) is doing something, like encrypting some buffer, for example. IRQ fires, OP-TEE immediately executes RPC return (right from interrupt handler), so NW can handle interrupt. Then NW returns control back to OP-TEE, if it wants to.
I understand this... But the map/unmap should be negligible over the rest of the context.
This is how long job in OP-TEE can be preempted by linux kernel, for example. Timer IRQ ensures that control will be returned to linux, scheduler schedules some other task and OP-TEE patiently waits until its caller is scheduled back, so it can resume the work.
However, I would like to see any performance comparison here to weight with the memory impact in Xen (Arm32 have limited amount of VA available).
With current configuration, this is maximum 16 pages per guest. As for performance comparison... This is doable, but will take some time.
Let me write it differently, I will always chose the safe side until this is strictly necessary or performance has been proven. I might be convinced for just 16 pages, although it feels like a premature optimization...
It feels quite suspicious to free the memory in Xen before calling OP-TEE. I think this need to be done afterwards.
No, it is OP-TEE asked to free buffer. This function is called, when NW returns from the RPC. So at this moment NW freed the buffer.
But you forward that call to OP-TEE after. So what would OP-TEE do with that?
Happily resume interrupted work. There is how RPC works:
- NW client issues STD call (or yielding call in terms of SMCCC)
- OP-TEE starts its work, but it is needed to be interrupted for some
reason: IRQ arrived, it wants to block on a mutex, it asks NW to do some work (like allocating memory or loading TA). This is called "RPC return". 3. OP-TEE suspends thread and does return from SMC call with code OPTEE_SMC_RPC_VAL(SOME_CMD) in a0, and some optional parameters in other registers 4. NW sees that this is a RPC, and not completed STD call, so it does SOME_CMD and issues another SMC with code OPTEE_SMC_CALL_RETURN_FROM_RPC in a0 5. OP-TEE wakes up suspended thread and continues execution 6. pts 2-5 are repeated until OP-TEE finishes the work 7. It returns from last SMC call with code OPTEE_SMC_RETURN_SUCCESS/ OPTEE_SMC_RETURN_some_error in a0. 8. optee driver sees that call from pt.1 is finished at least and returns control back to client
Thank you for the explanation. As I mentioned in another thread, it would be good to have some kind of highly level explanation in the tree and all those interaction. If it is already existing, then pointer in the code.
Looking at that code, I just noticed there potential race condition here. Nothing prevent a guest to call twice with the same optee_thread_id.
OP-TEE has internal check against this.
I am not sure how OP-TEE internal check would help here. The user may know that thread-id 1 exist and will call it from 2 vCPUs concurrently.
So handle_rpc will find a context associated to it and use it for execute_std_call. If OP-TEE return an error (or is done with it), you will end up to free twice the same context.
Did I miss anything?
So it would be possible for two vCPU to call concurrently the same command and free it.
Maybe you noticed that mediator uses shadow buffer to read cookie id.
I am not speaking about the cookie id but the thread_id... However this statement is wrong in the context we are discussing. Both thread_id and cookie are read from the guest registers.
So
it will free the buffer mentioned by OP-TEE. Basically what happened:
- OP-TEE asks "free buffer with cookie X" in RPC return
- guests says "I freed that buffer" in SMC call
- mediator frees buffer with cookie X on its side
In this particular order.
Cheers,