Skip to content

Dynamic App Loader Helper #387

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

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Conversation

viswajith-g
Copy link

This PR introduces the helper app to validate the kernel's dynamic process loading functionality (tock/tock#3941). The helper app triggers the dynamic process loading with a button press.

This application is located in /examples/tests/app_loader and was tested on the nRF52840DK. A previous version of the dynamic process loader was tested with the same application on an Imix board as well.

uint32_t app_size = 0; // variable to store app size

// Tock Application Binary to be flashed.
const uint8_t app_binary[] = {0x2, 0x0, 0x34, 0x0, 0x0, 0x8, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0xD7, 0x75, 0x50, 0x6E,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This application can build for any architecture. How does this data then work?

}

for (uint32_t offset = 0; offset < write_count; offset++) {
memcpy(write_buffer, &app_binary[FLASH_BUFFER_SIZE * offset], FLASH_BUFFER_SIZE); // copy binary to write buffer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if app_binary isn't a multiple of FLASH_BUFFER_SIZE?

@ppannuto ppannuto added the blocked-on-kernel Waiting for parallel changes in the kernel to stabilize label Oct 9, 2024
@viswajith-g viswajith-g force-pushed the app_loader branch 2 times, most recently from 42b0c51 to f653234 Compare February 1, 2025 23:40
@bradjc
Copy link
Contributor

bradjc commented Feb 12, 2025

Can you rebase this on master and

  • move the syscall wrappers to their own file
  • move app_binaries and the python script to the test directory

@viswajith-g
Copy link
Author

viswajith-g commented Feb 12, 2025

Can you rebase this on master and

I believe the repo was already rebased on master last time i pushed it.

  • move the syscall wrappers to their own file

Done. I also renamed the functions to start with libtock_ to keep in line with the other the new libtock format.

  • move app_binaries and the python script to the test directory

Done

@@ -0,0 +1,1304 @@
#include <examples/tests/app_loader/button-press-loading/app_binaries.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include <examples/tests/app_loader/button-press-loading/app_binaries.h>
#include "app_binaries.h"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move these to libtock/kernel

Comment on lines 2 to 7
// #include "libtock/tock.h"

// bool libtock_app_loader_exists(void) {
// syscall_return_t res = command(DRIVER_NUM_APP_LOADER, 0, 0, 0);
// return tock_command_return_novalue_to_returncode(res);
// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// #include "libtock/tock.h"
// bool libtock_app_loader_exists(void) {
// syscall_return_t res = command(DRIVER_NUM_APP_LOADER, 0, 0, 0);
// return tock_command_return_novalue_to_returncode(res);
// }

lvgl/lvgl Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i did not touch this though?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm i see it now

viswajith-g and others added 10 commits May 1, 2025 00:38
this app sends the tab file of an app that needs to be flashed to the capsule. once the app binary is written to flash, the userland app requests kernel to load the app.
… in internal/binaries.c.

cleaned up and formatted

removed old code

removed userspace code from a previous implementation that was giving rise to confusion.

added comments to app_loader.h

added binaries headers and updated code

added binaries in external files to make the app file more readable. Added a new subscribe and callback to support the async setup

update readme
added tock-dpl-hello and tock-welcomes-dpl. The first app is a simple c_hello analog that prints a message and terminates on boot. The second app is a helper app that installs tock-dpl-hello on the board during runtime. These apps were made to test the dynamic process loading using hw-ci.
@viswajith-g
Copy link
Author

ok, given the kernel part is now merged, i will leave this comment here:

I am all for removing button-press-loading and renaming abort-test to dynamic-process-loading-test because abort test is an extension of button press. We add the abort function and test it is all. I don't think it is worth two separate examples and a whole lot of code repeat for that reason. Thoughts @bradjc?

Copy link
Contributor

@bradjc bradjc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have one interactive app meant for trying out DPL, eg button-press-loading. The others should be more friendly for CI (ie not require user input). I think the abort test should be able to run and test abort without requiring a user to get the button timing right.

I also think we should come up with a folder and more general structure for storing the .tbf binaries. We should also make it clear the binaries are cortex-m4 binaries. They should match what tockloader generates.

* This function takes in the function that will be executed
* when the callback is triggered.
*/
returncode_t libtock_app_loader_set_setup_upcall(subscribe_upcall cb, void* userdata);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this app for? It seems the same as c_hello.

Copy link
Author

@viswajith-g viswajith-g May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the app that's compiled and fed to tock-welcomes-dpl.

@viswajith-g
Copy link
Author

Okay, I agree with that

@viswajith-g
Copy link
Author

viswajith-g commented May 2, 2025

I also think we should come up with a folder and more general structure for storing the .tbf binaries. We should also make it clear the binaries are cortex-m4 binaries. They should match what tockloader generates.

Okay, so I gave it some thought and the way I have it working right now is to move the binaries to a different directory and create a symlink so the Makefile can find and compile the binaries for each example. I still have them stored in a single app_binaries.c file but I am considering splitting each binary into its own file. This should hopefully reduce the size of the compiled test app, with people getting to choose and test their own binaries easily downstream. The downside to this would be multiple symlinks.

The structure looks like this right now:

│ 
├─ app-binaries/
│   ├─ cortex-m4/
│   	   ├─ app_binaries.h
│	   ├─ app_binaries.c
│
├─ abort-test/
│   ├─ app_binaries.c -> symlink
│   ├─ main.c
│   └─ Makefile
│
├─ button-press-loading/
│   ├─ app_binaries.c -> symlink
│   ├─ main.c
│   └─ Makefile

@viswajith-g
Copy link
Author

These pushes only address the directory structure changes and not the ci-ease concern that was raised. That will come with a separate commit

@viswajith-g
Copy link
Author

viswajith-g commented May 4, 2025

The current directory structure looks like this:

│ 
├─ app-binaries/
│   ├─ cortex-m4/
│   	   ├─ tock-apps.h
│	   ├─ tock-dpl-hello.c
│          ├─ blink.c
│          ├─ adc.c
│
├─ abort-test/
│   ├─ tock-dpl-hello.c  -> symlink
│   ├─ blink.c           -> symlink
│   ├─ adc.c             -> symlink
│   ├─ main.c
│   ├─ Readme
│   └─ Makefile
│
├─ button-press-loading/
│   ├─ blink.c  -> symlink
│   ├─ adc.c    -> symlink
│   ├─ main.c
│   ├─ Readme
│   └─ Makefile

The binaries match the ones generated by tockloader. I pulled from the examples in the repo for blink and adc. I wrote my own example for the third, but it is a simple variation of c-hello.

For the abort-test, the user can press the button whenever, and it'll still abort. I've tested it multiple times, the NonvolatileStorage driver is just fast.

However, I've changed the interface so that the app is listening for a console command instead of a button press. Essentially, the characters '0', '1', or '2' load a different app based on the command. This should eliminate those pesky debounce issues and whatnot. Plus it is easy to expand this anyway regardless of the board button support capabilities, while introducing the undefined behavior when multiple apps are listening however.
'2' installs adc, but the app is programmed so that at the 50% mark, the app sends an abort signal internally. One could write a script to automate this.

I have absolutely no idea why the ci-format failed on print statements labeled error only in abort-test and not in button_press_loading. Plus, this failure did not happen before either. (edit: fixed now)

@viswajith-g
Copy link
Author

I wonder, should I get rid of the other two apps and just have it run the adc + abort alone? While still requiring console input to make sure it does not just trigger on boot.

@bradjc bradjc removed the blocked-on-kernel Waiting for parallel changes in the kernel to stabilize label May 5, 2025
@bradjc
Copy link
Contributor

bradjc commented May 5, 2025

Why does the abort test need to be interactive at all? It is simpler and easier to run if all you have to do is install it and see if it prints success or not.

You could add a timer (potentially with some random delay) if you want to not run immediately at boot or randomize when the abort signal happens.

@bradjc
Copy link
Contributor

bradjc commented May 5, 2025

Okay, so I gave it some thought and the way I have it working right now is to move the binaries to a different directory and create a symlink so the Makefile can find and compile the binaries for each example. I still have them stored in a single app_binaries.c file but I am considering splitting each binary into its own file. This should hopefully reduce the size of the compiled test app, with people getting to choose and test their own binaries easily downstream. The downside to this would be multiple symlinks.

Why can't we just include in the makefile which binary apps to include?

@viswajith-g
Copy link
Author

I don't remember exactly why but the compiler kept complaining when I included the binaries in the make file. Something about a path issue. So I ended up making symlinks. I can try again.

Copy link
Member

@lschuermann lschuermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool.

@bradjc @viswajith-g Do you think it would make sense to also have some higher-level, reusable interface for process loading, such as the write_app function of the button-press-loading example available to all apps?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you maybe rename this directory to something other than tool? That doesn't really suggest much of what it does.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check in those binaries? This seems like something that'll go out of sync with the rest of the repository quickly. It would be nice if, instead, these could be automatically generated upon compilation of the DPL tests.

Also, using symlinks to reference these files makes me skittish; I don't think that typically interacts well with C include paths, no? It might be better to modify the Makefile to add these files into the include path (or, if you're generating them on the fly as part of compiling the DPL binaries, you could simply place them in the build/ output path).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They aren't generated on the fly. We have a complementary tockloader command that helps convert a .tab file into a struct whose contents can be copied over. I do think symlinks may cause issues with pathings across OSes.
These binaries are essentially c arrays that are passed as references to whatever app is loading them, they don't come from the original binaries in the example/tests directories.

I'm having some troubles with getting the makefile to recognize the path to these files because of the way it builds. I haven't fully delved into it yet.

Copy link
Member

@lschuermann lschuermann May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I'm no C toolchain expert so probably not the best person to ask about this. But I do think that adjusting the include paths is better than using symlinks.

They aren't generated on the fly. We have a complementary tockloader command that helps convert a .tab file into a struct whose contents can be copied over. I do think symlinks may cause issues with pathings across OSes.

I thought this was the job of the "tool" included in this PR?

My point is that I don't see a reason why they shouldn't be generated on the fly, as part of building the DPL example applications themselves. You should be able to simply run another make command in the respective application directories as part of the Makefiles of the DPL example applications themselves.

Pinging @bradjc and @ppannuto who know more about the Makefile magic here and how this could be supported most elegantly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tool here is a predecessor to the tockloader command.

My point is that I don't see a reason why they shouldn't be generated on the fly, as part of building the DPL example applications themselves. You should be able to simply run another make command in the respective application directories as part of the Makefiles of the DPL example applications themselves.

And I think that is what Brad got at in his last comment. But I too am no makefile expert haha 😅. Help would be appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is something elegant about having the binaries be generated on the fly, and it would be cool to have that. But, given we want tutorial participants to be able to use this, and to add their own binaries for flashing (with things like different signatures), I think we need this to be simple like what is here now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so I was starting down the road of making this more ergonomic / automatic with make stuff [which I can still do] — oorrrrrr, we can bump the Configuration.mk to C23 and use the #embed directive.

For N=1 apps, switching from -gnu11 to -gnu23 worked no-problem (and out-of-the-box on the compiler that happened to be on this machine (arm-none-eabi-gcc (GCC) 14.2.0, not the most new by any means). I'm testing this a bit more thoroughly now and will make a separate PR to libtock-c in a moment if everything is smooth.

@viswajith-g
Copy link
Author

Do you think it would make sense to also have some higher-level, reusable interface for process loading, such as the write_app function of the button-press-loading example available to all apps?

Hmm. I think there is merit in introducing an interface for the whole subsystem (setup + flash + load). That might reduce the implementation repeat. Another option, and something I am heavily leaning towards is to use a standalone app_loader application that functions as a background service (by making it a sticky app) which you can invoke via IPC.

@lschuermann
Copy link
Member

Hmm. I think there is merit in introducing an interface for the whole subsystem (setup + flash + load). That might reduce the implementation repeat. Another option, and something I am heavily leaning towards is to use a standalone app_loader application that functions as a background service (by making it a sticky app) which you can invoke via IPC.

I don't think these are mutually exclusive: you could have a high-level interface, and an IPC service that can perform app loading for other applications. However, I think that given the current state of IPC (both concerning problems with the interface, and the app loader application not being able to reliably determine whether the IPC client is a trustworth app), having this be a reusable function that you can embed in your own app seems more attractive. This app can then be verified by the kernel as a whole, before being given permission to manage applications on the board.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants