Skip to content
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

Enable Ledger and filesystem APIs on GCC platform #2737

Merged
merged 10 commits into from
Feb 19, 2024
Merged

Conversation

sergeuz
Copy link
Member

@sergeuz sergeuz commented Feb 13, 2024

This PR adds minimal implementations of exflash_hal and concurrent_hal for the GCC platform, sufficient to support the LittleFS and Ledger APIs on that platform.

By default, the contents of the external flash is stored in RAM and is not persistent, meaning that the virtual device will always start with an empty filesystem. The firmware executable can be provided with a --flash_file option to enable persistent storage:

./my_firmware --flash_file=flash.bin

In RAM and on disk, the contents of the external flash is stored in a sparse array to ensure that RAM and disk space are only allocated for the data actually in use by the firmware and we don't potentially spike the IO and RAM usage in load tests.

@sergeuz sergeuz force-pushed the lfs_gcc/sc-125175 branch 2 times, most recently from 3ea0054 to a693ef4 Compare February 13, 2024 19:56
@sergeuz sergeuz changed the title Support filesystem API on GCC platform Enable Ledger and filesystem APIs on GCC platform Feb 13, 2024
@sergeuz sergeuz marked this pull request as ready for review February 13, 2024 20:51
@@ -259,9 +260,12 @@ int LedgerManager::init() {
if (!buf) {
return SYSTEM_ERROR_NO_MEMORY;
}
auto fsInstance = filesystem_get_instance(FILESYSTEM_INSTANCE_DEFAULT, nullptr);
assert(fsInstance);
FsLock fs(fsInstance);
Copy link
Member

Choose a reason for hiding this comment

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

Why does this one need to change but not the one in LedgerManager::getLedgerNames?

Copy link
Member Author

Choose a reason for hiding this comment

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

The manager is expected to be initialized in getLedgerNames() and instance() will take care of that.

Copy link
Member Author

@sergeuz sergeuz Feb 13, 2024

Choose a reason for hiding this comment

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

Note that the main change in init() is that it now calls filesystem_mount() and the latter expects a filesystem_t instance, not lfs_t, hence the difference.

}

int hal_exflash_read(uintptr_t addr, uint8_t* data_buf, size_t data_size) {
try {
Copy link
Member

@avtolstoy avtolstoy Feb 19, 2024

Choose a reason for hiding this comment

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

While this pattern is not used in normal Device OS sources for real devices, would be nice to introduce some CHECK-style macro or something along those lines to forward SYSTEM_ERROR_XXX error codes on exceptions to improve readability.

static volatile bool initCalled = false;
if (!initCalled) {
static std::once_flag onceFlag;
std::call_once(onceFlag, []() {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@sergeuz sergeuz merged commit 60838a7 into develop Feb 19, 2024
13 checks passed
@sergeuz sergeuz deleted the lfs_gcc/sc-125175 branch February 19, 2024 17:25
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.

3 participants