Cortex-m MPU subregions for >64KB apps#103
Cortex-m MPU subregions for >64KB apps#103tyler-potyondy wants to merge 1 commit intotock:masterfrom
Conversation
9f5b5ac to
ef36f4f
Compare
| /// To prevent wasted space, only pad to: | ||
| /// (X - 1) * (TotalSizePowerOfTwo / 8) < size < X * (TotalSizePowerOfTwo) | ||
| MaskedPowerOfTwo, |
There was a problem hiding this comment.
I'm not sure I understand this.
The version below makes sense to me, but multiplying TotalSizePowerOfTwo by X doesn't.
| /// To prevent wasted space, only pad to: | |
| /// (X - 1) * (TotalSizePowerOfTwo / 8) < size < X * (TotalSizePowerOfTwo) | |
| MaskedPowerOfTwo, | |
| /// To prevent wasted space, only pad to the smallest integer `X` where | |
| /// `X * (TotalSizePowerOfTwo / 8) > size`. | |
| MaskedPowerOfTwo, |
There was a problem hiding this comment.
This was a mistake. It should be:
(X - 1) * (TotalSizePowerOfTwo / 8) < size < X * (TotalSizePowerOfTwo / 8)
| /// Specify how elf2tab should add trailing padding to the end of the TBF | ||
| /// file. | ||
| enum TrailingPadding { |
There was a problem hiding this comment.
Why did this move in the file?
There was a problem hiding this comment.
I moved it so all the comments are grouped to where the math is happening for this. With it being an enum def it makes sense for this to be where it was. I can revert this.
There was a problem hiding this comment.
Looking closer, the enum doesn't need to move but the let trailing_padding ... block needs to move lower since we need to know the binary size (to decide which padding to do based on the size).
| // Pad to the nearest multiple of (power of two / 8). | ||
| if target_size > binary_index { | ||
| target_size - binary_index | ||
| } else { | ||
| 0 | ||
| } |
There was a problem hiding this comment.
| // Pad to the nearest multiple of (power of two / 8). | |
| if target_size > binary_index { | |
| target_size - binary_index | |
| } else { | |
| 0 | |
| } | |
| // Pad to the nearest multiple of (power of two / 8). | |
| target_size - binary_index |
Takes advantage of subregions to lower the wasted space.
ef36f4f to
54ff1d6
Compare
| /// To prevent wasted space, only pad to: | ||
| /// (X - 1) * (TotalSizePowerOfTwo / 8) < size < X * (TotalSizePowerOfTwo / 8) | ||
| MaskedPowerOfTwo, |
There was a problem hiding this comment.
Is this supported for an unmodified, old kernel that doesn't know about these elf2tab changes yet?
There was a problem hiding this comment.
I think it should be. The only issue I forsee would be if the kernel can't assign an MPU config for this app size (in which case the kernel wouldn't load the app).
With that being said, the MaskedPowerOfTwo should always be able to be assigned an MPU config since it is just using subregions to mask out some of the wasted power of two region size.
There was a problem hiding this comment.
The only issue I forsee would be if the kernel can't assign an MPU config for this app size (in which case the kernel wouldn't load the app).
That's exactly what I'm concerned about.
With that being said, the
MaskedPowerOfTwoshould always be able to be assigned an MPU config since it is just using subregions to mask out some of the wasted power of two region size.
Got it. I'm not familiar with the Cortex-M MPU implementation, and so I wasn't sure whether that needs adjustment to support this new MaskedPowerOfTwo mode. If that works with the one that's upstream already (and hence won't break backwards-compatibility of elf2tab), then I'm conceptually on board with this change.
|
Looks like this PR is crossing with mine #102 |
|
Just following up on this... I agree with @ZhekaS that this has some overlap with #102. It seems #102 could also be used to fix this problem. The padding/alignment to powers of 2 has been particularly wasteful of space on cortex-m so it would be nice to have a default solution that helps in the common case without needing to pass another flag. |
Rounding to the nearest power of two for cortex-m apps results in a high amount of flash wasting for large applications. This adds logic to take advantage of MPU subregions for apps larger than 64kB.
Example
For an ~80kB application the tab was padded to 128kB before these changes.
Before Changes
After Changes