[+Volodymyr]
On Mon, Jun 19, 2017 at 4:29 PM, Stuart Yoder stuart.yoder@arm.com wrote:
On 6/19/17 2:24 AM, Jens Wiklander wrote:
Hi,
On Sat, Jun 17, 2017 at 5:18 PM, Stuart Yoder stuart.yoder@arm.com wrote:
On 6/16/17 7:31 PM, Thomas M Zeng wrote:
Hi Jens and Stuart:
#define OPTEE_SMC_SEC_CAP_DYNAMIC_SHM vs. UNRESITERED_SHM (1 << 1)
The topic of managing RichOS-and-TEE shared memory for *use case* scalability is of interest to Qualcomm as well. We might want to reserve "DYNAMIC_SHM" to mean that additional contiguous physical memory regions can be runtime registered / deregistered with Secure World, and the CA's can suballocate from these, just like they suballocate from the initial 1MB of carveout.
Meanwhile, the SMC_SEC_CAP_UNREGISTERED_SHM flag might be a better name to communicate the semantics that the Secure World, as a matter of policy, allows any NS memory to be mapped to TAs without restrictions.
That is to say, the semantics of SMC_CAP_DYNAMIC_REGISTER_SHM might better off set aside for future, when, post boot, some contiguous NS=1 regions can be added to the SHM pool, in order to lift the limitation of the initial 1MB SHM pool.
Granted it requires the addition of new SMC commands. That is why I chose the word, “future”.
Thoughts?
What I object to is using the word "register" in the same capabilities list in 2 completely different ways.
The capability OPTEE_SMC_SEC_CAP_CMD_REGISTER_SHM means-- op-tee supports the register/unregister client API commands. It allows a client app to "register" a buffer with op-tee.
OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM means as you suggest-- that op-tee allows any NS memory to be mapped.
So in order to support "REGISTER_SHM" op-tee must support "UNREGISTERED_SHM". That is seriously confusing.
We could try to be very explicit. Instead of "dynamic":
OPTEE_SMC_SEC_CAP_CAN_MAP_ALL_NS_MEMI would be happy with that.
Both OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM and OPTEE_SMC_SEC_CAP_REGISTER_SHM is actually not needed.
How about:
/*
- Secure world supports shared memory objects to represent
- physically non-contiguous memory and the commands
- "register/unregister shared memory object" for more efficient
- communication via physically non-contiguous memory.
*/ #define OPTEE_SMC_SEC_CAP_NONCONTIG_SHM (1 << 1)
Sounds fine.
But, if I understand correctly Volodymyr is working on a separate patch to add new "REGISTER/UNREGISTER" commands:
#define OPTEE_MSG_CMD_OPEN_SESSION 0 #define OPTEE_MSG_CMD_INVOKE_COMMAND 1 #define OPTEE_MSG_CMD_CLOSE_SESSION 2 #define OPTEE_MSG_CMD_CANCEL 3
- #define OPTEE_MSG_CMD_REGISTER_SHM 4
- #define OPTEE_MSG_CMD_UNREGISTER_SHM 5
You don't think we need a new capability to specify whether OP-TEE will support those new commands or not? Is supporting non-contiguous memory really the same thing as supporting CMD_REGISTER_SHM/CMD_UNREGISTER_SHM?
Yes, in practice that's the case because those functions should be introduced at the same time. Wouldn't you agree Volodymyr?
Thanks, Jens