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

Re: [Xen-devel] [PATCH v2 10/13] optee: add support for RPC commands



Hi Volodymyr,

On 09/11/2018 07:58 PM, Volodymyr Babchuk wrote:
On 11.09.18 16:56, Julien Grall wrote:
On 10/09/18 19:14, Volodymyr Babchuk wrote:
On 10.09.18 18:34, Julien Grall wrote:
On 03/09/18 17:54, Volodymyr Babchuk wrote:
1. OP-TEE issues RPC "allocate buffer"
2. NW returns list of pages
3. Mediator translates and stores address in non_contig[x]
4. OP-TEE begins to consume this list
5. IRQ arrives and OP-TEE forced to break the work
6. Mediator receives control back, but it should not free non_contig[x],
    because it is not sure of OP-TEE finished reading from it
7. Xen/guest handles the IRQ and returns control back to OP-TEE
8. OP-TEE finishes processing this buffers and asks for another one
9. NW returns list of pages for the next buffer
10. At this point mediator is sure that OP-TEE finished processing
     old non_contig[x], so it can free it and allocated another.

Thank you for the description of the protocol. However, it is still does not explain why you decided to free MAX_NONCONTIG_ENTRIES - 1. Why not 0 or 1 or n?
Okay. You can pass up to 4 arguments for TA in command buffer. Any of that
argument can be a shared memory reference, so we need at least 4 items
in non_contig array. Sadly, this constant (TEE_NUM_PARAMS) is defined not optee_msg.h or optee_smc.h, but in tee_api_defines.h, which is user-space (or TA) part of OP-TEE.
But, now I'm thinking, that maybe it is not so bad idea to add this file
to XEN... What is your opinion?

I would just introduce TEE_NUM_PARAMS in tee.h with a comment explaining this is coming from GlobalPlatform TEE.


Anyways, we need at least 4 items for arguments. But I defined MAX_NONCONTIG_ENTRIES as 5, because last item in array is used for RPC-allocated buffer. I also added comment. I'll copy it there:

/* Last entry in non_contig array is used to hold RPC-allocated  buffer */

So, first 4 items are used for arguments and last one used for RPC requests. Right way to define MAX_NONCONTIG_ENTRIES is (TEE_NUM_PARAMS + 1), but then I should add another header file, which defines GlobalPlatform TEE core API.

If I understand correctly your e-mail, a TEE call can never have more than 4 parameters. Right? If so, should not your code mostly use TEE_NUM_PARAMS and not MAX_NONCONTIG_ENTRIES?


Overall, it feels like to me you want to write more documentation about how the mediator is supposed to work.

Looks like I need to start on how OP-TEE protocol is supposed to work...
I tried to cover this in the commit messages, but looks like it is not sufficient.

I appreciate your effort to cover that in the commit message :). I tend to update commit message over revision when there are misunderstanding on the patch.

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®.