Allow cgp to be larger. #236
Conversation
We store the size of globals as a shifted value, which extends the maximum size of globals in a compartment from 64 KiB to 256 KiB. This adds two new tests. One is added to the misc test and just checks that we correctly handle globals that are larger than the range of a csetbounds with an immediate offset. The second checks that much larger globals work. This test is disabled because it uses so much RAM that we don't have enough for the allocator tests. At some point, we should refactor the test suite to allow subsets of the test suite to be built. Add test for large objects. Fixes #230
There was a problem hiding this comment.
I see the linker script catches single data structures that are too large but when the big data segment is something that overflows total memory the test suite links but fails at runtime (though I'm not using sail so this might be peculiar to my environment).
It appears simple to support even larger data but that would require a "non-standard" compartment def (and raise the minimum). It turns out my original test case used a 320K array which does not fit with a shift of 2. Probably good enough to require users to handle this but a comment explaining where all the "/4"s are would be helpful (maybe just a pointer to the eventual commit).
| static uint32_t data[16384]; | ||
| debug_log("Data: {}", data); | ||
| debug_log("CGP: {}", cgpRegister); | ||
| debug_log("Fill with for oop"); |
That's something I'd expect to be caught by something device specific. If you create a firmware file that is larger than the memory then you've done something wrong. It's probably a good idea to add a check in the loader though.
We can look at that. This is a simple fix that doesn't require significant changes and allows a single compartment to be as big as we expect the largest SRAM to be for most devices in the next year or so. If people are shipping devices with more than 256 KiB of SRAM then that's a good time to revisit it. On devices like that, the size savings from using 16-bit sizes are largely irrelevant and it's probably simpler to just use a 32-bit size. I would expect most use cases to want to use the heap for this kind of thing, rather than a static carve out. That would let you do things like use a big chunk of RAM for some post-quantum key exchange on boot to get a per-session symmetric key and then recycle the memory for your workload, for example, whereas a fixed carveout would require more total SRAM. As previously discussed, the heap would require some work to allow it to support larger total memory sizes. This isn't too difficult, but it is not urgent for us since it is not a feature that any of the current prototyping platforms or any ASICs that we expect to see in the near term can use. If it's something that you want to work on, I can help find the places in the code that will need changing. |
We store the size of globals as a shifted value, which extends the maximum size of globals in a compartment from 64 KiB to 256 KiB.
This adds two new tests. One is added to the misc test and just checks that we correctly handle globals that are larger than the range of a csetbounds with an immediate offset.
The second checks that much larger globals work. This test is disabled because it uses so much RAM that we don't have enough for the allocator tests.
At some point, we should refactor the test suite to allow subsets of the test suite to be built.
Add test for large objects.
Fixes #230