Create system capsules crate, move kernel implementations, update boards #3992
Conversation
These trait implementations do not need to be in the core kernel.
github-actions
bot
added
kernel
WG-OpenTitan
| pub flash: &'static [u8], | ||
|
|
||
| /// The footers of the process binary (may be zero-sized), which are metadata | ||
| /// about the process not covered by integrity. Used, among other things, to | ||
| /// store signatures. | ||
| pub(crate) footers: &'static [u8], | ||
| pub footers: &'static [u8], | ||
|
|
||
| /// Collection of pointers to the TBF header in flash. | ||
| pub(crate) header: tock_tbf::types::TbfHeader, | ||
| pub header: tock_tbf::types::TbfHeader, |
There was a problem hiding this comment.
While I don't think there's necessarily anything in a read-only reference to process flash that needs to be protected, this is a pretty big change in visibility. My instinct is that it's preferred to prevent any capsule from accessing this more willy-nilly.
Having only googled this briefly, can we use the pub (in path) syntax to restrict this to just the system capsules?
There was a problem hiding this comment.
If a capsule can get a ProcessBinary, I'm not sure there's any harm in accessing it.
There was a problem hiding this comment.
I think we should really think of pub(crate) as serving two possible roles:
- Protecting things that can break security (e.g. exposing a private field or allowing construction of a protected type) from other crates
- Avoiding exposing things to other crates because it would make maintaining things harder if those methods/fields are not stable.
We don't really have a general story for (2) so I'm not super worried about this. I don't think (1) applies here.
| //! This file contains definitions of policies the Tock kernel can use when | ||
| //! managing processes. For example, these policies control decisions such as | ||
| //! whether a specific process should be restarted. |
There was a problem hiding this comment.
This file / description is a bit weird now since it doesn't actually contain any policies anymore?
There was a problem hiding this comment.
It still defines what we need policies for.
| @@ -17,7 +14,7 @@ pub struct ProcessPrinterContext { | |||
| /// The overall print message is broken in to chunks so that it can be fit | |||
| /// in a small buffer that is called multiple times. This tracks which byte | |||
| /// we are at so we can ignore the text before and print the next bytes. | |||
| offset: usize, | |||
| pub offset: usize, | |||
There was a problem hiding this comment.
Same visibility comment here; more motivated by this being a somewhat tightly coupled interface than any concerns regarding access.
Spelling fix Co-authored-by: Pat Pannuto <pat.pannuto@gmail.com>
Pull Request Overview
Implements #3845.
The core kernel exposes several interfaces for implementations to customize the kernel. We have some default implementations, and for convenience we have kept those implementations in the core kernel. However, this code does not need to be in the kernel crate. This PR moves these implementations to a new capsule crate (
systemcapsules). This moves more code to crates whereunsafeis prohibited.Testing Strategy
travis
TODO or Help Wanted
I thought moving the schedulers would make sense too, but the scheduler trait has unsafe functions and uses internal kernel APIs.
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.