Add syscall driver support for mctp capsule #59
Conversation
… into pbhogaraju/mctp_syscall_driver
parvathib
requested review from
swenson,
mlvisaya,
mhatrevi,
vsonims and
helloxiling
runtime/capsules/src/mctp/driver.rs
Outdated
| } | ||
|
|
||
| impl OpContext { | ||
| fn for_me(&self, msg_tag: u8, peer_eid: u8, msg_type: u8) -> bool { |
There was a problem hiding this comment.
for_me is a little confusing. Maybe matches or is_compatible?
There was a problem hiding this comment.
Haha, I've been switching between for_me and is_matched before pushing the PR. Thanks for the help with the naming! I'll go with matches.
runtime/capsules/src/mctp/driver.rs
Outdated
| kernel_msg_buf, | ||
| ) { | ||
| Ok(_) => { | ||
| println!("MCTPDriver: send_msg success"); |
There was a problem hiding this comment.
Remove debugging printlns?
runtime/capsules/src/mctp/driver.rs
Outdated
| ); | ||
| kernel_msg_buf.slice(0..wpayload.len()); | ||
|
|
||
| match self.sender.send_msg( |
There was a problem hiding this comment.
This is getting deeply nested -- maybe break up this command function or some parts of it so it is easier to follow along? Some comments probably could help as well.
There was a problem hiding this comment.
I cleaned send message part of command a little and added some comments. This might need further refining once I start testing user apps. Let me know how it looks now.
runtime/capsules/src/mctp/driver.rs
Outdated
| match res { | ||
| Ok(()) => Ok(()), | ||
| Err(e) => Err(e), | ||
| } | ||
| // Ok(()) |
There was a problem hiding this comment.
| match res { | |
| Ok(()) => Ok(()), | |
| Err(e) => Err(e), | |
| } | |
| // Ok(()) | |
| res |
?
There was a problem hiding this comment.
Refactored this part of the code. Let me know if this looks okay now.
| MctpControl = 0, | ||
| Pldm = 1, | ||
| Spdm = 5, | ||
| SecureSPDM = 6, |
There was a problem hiding this comment.
nit: We're being a bit inconsistent with if we are capitalizing initialisms SPDM or Spdm.
I think Tock in general uses Spdm style, so we should stick with that.
There was a problem hiding this comment.
Noted! Changed them accordingly.
runtime/capsules/src/mctp/recv.rs
Outdated
| /// # Returns | ||
| /// True if the message type is expected, false otherwise. | ||
| pub fn is_receive_expected(&self, msg_type: MessageType) -> bool { | ||
| let msg_types = self.msg_types.get(); |
There was a problem hiding this comment.
I can't decide if this is easier to read, or if the functional style is:
self.msg_types.get().iter().find(|t| *t == msg_type).unwrap_or_default()There was a problem hiding this comment.
Changed to self.msg_types.get().iter().any(|&exp_type| exp_type == msg_type)
runtime/capsules/src/mctp/recv.rs
Outdated
| self.msg_terminus.replace(msg_terminus); | ||
| }) | ||
| .unwrap_or_else(|| { | ||
| println!( |
There was a problem hiding this comment.
Maybe panic! in that case?
There was a problem hiding this comment.
Right! corrected it.
runtime/capsules/src/mctp/send.rs
Outdated
| use zerocopy::IntoBytes; | ||
|
|
||
| use core::cell::Cell; | ||
|
|
There was a problem hiding this comment.
nit: clean up spaces between imports?
… into pbhogaraju/mctp_syscall_driver
This PR implements the syscall driver support for the MCTP capsule includes the instantiation of the MCTP capsule driver for SPDM, PLDM, and vendor-defined PCI message types.
I'll incorporate some MCTP MUX layer testing in the kernel in the upcoming PR.