[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 Julien,

On 18.09.18 19:50, Julien Grall wrote:
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.

[...]


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?

It is more complex. GP TEE API defines that there is 4 parameters maximum can be passed to TA in single call. OP-TEE can add additional, so called "meta" parameters, for own needs. Also, there is OP-TEE calls that does not map to GP API calls. So, I need to review this
piece of code. Probably, I'll add some dynamic structure to handle
used lists of pages.


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.

Thank you for the tip, this is good point. I'll extend commit messages to cover topics we discussed.

--
Volodymyr Babchuk

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