Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Kernel needs to be extended so that leases can span MPU regions #1672

Closed
cbiffle opened this issue Mar 20, 2024 · 2 comments
Closed

Kernel needs to be extended so that leases can span MPU regions #1672

cbiffle opened this issue Mar 20, 2024 · 2 comments
Labels
kernel Relates to the Hubris kernel robustness Fixing this would improve robustness of deployed firmware

Comments

@cbiffle
Copy link
Collaborator

cbiffle commented Mar 20, 2024

Currently, the kernel's memory access code won't let a task point it at a region that spans MPU descriptors. This was a deliberate simplification in the original system, where tasks had regions for "flash" and "RAM" which were often gigabytes away in the physical address space -- so spanning them was never correct.

However, the build system now (thanks to @mkeeter) smartly packs tasks into multiple region descriptors if possible. This has the side effect of inserting difficult to predict region descriptor boundaries into the middle of task flash and RAM.

@arjenroodselaar has just noticed that this is a ticking timebomb, since passing a data structure to the kernel directly or as a lease to another task may or may not fail depending on where those hard-to-predict boundaries fall.

We need to fix this oversimplification in the kernel and get it to tolerate such things. Meanwhile in #1671 @mkeeter has temporarily disabled packing in the build system to avoid triggering this.
@cbiffle cbiffle added kernel Relates to the Hubris kernel robustness Fixing this would improve robustness of deployed firmware labels Mar 20, 2024
@cbiffle
Copy link
Collaborator Author

cbiffle commented Mar 20, 2024

Alright, so, probably the easiest way of fixing this is to ensure the region table for each task is sorted in address order, and then do a linear sweep over it attempting to tile the given user slice with regions.

The order of descriptors in a task's region table is meaningful, but probably not observably so. On ARM M-profile, in the event that two regions overlap, the order is used to determine which takes precedence. We don't overlap regions --- there are data structures in the build system that prevent it --- so I'm pretty sure this isn't relevant to our system. Task region tables are also opaque to the tasks, which was how we were able to do better build packing in the first place without breaking code.

So, I think sorting them can be done trivially. Then it's a matter of writing the scan code.

cbiffle added a commit that referenced this issue Mar 20, 2024
@cbiffle
kern: permit USlices to span contiguous MPU regions
6e1a169 
Now that the build system has started packing tasks more cleverly, MPU
region boundaries can appear in the middle of task RAM and Flash in
positions that are difficult for the program to predict. This means the
program can't safely lend any section of RAM or Flash without risking
drawing the ire of the kernel, which --- owing to the limitations of
earlier versions of the build system --- would not accept any access
that spanned a region.

This fixes that. To make fixing that relatively inexpensive, this also
sorts task region tables by base address, so we can do a linear scan
with early exit.

See #1672 for more details and analysis.
@cbiffle cbiffle mentioned this issue Mar 20, 2024
Merged
cbiffle added a commit that referenced this issue Mar 21, 2024
@cbiffle
kern: permit USlices to span contiguous MPU regions
ff1e8b9 
Now that the build system has started packing tasks more cleverly, MPU
region boundaries can appear in the middle of task RAM and Flash in
positions that are difficult for the program to predict. This means the
program can't safely lend any section of RAM or Flash without risking
drawing the ire of the kernel, which --- owing to the limitations of
earlier versions of the build system --- would not accept any access
that spanned a region.

This fixes that. To make fixing that relatively inexpensive, this also
sorts task region tables by base address, so we can do a linear scan
with early exit.

See #1672 for more details and analysis.
cbiffle added a commit that referenced this issue Mar 21, 2024
@cbiffle
kern: permit USlices to span contiguous MPU regions
32daba4 
Now that the build system has started packing tasks more cleverly, MPU
region boundaries can appear in the middle of task RAM and Flash in
positions that are difficult for the program to predict. This means the
program can't safely lend any section of RAM or Flash without risking
drawing the ire of the kernel, which --- owing to the limitations of
earlier versions of the build system --- would not accept any access
that spanned a region.

This fixes that. To make fixing that relatively inexpensive, this also
sorts task region tables by base address, so we can do a linear scan
with early exit.

See #1672 for more details and analysis.
cbiffle added a commit that referenced this issue Mar 21, 2024
@cbiffle
kern: permit USlices to span contiguous MPU regions
cdbdbd3 
Now that the build system has started packing tasks more cleverly, MPU
region boundaries can appear in the middle of task RAM and Flash in
positions that are difficult for the program to predict. This means the
program can't safely lend any section of RAM or Flash without risking
drawing the ire of the kernel, which --- owing to the limitations of
earlier versions of the build system --- would not accept any access
that spanned a region.

This fixes that. To make fixing that relatively inexpensive, this also
sorts task region tables by base address, so we can do a linear scan
with early exit.

See #1672 for more details and analysis.
cbiffle added a commit that referenced this issue Mar 21, 2024
@cbiffle
kern: permit USlices to span contiguous MPU regions
b44e677 
Now that the build system has started packing tasks more cleverly, MPU
region boundaries can appear in the middle of task RAM and Flash in
positions that are difficult for the program to predict. This means the
program can't safely lend any section of RAM or Flash without risking
drawing the ire of the kernel, which --- owing to the limitations of
earlier versions of the build system --- would not accept any access
that spanned a region.

This fixes that. To make fixing that relatively inexpensive, this also
sorts task region tables by base address, so we can do a linear scan
with early exit.

See #1672 for more details and analysis.
@cbiffle
Copy link
Collaborator Author

cbiffle commented Mar 21, 2024

Alright, we believe this to be fixed. Packing is still disabled in the build system at this time, but, we can probably turn it back on safely.

We have actual unit tests for the new algorithm in the new kerncore crate, at @hawkw's prodding.

@cbiffle cbiffle closed this as completed Mar 21, 2024
lzrd pushed a commit that referenced this issue Mar 26, 2024
@cbiffle @lzrd
kern: permit USlices to span contiguous MPU regions
f2d0522 
Now that the build system has started packing tasks more cleverly, MPU
region boundaries can appear in the middle of task RAM and Flash in
positions that are difficult for the program to predict. This means the
program can't safely lend any section of RAM or Flash without risking
drawing the ire of the kernel, which --- owing to the limitations of
earlier versions of the build system --- would not accept any access
that spanned a region.

This fixes that. To make fixing that relatively inexpensive, this also
sorts task region tables by base address, so we can do a linear scan
with early exit.

See #1672 for more details and analysis.
cbiffle added a commit that referenced this issue Mar 29, 2024
@cbiffle
build: re-enable task packing.
2163e26 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
@cbiffle cbiffle mentioned this issue Mar 29, 2024
Merged
cbiffle added a commit that referenced this issue Mar 29, 2024
@cbiffle
build: re-enable task packing.
3876c15 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
cbiffle added a commit that referenced this issue Mar 29, 2024
@cbiffle
build: re-enable task packing.
475833b 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
cbiffle added a commit that referenced this issue Mar 29, 2024
@cbiffle
build: re-enable task packing.
5bc0126 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
cbiffle added a commit that referenced this issue Mar 29, 2024
@cbiffle
build: re-enable task packing.
c899dac 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
cbiffle added a commit that referenced this issue Mar 29, 2024
@cbiffle
build: re-enable task packing.
a285f62 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
cbiffle added a commit that referenced this issue Apr 1, 2024
@cbiffle
build: re-enable task packing.
0d614d8 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
hawkw pushed a commit that referenced this issue Apr 5, 2024
@cbiffle @hawkw
build: re-enable task packing.
4332977 
Now that release 7 is closer to finalized and we've gained some
confidence in the kernel fixes for #1672, it would be nice to regain the
benefits of task packing and stop fretting so much about fragmentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kernel Relates to the Hubris kernel robustness Fixing this would improve robustness of deployed firmware
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@cbiffle