DemesneGH commented on issue #153: URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/issues/153#issuecomment-2520013238
Thanks for the solution. Let’s start by clarifying a few points: What we need to do: 1. The current implementation has issues that we need to fix. 2. Additionally, there is a requirement for zero-copy handling of large outputs, which we should incorporate. We are open to introducing breaking changes if necessary. Seems there're multiple ways to call the `plugin.invoke()` in the above solution. Developers may be confused about which way should they choose. I prefer re-design the API even if sacrificing backward compatibility. > ```rust > let immutable_buffer = [0_u8; 32]; > let mut mutable_buffer = [0_u8; 32]; > // =================== old feature, pass immutable slice ================== > // response1 infer to Vec<u8> automatically, keeps compatibility. > let response1 = plugin.invoke(1, 1, &immutable_buffer).unwrap(); > // this won't compile, as we didn't imply the Wire trait for the type pair > // and I think codes like this must be mistake from developers, developers > // must get the buffer back > let response2: usize = plugin.invoke(1, 1, &immutable_buffer).unwrap(); > > // =================== new feature, pass mutable slice ==================== > // developers must specific what they want, keeping developers from misuse > // currently the valid return types are: Vec<u8> or usize > let response3: Vec<u8> = plugin.invoke(1, 1, &mut mutable_buffer).unwrap(); > let response4: Vec<u8> = plugin.invoke(1, 1, mutable_buffer.as_mut_slice()).unwrap(); > // this won't compile, as we need developers to specific the input type to &mut [u8] > let response5: usize = plugin.invoke(1, 1, &mut mutable_buffer).unwrap(); > // this is working like std::io::Read > let response5: usize = plugin.invoke(1, 1, mutable_buffer.as_mut_slice()).unwrap(); > ``` So let’s first sync the requirements and then discuss the new design. Please let me know if I’ve misunderstood anything: The requirement is: TA send a small buffer for request and received a large buffer from NW. The buffer cannot be copied due to memory limitation. For the design let's discuss the questions: - Should developers care about the shared buffer? - Yes? Because developers need dynamic shared buffer. Developers know the approximate output size and they should alloc a shared buffer when preparing the input. - The `plugin_invoke()` takes these two parameters `data_ptr` and `data_len`. they are description of the shared_buffer, or the data_len is size of `input_buffer` (as the comment for tee_invoke_supp_plugin)? - Seems the `len` is size of shared_buffer, when invoke it contains the input data, so TA also need to send the input_data size, like `request_size` you mentioned above but using `sub_cmd` to send the input size seems slightly confusing, but we can implement it in any suitable way. - Should developers manually set the input_data size, or should our SDK handle it automatically? - Would zero-copy of Vec in Rust solve the problem? - Compared with returning `usize`, I prefer Vec<u8> which is more Rust way and avoid unsafe memory operations by developers themselves. We may seek ways for zero-copy (maybe unsafe, carefully handled by SDK) and return the Vec<u8> to developers. -- 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-unsubscr...@teaclave.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org For additional commands, e-mail: dev-h...@teaclave.apache.org