-
Notifications
You must be signed in to change notification settings - Fork 4
Add handlers for OP-TEE message protocol and SMC #555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
2902dd9 to
102cfa9
Compare
wdcui
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding the support for OP-TEE step-by-step. I left some comments below. I suggest @jaybosamiya-ms to take a look at ptr.rs.
f2691a8 to
16850b8
Compare
jaybosamiya-ms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only looked at litebox_shim_optee/src/ptr.rs, a few comments from my pass over it:
58476ce to
27152aa
Compare
jaybosamiya-ms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new ptr.rs is a significant rewrite over the prior version. I have a few minor comments/clarifications, but no major concerns with it as it currently stands (the more major changes are things that #505 already has in mind, so I'll just make a quick comment there to re-look at this PR when cleaning things up with that).
Minor readability note: it appears all the methods in this are right up against each other with no blank lines separating them; usually an additional newline between functions (before the doc string for the function) helps with readability. Not major and not a blocker. Funnily, rustfmt has an (unstable) pair of options blank_lines_*_bound that are the closest to what would guarantee this but they are hilariously broken and would be a bad idea for us to enable lol
aa93994 to
b4ab389
Compare
|
If we merge #572 first, we can get rid of |
wdcui
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments below.
jaybosamiya-ms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've focused my review on the VMapProvider. Comments inline, but I'll just bubble the high-level comment to the top here too: if it helps unblock this PR, please feel free to move VmapProvider into litebox_common_optee. Since the current usage of it is only to provide litebox_shim_optee::ptr::Phys*Ptr, there is no reason to block this otherwise fairly-large PR on making sure the design is general enough to sit inside the main litebox crate.
20a424a to
42500bb
Compare
4cecf50 to
4d0df5e
Compare
|
🤖 SemverChecks 🤖 Click for details |
This PR adds OP-TEE message protocol and SMC handlers (a handler for TA request is
covered in another PR).
Also, this PR adds traits for physical memory access to (copy from/to) the memory of
a remote environment (e.g., VTL0, normal world, ...),
PhysMutPtrandPhysConstPtr.These traits expect platforms will implement Linux kernel's
vmap-like functions toenable virtually contiguous mapping of non-contiguous physical pages on demand.