+tee-dev
On Mon, Jun 12, 2017 at 10:11 AM, Jens Wiklander jens.wiklander@linaro.org wrote:
Hi Stuart,
On Sat, Jun 10, 2017 at 12:12 AM, Stuart Yoder stuart.yoder@arm.com wrote:
+Volodymyr (correcting corrupted email address)
On 6/9/17 11:51 AM, Stuart Yoder wrote:
All,
I've been reviewing and trying out Volodymyr's patches related to dynamic shared memory.
One thing I found thoroughly confusing was that in the same capabilities list we use the word "register" in two completely different ways.
/* Secure world can communicate via previously unregistered shared memory */ #define OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM (1 << 1) /* Secure world supporst commands "register/unregister shared memory" */ #define OPTEE_SMC_SEC_CAP_REGISTER_SHM (1 << 2)
As far as I can tell "unregistered shared memory" simply means "dynamic shared memory" vs a pre-allocated static region.
It would be much clearer if we tweaked the names:
/* Secure world can communicate via previously unregistered shared memory */ #define OPTEE_SMC_SEC_CAP_DYNAMIC_SHM (1 << 1) /* Secure world supporst commands "register/unregister shared memory" */ #define OPTEE_SMC_SEC_CAP_CMD_REGISTER_SHM (1 << 2)
Are you open to that change?
I agree, what you're proposing is more clear. It's not an ABI change, only how it's documented. Since we already have OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM defined on the (optee_os) master branch we do the change there via a pull request.
If possible please create a pull request at github.
Thanks, Jens
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?
Thanks, -thomas
-----Original Message----- From: Tee-dev [mailto:tee-dev-bounces@lists.linaro.org] On Behalf Of Jens Wiklander Sent: Monday, June 12, 2017 1:12 AM To: Stuart Yoder stuart.yoder@arm.com Cc: tee-dev@lists.linaro.org Subject: Re: [Tee-dev] comment about "unregistered" vs "register" shared memory
+tee-dev
On Mon, Jun 12, 2017 at 10:11 AM, Jens Wiklander jens.wiklander@linaro.org wrote:
Hi Stuart,
On Sat, Jun 10, 2017 at 12:12 AM, Stuart Yoder stuart.yoder@arm.com wrote:
+Volodymyr (correcting corrupted email address)
On 6/9/17 11:51 AM, Stuart Yoder wrote:
All,
I've been reviewing and trying out Volodymyr's patches related to dynamic shared memory.
One thing I found thoroughly confusing was that in the same capabilities list we use the word "register" in two completely different ways.
/* Secure world can communicate via previously unregistered shared memory */ #define OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM (1 << 1) /* Secure world supporst commands "register/unregister shared memory" */ #define OPTEE_SMC_SEC_CAP_REGISTER_SHM (1 << 2)
As far as I can tell "unregistered shared memory" simply means "dynamic shared memory" vs a pre-allocated static region.
It would be much clearer if we tweaked the names:
/* Secure world can communicate via previously unregistered shared memory */ #define OPTEE_SMC_SEC_CAP_DYNAMIC_SHM (1 << 1) /* Secure world supporst commands "register/unregister shared memory" */ #define OPTEE_SMC_SEC_CAP_CMD_REGISTER_SHM (1 << 2)
Are you open to that change?
I agree, what you're proposing is more clear. It's not an ABI change, only how it's documented. Since we already have OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM defined on the (optee_os) master branch we do the change there via a pull request.
If possible please create a pull request at github.
Thanks, Jens
_______________________________________________ Tee-dev mailing list Tee-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/tee-dev
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_MEM
I would be happy with that.
Stuart
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_MEM
I 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)
If there's ever an option to register additional shared memory pools:
/* Secure world supports commands "register/unregister shared memory pool" */ #define OPTEE_SMC_SEC_CAP_CMD_REGISTER_SHM_POOL (1 << 2)
But let's wait with the latter until it's really needed.
Thanks, Jens
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?
Thanks, Stuart
[+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
Hi Jens,
#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?
Actually, I planed to use two caps:
- one to indicate that REE can use register/unregister SHM commands and registered memory references - another one to indicate that REE can allocate command buffers from own memory.
But those two capabilities are so entangled with each other, so I'll be very glad to merge them in one, if there are no other objections.
Hi Volodymyr,
This is somewhat slippery so to eliminate potential misunderstandings, can you please clarify a few things?
When you said:
> - another one to indicate that REE can allocate command buffers from own memory.
1. By REE do you mean one of the several Guest VMs in the NS=1 world, or just Linux as the only Guest VM in the NS=1 world? I recall you said in the past that you work on the Automotive technology with some virtual machine hypervisor requirement, that is why I wanted to understand whether your NS=1 configurations might require a Multi VM flavor or not.
2. Do you mean that REE can use command buffers in SMC messages to a TEE that is not from "tee_shm_alloc()"?
3. If 2 is true, do you mean to use the CAPABILITY flag to signal the policy stating that the TAs in the NS=0 world are allowed by SEL1 to map any memory pages in the NS=1 world?
Cheers, -thomas
-----Original Message----- From: Volodymyr Babchuk [mailto:vlad.babchuk@gmail.com] Sent: Monday, June 19, 2017 7:56 AM To: Jens Wiklander jens.wiklander@linaro.org Cc: Stuart Yoder stuart.yoder@arm.com; Thomas M Zeng tzeng@codeaurora.org; tee-dev@lists.linaro.org; Zhen Kong zkong@quicinc.com Subject: Re: [Tee-dev] comment about "unregistered" vs "register" shared memory
Hi Jens,
#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?
Actually, I planed to use two caps:
- one to indicate that REE can use register/unregister SHM commands and registered memory references - another one to indicate that REE can allocate command buffers from own memory.
But those two capabilities are so entangled with each other, so I'll be very glad to merge them in one, if there are no other objections.
-- WBR Volodymyr Babchuk aka lorc [+380976646013] mailto: vlad.babchuk@gmail.com
-
Jens Wiklander -
Stuart Yoder -
Thomas M Zeng -
Volodymyr Babchuk