-
Notifications
You must be signed in to change notification settings - Fork 747
boards: add a new linker script that includes the .apps section #4158
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.
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
Conversation
This gives boards the flexibility to choose whether they want a .apps section to end up in the elf or not. The downside to leaving the .apps section in the elf is that loading the elf directly erases all apps.
No need for the old .apps documentation.
This requires using the kernel_layout_apps.ld linker script.
Now works on recursive relative imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense. It's exactly the kind of little quality-of-life thing that is easy to not notice, but reduces friction for using Tock.
The changes look good to me.
/* Include placeholder bytes in this section so that the linker | ||
* includes a segment for it. Otherwise the section will be empty and | ||
* the linker will ignore it when defining the segments. | ||
* If less then 4 bytes, some linkers set this section to size 0 | ||
* and openocd fails to write it. | ||
* | ||
* An issue has been submitted https://github.com/raspberrypi/openocd/issues/25 | ||
*/ | ||
BYTE(0xFF) | ||
BYTE(0xFF) | ||
BYTE(0xFF) | ||
BYTE(0xFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexandruradovici Do you know whether this is still necessary?
It looks like the issue was closed a while ago: raspberrypi/openocd#25
I guess we could just leave these four bytes as-is. Previously there was a single BYTE(0)
here, so this isn't worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just removing the link to the issue is the right thing to do at this point.
For relative imports to work in linker scripts we must include all of the locations of the linker scripts in the linker search path. Since we are already tracing through linker scripts we can do this fairly easily.
Instead of maintaining two linker files, I believe we can just modify the elf instead? i.e.,
should prevent the section from being overwritten. Or, perhaps more in line with the common case for tock, we mark the |
Is there a good way to do this without making builds only work if you build using |
Have you known about this the whole time!?!?! I feel like you've been holding out! Why didn't we do this from the beginning. |
Hah, no and yes? I was reviewing this, and I actually started by re-investigating the "how do to a conditional in a linker script" rabbit hole [where the consensus solution is apparently to run the C preprocessor over your linker script via
Without reviewing the machinery, don't we already need other "extra commands" to set up the binary image for apps that will be linked for the unified flash case? That's already a bit of a hack that I think we'd rather purge from the tree wholly eventually generally... |
Alternative in #4169. |
Pull Request Overview
The
.apps
section is used as a placeholder for replacing with a compiled app binary to create a binary that includes both the kernel in the app. This is used by some boards and avoids needing to flash several times.As a side effect, if the .elf with .apps is used to flash the kernel, this also (effectively) removes all installed apps.
This PR makes this optional rather than the default. Boards that want this can use the
kernel_layout_apps.ld
linker script.For the stm32 discovery boards I removed the docs from the readme because those have tockloader support.
I also had to expand the build.rs code to handle relative imports outside or the board's directory. I don't think it's the prettiest code.
Testing Strategy
Installing the microbit kernel and verifying it does not erase my apps any more.
TODO or Help Wanted
Nothing
Documentation Updated
/docs
, or no updates are required.Formatting
make prepush
.