pkg/xipfs: add XIPFS as vfs module #21197
Conversation
|
Interesting! |
|
I think something with your rebase went wrong, you have two commits in the PR that are part of the master branch from benpicco and maribu. |
Added shell commands to create XIPFS executable files and to execute them. Demonstrator has been commented with an execution trace in examples/xipfs.
1787749 to
c588492
Compare
Yeah I should have checked twice, I tried a a git rebase -i and dropped the two commits you cited. Is it ok with you ? |
|
Yes, it looks good now. Unfortunately I don't have a dwm1001 board for testing this. I'm not a maintainer, so this is not an official review. I took a quick look at the code and I think it would be possible to eliminate the need for dedicated board header files in your xipfs package. And the rest of the constants seem to be pretty much the same for all platforms. There is also a bug that it won't allow you to build for other boards than the Before compiling for the dwm1001After compiling for the dwm1001If the package only supports certain boards, it would be best to add a whitelist for supported boards: https://doc.riot-os.org/creating-an-application.html However I am not sure if that is intended to work for packages as well, I guess a maintainer could answer that. Adding But in general I think it would be better to add general support for all boards (that support flashpage). |
There are some dwm1001 boards deployed in the IoT-LAB testbed (in Lille and Toulouse sites). You can:
(I just tried and it still works!) |
|
For starters, thanks to @crasbe and @aabadie for their feeback; they are much appreciated. We were, and still are, quite newbies into the RIOT community. @crasbe : I am giving a try at the generic code path that you propose, ie relying to FLASH definitions in But now, I am facing an error from the build system where my CPU type is not found anymore : I've searched into RIOT only to find a definition in generated Am I missing something ? Thanks for reading. |
|
I think you don't have to make any significant changes to the original project to be compatible with the RIOT definitions. I can't reproduce the issue you're describing. The error comes from the RIOT/cpu/nrf52/include/cpu_conf.h Line 58 in 7027fd3 What's odd is that the RIOT/boards/dwm1001/Makefile.features Line 1 in 7027fd3 To reproduce the issue I included If you include You could try to run |
|
@crasbe I scratched my head a bit to get how the build system works, while keeping at the same time the riot integration code as a template for other ones. I don't know if changes will be ok for reviewers, but now when building xipfs for RIOT, xipfs relies on I compiled both Please, could you give it a try for your board (nRF52840DK) and let me know if the compilation works for you now ? |
|
Unfortunately it still fails, it appears that https://github.com/2xs/xipfs/blob/main/include/xipfs.h#L40 Removing that include makes it somewhat work, but of couse, there is no place now that defines Also I don't see any includes for Compiling for the |
sys/shell/cmds/xipfs.c
Outdated
| #ifdef RIOT_OS | ||
| #include "include/xipfs.h" | ||
| #else /* RIOT_OS */ | ||
| #include "xipfs_config.h" | ||
| #endif /* RIOT_OS */ |
There was a problem hiding this comment.
This file is only present in RIOT, isn't it? Therefore, no include guards are required to check if it is RIOT or not.
| * @} | ||
| */ | ||
|
|
||
| #if defined(MODULE_XIPFS_FS) || defined(MODULE_XIPFS) |
There was a problem hiding this comment.
These include guards should not be necessary here. The build system should ensure that only those files are compiled and linked that are actually used.
There was a problem hiding this comment.
Ok, I remove them. Thanks !
examples/xipfs-tests/Makefile
Outdated
| @@ -23,4 +23,6 @@ USEMODULE += ps | |||
| # Use the default file system | |||
| USEMODULE += xipfs | |||
|
|
|||
| CFLAGS +=-DRIOT_OS | |||
There was a problem hiding this comment.
You can just use the existing RIOT_VERSION define
There was a problem hiding this comment.
Thanks for the tip.
I've followed your advice and relied on this preprocessor symbol in the library : xipfs library
|
I think the easiest approach would be to create a #ifndef XIPFS_CONFIG_H
#define XIPFS_CONFIG_H
#include "cpu.h"
/**
* @def XIPFS_PATH_MAX
*
* @brief The maximum length of an xipfs path
*/
#define XIPFS_PATH_MAX (64)
/**
* @def XIPFS_MAGIC
*
* @brief The magic number of an xipfs file system
*/
#define XIPFS_MAGIC (0xf9d3b6cb)
/**
* @def XIPFS_FILESIZE_SLOT_MAX
*
* @brief The maximum slot number for the list holding file
* sizes
*/
#define XIPFS_FILESIZE_SLOT_MAX (86)
/**
* @def XIPFS_EXEC_ARGC_MAX
*
* @brief The maximum number of arguments on the command line
*/
#define XIPFS_EXEC_ARGC_MAX (64)
/**
* @def XIPFS_NVM_BASE
*
* @brief The non-volatile memory base address
*/
#define XIPFS_NVM_BASE (0)
/**
* @def XIPFS_NVM_ERASE_STATE
*
* @brief The non-volatile memory erased state
*/
#define XIPFS_NVM_ERASE_STATE (0xff)
/**
* @def XIPFS_NVM_NUMOF
*
* @brief The non-volatile memory flash page number
*/
#define XIPFS_NVM_NUMOF FLASHPAGE_NUMOF
/**
* @def XIPFS_NVM_WRITE_BLOCK_ALIGNMENT
*
* @brief The write alignment for the non-volatile memory
*/
#define XIPFS_NVM_WRITE_BLOCK_ALIGNMENT FLASHPAGE_WRITE_BLOCK_ALIGNMENT
/**
* @def XIPFS_NVM_WRITE_BLOCK_SIZE
*
* @brief The write size for the non-volatile memory
*/
#define XIPFS_NVM_WRITE_BLOCK_SIZE FLASHPAGE_WRITE_BLOCK_SIZE
/**
* @def XIPFS_NVM_PAGE_SIZE
*
* @brief The non-volatile memory flash page size
*/
#define XIPFS_NVM_PAGE_SIZE FLASHPAGE_SIZE
/**
* @def XIPFS_MAX_OPEN_DESC
*
* @brief The maximum number of opened descriptors
*/
#define XIPFS_MAX_OPEN_DESC (16)
#endif /* XIPFS_CONFIG_H */And in your Makefile, you do the following changes: ifdef RIOT_OS
CFLAGS += -DRIOT_OS=1
CFLAGS += $(RIOT_INCLUDES)
CFLAGS += $(RIOT_CFLAGS)
+ CFLAGS += -Iboards/riot
else # RIOT_OS
CFLAGS += -Iboards/$(BOARD)
endif # RIOT_OS
Then you can pretty much drop all the (I did not test this yet, but you should get the idea). |
examples/xipfs/main.c
Outdated
| * Then modify the Makefile to suit your needs/sources and call make. | ||
| * You should end up with a *.bin file ready to be uploaded. | ||
| */ | ||
| const unsigned char hello_world_bin[896] = { |
There was a problem hiding this comment.
BLOBS += hello_world.bin
in Makefile would generate a header file from your binary that can be included
this bin seem architecture (maybe mcu, maybe even board) specific please ensure to mark it accordingly
might be possible to reduce the sourcecode for an example binary and generate it while compiling the test
There was a problem hiding this comment.
Well spotted, thanks.
You're correct, the bin file is indeed architecture specific, and it should be available only for the dwm1001 board.
I will then use the BLOBS feature and guard all related code with #ifdef/#else/#endif BOARD_DWM1001 in C file, which should be available at compilation time from CFLAGS.
This will remove the exec command, but will format and mount the two mount points in the filesystem.
Reviewers/users will still be able to manipulate files manually.
Is that all right with what you have in mind ?
|
@crasbe We had quite similar ideas, but I did it on the library side, which seemed pretty legitimate in my opinion. BUT... cc1: error: /home/greg/Documents/code/RIOT-Github/build/pkg/xipfs/boards/nrf52840dk: No such file or directory [-Werror=missing-include-dirs]TBH, I don't know where this path comes from, and my fix on it is to create an empty directory; for now. With it, I am able to complete a build for nrf52840dk : arm-none-eabi-size /home/greg/Documents/code/RIOT-Github/examples/xipfs/bin/nrf52840dk/xipfs.elf
text data bss dec hex filename
137544 4368 5716 147628 240ac /home/greg/Documents/code/RIOT-Github/examples/xipfs/bin/nrf52840dk/xipfs.elfI tried to collect the community's opinions in the forum about this workaround, but no luck yet. Before pushing this change in the PR, I'll be interested if you or someone would have a proper way to fix it. |
|
I would assume that this line is creating the error: The fake folder is not a good solution IMO. |
|
My bad. I feel so stupid. |
|
No need to apologize. The RIOT build system can be quite a beast and integrating a package that wasn't designed for RIOT is not an easy task. Unfortunately I don't know when exactly I'll have access to my devboards again, so I can't test it at the moment. At some point this week or next week for sure though. I think the Also, with the recent merge of #21135 which restructured the |
| @@ -142,3 +142,5 @@ Here is a quick overview of the examples available in the RIOT: | |||
| | [twr_aloha](./twr_aloha/README.md) | This example allows testing different two-way ranging algorithms between two boards supporting a dw1000 device. This makes use of the uwb-core pkg. | | |||
| | [senml_saul](./senml_saul/README.md) | This example demonstrates the usage of the SAUL (Sensor Actuator Uber Layer) module with the SenML (Sensor Measurement Lists) format. | | |||
| | [opendsme](./opendsme/README.md) | This example demonstrates the usage of the OpenDSME module in RIOT. | | |||
| | [xipfs](./xipfs/README.md) | This simple example demonstrates the usage of XIPFS for creating and executing an executable file. | | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
3b1d69a to
4cd4f58
Compare
| @@ -0,0 +1,28 @@ | |||
| # name of your application | |||
There was a problem hiding this comment.
| # name of your application | |
| include ../Makefile.pkg_common | |
| # name of your application |
You can use the common pkg-test makefile which defines the $(RIOTBASE).
| # name of your application | ||
| APPLICATION = xipfs-test | ||
|
|
There was a problem hiding this comment.
| # name of your application | |
| APPLICATION = xipfs-test |
Tests don't necessarily need a name, it will be automatically generated.
| # This has to be the absolute path to the RIOT base directory: | ||
| RIOTBASE ?= $(CURDIR)/../.. |
There was a problem hiding this comment.
| # This has to be the absolute path to the RIOT base directory: | |
| RIOTBASE ?= $(CURDIR)/../.. |
See above, $(RIOTBASE) is defined in the common Makefile.
|
|
||
| int ret = xipfs_extended_driver_new_file(argv[1], (uint32_t)file_size, 1); | ||
| if (ret != 0) { | ||
| printf("Failed to create '%s' as an XIPFS executable file.", argv[1]); |
There was a problem hiding this comment.
| printf("Failed to create '%s' as an XIPFS executable file.", argv[1]); | |
| printf("Failed to create '%s' as an XIPFS executable file.\n", argv[1]); |
A newline is missing here.
|
I gave this a test with an nRF52DK devboard and the tests run successfully: Then I tested the example and it did work for small executables (10 bytes), but resulted in a stack overflow for larger executables (123 bytes). In this log, the file What's a bit odd is that |
Contribution description
This PR aims at integrating the eXecutable In-Place FileSystem into RIOT operating system.
XIPFS implements regular filesystem storage manipulation capabilities, but also allows
to run code almost directly from the FLASH memory.
Just a little bit of RAM is used to relocate what's needed for the execution (data and bss).
It is provided like any other vfs module, and can be called through the defined vfs API.
Nonetheless, it can also be called directly through its own API (xipfs_* functions).
No matter the actual file size is, XIPFS deals with Flash pages granularity of 4KiB.
That means that with two files with different sizes less than 4 Kib,
vfs dfwill display8 Kib used.
Due to XIPFS design, directories and files are stored in a similar way.
Echoing the remark above, that also means that an empty directory will cost 4 Kib to the
Flash memory.
This implementation has been developed and tested on a Decawave DW1001 board (Quorvo).
However, the Flash memory handling relies on available periph/flashpage.h, and should thus
run on any platform supporting addressable Flash memory.
Testing procedure
Manual and automated tests are provided.
Both are great resources to showcase XIPFS api and abilities.
Manual tests also demonstrate the integration of XIPFS as a VFS submodule.
To run the automated tests :
pyterm, listening to the device.To run the manual tests :
Please open a terminal with a
pyterm, listening to the device.Open another one and move to the examples\xipfs directory.
Now, please enter the following in this shell :
Come back to the
pytermterminal.The first time XIPFS is run onto the board the resulting display should be similar to :
, or to
when XIPFS has already be run and has initialized the two mount points of the filesystem.
Now, please enter
helpexecis a more a demonstration than a regular shell command.However it will :
hello-world.bin, when none is stored onto/dev/nvme0p0;hello-world.binthen.executeis the XIPFS execution shell command to run an executable file.create_executableis the shell command to create an executable_file.Though
create_executabletakes a name and a bytesize, please remember that it will contain no code until filled by a write command.