Uh oh!
There was an error while loading. Please reload this page.
Remove platform singletons from the LVBS platform and OP-TEE shim - #1127
Remove platform singletons from the LVBS platform and OP-TEE shim#1127Sangho Lee (sangho2) wants to merge 2 commits into
Conversation
c3d9103 to
e7364baCompareSangho Lee (sangho2)
commented
Aug 5, 2026
A sequel of #1056 |
e7364ba to
00bf3efCompare00bf3ef to
4273ff1ComparePraveen K Paladugu (praveen-pk)
commented
Aug 12, 2026
This is rather a large PR. would be nice if this can be broken down to smaller commits. Overall, the PR LGTM! |
Weidong Cui (wdcui)
commented
Aug 12, 2026
Jay Bosamiya (Microsoft) (@jaybosamiya-ms) , can you help review this PR? Thanks! |
Weidong Cui (wdcui)
commented
Aug 12, 2026
Sangho Lee (@sangho2) , the agent found the following issue: Medium — OpteeShim unintentionally loses Clone — litebox_shim_optee/src/lib.rs:292 . #[derive(Clone)] adds Platform: Clone , but neither platform implements it. Replace the derive with a manual Clone impl that clones the inner Arc . |
4273ff1 to
4543b4bCompare🤖 SemverChecks 🤖 Click for details |
Jay Bosamiya (Microsoft) (jaybosamiya-ms)
left a comment
There was a problem hiding this comment.
Thanks Sangho for getting rid of the final bits of the multiplex! The PR generally looks good to me. Most of it is fairly mechanical, so I haven't reviewed every line, but have done a look through for any bits that stood out to me, and have pointed out a few very minor nits. If there are any parts that you think I should specifically take a closer look at or sanity check, let me know.
| unsafe { crate::platform_low().vmap_privileged(pages, perms) } | ||
| unsafe { self.0.vmap_privileged(pages, perms) } |
There was a problem hiding this comment.
Nit: do we want to name it as self.platform_low for consistency/readability? Or just self.platform? It is possible .0 good enough. No response needed here, feel free to mark as resolved either way.
There was a problem hiding this comment.
yes, self.0 sometimes looks bad. self.platform might work well. Let me figure out better option.
| // NOTE: the session manager singleton lives in the composition root (each | ||
| // runner), not here. A `static` cannot name a generic parameter, and a shim | ||
| // instance is built per session, so the shim has nowhere to put it. The runner | ||
| // knows its concrete platform, so it can hold the `static` and hand out | ||
| // `&'static SessionManager<ConcretePlatform>`. |
There was a problem hiding this comment.
Nit: this feels like a comment left in by an agent removing code leaving breadcrumbs; not sure it needs to stay?
There was a problem hiding this comment.
Good catch. Let me go over all comments again.
Sangho Lee (sangho2)
commented
Aug 18, 2026
Let me merge this one later because it might conflict with ongoing dynamic TA loading efforts. |
This PR removes platform singletons from the LVBS platform and OP-TEE shim crates, allowing us to drop
litebox_platform_multiplexcompletely. Refactor both crates at once because they relied on the sameGlobalVmapManagerwhich is now removed as well.