Refactor OP-TEE message types and add RPC args support#664
Refactor OP-TEE message types and add RPC args support#664
Conversation
275c28f to
e2e91e2
Compare
wdcui
left a comment
There was a problem hiding this comment.
Overall the code looks good to me. I left some minor comments below.
litebox_common_optee/src/lib.rs
Outdated
| } | ||
| } | ||
| pub use workaround_identity_op_suppression::UteeParamsTypes; | ||
| /// Number of RPC parameters that LiteBox defined and reported to the normal-world |
|
|
||
| /// `OPTEE_MSG_RPC_CMD_*` from `optee_os/core/include/optee_msg.h` | ||
| /// | ||
| /// These are the command IDs used in the `cmd` field of the RPC `optee_msg_arg`. |
There was a problem hiding this comment.
The comment here overlaps quite a bit with the comment above?
litebox_common_optee/src/lib.rs
Outdated
| impl OpteeMsgAttr { | ||
| /// Returns the attribute type (bits 0–7). | ||
| #[allow(clippy::cast_possible_truncation)] | ||
| pub fn typ(&self) -> u8 { |
There was a problem hiding this comment.
OPTEE calls this typ instead of type?
There was a problem hiding this comment.
OP-TEE calls it type but type is a reserved keyword in Rust.
There was a problem hiding this comment.
Can we use a longer name like "xxx_type"?
litebox_common_optee/src/lib.rs
Outdated
| /// Maximum number of parameters that `OpteeMsgArgs` can hold. | ||
| /// | ||
| /// This is `TEE_NUM_PARAMS + 2` = 6, matching the Linux driver's `MAX_ARG_PARAM_COUNT`. | ||
| pub const MAX_PARAMS: usize = TEE_NUM_PARAMS + 2; |
There was a problem hiding this comment.
NIT: MAX_PARAMS is public and sounds very general. Maybe TEE_MAX_NUMBER_PARAME`?
There was a problem hiding this comment.
This is a member constant of OpteeMsgArgs. Can rename it if it looks too general.
There was a problem hiding this comment.
It may help to make the name more specific.
There was a problem hiding this comment.
MAX_ARG_PARAM_COUNT? This is what OP-TEE driver is using.
litebox_common_optee/src/lib.rs
Outdated
| /// Maximum number of RPC parameters this struct can hold. | ||
| /// | ||
| /// This is [`NUM_RPC_PARAMS`], the count negotiated during `EXCHANGE_CAPABILITIES`. | ||
| pub const MAX_PARAMS: usize = NUM_RPC_PARAMS; |
There was a problem hiding this comment.
same. this is a member constant of OpteeRpcArgs.
litebox_common_optee/src/lib.rs
Outdated
| if num > Self::MAX_PARAMS { | ||
| return Err(OpteeSmcReturnCode::EBadCmd); | ||
| } | ||
| let needed = num * size_of::<OpteeMsgParam>(); |
There was a problem hiding this comment.
Do we need to include the header size here?
There was a problem hiding this comment.
This variable checks whether the length of new_params is enough to store parameters. Let me rename the variable name.
| // Compute copy size from known-good upper bounds — no untrusted data involved. | ||
| let main_max = optee_msg_arg_total_size(OpteeMsgArgs::MAX_PARAMS.truncate()); | ||
| let copy_size = if has_rpc_arg { | ||
| main_max + optee_msg_arg_total_size(NUM_RPC_PARAMS.truncate()) |
There was a problem hiding this comment.
why do use NUM_RPC_PARAMS here instead of MAX_PARAMS?
There was a problem hiding this comment.
Let me use OpteeRpcArgs::MAX_PARAMS instead of NUM_RPC_PARAMS.
002a576 to
28d0a36
Compare
|
🤖 SemverChecks 🤖 Click for details |
This PR refactors the OP-TEE message types and related functions to handle variable-length parameters. It also supports OP-TEE RPC message types to enable future RPC implementation. In addition, it leverages
zerocopyto remove someunsafeoperations.#641