You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@teaclave.apache.org by "DemesneGH (via GitHub)" <gi...@apache.org> on 2025/04/08 10:44:09 UTC
[PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
DemesneGH opened a new pull request, #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178
Compared to https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/177, the Params and TaSession are re-organized and can be used in a more Rust style.
It also addressed some comments from the previous PR https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/177.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2033109561
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ ParamContent::ValueOutput { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ _ => {}
+ }
+ }
+}
+
+/// The TeeParams struct is used to manage the parameters for TEE commands.
+pub struct TeeParams<'a> {
+ params: [Param<'a>; 4],
+}
+
+impl<'a> TeeParams<'a> {
+ pub fn new() -> Self {
+ Self {
+ params: [Param::new(), Param::new(), Param::new(), Param::new()],
+ }
+ }
+
+ // Method-chaining parameter setters
+ pub fn with_memref_in(mut self, idx: ParamIndex, buffer: &'a [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn with_memref_out(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_memref_inout(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_value_in(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn with_value_out(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn with_value_inout(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub fn set_memref_in(&mut self, idx: ParamIndex, buffer: &'a [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn set_memref_out(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_memref_inout(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_value_in(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn set_value_out(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn set_value_inout(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub(crate) fn raw_param_types(&self) -> u32 {
+ let mut param_types = 0;
+ for (i, param) in self.params.iter().enumerate() {
+ param_types |= param.get_raw_type() << (i * 4);
+ }
+ param_types
+ }
+
+ pub(crate) fn as_raw(&mut self) -> [raw::TEE_Param; 4] {
+ [
+ self.params[0].as_raw(),
+ self.params[1].as_raw(),
+ self.params[2].as_raw(),
+ self.params[3].as_raw(),
+ ]
+ }
+
+ pub(crate) fn update_from_raw(&mut self, raw_params: &[raw::TEE_Param; 4]) {
+ // update the content for memref inout/out, and value inout/out
+ for (i, param) in self.params.iter_mut().enumerate() {
+ let raw_param = &raw_params[i];
+ match param.get_type() {
+ ParamType::MemrefOutput => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::MemrefInout => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::ValueInput => {
Review Comment:
This should be ValueOutput.
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ ParamContent::ValueOutput { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ _ => {}
+ }
+ }
+}
+
+/// The TeeParams struct is used to manage the parameters for TEE commands.
+pub struct TeeParams<'a> {
+ params: [Param<'a>; 4],
+}
+
+impl<'a> TeeParams<'a> {
+ pub fn new() -> Self {
+ Self {
+ params: [Param::new(), Param::new(), Param::new(), Param::new()],
+ }
+ }
+
+ // Method-chaining parameter setters
+ pub fn with_memref_in(mut self, idx: ParamIndex, buffer: &'a [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn with_memref_out(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_memref_inout(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_value_in(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn with_value_out(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn with_value_inout(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub fn set_memref_in(&mut self, idx: ParamIndex, buffer: &'a [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn set_memref_out(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_memref_inout(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_value_in(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn set_value_out(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn set_value_inout(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub(crate) fn raw_param_types(&self) -> u32 {
+ let mut param_types = 0;
+ for (i, param) in self.params.iter().enumerate() {
+ param_types |= param.get_raw_type() << (i * 4);
+ }
+ param_types
+ }
+
+ pub(crate) fn as_raw(&mut self) -> [raw::TEE_Param; 4] {
+ [
+ self.params[0].as_raw(),
+ self.params[1].as_raw(),
+ self.params[2].as_raw(),
+ self.params[3].as_raw(),
+ ]
+ }
+
+ pub(crate) fn update_from_raw(&mut self, raw_params: &[raw::TEE_Param; 4]) {
+ // update the content for memref inout/out, and value inout/out
+ for (i, param) in self.params.iter_mut().enumerate() {
+ let raw_param = &raw_params[i];
+ match param.get_type() {
+ ParamType::MemrefOutput => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::MemrefInout => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::ValueInput => {
+ param.update_value_from_raw(raw_param);
+ }
+ ParamType::ValueInout => {
+ param.update_value_from_raw(raw_param);
+ }
+ _ => {}
Review Comment:
Should we check against all param types? We can at least ensure
1. `In` types' content are never changed; if not, raise an error;
2. out size for MemRef Out/InOut types always within the original bound; if not, raise an error;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034936751
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
Review Comment:
done
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ ParamContent::ValueOutput { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ _ => {}
+ }
+ }
+}
+
+/// The TeeParams struct is used to manage the parameters for TEE commands.
+pub struct TeeParams<'a> {
+ params: [Param<'a>; 4],
+}
+
+impl<'a> TeeParams<'a> {
+ pub fn new() -> Self {
+ Self {
+ params: [Param::new(), Param::new(), Param::new(), Param::new()],
+ }
+ }
+
+ // Method-chaining parameter setters
+ pub fn with_memref_in(mut self, idx: ParamIndex, buffer: &'a [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "ivila (via GitHub)" <gi...@apache.org>.
ivila commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034329778
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ 0,
+ ptr::null_mut(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Open the session with parameters.
+ pub fn open_with_params(mut self, params: &mut TeeParams) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with any results
+ params.update_from_raw(&raw_params);
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Get the default timeout value for this session.
+ pub fn get_default_timeout(&self) -> u32 {
+ self.default_timeout
+ }
+
+ /// Invokes a command with the provided parameters using the session's default timeout.
+ /// Returns the result directly without allowing further method chaining.
+ pub fn invoke_command(&mut self, command_id: u32, params: &mut TeeParams) -> Result<()> {
+ let mut err_origin: u32 = 0;
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_InvokeTACommand(
+ self.raw,
+ self.default_timeout,
+ command_id,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with the results
+ params.update_from_raw(&raw_params);
+ trace_println!("TEE_InvokeTACommand: command_id: {} finished", command_id);
+ Ok(())
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ pub fn as_mut_raw_ptr(&mut self) -> *mut raw::TEE_TASessionHandle {
Review Comment:
Shouldn't exposed to developers, maybe `pub(crate)`?
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ 0,
+ ptr::null_mut(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Open the session with parameters.
+ pub fn open_with_params(mut self, params: &mut TeeParams) -> Result<Self> {
+ let mut err_origin: u32 = 0;
Review Comment:
The `err_origin` never return, but I think some of the developers might read it (same as the `ERROR_TARGET_DEAD` problem in teec).
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
Review Comment:
Should keep the same naming strategy with other files(like `tee_parameter.rs`).
1. `pub fn timeout(mut self)` => `pub fn set_timeout(&mut self)`
prefix with `set_`, and change `mut self` to `&mut self`
3. `pub fn with_timeout(&mut self)` => `pub fn with_timeout(mut self)`
change `&mut self` to `mut self`
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
Review Comment:
I think we should remove the `new` method and make `open` and `open_with_params` static methods instead. The current new method returns an invalid instance, and you must call `open` or `open_with_params` before doing anything useful—so it’s cleaner to just drop new altogether.
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ 0,
+ ptr::null_mut(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Open the session with parameters.
+ pub fn open_with_params(mut self, params: &mut TeeParams) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with any results
+ params.update_from_raw(&raw_params);
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Get the default timeout value for this session.
+ pub fn get_default_timeout(&self) -> u32 {
+ self.default_timeout
+ }
+
+ /// Invokes a command with the provided parameters using the session's default timeout.
+ /// Returns the result directly without allowing further method chaining.
+ pub fn invoke_command(&mut self, command_id: u32, params: &mut TeeParams) -> Result<()> {
+ let mut err_origin: u32 = 0;
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_InvokeTACommand(
+ self.raw,
+ self.default_timeout,
+ command_id,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with the results
+ params.update_from_raw(&raw_params);
+ trace_println!("TEE_InvokeTACommand: command_id: {} finished", command_id);
+ Ok(())
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ pub fn as_mut_raw_ptr(&mut self) -> *mut raw::TEE_TASessionHandle {
+ &mut self.raw
+ }
+}
+
+// Drop implementation to close the session
+impl Drop for TaSession {
+ fn drop(&mut self) {
+ if !self.raw.is_null() {
Review Comment:
By removing the `new` method, we wouldn't need to check if `self.raw` is valid any more.
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
Review Comment:
Maybe return a `Result` type with `TEE_ERROR_BAD_FORMAT` or `TEE_ERROR_BAD_PARAMETERS`, and developers can easily convert the `Result` to `Option` using `ok()` or `err()` method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2033022953
##########
examples/inter_ta-rs/ta/src/main.rs:
##########
@@ -0,0 +1,126 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#![no_std]
+#![no_main]
+
+use optee_utee::{
+ ta_close_session, ta_create, ta_destroy, ta_invoke_command, ta_open_session, trace_println,
+};
+use optee_utee::{Error, ErrorKind, Parameters, Result, Uuid};
+use optee_utee::{ParamIndex, TaSession, TeeParams};
+use proto::{
+ Command, HelloWorldTaCommand, SystemPtaCommand, HELLO_WORLD_USER_TA_UUID, SYSTEM_PTA_UUID,
+};
+
+#[ta_create]
+fn create() -> Result<()> {
+ trace_println!("[+] TA create");
+ Ok(())
+}
+
+#[ta_open_session]
+fn open_session(_params: &mut Parameters) -> Result<()> {
+ trace_println!("[+] TA open session");
+ Ok(())
+}
+
+#[ta_close_session]
+fn close_session() {
+ trace_println!("[+] TA close session");
+}
+
+#[ta_destroy]
+fn destroy() {
+ trace_println!("[+] TA destroy");
+}
+
+fn test_invoke_system_pta() -> Result<()> {
+ let system_pta_uuid =
+ Uuid::parse_str(SYSTEM_PTA_UUID).map_err(|_| Error::from(ErrorKind::BadParameters))?;
+ let mut session = TaSession::new(system_pta_uuid).open()?;
+ trace_println!("[+] TA open PTA session success");
+
+ let input: [u8; 32] = [0; 32];
+ let mut output: [u8; 32] = [0; 32];
+
+ let mut params = TeeParams::new()
+ .with_memref_in(ParamIndex::Arg0, &input)
+ .with_memref_out(ParamIndex::Arg1, &mut output);
+
+ session.invoke_command(SystemPtaCommand::DeriveTaUniqueKey as u32, &mut params)?;
+
+ if let Some(written_slice) = params[ParamIndex::Arg1].written_slice() {
Review Comment:
For demonstrations of using the inout `written_slice`, this might be further improved by converting the option to Result using `.ok_or()?` or anyhow::context, to avoid nested if/else {return Err} pattern.
##########
examples/inter_ta-rs/ta/src/main.rs:
##########
@@ -0,0 +1,126 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#![no_std]
+#![no_main]
+
+use optee_utee::{
+ ta_close_session, ta_create, ta_destroy, ta_invoke_command, ta_open_session, trace_println,
+};
+use optee_utee::{Error, ErrorKind, Parameters, Result, Uuid};
+use optee_utee::{ParamIndex, TaSession, TeeParams};
+use proto::{
+ Command, HelloWorldTaCommand, SystemPtaCommand, HELLO_WORLD_USER_TA_UUID, SYSTEM_PTA_UUID,
+};
+
+#[ta_create]
+fn create() -> Result<()> {
+ trace_println!("[+] TA create");
+ Ok(())
+}
+
+#[ta_open_session]
+fn open_session(_params: &mut Parameters) -> Result<()> {
+ trace_println!("[+] TA open session");
+ Ok(())
+}
+
+#[ta_close_session]
+fn close_session() {
+ trace_println!("[+] TA close session");
+}
+
+#[ta_destroy]
+fn destroy() {
+ trace_println!("[+] TA destroy");
+}
+
+fn test_invoke_system_pta() -> Result<()> {
+ let system_pta_uuid =
+ Uuid::parse_str(SYSTEM_PTA_UUID).map_err(|_| Error::from(ErrorKind::BadParameters))?;
+ let mut session = TaSession::new(system_pta_uuid).open()?;
+ trace_println!("[+] TA open PTA session success");
+
+ let input: [u8; 32] = [0; 32];
+ let mut output: [u8; 32] = [0; 32];
+
+ let mut params = TeeParams::new()
+ .with_memref_in(ParamIndex::Arg0, &input)
+ .with_memref_out(ParamIndex::Arg1, &mut output);
+
+ session.invoke_command(SystemPtaCommand::DeriveTaUniqueKey as u32, &mut params)?;
+
+ if let Some(written_slice) = params[ParamIndex::Arg1].written_slice() {
+ trace_println!("[+] TA invoke PTA command, output: {:?}", written_slice);
+ if written_slice.len() != 32 {
+ return Err(Error::new(ErrorKind::Generic));
+ }
+ // check if it's same as the output
+ if written_slice.iter().all(|&x| x == 0) {
Review Comment:
According to the comment, it seems that the written_slice should be checked against output. But I don't think that is allowed as output is still mut borrowed by params[Arg1].
So maybe we can just change the comment to ensure the written_slice contains updated content, which are not all zeros.
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
Review Comment:
I am curious why we consider accessing `raw::TEE_Param` is `unsafe` here. Can you add comment about the reason to the code?
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
Review Comment:
I am curious why we consider accessing `raw::TEE_Param` is `unsafe` here. Can you add comment about the reason to the code?
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ 0,
+ ptr::null_mut(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Open the session with parameters.
+ pub fn open_with_params(mut self, params: &mut TeeParams) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with any results
+ params.update_from_raw(&raw_params);
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Get the default timeout value for this session.
+ pub fn get_default_timeout(&self) -> u32 {
+ self.default_timeout
+ }
+
+ /// Invokes a command with the provided parameters using the session's default timeout.
+ /// Returns the result directly without allowing further method chaining.
+ pub fn invoke_command(&mut self, command_id: u32, params: &mut TeeParams) -> Result<()> {
+ let mut err_origin: u32 = 0;
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_InvokeTACommand(
+ self.raw,
+ self.default_timeout,
+ command_id,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with the results
+ params.update_from_raw(&raw_params);
+ trace_println!("TEE_InvokeTACommand: command_id: {} finished", command_id);
+ Ok(())
Review Comment:
We'd better avoid using `println!` by default inside SDK APIs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2033041441
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
Review Comment:
I am curious why we consider accessing `raw::TEE_Param` is `unsafe` here. Can you add comment about the reason to the code? Or can we provide getter functions for `raw::TEE_Param` and discuss the safe/unsafe behavior inside the `raw` module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034375311
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ 0,
+ ptr::null_mut(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Open the session with parameters.
+ pub fn open_with_params(mut self, params: &mut TeeParams) -> Result<Self> {
+ let mut err_origin: u32 = 0;
Review Comment:
Agree, will follow the definition in optee-teec:
```
pub struct Error {
kind: ErrorKind,
origin: Option<ErrorOrigin>,
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#issuecomment-2832930983
Any other comments? @m4sterchain
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034930016
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
Review Comment:
> Also, maybe consider supporting timeout configuration in `TeeParams`.
If a developer wants to configure both parameters and timeout, it makes sense to include the timeout in TeeParams and call `open_with_params(params)`.
But if a developer only wants to set a timeout, requiring them to create a TeeParams just for that feels unintuitive.
I've updated a more clear abstraction for TaSessionBuilder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2033106357
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
Review Comment:
Should we check against the out `size` before assign to `written`? This function could be improved to return Result<()>.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034427335
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
Review Comment:
Will add `SAFETY` comment for the modules inside this PR. Since `cargo clippy` infects many other modules, there will be another PR to address all rules.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "ivila (via GitHub)" <gi...@apache.org>.
ivila commented on PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#issuecomment-2832874159
Should we merge this PR or are we waiting for something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#issuecomment-2833196734
LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034367397
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ 0,
+ ptr::null_mut(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Open the session with parameters.
+ pub fn open_with_params(mut self, params: &mut TeeParams) -> Result<Self> {
+ let mut err_origin: u32 = 0;
+ let mut raw_session: raw::TEE_TASessionHandle = core::ptr::null_mut();
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_OpenTASession(
+ self.target_uuid.as_raw_ptr(),
+ self.default_timeout,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut raw_session,
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with any results
+ params.update_from_raw(&raw_params);
+ self.raw = raw_session;
+ Ok(self)
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ /// Get the default timeout value for this session.
+ pub fn get_default_timeout(&self) -> u32 {
+ self.default_timeout
+ }
+
+ /// Invokes a command with the provided parameters using the session's default timeout.
+ /// Returns the result directly without allowing further method chaining.
+ pub fn invoke_command(&mut self, command_id: u32, params: &mut TeeParams) -> Result<()> {
+ let mut err_origin: u32 = 0;
+ let mut raw_params = params.as_raw();
+ let param_types = params.raw_param_types();
+
+ match unsafe {
+ raw::TEE_InvokeTACommand(
+ self.raw,
+ self.default_timeout,
+ command_id,
+ param_types,
+ raw_params.as_mut_ptr(),
+ &mut err_origin,
+ )
+ } {
+ raw::TEE_SUCCESS => {
+ // Update the parameters with the results
+ params.update_from_raw(&raw_params);
+ trace_println!("TEE_InvokeTACommand: command_id: {} finished", command_id);
+ Ok(())
+ }
+ code => Err(Error::from_raw_error(code)),
+ }
+ }
+
+ pub fn as_mut_raw_ptr(&mut self) -> *mut raw::TEE_TASessionHandle {
Review Comment:
It is not used, removed the function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "ivila (via GitHub)" <gi...@apache.org>.
ivila commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034522859
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
Review Comment:
Got it, in that case, could you please add some comments explaining the design at the top of the Param struct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "ivila (via GitHub)" <gi...@apache.org>.
ivila commented on PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#issuecomment-2791278634
`Reviewed-by: Zehui Chen <iv...@apache.org>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH merged PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2033115250
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ ParamContent::ValueOutput { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ _ => {}
+ }
+ }
+}
+
+/// The TeeParams struct is used to manage the parameters for TEE commands.
+pub struct TeeParams<'a> {
+ params: [Param<'a>; 4],
+}
+
+impl<'a> TeeParams<'a> {
+ pub fn new() -> Self {
+ Self {
+ params: [Param::new(), Param::new(), Param::new(), Param::new()],
+ }
+ }
+
+ // Method-chaining parameter setters
+ pub fn with_memref_in(mut self, idx: ParamIndex, buffer: &'a [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn with_memref_out(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_memref_inout(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_value_in(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn with_value_out(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn with_value_inout(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub fn set_memref_in(&mut self, idx: ParamIndex, buffer: &'a [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn set_memref_out(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_memref_inout(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_value_in(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn set_value_out(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn set_value_inout(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub(crate) fn raw_param_types(&self) -> u32 {
+ let mut param_types = 0;
+ for (i, param) in self.params.iter().enumerate() {
+ param_types |= param.get_raw_type() << (i * 4);
+ }
+ param_types
+ }
+
+ pub(crate) fn as_raw(&mut self) -> [raw::TEE_Param; 4] {
+ [
+ self.params[0].as_raw(),
+ self.params[1].as_raw(),
+ self.params[2].as_raw(),
+ self.params[3].as_raw(),
+ ]
+ }
+
+ pub(crate) fn update_from_raw(&mut self, raw_params: &[raw::TEE_Param; 4]) {
+ // update the content for memref inout/out, and value inout/out
+ for (i, param) in self.params.iter_mut().enumerate() {
+ let raw_param = &raw_params[i];
+ match param.get_type() {
+ ParamType::MemrefOutput => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::MemrefInout => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::ValueInput => {
+ param.update_value_from_raw(raw_param);
+ }
+ ParamType::ValueInout => {
+ param.update_value_from_raw(raw_param);
+ }
+ _ => {}
Review Comment:
Should we check against other param types? We can at least ensure
1. `ValueInput` contents are never changed; if not, raise an error;
2. out size for MemRef Out/InOut types always within the original bound; if not, raise an error;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2033115250
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ ParamContent::ValueOutput { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ _ => {}
+ }
+ }
+}
+
+/// The TeeParams struct is used to manage the parameters for TEE commands.
+pub struct TeeParams<'a> {
+ params: [Param<'a>; 4],
+}
+
+impl<'a> TeeParams<'a> {
+ pub fn new() -> Self {
+ Self {
+ params: [Param::new(), Param::new(), Param::new(), Param::new()],
+ }
+ }
+
+ // Method-chaining parameter setters
+ pub fn with_memref_in(mut self, idx: ParamIndex, buffer: &'a [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn with_memref_out(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_memref_inout(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_value_in(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn with_value_out(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn with_value_inout(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub fn set_memref_in(&mut self, idx: ParamIndex, buffer: &'a [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn set_memref_out(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_memref_inout(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_value_in(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn set_value_out(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn set_value_inout(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub(crate) fn raw_param_types(&self) -> u32 {
+ let mut param_types = 0;
+ for (i, param) in self.params.iter().enumerate() {
+ param_types |= param.get_raw_type() << (i * 4);
+ }
+ param_types
+ }
+
+ pub(crate) fn as_raw(&mut self) -> [raw::TEE_Param; 4] {
+ [
+ self.params[0].as_raw(),
+ self.params[1].as_raw(),
+ self.params[2].as_raw(),
+ self.params[3].as_raw(),
+ ]
+ }
+
+ pub(crate) fn update_from_raw(&mut self, raw_params: &[raw::TEE_Param; 4]) {
+ // update the content for memref inout/out, and value inout/out
+ for (i, param) in self.params.iter_mut().enumerate() {
+ let raw_param = &raw_params[i];
+ match param.get_type() {
+ ParamType::MemrefOutput => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::MemrefInout => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::ValueInput => {
+ param.update_value_from_raw(raw_param);
+ }
+ ParamType::ValueInout => {
+ param.update_value_from_raw(raw_param);
+ }
+ _ => {}
Review Comment:
Should we check against other param types? We can at least ensure
1. `ValueInput` content are never changed; if not, raise an error;
2. out size for MemRef Out/InOut types always within the original bound; if not, raise an error;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "m4sterchain (via GitHub)" <gi...@apache.org>.
m4sterchain commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2033124940
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ ParamContent::ValueOutput { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ _ => {}
+ }
+ }
+}
+
+/// The TeeParams struct is used to manage the parameters for TEE commands.
+pub struct TeeParams<'a> {
+ params: [Param<'a>; 4],
+}
+
+impl<'a> TeeParams<'a> {
+ pub fn new() -> Self {
+ Self {
+ params: [Param::new(), Param::new(), Param::new(), Param::new()],
+ }
+ }
+
+ // Method-chaining parameter setters
+ pub fn with_memref_in(mut self, idx: ParamIndex, buffer: &'a [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
Review Comment:
We can optimize all the `idx.to_usize()` if the `ops::IndexMut` is implemented for `[Param<'a>; 4]` or related wrapped type.
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
Review Comment:
I suggest we follow `https://github.com/rust-lang/rust-clippy/issues/9330` to explicitly add comment to `unsafe` block; and run `cargo lint` to help automatically enforce available rules.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034371745
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
Review Comment:
> developers can easily convert the `Result` to `Option` using `ok()` or `err()` method.
Yes it also works but follows a different design choice.
In current strategy, we choose to return an Option to indicate whether a written slice is available, allowing developers to decide whether to raise an error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "ivila (via GitHub)" <gi...@apache.org>.
ivila commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034520603
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
Review Comment:
Maybe consider introducing a `TaSessionBuilder` struct? Mixing them together isn't very ergonomic.
Also, maybe consider supporting timeout configuration in `TeeParams`. That way, developers can use `open_with_params` to set the timeout, and we wouldn't need a separate timeout argument for `open`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#issuecomment-2789013060
Updated for all above comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034364957
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ ParamContent::MemrefInout { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
+ }
+ _ => {}
+ }
+ }
+
+ fn update_value_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::ValueInout { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ ParamContent::ValueOutput { a, b } => {
+ *a = unsafe { raw_param.value.a };
+ *b = unsafe { raw_param.value.b };
+ }
+ _ => {}
+ }
+ }
+}
+
+/// The TeeParams struct is used to manage the parameters for TEE commands.
+pub struct TeeParams<'a> {
+ params: [Param<'a>; 4],
+}
+
+impl<'a> TeeParams<'a> {
+ pub fn new() -> Self {
+ Self {
+ params: [Param::new(), Param::new(), Param::new(), Param::new()],
+ }
+ }
+
+ // Method-chaining parameter setters
+ pub fn with_memref_in(mut self, idx: ParamIndex, buffer: &'a [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn with_memref_out(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_memref_inout(mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn with_value_in(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn with_value_out(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn with_value_inout(mut self, idx: ParamIndex, a: u32, b: u32) -> Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub fn set_memref_in(&mut self, idx: ParamIndex, buffer: &'a [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInput { buffer };
+ self
+ }
+
+ pub fn set_memref_out(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefOutput { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_memref_inout(&mut self, idx: ParamIndex, buffer: &'a mut [u8]) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::MemrefInout { buffer, written: 0 };
+ self
+ }
+
+ pub fn set_value_in(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInput { a, b };
+ self
+ }
+
+ pub fn set_value_out(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueOutput { a, b };
+ self
+ }
+
+ pub fn set_value_inout(&mut self, idx: ParamIndex, a: u32, b: u32) -> &mut Self {
+ self.params[idx.to_usize()].content = ParamContent::ValueInout { a, b };
+ self
+ }
+
+ pub(crate) fn raw_param_types(&self) -> u32 {
+ let mut param_types = 0;
+ for (i, param) in self.params.iter().enumerate() {
+ param_types |= param.get_raw_type() << (i * 4);
+ }
+ param_types
+ }
+
+ pub(crate) fn as_raw(&mut self) -> [raw::TEE_Param; 4] {
+ [
+ self.params[0].as_raw(),
+ self.params[1].as_raw(),
+ self.params[2].as_raw(),
+ self.params[3].as_raw(),
+ ]
+ }
+
+ pub(crate) fn update_from_raw(&mut self, raw_params: &[raw::TEE_Param; 4]) {
+ // update the content for memref inout/out, and value inout/out
+ for (i, param) in self.params.iter_mut().enumerate() {
+ let raw_param = &raw_params[i];
+ match param.get_type() {
+ ParamType::MemrefOutput => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::MemrefInout => {
+ param.update_size_from_raw(raw_param);
+ }
+ ParamType::ValueInput => {
+ param.update_value_from_raw(raw_param);
+ }
+ ParamType::ValueInout => {
+ param.update_value_from_raw(raw_param);
+ }
+ _ => {}
Review Comment:
> 1. `ValueInput` contents are never changed; if not, raise an error;
For ValueInput, we will not read the TEEParam buffer after invoke, which means if it was changed we will ignore it. It is the same strategy as MemrefInput. Should we add the check for those input?
For `2.`, agree and will update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034364145
##########
optee-utee/src/tee_parameter.rs:
##########
@@ -0,0 +1,302 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ParamType;
+use core::ops::{Index, IndexMut};
+use optee_utee_sys as raw;
+
+#[derive(Copy, Clone, Debug)]
+pub enum ParamIndex {
+ Arg0,
+ Arg1,
+ Arg2,
+ Arg3,
+}
+
+impl ParamIndex {
+ fn to_usize(self) -> usize {
+ match self {
+ ParamIndex::Arg0 => 0,
+ ParamIndex::Arg1 => 1,
+ ParamIndex::Arg2 => 2,
+ ParamIndex::Arg3 => 3,
+ }
+ }
+}
+
+enum ParamContent<'a> {
+ None,
+ MemrefInput {
+ buffer: &'a [u8],
+ },
+ MemrefOutput {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ MemrefInout {
+ buffer: &'a mut [u8],
+ written: usize,
+ },
+ ValueInput {
+ a: u32,
+ b: u32,
+ },
+ ValueOutput {
+ a: u32,
+ b: u32,
+ },
+ ValueInout {
+ a: u32,
+ b: u32,
+ },
+}
+
+pub struct Param<'a> {
+ content: ParamContent<'a>,
+}
+
+impl<'a> Param<'a> {
+ fn new() -> Self {
+ Self {
+ content: ParamContent::None,
+ }
+ }
+
+ pub fn written_slice(&self) -> Option<&[u8]> {
+ match &self.content {
+ ParamContent::MemrefOutput { buffer, written } => Some(&buffer[..*written]),
+ ParamContent::MemrefInout { buffer, written } => Some(&buffer[..*written]),
+ _ => None,
+ }
+ }
+
+ pub fn output_value(&self) -> Option<(u32, u32)> {
+ match &self.content {
+ ParamContent::ValueOutput { a, b } => Some((*a, *b)),
+ ParamContent::ValueInout { a, b } => Some((*a, *b)),
+ _ => None,
+ }
+ }
+
+ fn get_type(&self) -> ParamType {
+ match &self.content {
+ ParamContent::None => ParamType::None,
+ ParamContent::MemrefInput { .. } => ParamType::MemrefInput,
+ ParamContent::MemrefOutput { .. } => ParamType::MemrefOutput,
+ ParamContent::MemrefInout { .. } => ParamType::MemrefInout,
+ ParamContent::ValueInput { .. } => ParamType::ValueInput,
+ ParamContent::ValueOutput { .. } => ParamType::ValueOutput,
+ ParamContent::ValueInout { .. } => ParamType::ValueInout,
+ }
+ }
+
+ fn get_raw_type(&self) -> u32 {
+ self.get_type() as u32
+ }
+
+ fn as_raw(&mut self) -> raw::TEE_Param {
+ match &mut self.content {
+ ParamContent::None => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: core::ptr::null_mut(),
+ size: 0,
+ },
+ },
+ ParamContent::MemrefInput { buffer } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefOutput { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::MemrefInout { buffer, written: _ } => raw::TEE_Param {
+ memref: raw::Memref {
+ buffer: (*buffer).as_mut_ptr() as *mut core::ffi::c_void,
+ size: buffer.len(),
+ },
+ },
+ ParamContent::ValueInput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueInout { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ ParamContent::ValueOutput { a, b } => raw::TEE_Param {
+ value: raw::Value { a: *a, b: *b },
+ },
+ }
+ }
+
+ fn update_size_from_raw(&mut self, raw_param: &raw::TEE_Param) {
+ match &mut self.content {
+ ParamContent::MemrefOutput { buffer: _, written } => {
+ *written = unsafe { raw_param.memref.size };
Review Comment:
Accessing `union` should be unsafe in Rust:
error[E0133]: access to union field is unsafe and requires unsafe function or block
--> /teaclave/optee-utee/src/tee_parameter.rs:152:28
|
152 | *written = raw_param.memref.size;
| ^^^^^^^^^^^^^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org
Re: [PR] New Design for TA-to-TA Invocation [incubator-teaclave-trustzone-sdk]
Posted by "DemesneGH (via GitHub)" <gi...@apache.org>.
DemesneGH commented on code in PR #178:
URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/178#discussion_r2034362346
##########
optee-utee/src/ta_session.rs:
##########
@@ -0,0 +1,150 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use core::ptr;
+use optee_utee_sys as raw;
+
+use crate::{Error, Result, TeeParams, Uuid};
+
+/// Represents a connection between a trusted application and another trusted application (can be user TA or pseudo TA).
+pub struct TaSession {
+ raw: raw::TEE_TASessionHandle,
+ target_uuid: Uuid,
+ default_timeout: u32,
+}
+
+impl TaSession {
+ /// Start configuring a new TA session with infinite timeout by default.
+ pub fn new(uuid: Uuid) -> Self {
+ Self {
+ raw: core::ptr::null_mut(),
+ target_uuid: uuid,
+ default_timeout: raw::TEE_TIMEOUT_INFINITE,
+ }
+ }
+
+ /// Set the default timeout for this session.
+ /// Can be called both before and after opening the session.
+ pub fn timeout(mut self, timeout: u32) -> Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Set the default timeout for this session (mutable version).
+ /// Returns self to allow method chaining with invoke().
+ pub fn with_timeout(&mut self, timeout: u32) -> &mut Self {
+ self.default_timeout = timeout;
+ self
+ }
+
+ /// Open the session without parameters.
+ pub fn open(mut self) -> Result<Self> {
Review Comment:
I want to follow the builder's pattern, the configuration it can take is `timeout`. We can use default timeout:
```
let mut session = TaSession::new(system_pta_uuid).open()?;
```
Or set a timeout before open:
```
let mut session = TaSession::new(system_pta_uuid).timeout(1000).open()?;
```
For invoke we can overwrite the timeout:
```
let mut session = TaSession::new(system_pta_uuid).open()?;
session.with_timeout(2000).invoke_command(xxxx)
```
Maybe I should add some comments and one example usage in this inter_ta-rs/ta
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@teaclave.apache.org
For additional commands, e-mail: dev-help@teaclave.apache.org