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

[Gen 3] 256KB application support #2322

Merged
merged 12 commits into from
May 27, 2021
Merged

[Gen 3] 256KB application support #2322

merged 12 commits into from
May 27, 2021

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented May 26, 2021

Description

Refer to the SDD

NOTE: this PR also enables LTO on all Gen 3 platforms when building system parts.

NOTE: do not use current version of CLI to flash new 256KB binaries over DFU, this will fail

Steps to Test

NOTE: 3.1.0-alpha.1 and 3.1.0-alpha.2 releases available on staging can be used for testing (test/v3.1.0-alpha.1 and test/v3.1.0-alpha.2 are the respective branches). The releases are fully available on staging and appropriate version of DS is also deployed there.

  1. Building larger than 128KB applications should be possible with this branch. They should also run just fine
  2. 128KB older applications should still work correctly
  3. particle serial inspect should correctly report either type of an application (index = 1, 128KB max) (index = 2, 256KB max)
  4. Flashing 128KB older app should override the current 256KB app, even if they don't overlap
  5. Flashing 256KB app should override (invalidate) the older 128KB app residing on the device even if they don't overlap
  6. Bootloader should function correctly with older versions of system-part1 on the device (mainly the bootloader updates through MBR are a concern and application updates)

Example App

N/A

References

  • [CH60338]
  • There is an appropriate PR in DS repo

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@avtolstoy avtolstoy added this to the 3.1.0 milestone May 26, 2021
@avtolstoy avtolstoy requested review from sergeuz and XuGuohui May 26, 2021 05:12
@XuGuohui
Copy link
Member

XuGuohui commented May 26, 2021

  • They should also run just fine 128KB older applications should still work correctly
  • Building larger than 128KB applications should be possible with this branch.
constexpr uint32_t BUF_SIZE = 250 * 1024;
const uint8_t buf[BUF_SIZE] = {};

void setup() {
    for (uint32_t i = 0; i < BUF_SIZE; i++) {
        Serial.printf("%d", buf[i]);
    }
}
  • particle serial inspect should correctly report either type of an application (index = 1, 128KB max) (index = 2, 256KB max)
  • Flashing 128KB older app should override the current 256KB app, even if they don't overlap
  • Flashing 256KB app should override (invalidate) the older 128KB app residing on the device even if they don't overlap
  • Bootloader should function correctly with older versions of system-part1 on the device. This may surprise user that the device enters safe mode after downgrading the pre Device OS versions. This is an expected behavior and it would be better to document this somewhere.

Also, after updating the bootloader, system part1 and the 256K user application, their module version isn't bumped. Should we bump the versions in this branch or postpone to when we cut a release?

fetch_module(module, bounds, false, MODULE_VALIDATION_INTEGRITY);
if (bounds->module_function == MODULE_FUNCTION_USER_PART && user_module_found) {
// Make sure that we report only single user part (either 128KB or 256KB) in the
// list of modules.
Copy link
Member

Choose a reason for hiding this comment

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

Let's make it clear in this comment that if both user modules are present, the compatibility one will take precedence and be the only user module returned in the modules info.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

{
return module_user_part_validated;
return system_mode() != SAFE_MODE;
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -368,23 +368,31 @@ int FLASH_CopyMemory(flash_device_t sourceDeviceID, uint32_t sourceAddress,
return FLASH_ACCESS_RESULT_BADARG;
}
#if SOFTDEVICE_MBR_UPDATES
if (module_function == MODULE_FUNCTION_BOOTLOADER && destinationAddress == USER_FIRMWARE_IMAGE_LOCATION) {
if (module_function == MODULE_FUNCTION_BOOTLOADER && destinationAddress == USER_FIRMWARE_IMAGE_LOCATION_COMPAT) {
Copy link
Member

@sergeuz sergeuz May 26, 2021

Choose a reason for hiding this comment

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

Note that verify_module() still checks that the bootloader's destination address is USER_FIRMWARE_IMAGE_LOCATION and not USER_FIRMWARE_IMAGE_LOCATION_COMPAT.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Double checked bootloader updates. Works fine with system-part out of this branch and older releases as well.

@avtolstoy avtolstoy requested a review from sergeuz May 27, 2021 15:27
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