Add basic memory allocation outside Python runtime#1057
Conversation
|
The mode on some of the files are changed from 0644 to 0755, which I suppose is unintentional. |
This allows for the heap to fill all space but the stack. It also allows us to designate space for memory outside the runtime for things such as USB descriptors, flash cache and main filename. Fixes micropython#754
|
Ok this should be good to go after travis gives the ok. Please take another look. |
dhalbert
left a comment
dhalbert
left a comment
There was a problem hiding this comment.
Excellent idea! Some questions and comments.
| found = allocation == &allocations[index]; | ||
| if (found) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Supposer the allocation is not found. Wouldn't that be a fatal error of some kind?
There was a problem hiding this comment.
Yes but I couldn't decide how to convey it since the Python VM isn't running. Maybe an assert?
There was a problem hiding this comment.
Maybe we should have a general "terrible thing happened -- blink the status LED?" loop? Or do we need the LED set up to do that?
There was a problem hiding this comment.
We could use your "blink variable number of times" code and have some ifdef's for the values. This could be a todo. Right now you could just while { } like we do for some other errors, and mark it as a TODO.
There was a problem hiding this comment.
TODO added. Sounds like we'd want to kick into safe mode.
| return allocate_memory((high_address - low_address) * 4, false); | ||
| } | ||
|
|
||
| supervisor_allocation* allocate_memory(uint32_t length, bool high) { |
There was a problem hiding this comment.
Could you add some a comment here about what the high bool means? it looks likethis is for blocks that are addressed downward, is that right?
There was a problem hiding this comment.
Added it to the .h at the top level. (Will push soon.)
| } | ||
| supervisor_allocation* alloc = &allocations[index]; | ||
| if (high) { | ||
| high_address -= length / 4; |
There was a problem hiding this comment.
If the requested length is not a multiple of 4, then I think you'll allocate a region that's length % 4 bytes too short. So maybe check that length % 4 == 0.
|
|
||
| #define CIRCUITPY_SUPERVISOR_ALLOC_COUNT 8 | ||
|
|
||
| static supervisor_allocation allocations[CIRCUITPY_SUPERVISOR_ALLOC_COUNT]; |
There was a problem hiding this comment.
This nrf/supervisor/memory.c looks pretty identical to the atmel-samd version. Can you share code? Hoist this into the common supervisor/ dir? add #if's for arch-specific if needed?
There was a problem hiding this comment.
Shared. I'm not sure if ESP will fit but we can change it when the time comes.
| } | ||
|
|
||
| inline void stack_init(void) { | ||
| allocate_stack(); |
There was a problem hiding this comment.
Do these have to be inline or are you suggesting? If it's required, then we need to do the volatile asm(""); trick.
There was a problem hiding this comment.
Nope. I must have been experimenting.
3 participants
This allows for the heap to fill all space but the stack. It also allows us to designate space for memory outside the runtime for things such as USB descriptors, flash cache and main filename.
Fixes #754