[Draft] optee: share TA binaries through a global UUID map - #1142
[Draft] optee: share TA binaries through a global UUID map#1142Praveen K Paladugu (praveen-pk) wants to merge 1 commit into
Conversation
Move the TA UUID-to-binary map out of individual shim instances so TA binaries can be registered once and reused across instances. This global map is required to support Dyanamically Loading TAs and using them across sessions. Protect the map with a spin-based RwLock and expose shim methods for storing and retrieving TA binaries. Register the LVBS runner's embedded TA during BSP initialization and resolve new TA instances through the shared map. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Praveen K Paladugu (praveen-pk)
changed the title
Sangho Lee (sangho2)
left a comment
Sangho Lee (sangho2)
left a comment
There was a problem hiding this comment.
Thanks for your effort! Yes, we should differentiate shim-global from runner-global. I left some comments/suggestions.
| if is_bsp { | ||
| let shim = litebox_shim_optee::OpteeShimBuilder::new().build(); | ||
| register_embedded_tas(&shim); | ||
| } |
There was a problem hiding this comment.
It is a bit unclear whether this is a good place to register TA binaries. a bit early. Probably fine because this is a temporary solution.
|
|
||
| /// Register all TA binaries embedded in the runner image. | ||
| fn register_embedded_tas(shim: &litebox_shim_optee::OpteeShim) { | ||
| static REGISTERED: spin::Once<()> = spin::Once::new(); |
There was a problem hiding this comment.
Do we need this static REGISTERED? ta_uuid_map itself should be able to reject redundant registration.
| fn register_embedded_tas(shim: &litebox_shim_optee::OpteeShim) { | ||
| static REGISTERED: spin::Once<()> = spin::Once::new(); | ||
| REGISTERED.call_once(|| { | ||
| assert!(register_embedded_ta( |
There was a problem hiding this comment.
It registers only a single TA, regression. We could keep using const TA_BINARIES.
| /// The TA UUID to binary map for TA loading. | ||
| ta_uuid_map: TaUuidMap, |
There was a problem hiding this comment.
Dropping ta_uuid_map is here is a bit concerning because we can no longer use shim's self. Instead, we could store &'static TaUuidMap here. This is an outstanding PR's direction (#1127).
|
|
||
| /// Get the global TA UUID map. | ||
| fn ta_uuid_map() -> Arc<TaUuidMap> { | ||
| static TA_UUID_MAP: once_cell::race::OnceBox<Arc<TaUuidMap>> = once_cell::race::OnceBox::new(); |
There was a problem hiding this comment.
Using Arc in general a good idea, but using it against TaUuidMap itself is not a good call IMO. This map is already protected by a lock and reference counting it less meaningful because we would not drop this map anyhow. Instead, we can Arc each TA binary which might be helpful to deal with future TA binary update as well.
| /// Entry in the TA UUID map containing binary data and parsed flags. | ||
| struct TaInfo { | ||
| /// The raw TA binary | ||
| binary: alloc::boxed::Box<[u8]>, |
There was a problem hiding this comment.
As noted, we could do binary: Arc<[u8]> here.
| } | ||
|
|
||
| /// Get the TA binary associated with the given TA UUID. | ||
| pub(crate) fn get_ta_bin(&self, ta_uuid: &TeeUuid) -> Option<alloc::boxed::Box<[u8]>> { |
There was a problem hiding this comment.
once we do use Arc<[u8]>, functions like this one can return Arc, avoiding deep heap copy.
2 participants
Move the TA UUID-to-binary map out of individual shim instances so TA binaries can be registered once and reused across instances. This global map is required to support Dyanamically Loading TAs and using them across sessions.
Protect the map with a spin-based RwLock and expose shim methods for storing and retrieving TA binaries.
Register the LVBS runner's embedded TA during BSP initialization and resolve new TA instances through the shared map.