From 1a0472c5495df63cb2f4ade35504199b8339f70a Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Wed, 15 Apr 2026 11:35:46 -0700 Subject: [PATCH 01/10] fix: use explicit host target for cargo test in pre-commit hook cargo test without --target fails locally when rust-toolchain.toml installs aarch64-unknown-none-softfloat, because no_std-incompatible transitive deps (futures-timer, slab) are resolved for that target. Using the host target from rustc -vV ensures portability across Linux, macOS, and Windows. --- .husky/pre-commit | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index c7ca1af..c8f1100 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -43,11 +43,17 @@ check_rustup_target "aarch64-unknown-none" set -x # cargo deny --log-level=warn check -cargo clippy --target=aarch64-unknown-none -p ihv1-ec-sp -p qemu-ec-sp -cargo clippy +cargo clippy --target=aarch64-unknown-none -p ihv1-ec-sp -p qemu-ec-sp -- -D warnings +cargo clippy -- -D warnings cargo fmt --check cargo hack check --feature-powerset --exclude=ihv1-ec-sp --exclude=qemu-ec-sp cargo hack check --feature-powerset --target=aarch64-unknown-none -cargo test -q +HOST_TARGET=$(rustc -vV | sed -n 's/host: //p') +if [ -z "$HOST_TARGET" ]; then + echo "Failed to determine Rust host target from \`rustc -vV\` output." + echo "Please verify that \`rustc -vV\` prints a valid \`host:\` line." + exit 1 +fi +cargo test --target "$HOST_TARGET" -q cd "$PROJECT_DIR/platform/qemu-sp" && cargo build --target=aarch64-unknown-none cd "$PROJECT_DIR/platform/ihv1-sp" && cargo build --target=aarch64-unknown-none From 7377e61a1231fd7dc8f01894adde1544ff22ce63 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:30:34 -0700 Subject: [PATCH 02/10] feat(03-01): FwMgmt error handling + process_indirect NotSupported - process_indirect returns Err("process_indirect not supported") instead of hardcoded 0x0 - map_share uses ? operator on MemRetrieveReq::exec() instead of .unwrap() - test_notify uses ? operator on NotificationSet::exec() instead of .unwrap() - Dispatch match arms updated to use ? propagation - Removed old commented-out FfaNotify code block --- ec-service-lib/src/services/fw_mgmt.rs | 58 ++++++-------------------- 1 file changed, 12 insertions(+), 46 deletions(-) diff --git a/ec-service-lib/src/services/fw_mgmt.rs b/ec-service-lib/src/services/fw_mgmt.rs index 216e221..28696cb 100644 --- a/ec-service-lib/src/services/fw_mgmt.rs +++ b/ec-service-lib/src/services/fw_mgmt.rs @@ -119,58 +119,24 @@ impl FwMgmt { } } - fn map_share(&self, _address: u64, _length: u64) -> GenericRsp { + fn map_share(&self, _address: u64, _length: u64) -> Result { // TODO - do not hardcode address and length in MemRetrieveReq - MemRetrieveReq::new().exec().unwrap(); - GenericRsp { _status: 0x0 } + MemRetrieveReq::new().exec()?; + Ok(DirectMessagePayload::from(GenericRsp { _status: 0x0 })) } - fn test_notify(&self, msg: MsgSendDirectReq2) -> GenericRsp { - // let nfy = FfaNotify { - // function_id: FunctionId::NotificationSet.into(), - // source_id: msg.destination_id, - // destination_id: msg.source_id, - // args64: [ - // 0x2, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, - // ], - // }; - - // let _result = nfy.exec(); - + fn test_notify(&self, msg: MsgSendDirectReq2) -> Result { let flags = 0b10; let notification_bitmap = 0b10; - NotificationSet::new(msg.destination_id(), msg.source_id(), flags, notification_bitmap) - .exec() - .unwrap(); + NotificationSet::new(msg.destination_id(), msg.source_id(), flags, notification_bitmap).exec()?; // Return status success - GenericRsp { _status: 0x0 } + Ok(DirectMessagePayload::from(GenericRsp { _status: 0x0 })) } - fn process_indirect(&self, seq_num: u16, _rx_buffer: u64, _tx_buffer: u64) -> GenericRsp { + fn process_indirect(&self, seq_num: u16, _rx_buffer: u64, _tx_buffer: u64) -> Result { debug!("Processing indirect message: 0x{:x}", seq_num); - // let msg = FfaIndirectMsg::new(); - // let mut in_buf: [u8; 256] = [0; 256]; - // let mut status; - - // unsafe { - // status = msg.read_indirect_msg(rx_buffer, seq_num, &mut in_buf); - // }; - - // if status == FfaError::Ok { - // error!("Indirect Message: {:?}", in_buf); - // } - - // // Populate TX buffer with response and matching seq num - // let buf: [u8; 16] = [ - // 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xA, 0xB, 0xC, 0xD, 0xE, 0xF, - // ]; - // unsafe { - // status = msg.write_indirect_msg(tx_buffer, seq_num, &buf); - // }; - - // GenericRsp { _status: status.into() } - GenericRsp { _status: 0x0 } + Err(odp_ffa::Error::Other("process_indirect not supported")) } } @@ -183,18 +149,18 @@ impl Service for FwMgmt { debug!("Received FwMgmt command 0x{:x}", cmd); let payload = match cmd { - EC_CAP_INDIRECT_MSG => DirectMessagePayload::from(self.process_indirect( + EC_CAP_INDIRECT_MSG => self.process_indirect( msg.payload().u8_at(1) as u16, msg.payload().register_at(4), msg.payload().register_at(5), - )), + )?, EC_CAP_GET_FW_STATE => DirectMessagePayload::from(self.get_fw_state()), EC_CAP_GET_SVC_LIST => DirectMessagePayload::from(self.get_svc_list()), EC_CAP_GET_BID => DirectMessagePayload::from(self.get_bid()), - EC_CAP_TEST_NFY => DirectMessagePayload::from(self.test_notify(msg.clone())), + EC_CAP_TEST_NFY => self.test_notify(msg.clone())?, EC_CAP_MAP_SHARE => { // First parameter is pointer to memory descriptor - DirectMessagePayload::from(self.map_share(msg.payload().register_at(1), msg.payload().register_at(2))) + self.map_share(msg.payload().register_at(1), msg.payload().register_at(2))? } _ => { error!("Unknown FwMgmt Command: {}", cmd); From 9f479908273948965b653756cb3af2361803210e Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:32:35 -0700 Subject: [PATCH 03/10] feat(03-01): Notify Add/Remove/Assign/Unassign state-tracking - nfy_add: find-or-create entry, register mappings (mirrors nfy_setup logic) - nfy_remove: find entry, unregister mappings (mirrors nfy_destroy logic) - nfy_assign: update id/ntype on existing cookie-matched mappings with bitmap conflict check - nfy_unassign: clear id/ntype on cookie-matched mappings, keep slot reserved - Replace catch-all NotSupported arm with four individual dispatch arms --- ec-service-lib/src/services/notify.rs | 174 ++++++++++++++++++++++++-- 1 file changed, 167 insertions(+), 7 deletions(-) diff --git a/ec-service-lib/src/services/notify.rs b/ec-service-lib/src/services/notify.rs index fadd276..863cad1 100644 --- a/ec-service-lib/src/services/notify.rs +++ b/ec-service-lib/src/services/notify.rs @@ -426,6 +426,169 @@ impl Notify { status: res, } } + + fn nfy_add(&mut self, req: NotifyReq) -> NfySetupRsp { + if req.count == 0 || req.count >= NOTIFY_MAX_MAPPINGS_PER_REQ as u8 { + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Add as u64, + status: ErrorCode::InvalidParameters, + }; + } + + let entry_index = if let Some(idx) = self.nfy_find_entry(req.receiver_uuid) { + idx + } else if let Some(empty) = self.nfy_find_empty_slot() { + self.entries[empty].in_use = true; + self.entries[empty].service_uuid = req.receiver_uuid; + self.entries[empty].mappings = [NfyMapping::default(); NOTIFY_MAX_MAPPINGS]; + empty + } else { + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Add as u64, + status: ErrorCode::NoMemory, + }; + }; + + let res = self.nfy_register_mapping(entry_index, req); + NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Add as u64, + status: res, + } + } + + fn nfy_remove(&mut self, req: NotifyReq) -> NfySetupRsp { + let entry = match self.nfy_find_entry(req.receiver_uuid) { + Some(idx) => idx, + None => { + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Remove as u64, + status: ErrorCode::InvalidParameters, + }; + } + }; + + let res = self.nfy_unregister_mapping(entry, req); + NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Remove as u64, + status: res, + } + } + + fn nfy_assign(&mut self, req: NotifyReq) -> NfySetupRsp { + let entry_index = match self.nfy_find_entry(req.receiver_uuid) { + Some(idx) => idx, + None => { + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, + status: ErrorCode::InvalidParameters, + }; + } + }; + + for (cookie, id, ntype) in req.notifications.iter().take(req.count as usize) { + match self.nfy_find_matching_cookie(entry_index, *cookie) { + Some(mapping_index) => { + let mapping = &mut self.entries[entry_index].mappings[mapping_index]; + // Clear old bit, set new bit + self.global_bitmap &= !(1 << mapping.id); + if self.global_bitmap & (1 << id) != 0 { + error!("Bitmask conflict during assign for id: {id}"); + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, + status: ErrorCode::InvalidParameters, + }; + } + mapping.id = *id; + mapping.ntype = *ntype; + self.global_bitmap |= 1 << id; + } + None => { + error!("No matching cookie for assign: {cookie}"); + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, + status: ErrorCode::InvalidParameters, + }; + } + } + } + + NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, + status: ErrorCode::Ok, + } + } + + fn nfy_unassign(&mut self, req: NotifyReq) -> NfySetupRsp { + let entry_index = match self.nfy_find_entry(req.receiver_uuid) { + Some(idx) => idx, + None => { + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Unassign as u64, + status: ErrorCode::InvalidParameters, + }; + } + }; + + for (cookie, _id, _ntype) in req.notifications.iter().take(req.count as usize) { + match self.nfy_find_matching_cookie(entry_index, *cookie) { + Some(mapping_index) => { + let mapping = &mut self.entries[entry_index].mappings[mapping_index]; + self.global_bitmap &= !(1 << mapping.id); + mapping.id = 0; + mapping.ntype = NotifyType::Global; + // Keep in_use = true and cookie — slot is reserved, just unassigned + } + None => { + error!("No matching cookie for unassign: {cookie}"); + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Unassign as u64, + status: ErrorCode::InvalidParameters, + }; + } + } + } + + NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Unassign as u64, + status: ErrorCode::Ok, + } + } } impl Service for Notify { @@ -439,13 +602,10 @@ impl Service for Notify { let payload = match req.msg_info.message_id() { MessageID::Setup => DirectMessagePayload::from(self.nfy_setup(req)), MessageID::Destroy => DirectMessagePayload::from(self.nfy_destroy(req)), - MessageID::Add | MessageID::Remove | MessageID::Assign | MessageID::Unassign => { - // For Add, Remove, Assign, and Unassign, we just return unsupported - NfyGenericRsp { - status: ErrorCode::NotSupported as i64, - } - .into() - } + MessageID::Add => DirectMessagePayload::from(self.nfy_add(req)), + MessageID::Remove => DirectMessagePayload::from(self.nfy_remove(req)), + MessageID::Assign => DirectMessagePayload::from(self.nfy_assign(req)), + MessageID::Unassign => DirectMessagePayload::from(self.nfy_unassign(req)), }; Ok(MsgSendDirectResp2::from_req_with_payload(&msg, payload)) From 1218745c40110135292f93999e1f0e9b52a795c7 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:35:07 -0700 Subject: [PATCH 04/10] feat(03-01): TPM get_feature_info, register, unregister, finish + deinit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add notification_registered field to TpmService struct - get_feature_info_handler returns capability flags for notification feature - register_handler tracks notification registration state (Already on double-register) - unregister_handler validates registration (Denied if not registered) - finish_handler validates registration (Denied if not registered) - deinit resets all state (current_state, active_locality, locality_states, notification_registered) - init also resets notification_registered - Update 3 test assertions: register→Ok, unregister→Denied, finish→Denied - Apply cargo fmt fixup for fw_mgmt.rs --- ec-service-lib/src/services/tpm.rs | 59 ++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/ec-service-lib/src/services/tpm.rs b/ec-service-lib/src/services/tpm.rs index 66a140b..d6cefb0 100644 --- a/ec-service-lib/src/services/tpm.rs +++ b/ec-service-lib/src/services/tpm.rs @@ -223,6 +223,7 @@ pub struct TpmService { interface_id_default: PtpCrbInterfaceIdentifier, locality_states: [TpmLocalityState; NUM_LOCALITIES as usize], tpm_internal_crb_address: u64, + notification_registered: bool, pub sst: S, } @@ -236,6 +237,7 @@ impl TpmService { interface_id_default: PtpCrbInterfaceIdentifier::new(), locality_states: [TpmLocalityState::Closed; NUM_LOCALITIES as usize], tpm_internal_crb_address: 0x10000200000, + notification_registered: false, sst, } } @@ -499,9 +501,17 @@ impl TpmService { TpmStatus::OkResultsReturned } - fn get_feature_info_handler(&self, _request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { - error!("Unsupported Function"); - TpmStatus::NotSup + fn get_feature_info_handler(&self, request: &TpmRequest, response: &mut TpmResponse) -> TpmStatus { + // Feature ID is in the function field (Arg1) + // Currently only notification feature (RegisterForNotification) is queryable + if request.function == TpmFunction::RegisterForNotification as u64 { + // Return feature flags in payload: bit 0 = notifications supported + response.tpm_payload = 0x1; + TpmStatus::OkResultsReturned + } else { + // Unknown feature ID + TpmStatus::NotSup + } } // SAFETY: Function invokes both handle_command and handle_locality_request which can @@ -538,19 +548,32 @@ impl TpmService { return_val } - fn register_handler(&self, _request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { - error!("Unsupported Function"); - TpmStatus::NotSup + fn register_handler(&mut self, _request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { + if self.notification_registered { + TpmStatus::Already + } else { + self.notification_registered = true; + TpmStatus::Ok + } } - fn unregister_handler(&self, _request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { - error!("Unsupported Function"); - TpmStatus::NotSup + fn unregister_handler(&mut self, _request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { + if !self.notification_registered { + TpmStatus::Denied + } else { + self.notification_registered = false; + TpmStatus::Ok + } } - fn finish_handler(&self, _request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { - error!("Unsupported Function"); - TpmStatus::NotSup + fn finish_handler(&mut self, _request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { + if !self.notification_registered { + TpmStatus::Denied + } else { + // Acknowledge notification completion — no additional state change needed + // since notifications are edge-triggered in this stub implementation + TpmStatus::Ok + } } fn manage_locality_handler(&mut self, request: &TpmRequest, _response: &mut TpmResponse) -> TpmStatus { @@ -605,11 +628,15 @@ impl TpmService { self.current_state = TpmState::Idle; self.active_locality = NO_ACTIVE_LOCALITY; + self.notification_registered = false; } /// De-initializes the TPM service (equivalent to `TpmServiceDeInit`). pub fn deinit(&mut self) { - // Nothing to de-init. + self.current_state = TpmState::Idle; + self.active_locality = NO_ACTIVE_LOCALITY; + self.locality_states = [TpmLocalityState::Closed; NUM_LOCALITIES as usize]; + self.notification_registered = false; } /// Test-only: write a specific value to a CRB register in the internal CRB. @@ -1220,7 +1247,7 @@ mod tests { let mut service = TpmService::new(MockTpmSst::new()); let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::RegisterForNotification as u64, 0x00, 0x00); let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); - assert_eq!(resp_status(&resp), TpmStatus::NotSup as u64); + assert_eq!(resp_status(&resp), TpmStatus::Ok as u64); } // ======================================================================= @@ -1237,7 +1264,7 @@ mod tests { 0x00, ); let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); - assert_eq!(resp_status(&resp), TpmStatus::NotSup as u64); + assert_eq!(resp_status(&resp), TpmStatus::Denied as u64); } // ======================================================================= @@ -1248,7 +1275,7 @@ mod tests { let mut service = TpmService::new(MockTpmSst::new()); let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::FinishNotified as u64, 0x00, 0x00); let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); - assert_eq!(resp_status(&resp), TpmStatus::NotSup as u64); + assert_eq!(resp_status(&resp), TpmStatus::Denied as u64); } // ======================================================================= From 9728dd01cd49471928d7e1a1da1a008a544651f4 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:43:40 -0700 Subject: [PATCH 05/10] test(03-02): add FwMgmt and Notify unit test modules - FwMgmt: 5 tests covering get_fw_state, get_svc_list, get_bid, process_indirect error, unknown command - Notify: 12 tests covering Setup, Destroy, Add, Remove, Assign, Unassign + error cases - UUID round-trip via to_u128_le() for correct NotifyReq deserialization --- ec-service-lib/src/services/fw_mgmt.rs | 91 +++++++++++ ec-service-lib/src/services/notify.rs | 208 +++++++++++++++++++++++++ 2 files changed, 299 insertions(+) diff --git a/ec-service-lib/src/services/fw_mgmt.rs b/ec-service-lib/src/services/fw_mgmt.rs index 28696cb..884cc9f 100644 --- a/ec-service-lib/src/services/fw_mgmt.rs +++ b/ec-service-lib/src/services/fw_mgmt.rs @@ -171,3 +171,94 @@ impl Service for FwMgmt { Ok(MsgSendDirectResp2::from_req_with_payload(&msg, payload)) } } + +// =========================================================================== +// FwMgmt Unit Tests +// =========================================================================== +#[cfg(test)] +mod tests { + use super::*; + use odp_ffa::{DirectMessagePayload, HasRegisterPayload}; + + const FWMGMT_UUID: Uuid = uuid!("330c1273-fde5-4757-9819-5b6539037502"); + + /// Build a FwMgmt request with the given command byte. + fn fwmgmt_req(cmd: u8) -> MsgSendDirectReq2 { + let mut bytes = [0u8; 14 * 8]; + bytes[0] = cmd; + let payload = DirectMessagePayload::from_iter(bytes); + MsgSendDirectReq2::new(0x0001, 0x8001, FWMGMT_UUID, payload) + } + + /// Extract status (i64) from response payload register 0. + fn resp_status_i64(resp: &MsgSendDirectResp2) -> i64 { + resp.payload().u64_at(0) as i64 + } + + // =================================================================== + // FwMgmt::get_fw_state Test + // =================================================================== + #[test] + fn test_get_fw_state() { + let mut svc = FwMgmt::new(); + let resp = svc.ffa_msg_send_direct_req2(fwmgmt_req(EC_CAP_GET_FW_STATE)).unwrap(); + let p = resp.payload(); + // FwStateRsp serializes: fw_version(u16 LE) + secure_state(u8) + boot_status(u8) = 4 bytes + assert_eq!(p.u8_at(0), 0x00); // fw_version low byte + assert_eq!(p.u8_at(1), 0x01); // fw_version high byte (0x0100 LE) + assert_eq!(p.u8_at(2), 0x00); // secure_state + assert_eq!(p.u8_at(3), 0x01); // boot_status + } + + // =================================================================== + // FwMgmt::get_svc_list Test + // =================================================================== + #[test] + fn test_get_svc_list() { + let mut svc = FwMgmt::new(); + let resp = svc.ffa_msg_send_direct_req2(fwmgmt_req(EC_CAP_GET_SVC_LIST)).unwrap(); + // ServiceListRsp: status(i64) + debug_mask(u16) + battery_mask(u8) + fan_mask(u8) + + // thermal_mask(u8) + hid_mask(u8) + key_mask(u16) + assert_eq!(resp_status_i64(&resp), 0x0); // status + let p = resp.payload(); + assert_eq!(p.u8_at(8), 0x01); // debug_mask low byte + assert_eq!(p.u8_at(10), 0x01); // battery_mask + assert_eq!(p.u8_at(11), 0x01); // fan_mask + assert_eq!(p.u8_at(12), 0x01); // thermal_mask + assert_eq!(p.u8_at(13), 0x00); // hid_mask + assert_eq!(p.u8_at(14), 0x07); // key_mask low byte + } + + // =================================================================== + // FwMgmt::get_bid Test + // =================================================================== + #[test] + fn test_get_bid() { + let mut svc = FwMgmt::new(); + let resp = svc.ffa_msg_send_direct_req2(fwmgmt_req(EC_CAP_GET_BID)).unwrap(); + assert_eq!(resp_status_i64(&resp), 0x0); + let p = resp.payload(); + let bid = p.u64_at(8); // _bid starts at offset 8 (after i64 status) + assert_eq!(bid, 0xdead0001); + } + + // =================================================================== + // FwMgmt::process_indirect Test + // =================================================================== + #[test] + fn test_process_indirect_returns_error() { + let mut svc = FwMgmt::new(); + let result = svc.ffa_msg_send_direct_req2(fwmgmt_req(EC_CAP_INDIRECT_MSG)); + assert!(result.is_err(), "process_indirect should return an error"); + } + + // =================================================================== + // FwMgmt Unknown Command Test + // =================================================================== + #[test] + fn test_unknown_command_returns_error() { + let mut svc = FwMgmt::new(); + let result = svc.ffa_msg_send_direct_req2(fwmgmt_req(0xFF)); + assert!(result.is_err()); + } +} diff --git a/ec-service-lib/src/services/notify.rs b/ec-service-lib/src/services/notify.rs index 863cad1..f4ace2c 100644 --- a/ec-service-lib/src/services/notify.rs +++ b/ec-service-lib/src/services/notify.rs @@ -611,3 +611,211 @@ impl Service for Notify { Ok(MsgSendDirectResp2::from_req_with_payload(&msg, payload)) } } + +// =========================================================================== +// Notify Unit Tests +// =========================================================================== +#[cfg(test)] +mod tests { + use super::*; + use odp_ffa::{DirectMessagePayload, HasRegisterPayload}; + use uuid::uuid; + + const NOTIFY_UUID: Uuid = uuid!("e474d87e-5731-4044-a727-cb3e8cf3c8df"); + const SENDER_UUID: Uuid = uuid!("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + const RECEIVER_UUID: Uuid = uuid!("11111111-2222-3333-4444-555555555555"); + + /// Build a notify request with given message_id, count, and notification tuples. + /// Each tuple is (cookie: u32, id: u16, ntype: 0=Global / 1=PerVcpu). + fn notify_req(msg_id: MessageID, count: u8, notifs: &[(u32, u16, u8)]) -> MsgSendDirectReq2 { + let mut regs = [0u64; 14]; + // regs[1-2] = sender_uuid as LE u128 split into two u64s + let sender_le = SENDER_UUID.to_u128_le(); + regs[1] = sender_le as u64; + regs[2] = (sender_le >> 64) as u64; + // regs[3-4] = receiver_uuid as LE u128 split into two u64s + let receiver_le = RECEIVER_UUID.to_u128_le(); + regs[3] = receiver_le as u64; + regs[4] = (receiver_le >> 64) as u64; + // regs[5] = msg_info (bits 0-2 = message_id) + regs[5] = msg_id as u64; + // regs[6] = count (lower 9 bits) + regs[6] = count as u64; + // regs[7..] = notification tuples: cookie(bits63:32) | id(bits31:23) | type(bit0) + for (i, (cookie, id, ntype)) in notifs.iter().enumerate().take(7) { + regs[7 + i] = ((*cookie as u64) << 32) | ((*id as u64) << 23) | (*ntype as u64); + } + let payload_bytes = regs.iter().flat_map(|r| r.to_le_bytes()); + let payload = DirectMessagePayload::from_iter(payload_bytes); + MsgSendDirectReq2::new(0x0001, 0x8001, NOTIFY_UUID, payload) + } + + /// Extract the ErrorCode status from a NfySetupRsp-shaped response. + /// NfySetupRsp layout: reg0=reserved, reg1-2=sender_uuid, reg3-4=receiver_uuid, + /// reg5=msg_info, reg6=status. + fn resp_error_code(resp: &MsgSendDirectResp2) -> i64 { + resp.payload().register_at(6) as i64 + } + + /// Extract msg_info from response. + fn resp_msg_info(resp: &MsgSendDirectResp2) -> u64 { + resp.payload().register_at(5) + } + + // =================================================================== + // Notify::Setup Test(s) + // =================================================================== + #[test] + fn test_setup_registers_service() { + let mut svc = Notify::new(); + let msg = notify_req(MessageID::Setup, 1, &[(100, 1, 0)]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::Ok as i64); + assert_eq!(resp_msg_info(&resp), MESSAGE_INFO_DIR_RESP + MessageID::Setup as u64); + } + + #[test] + fn test_setup_invalid_count_zero() { + let mut svc = Notify::new(); + let msg = notify_req(MessageID::Setup, 0, &[]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::InvalidParameters as i64); + } + + #[test] + fn test_setup_overflow_count() { + let mut svc = Notify::new(); + // NOTIFY_MAX_MAPPINGS_PER_REQ is 8, so count >= 8 should fail + let msg = notify_req(MessageID::Setup, NOTIFY_MAX_MAPPINGS_PER_REQ as u8, &[(100, 1, 0)]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::InvalidParameters as i64); + } + + // =================================================================== + // Notify::Destroy Test(s) + // =================================================================== + #[test] + fn test_destroy_after_setup() { + let mut svc = Notify::new(); + // Setup first + let setup_msg = notify_req(MessageID::Setup, 1, &[(100, 1, 0)]); + let setup_resp = svc.ffa_msg_send_direct_req2(setup_msg).unwrap(); + assert_eq!(resp_error_code(&setup_resp), ErrorCode::Ok as i64); + + // Destroy with matching cookie/id/ntype + let destroy_msg = notify_req(MessageID::Destroy, 1, &[(100, 1, 0)]); + let destroy_resp = svc.ffa_msg_send_direct_req2(destroy_msg).unwrap(); + assert_eq!(resp_error_code(&destroy_resp), ErrorCode::Ok as i64); + assert_eq!( + resp_msg_info(&destroy_resp), + MESSAGE_INFO_DIR_RESP + MessageID::Destroy as u64 + ); + } + + #[test] + fn test_destroy_unregistered_returns_error() { + let mut svc = Notify::new(); + let msg = notify_req(MessageID::Destroy, 1, &[(100, 1, 0)]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::InvalidParameters as i64); + } + + // =================================================================== + // Notify::Add Test(s) + // =================================================================== + #[test] + fn test_add_registers_mapping() { + let mut svc = Notify::new(); + let msg = notify_req(MessageID::Add, 1, &[(200, 2, 0)]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::Ok as i64); + assert_eq!(resp_msg_info(&resp), MESSAGE_INFO_DIR_RESP + MessageID::Add as u64); + } + + // =================================================================== + // Notify::Remove Test(s) + // =================================================================== + #[test] + fn test_add_then_remove() { + let mut svc = Notify::new(); + // First: Add a mapping + let add_msg = notify_req(MessageID::Add, 1, &[(200, 2, 0)]); + let add_resp = svc.ffa_msg_send_direct_req2(add_msg).unwrap(); + assert_eq!(resp_error_code(&add_resp), ErrorCode::Ok as i64); + + // Then: Remove it + let remove_msg = notify_req(MessageID::Remove, 1, &[(200, 2, 0)]); + let remove_resp = svc.ffa_msg_send_direct_req2(remove_msg).unwrap(); + assert_eq!(resp_error_code(&remove_resp), ErrorCode::Ok as i64); + assert_eq!( + resp_msg_info(&remove_resp), + MESSAGE_INFO_DIR_RESP + MessageID::Remove as u64 + ); + } + + #[test] + fn test_remove_without_add_returns_error() { + let mut svc = Notify::new(); + let msg = notify_req(MessageID::Remove, 1, &[(200, 2, 0)]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::InvalidParameters as i64); + } + + // =================================================================== + // Notify::Assign Test(s) + // =================================================================== + #[test] + fn test_assign_updates_mapping() { + let mut svc = Notify::new(); + // Setup with 1 notification (cookie=100, id=1) + let setup_msg = notify_req(MessageID::Setup, 1, &[(100, 1, 0)]); + let setup_resp = svc.ffa_msg_send_direct_req2(setup_msg).unwrap(); + assert_eq!(resp_error_code(&setup_resp), ErrorCode::Ok as i64); + + // Assign same cookie with new id=5 + let assign_msg = notify_req(MessageID::Assign, 1, &[(100, 5, 0)]); + let assign_resp = svc.ffa_msg_send_direct_req2(assign_msg).unwrap(); + assert_eq!(resp_error_code(&assign_resp), ErrorCode::Ok as i64); + assert_eq!( + resp_msg_info(&assign_resp), + MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64 + ); + } + + #[test] + fn test_assign_without_entry_returns_error() { + let mut svc = Notify::new(); + let msg = notify_req(MessageID::Assign, 1, &[(100, 5, 0)]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::InvalidParameters as i64); + } + + // =================================================================== + // Notify::Unassign Test(s) + // =================================================================== + #[test] + fn test_unassign_clears_event() { + let mut svc = Notify::new(); + // Setup with 1 notification (cookie=100, id=1) + let setup_msg = notify_req(MessageID::Setup, 1, &[(100, 1, 0)]); + let setup_resp = svc.ffa_msg_send_direct_req2(setup_msg).unwrap(); + assert_eq!(resp_error_code(&setup_resp), ErrorCode::Ok as i64); + + // Unassign by cookie + let unassign_msg = notify_req(MessageID::Unassign, 1, &[(100, 1, 0)]); + let unassign_resp = svc.ffa_msg_send_direct_req2(unassign_msg).unwrap(); + assert_eq!(resp_error_code(&unassign_resp), ErrorCode::Ok as i64); + assert_eq!( + resp_msg_info(&unassign_resp), + MESSAGE_INFO_DIR_RESP + MessageID::Unassign as u64 + ); + } + + #[test] + fn test_unassign_without_entry_returns_error() { + let mut svc = Notify::new(); + let msg = notify_req(MessageID::Unassign, 1, &[(100, 1, 0)]); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::InvalidParameters as i64); + } +} From 97dc5721e93f302288f341e4e2372502119b02f2 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:44:22 -0700 Subject: [PATCH 06/10] test(03-02): add MessageHandler dispatch unit tests - 6 tests: UUID routing to MockServiceA/B, unknown UUID error, empty handler, chain ordering - Mock services use echo pattern with distinct markers (0xAA, 0xBB) - Verifies LIFO linked-list dispatch and Error::Other("Unknown UUID") terminal --- ec-service-lib/src/message_handler.rs | 134 ++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/ec-service-lib/src/message_handler.rs b/ec-service-lib/src/message_handler.rs index 2646870..4ff60c3 100644 --- a/ec-service-lib/src/message_handler.rs +++ b/ec-service-lib/src/message_handler.rs @@ -70,3 +70,137 @@ where } } } + +// =========================================================================== +// MessageHandler Unit Tests +// =========================================================================== +#[cfg(test)] +mod tests { + use super::*; + use crate::Service; + use odp_ffa::{DirectMessagePayload, HasRegisterPayload}; + use uuid::{uuid, Uuid}; + + // =================================================================== + // Mock Services + // =================================================================== + + /// A mock service that echoes the first payload register + marker 0xAA. + struct MockServiceA; + + impl Service for MockServiceA { + const UUID: Uuid = uuid!("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); + const NAME: &'static str = "MockA"; + + fn ffa_msg_send_direct_req2( + &mut self, + msg: MsgSendDirectReq2, + ) -> core::result::Result { + let input = msg.payload().register_at(0); + let regs = [input, 0xAA]; + let payload = DirectMessagePayload::from_iter(regs.iter().flat_map(|r| r.to_le_bytes())); + Ok(MsgSendDirectResp2::from_req_with_payload(&msg, payload)) + } + } + + /// A mock service that echoes the first payload register + marker 0xBB. + struct MockServiceB; + + impl Service for MockServiceB { + const UUID: Uuid = uuid!("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"); + const NAME: &'static str = "MockB"; + + fn ffa_msg_send_direct_req2( + &mut self, + msg: MsgSendDirectReq2, + ) -> core::result::Result { + let input = msg.payload().register_at(0); + let regs = [input, 0xBB]; + let payload = DirectMessagePayload::from_iter(regs.iter().flat_map(|r| r.to_le_bytes())); + Ok(MsgSendDirectResp2::from_req_with_payload(&msg, payload)) + } + } + + // =================================================================== + // Helpers + // =================================================================== + + fn make_msg(uuid: Uuid, value: u64) -> MsgSendDirectReq2 { + let regs = [value]; + let payload = DirectMessagePayload::from_iter(regs.iter().flat_map(|r| r.to_le_bytes())); + MsgSendDirectReq2::new(0x0001, 0x8001, uuid, payload) + } + + // =================================================================== + // UUID Routing Tests (MHD-01) + // =================================================================== + #[test] + fn test_routes_to_correct_service_a() { + let mut handler = MessageHandler::new().append(MockServiceA).append(MockServiceB); + let msg = make_msg(MockServiceA::UUID, 42); + let resp = handler.node.handle(msg).unwrap(); + assert_eq!(resp.payload().register_at(0), 42); // echoed value + assert_eq!(resp.payload().register_at(1), 0xAA); // MockA marker + } + + #[test] + fn test_routes_to_correct_service_b() { + let mut handler = MessageHandler::new().append(MockServiceA).append(MockServiceB); + let msg = make_msg(MockServiceB::UUID, 99); + let resp = handler.node.handle(msg).unwrap(); + assert_eq!(resp.payload().register_at(0), 99); + assert_eq!(resp.payload().register_at(1), 0xBB); + } + + // =================================================================== + // Unknown UUID Tests (MHD-02) + // =================================================================== + #[test] + fn test_unknown_uuid_returns_error() { + let mut handler = MessageHandler::new().append(MockServiceA).append(MockServiceB); + let unknown_uuid = uuid!("cccccccc-cccc-cccc-cccc-cccccccccccc"); + let msg = make_msg(unknown_uuid, 0); + let result = handler.node.handle(msg); + assert!(result.is_err()); + match result.unwrap_err() { + odp_ffa::Error::Other(s) => assert_eq!(s, "Unknown UUID"), + e => panic!("Expected Error::Other(\"Unknown UUID\"), got {:?}", e), + } + } + + #[test] + fn test_empty_handler_returns_error() { + let mut handler = MessageHandler::new(); + let msg = make_msg(MockServiceA::UUID, 0); + let result = handler.node.handle(msg); + assert!(result.is_err()); + } + + // =================================================================== + // Service Chain Ordering Tests (MHD-03) + // =================================================================== + #[test] + fn test_service_chain_ordering_both_reachable() { + // append order: A first, B second + // linked list order: B -> A -> Terminal (LIFO) + let mut handler = MessageHandler::new().append(MockServiceA).append(MockServiceB); + + // Both should be reachable regardless of position in chain + let resp_a = handler.node.handle(make_msg(MockServiceA::UUID, 1)).unwrap(); + assert_eq!(resp_a.payload().register_at(1), 0xAA); + + let resp_b = handler.node.handle(make_msg(MockServiceB::UUID, 2)).unwrap(); + assert_eq!(resp_b.payload().register_at(1), 0xBB); + } + + #[test] + fn test_single_service_routes_correctly() { + let mut handler = MessageHandler::new().append(MockServiceA); + let resp = handler.node.handle(make_msg(MockServiceA::UUID, 7)).unwrap(); + assert_eq!(resp.payload().register_at(1), 0xAA); + + // Unknown UUID should still error + let unknown = uuid!("dddddddd-dddd-dddd-dddd-dddddddddddd"); + assert!(handler.node.handle(make_msg(unknown, 0)).is_err()); + } +} From 90742452e0f2ef43e1977ba479eae77b55ebd21f Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:45:52 -0700 Subject: [PATCH 07/10] test(03-02): add TPM extended handler tests + cargo fmt - 9 new TPM tests: get_feature_info (notification/unknown), register/unregister state transitions, double register -> Already, finish with/without registration, deinit state reset, register after deinit - cargo fmt fixups for all test modules (alignment/line-length) - Total: 94 tests passing (62 existing + 32 new), zero regressions --- ec-service-lib/src/services/tpm.rs | 151 +++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/ec-service-lib/src/services/tpm.rs b/ec-service-lib/src/services/tpm.rs index d6cefb0..b3bf90c 100644 --- a/ec-service-lib/src/services/tpm.rs +++ b/ec-service-lib/src/services/tpm.rs @@ -1401,4 +1401,155 @@ mod tests { let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); assert_eq!(resp_status(&resp), TpmStatus::NoFunc as u64); } + + // ======================================================================= + // TpmService::GetFeatureInfo Extended Test(s) + // ======================================================================= + #[test] + fn test_get_feature_info_notification() { + let mut service = TpmService::new(MockTpmSst::new()); + let msg = direct_req2_msg( + 0x0000, + 0x0000, + TpmFunction::GetFeatureInfo as u64, + TpmFunction::RegisterForNotification as u64, // Query notification feature + 0x00, + ); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::OkResultsReturned as u64); + assert_eq!(resp_payload(&resp) & 0x1, 0x1); // Notifications supported + } + + #[test] + fn test_get_feature_info_unknown_feature() { + // Mirrors existing test_get_feature_info — unknown feature returns NotSup + let mut service = TpmService::new(MockTpmSst::new()); + let msg = direct_req2_msg( + 0x0000, + 0x0000, + TpmFunction::GetFeatureInfo as u64, + 0x00, // Unknown feature ID + 0x00, + ); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::NotSup as u64); + } + + // ======================================================================= + // TpmService::Register/Unregister State Transition Test(s) + // ======================================================================= + #[test] + fn test_register_then_unregister() { + let mut service = TpmService::new(MockTpmSst::new()); + // Register + let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::RegisterForNotification as u64, 0, 0); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::Ok as u64); + assert!(service.notification_registered); + + // Unregister + let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::UnregisterForNotification as u64, 0, 0); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::Ok as u64); + assert!(!service.notification_registered); + } + + #[test] + fn test_register_double_returns_already() { + let mut service = TpmService::new(MockTpmSst::new()); + let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::RegisterForNotification as u64, 0, 0); + service.ffa_msg_send_direct_req2(msg.clone()).unwrap(); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::Already as u64); + } + + #[test] + fn test_unregister_without_register_returns_denied() { + let mut service = TpmService::new(MockTpmSst::new()); + let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::UnregisterForNotification as u64, 0, 0); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::Denied as u64); + } + + // ======================================================================= + // TpmService::Finish Test(s) — Extended + // ======================================================================= + #[test] + fn test_finish_after_register() { + let mut service = TpmService::new(MockTpmSst::new()); + // Register first + let reg_msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::RegisterForNotification as u64, 0, 0); + service.ffa_msg_send_direct_req2(reg_msg).unwrap(); + + // Finish should succeed + let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::FinishNotified as u64, 0, 0); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::Ok as u64); + } + + #[test] + fn test_finish_without_register_returns_denied() { + let mut service = TpmService::new(MockTpmSst::new()); + let msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::FinishNotified as u64, 0, 0); + let resp = service.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::Denied as u64); + } + + // ======================================================================= + // TpmService::Deinit Test(s) + // ======================================================================= + #[test] + fn test_deinit_resets_state() { + let (buff, addr) = alloc_crb_region(); + let mut service = TpmService::new(MockTpmSst::new()); + unsafe { service.init(addr) }; + + // Open a locality + let open_msg = direct_req2_msg( + 0xFF00, + 0x0000, + TpmFunction::ManageLocality as u64, + TPM2_FFA_MANAGE_LOCALITY_OPEN as u64, + 0, + ); + service.ffa_msg_send_direct_req2(open_msg).unwrap(); + + // Register notification + let reg_msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::RegisterForNotification as u64, 0, 0); + service.ffa_msg_send_direct_req2(reg_msg).unwrap(); + + // Verify state is modified + assert_eq!(service.locality_states[0], TpmLocalityState::Open); + assert!(service.notification_registered); + + // Deinit should reset everything + service.deinit(); + assert_eq!(service.current_state, TpmState::Idle); + assert_eq!(service.active_locality, NO_ACTIVE_LOCALITY); + assert_eq!( + service.locality_states, + [TpmLocalityState::Closed; NUM_LOCALITIES as usize] + ); + assert!(!service.notification_registered); + + drop(buff); // prevent early deallocation + } + + #[test] + fn test_register_after_deinit() { + let mut service = TpmService::new(MockTpmSst::new()); + // Register, then deinit + let reg_msg = direct_req2_msg(0x0000, 0x0000, TpmFunction::RegisterForNotification as u64, 0, 0); + service.ffa_msg_send_direct_req2(reg_msg).unwrap(); + assert!(service.notification_registered); + + service.deinit(); + assert!(!service.notification_registered); + + // Re-register should work (state was properly reset) + let reg_msg2 = direct_req2_msg(0x0000, 0x0000, TpmFunction::RegisterForNotification as u64, 0, 0); + let resp = service.ffa_msg_send_direct_req2(reg_msg2).unwrap(); + assert_eq!(resp_status(&resp), TpmStatus::Ok as u64); + assert!(service.notification_registered); + } } From 3f5ffe117382143ce5db565ed519081573fd1ed6 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:57:13 -0700 Subject: [PATCH 08/10] fix(03): prevent panic on invalid MessageID and shift overflow in notify CR-01: MessageInfo::message_id() now returns Option instead of panicking via .expect(). Invalid bit patterns (6, 7) return an error response rather than crashing the secure partition. CR-02: All 1<= 64, preventing shift overflow panics in debug and silent bitmap corruption in release builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ec-service-lib/src/services/notify.rs | 62 +++++++++++++++++++++------ 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/ec-service-lib/src/services/notify.rs b/ec-service-lib/src/services/notify.rs index f4ace2c..f62818c 100644 --- a/ec-service-lib/src/services/notify.rs +++ b/ec-service-lib/src/services/notify.rs @@ -123,9 +123,9 @@ impl From for DirectMessagePayload { struct MessageInfo(u64); impl MessageInfo { - /// Get the message ID (bits 0–2). - fn message_id(&self) -> MessageID { - ((self.0 & 0b111) as u8).try_into().expect("Invalid Message ID") + /// Get the message ID (bits 0–2). Returns None for invalid bit patterns. + fn message_id(&self) -> Option { + ((self.0 & 0b111) as u8).try_into().ok() } /// Construct from a raw u64. @@ -137,12 +137,21 @@ impl MessageInfo { #[derive(Default, Debug, Copy, Clone)] struct NfyMapping { cookie: u32, // Cookie for the notification - id: u16, // Global bitmask value + id: u16, // Global bitmask value (must be < 64 for u64 bitmap) ntype: NotifyType, // Type of notification (Global or PerVcpu) src_id: u16, // Source ID for the notification in_use: bool, // Whether the notification mapping is currently in use } +/// Safely compute a bitmask for a notification ID, returning None if id >= 64. +fn nfy_bitmask(id: u16) -> Option { + if id < 64 { + Some(1u64 << id) + } else { + None + } +} + #[derive(Debug, Copy, Clone)] struct NfyEntry { service_uuid: Uuid, @@ -217,9 +226,9 @@ impl Notify { // If we found a matching cookie, this does not make sense, so we return an error error!("Found matching cookie for entry {entry_index}: {cookie}"); return ErrorCode::InvalidParameters; - } else if temp_bitmask & (1 << id) != 0 { - // If the bit is already set, we cannot register this mapping - error!("Bitmask already set for entry {entry_index}: {id}"); + } else if nfy_bitmask(*id).is_none_or(|b| temp_bitmask & b != 0) { + // If the id is out of range or bit is already set, reject + error!("Bitmask conflict or out-of-range id for entry {entry_index}: {id}"); return ErrorCode::InvalidParameters; } else { // No matching cookie found, we can register this mapping @@ -237,7 +246,7 @@ impl Notify { mapping.ntype = ntype; mapping.src_id = req.src_id; mapping.in_use = true; - temp_bitmask |= 1 << id; // Set the bit in the global bitmap + temp_bitmask |= nfy_bitmask(id).unwrap(); // Safe: validated above applied = true; break; } @@ -507,9 +516,24 @@ impl Notify { match self.nfy_find_matching_cookie(entry_index, *cookie) { Some(mapping_index) => { let mapping = &mut self.entries[entry_index].mappings[mapping_index]; - // Clear old bit, set new bit - self.global_bitmap &= !(1 << mapping.id); - if self.global_bitmap & (1 << id) != 0 { + // Clear old bit, set new bit (safe: mapping.id was validated on insert) + if let Some(old_bit) = nfy_bitmask(mapping.id) { + self.global_bitmap &= !old_bit; + } + let new_bit = match nfy_bitmask(*id) { + Some(b) => b, + None => { + error!("Assign id out of range: {id}"); + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, + status: ErrorCode::InvalidParameters, + }; + } + }; + if self.global_bitmap & new_bit != 0 { error!("Bitmask conflict during assign for id: {id}"); return NfySetupRsp { reserved: 0, @@ -521,7 +545,7 @@ impl Notify { } mapping.id = *id; mapping.ntype = *ntype; - self.global_bitmap |= 1 << id; + self.global_bitmap |= new_bit; } None => { error!("No matching cookie for assign: {cookie}"); @@ -563,7 +587,9 @@ impl Notify { match self.nfy_find_matching_cookie(entry_index, *cookie) { Some(mapping_index) => { let mapping = &mut self.entries[entry_index].mappings[mapping_index]; - self.global_bitmap &= !(1 << mapping.id); + if let Some(bit) = nfy_bitmask(mapping.id) { + self.global_bitmap &= !bit; + } mapping.id = 0; mapping.ntype = NotifyType::Global; // Keep in_use = true and cookie — slot is reserved, just unassigned @@ -599,7 +625,15 @@ impl Service for Notify { let req: NotifyReq = msg.clone().into(); debug!("Received notify command: {:?}", req.msg_info.message_id()); - let payload = match req.msg_info.message_id() { + let message_id = match req.msg_info.message_id() { + Some(id) => id, + None => { + error!("Invalid notify message ID: {}", req.msg_info.0 & 0b111); + return Err(odp_ffa::Error::Other("Invalid notify message ID")); + } + }; + + let payload = match message_id { MessageID::Setup => DirectMessagePayload::from(self.nfy_setup(req)), MessageID::Destroy => DirectMessagePayload::from(self.nfy_destroy(req)), MessageID::Add => DirectMessagePayload::from(self.nfy_add(req)), From cf8118dde8a2531761f8ce179d41dab63a7d2d13 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 23:29:30 -0700 Subject: [PATCH 09/10] feat(tpm): add TPM2_Startup test helper for E2E command execution coverage Add test_write_crb operation 5 that prepares a TPM2_Startup(CLEAR) command in the CRB data buffer with correct command_size. This enables E2E tests to exercise the full sst.start() pipeline (copy_command_data, start_command, copy_response_data) through the emulated TPM CRB interface. Only available when test-bypass-locality-check feature is enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ec-service-lib/src/services/tpm.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ec-service-lib/src/services/tpm.rs b/ec-service-lib/src/services/tpm.rs index b3bf90c..b6e1847 100644 --- a/ec-service-lib/src/services/tpm.rs +++ b/ec-service-lib/src/services/tpm.rs @@ -646,6 +646,7 @@ impl TpmService { /// 2 = crb_control_request = CMD_READY /// 3 = crb_control_request = GO_IDLE /// 4 = crb_control_start = START + /// 5 = prepare TPM2_Startup command in CRB data buffer #[cfg(feature = "test-bypass-locality-check")] fn test_write_crb(&mut self, request: &TpmRequest) -> TpmStatus { let operation = request.function as u16; @@ -663,6 +664,14 @@ impl TpmService { 2 => crb.crb_control_request = PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY, 3 => crb.crb_control_request = PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE, 4 => crb.crb_control_start = PTP_CRB_CONTROL_START, + 5 => { + // Write a TPM2_Startup(CLEAR) command into the CRB data buffer. + // TPM2_Startup header (big-endian): + // tag=0x8001, size=12, code=0x144, startupType=0x0000 + let tpm2_startup: [u8; 12] = [0x80, 0x01, 0x00, 0x00, 0x00, 0x0C, 0x00, 0x00, 0x01, 0x44, 0x00, 0x00]; + crb.crb_data_buffer[..12].copy_from_slice(&tpm2_startup); + crb.crb_control_command_size = 12; + } _ => { error!("test_write_crb: Invalid Operation"); return TpmStatus::InvArg; From fff1bd24afd662bcecc235d00c1426bfa8e43810 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Wed, 15 Apr 2026 12:33:19 -0700 Subject: [PATCH 10/10] fix: make nfy_assign/nfy_unassign atomic; fix overflow count test nfy_assign: pre-validate all tuples (cookie exists, id in range, no bitmap conflicts) using a simulated bitmap before applying any mutations. On error, no state is modified. nfy_unassign: pre-validate all cookies exist before clearing any mappings. Bitmap is updated atomically after all mutations. test_setup_overflow_count: NotifyReq::from() clamps count with .min(7), so sending count=8 never reached the >= 8 guard. Replace with two tests: one verifying clamped count=8 succeeds with 7 valid tuples, another exercising count=7 directly as the effective maximum. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ec-service-lib/src/services/notify.rs | 159 ++++++++++++++++---------- 1 file changed, 101 insertions(+), 58 deletions(-) diff --git a/ec-service-lib/src/services/notify.rs b/ec-service-lib/src/services/notify.rs index f62818c..53d7c99 100644 --- a/ec-service-lib/src/services/notify.rs +++ b/ec-service-lib/src/services/notify.rs @@ -512,43 +512,31 @@ impl Notify { } }; - for (cookie, id, ntype) in req.notifications.iter().take(req.count as usize) { - match self.nfy_find_matching_cookie(entry_index, *cookie) { - Some(mapping_index) => { - let mapping = &mut self.entries[entry_index].mappings[mapping_index]; - // Clear old bit, set new bit (safe: mapping.id was validated on insert) - if let Some(old_bit) = nfy_bitmask(mapping.id) { - self.global_bitmap &= !old_bit; - } - let new_bit = match nfy_bitmask(*id) { - Some(b) => b, - None => { - error!("Assign id out of range: {id}"); - return NfySetupRsp { - reserved: 0, - sender_uuid: req.sender_uuid, - receiver_uuid: req.receiver_uuid, - msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, - status: ErrorCode::InvalidParameters, - }; - } + // Pre-validation pass: check all tuples before mutating any state. + // Track a simulated bitmap to detect cross-tuple conflicts. + let mut sim_bitmap = self.global_bitmap; + for (cookie, id, _ntype) in req.notifications.iter().take(req.count as usize) { + let mapping_index = match self.nfy_find_matching_cookie(entry_index, *cookie) { + Some(idx) => idx, + None => { + error!("No matching cookie for assign: {cookie}"); + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, + status: ErrorCode::InvalidParameters, }; - if self.global_bitmap & new_bit != 0 { - error!("Bitmask conflict during assign for id: {id}"); - return NfySetupRsp { - reserved: 0, - sender_uuid: req.sender_uuid, - receiver_uuid: req.receiver_uuid, - msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, - status: ErrorCode::InvalidParameters, - }; - } - mapping.id = *id; - mapping.ntype = *ntype; - self.global_bitmap |= new_bit; } + }; + let mapping = &self.entries[entry_index].mappings[mapping_index]; + if let Some(old_bit) = nfy_bitmask(mapping.id) { + sim_bitmap &= !old_bit; + } + let new_bit = match nfy_bitmask(*id) { + Some(b) => b, None => { - error!("No matching cookie for assign: {cookie}"); + error!("Assign id out of range: {id}"); return NfySetupRsp { reserved: 0, sender_uuid: req.sender_uuid, @@ -557,8 +545,28 @@ impl Notify { status: ErrorCode::InvalidParameters, }; } + }; + if sim_bitmap & new_bit != 0 { + error!("Bitmask conflict during assign for id: {id}"); + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Assign as u64, + status: ErrorCode::InvalidParameters, + }; } + sim_bitmap |= new_bit; + } + + // All validated — apply mutations atomically. + for (cookie, id, ntype) in req.notifications.iter().take(req.count as usize) { + let mapping_index = self.nfy_find_matching_cookie(entry_index, *cookie).unwrap(); + let mapping = &mut self.entries[entry_index].mappings[mapping_index]; + mapping.id = *id; + mapping.ntype = *ntype; } + self.global_bitmap = sim_bitmap; NfySetupRsp { reserved: 0, @@ -583,30 +591,34 @@ impl Notify { } }; + // Pre-validation pass: ensure all cookies exist before mutating. for (cookie, _id, _ntype) in req.notifications.iter().take(req.count as usize) { - match self.nfy_find_matching_cookie(entry_index, *cookie) { - Some(mapping_index) => { - let mapping = &mut self.entries[entry_index].mappings[mapping_index]; - if let Some(bit) = nfy_bitmask(mapping.id) { - self.global_bitmap &= !bit; - } - mapping.id = 0; - mapping.ntype = NotifyType::Global; - // Keep in_use = true and cookie — slot is reserved, just unassigned - } - None => { - error!("No matching cookie for unassign: {cookie}"); - return NfySetupRsp { - reserved: 0, - sender_uuid: req.sender_uuid, - receiver_uuid: req.receiver_uuid, - msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Unassign as u64, - status: ErrorCode::InvalidParameters, - }; - } + if self.nfy_find_matching_cookie(entry_index, *cookie).is_none() { + error!("No matching cookie for unassign: {cookie}"); + return NfySetupRsp { + reserved: 0, + sender_uuid: req.sender_uuid, + receiver_uuid: req.receiver_uuid, + msg_info: MESSAGE_INFO_DIR_RESP + MessageID::Unassign as u64, + status: ErrorCode::InvalidParameters, + }; } } + // All validated — apply mutations atomically. + let mut new_bitmap = self.global_bitmap; + for (cookie, _id, _ntype) in req.notifications.iter().take(req.count as usize) { + let mapping_index = self.nfy_find_matching_cookie(entry_index, *cookie).unwrap(); + let mapping = &mut self.entries[entry_index].mappings[mapping_index]; + if let Some(bit) = nfy_bitmask(mapping.id) { + new_bitmap &= !bit; + } + mapping.id = 0; + mapping.ntype = NotifyType::Global; + // Keep in_use = true and cookie — slot is reserved, just unassigned + } + self.global_bitmap = new_bitmap; + NfySetupRsp { reserved: 0, sender_uuid: req.sender_uuid, @@ -717,12 +729,43 @@ mod tests { } #[test] - fn test_setup_overflow_count() { + fn test_setup_overflow_count_clamped() { + // NotifyReq::from() clamps count with .min(7), so a raw count of + // NOTIFY_MAX_MAPPINGS_PER_REQ (8) becomes 7. Verify the request + // succeeds with 7 distinct valid tuples rather than hitting the + // `count >= NOTIFY_MAX_MAPPINGS_PER_REQ` guard. let mut svc = Notify::new(); - // NOTIFY_MAX_MAPPINGS_PER_REQ is 8, so count >= 8 should fail - let msg = notify_req(MessageID::Setup, NOTIFY_MAX_MAPPINGS_PER_REQ as u8, &[(100, 1, 0)]); + let tuples: [(u32, u16, u8); 7] = [ + (100, 1, 0), + (101, 2, 0), + (102, 3, 0), + (103, 4, 0), + (104, 5, 0), + (105, 6, 0), + (106, 7, 0), + ]; + let msg = notify_req(MessageID::Setup, NOTIFY_MAX_MAPPINGS_PER_REQ as u8, &tuples); let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); - assert_eq!(resp_error_code(&resp), ErrorCode::InvalidParameters as i64); + assert_eq!(resp_error_code(&resp), ErrorCode::Ok as i64); + } + + #[test] + fn test_setup_max_valid_count() { + // count=7 is the effective maximum (from() clamps to .min(7)). + // Verify 7 distinct tuples register successfully. + let mut svc = Notify::new(); + let tuples: [(u32, u16, u8); 7] = [ + (200, 10, 0), + (201, 11, 0), + (202, 12, 0), + (203, 13, 0), + (204, 14, 0), + (205, 15, 0), + (206, 16, 0), + ]; + let msg = notify_req(MessageID::Setup, 7, &tuples); + let resp = svc.ffa_msg_send_direct_req2(msg).unwrap(); + assert_eq!(resp_error_code(&resp), ErrorCode::Ok as i64); } // ===================================================================