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

Adds bootloader as a secondary dependency to Photon/P1/Electron system-part2 #1306

Merged
merged 22 commits into from
May 10, 2017

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Apr 23, 2017

Problem

Bootloader needs to be a dependency of some system-module.

Solution

This PR is based on feature/bootloader_dct branch and #1289.

  • Adds HAL_Core_Validate_Modules function
  • When initializing system-part2, apart from verifying user module, this PR additionally checks bootloader and if dependency or integrity check fails, the device is forced into safe mode.

Steps to Test

N/A

Example App

N/A

References

https://github.com/spark/firmware/tree/feature/bootloader_dct


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)

Enhancement

  • [PR #1306] Bootloader module dependency and integrity checks have been added to system-part2. If they fail, the device is forced into safe mode and a new bootloader will be OTA transferred to the device.

@technobly
Copy link
Member

@avtolstoy to help the Cloud team develop the support for this feature in the Safe Mode Healer, would you please post an example of what the describe message would look like with the new bootloader dependency, and also provide binaries of the bootloader, and system modules? That would be super helpful for testing.

system-part2 now depends both on system-part1 and bootloader
@avtolstoy
Copy link
Member Author

avtolstoy commented Apr 26, 2017

Binaries: https://drive.google.com/drive/folders/0BySBnweX26fwRldJVGFuU0RJRU0?usp=sharing

Note: system-module binaries provided here are version 107, bootloader is 13

Bootloader is now a second dependency of system-part2 both on Electron and Photon/P1 (binary inspector probably needs support for this).

Electron

On develop (or 0.6.2-rc.1):

{
  "p": 10,
  "imei": "XXXXXXXXXXXXX",
  "iccid": "YYYYYYYYYYYYYYY",
  "m": [
    {
      "s": 16384,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "b",
      "n": "0",
      "v": 12,
      "d": []
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "1",
      "v": 106,
      "d": [
        {
          "f": "s",
          "n": "3",
          "v": 106,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "2",
      "v": 106,
      "d": [
        {
          "f": "s",
          "n": "1",
          "v": 106,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "3",
      "v": 106,
      "d": []
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "u": "25DC0C3F1769382A0494BE93AC6491F5918DB252912CC0E91B29BD28D27C44CE",
      "f": "u",
      "n": "1",
      "v": 4,
      "d": [
        {
          "f": "s",
          "n": "2",
          "v": 106,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "f",
      "vc": 30,
      "vv": 0,
      "d": []
    }
  ]
}

After updating system-part1, system-part2, system-part3, but not bootloader (device is in safe mode):

{
  "p": 10,
  "imei": "XXXXXXXXXXX",
  "iccid": "YYYYYYYYYYYYYYY",
  "m": [
    {
      "s": 16384,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "b",
      "n": "0",
      "v": 12,
      "d": []
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "1",
      "v": 107,
      "d": [
        {
          "f": "s",
          "n": "3",
          "v": 107,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 26,
      "f": "s",
      "n": "2",
      "v": 107,
      "d": [
        {
          "f": "s",
          "n": "1",
          "v": 107,
          "_": ""
        },
        {
          "f": "b",
          "n": "0",
          "v": 13,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "3",
      "v": 107,
      "d": []
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "u": "3FE3D246AB659DC01C4CFE030B89EE23FFBF54EDBAA402957CBC47078E631341",
      "f": "u",
      "n": "1",
      "v": 4,
      "d": [
        {
          "f": "s",
          "n": "2",
          "v": 107,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "f",
      "vc": 30,
      "vv": 0,
      "d": []
    }
  ]
}
Platform: 10 - Electron
Modules
  Bootloader module #0 - version 12, main location, 16384 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  System module #1 - version 107, main location, 131072 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #3 - version 107
  System module #2 - version 107, main location, 131072 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: FAIL
      System module #1 - version 107
      Bootloader module #0 - version 13
  System module #3 - version 107, main location, 131072 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  User module #1 - version 4, main location, 131072 bytes max size
    UUID: 3FE3D246AB659DC01C4CFE030B89EE23FFBF54EDBAA402957CBC47078E631341
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #2 - version 107
  empty - factory location, 131072 bytes max size

Updated bootloader:

{
  "p": 10,
  "imei": "XXXXXXXXX",
  "iccid": "YYYYYYYYYYY",
  "m": [
    {
      "s": 16384,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "b",
      "n": "0",
      "v": 13,
      "d": []
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "1",
      "v": 107,
      "d": [
        {
          "f": "s",
          "n": "3",
          "v": 107,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "2",
      "v": 107,
      "d": [
        {
          "f": "s",
          "n": "1",
          "v": 107,
          "_": ""
        },
        {
          "f": "b",
          "n": "0",
          "v": 13,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "f": "s",
      "n": "3",
      "v": 107,
      "d": []
    },
    {
      "s": 131072,
      "l": "m",
      "vc": 30,
      "vv": 30,
      "u": "3FE3D246AB659DC01C4CFE030B89EE23FFBF54EDBAA402957CBC47078E631341",
      "f": "u",
      "n": "1",
      "v": 4,
      "d": [
        {
          "f": "s",
          "n": "2",
          "v": 107,
          "_": ""
        }
      ]
    },
    {
      "s": 131072,
      "l": "f",
      "vc": 30,
      "vv": 0,
      "d": []
    }
  ]
}
Platform: 10 - Electron
Modules
  Bootloader module #0 - version 13, main location, 16384 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  System module #1 - version 107, main location, 131072 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #3 - version 107
  System module #2 - version 107, main location, 131072 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #1 - version 107
      Bootloader module #0 - version 13
  System module #3 - version 107, main location, 131072 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  User module #1 - version 4, main location, 131072 bytes max size
    UUID: 3FE3D246AB659DC01C4CFE030B89EE23FFBF54EDBAA402957CBC47078E631341
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #2 - version 107
  empty - factory location, 131072 bytes max size

Photon

On develop (or 0.6.2-rc.1):

{"p":6,"m":[{"s":16384,"l":"m","vc":30,"vv":30,"f":"b","n":"0","v":12,"d":[]},{"s":262144,"l":"m","vc":30,"vv":30,"f":"s","n":"1","v":106,"d":[]},{"s":262144,"l":"m","vc":30,"vv":30,"f":"s","n":"2","v":106,"d":[{"f":"s","n":"1","v":106,"_":""}]},{"s":131072,"l":"m","vc":30,"vv":30,"u":"26488F71FF958591C887EA4609F3F70A0B067544977EDFBBF4EAD2656070394E","f":"u","n":"1","v":4,"d":[{"f":"s","n":"2","v":106,"_":""}]},{"s":131072,"l":"f","vc":30,"vv":0,"d":[]}]}

After updating system-part1, system-part2, but not bootloader (device is in safe mode):

{"p":6,"m":[{"s":16384,"l":"m","vc":30,"vv":30,"f":"b","n":"0","v":12,"d":[]},{"s":262144,"l":"m","vc":30,"vv":30,"f":"s","n":"1","v":107,"d":[]},{"s":262144,"l":"m","vc":30,"vv":26,"f":"s","n":"2","v":107,"d":[{"f":"s","n":"1","v":107,"_":""},{"f":"b","n":"0","v":13,"_":""}]},{"s":131072,"l":"m","vc":30,"vv":30,"u":"93E33C9310485C2E5D2390FBA9B101B3A8168EABA77AFC99C890CC53E596E265","f":"u","n":"1","v":4,"d":[{"f":"s","n":"2","v":107,"_":""}]},{"s":131072,"l":"f","vc":30,"vv":0,"d":[]}]}
Platform: 6 - Photon
Modules
  Bootloader module #0 - version 12, main location, 16384 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  System module #1 - version 107, main location, 262144 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  System module #2 - version 107, main location, 262144 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: FAIL
      System module #1 - version 107
      Bootloader module #0 - version 13
  User module #1 - version 4, main location, 131072 bytes max size
    UUID: 93E33C9310485C2E5D2390FBA9B101B3A8168EABA77AFC99C890CC53E596E265
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #2 - version 107
  empty - factory location, 131072 bytes max size

Updated bootloader:

{"p":6,"m":[{"s":16384,"l":"m","vc":30,"vv":30,"f":"b","n":"0","v":13,"d":[]},{"s":262144,"l":"m","vc":30,"vv":30,"f":"s","n":"1","v":107,"d":[]},{"s":262144,"l":"m","vc":30,"vv":30,"f":"s","n":"2","v":107,"d":[{"f":"s","n":"1","v":107,"_":""},{"f":"b","n":"0","v":13,"_":""}]},{"s":131072,"l":"m","vc":30,"vv":30,"u":"93E33C9310485C2E5D2390FBA9B101B3A8168EABA77AFC99C890CC53E596E265","f":"u","n":"1","v":4,"d":[{"f":"s","n":"2","v":107,"_":""}]},{"s":131072,"l":"f","vc":30,"vv":0,"d":[]}]}
Platform: 6 - Photon
Modules
  Bootloader module #0 - version 13, main location, 16384 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  System module #1 - version 107, main location, 262144 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
  System module #2 - version 107, main location, 262144 bytes max size
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #1 - version 107
      Bootloader module #0 - version 13
  User module #1 - version 4, main location, 131072 bytes max size
    UUID: 93E33C9310485C2E5D2390FBA9B101B3A8168EABA77AFC99C890CC53E596E265
    Integrity: PASS
    Address Range: PASS
    Platform: PASS
    Dependencies: PASS
      System module #2 - version 107
  empty - factory location, 131072 bytes max size

@suda
Copy link
Contributor

suda commented Apr 26, 2017

Thanks Andrey! I see that bootloader shows up in the dependencies when running inspector and I also created a test case for binary-version-reader: particle-iot/binary-version-reader#15 so that part should be sorted. I'll move to testing SMH itself

@avtolstoy avtolstoy changed the title Adds bootloader as a dependency to Photon/P1 system-part1 and Electron system-part3 Adds bootloader as a secondary dependency to Photon/P1/Electron system-part2 Apr 26, 2017
@m-mcgowan
Copy link
Contributor

Rather than adding a 2nd dependency, why not make the bootloader the root dependency (a dependency of system-part1 on the photon.)

@avtolstoy
Copy link
Member Author

avtolstoy commented Apr 26, 2017

Just to duplicate what we discussed earlier: it is not possible to have bootloader as a dependency of system-part1 as I initially intended, as it will fail the dependency check when upgrading OTA/Ymodem. Whereas if we add it as a secondary dependency of system-part2, which is flashed last, we can safely let SMH upgrade the bootloader for us.

@suda
Copy link
Contributor

suda commented Apr 27, 2017

Second check in SMH itself also confirms this should work: https://github.com/spark/workerd/pull/70

Once we have a ready bootloader image (I assumed the one Andrey posted isn't final yet), we can add it to the database and test it live.

@technobly
Copy link
Member

Just an FYI, 0.6.2-rc.2 will be v107, and v0.7.0-rc.1 will be v200.

@technobly technobly added this to the 0.7.0 milestone May 9, 2017
#define MODULAR_FIRMWARE 1
#include "../../../hal/src/photon/ota_module_bounds.c"

#define SYSTEM_PART2_MIN_MODULE_VERSION 107 // 0.7.0-rc.1
Copy link
Member

Choose a reason for hiding this comment

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

v0.7.0-rc.1 will start at module version 200

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in ca75375

const module_bounds_t* bounds = NULL;
hal_module_t mod;
bool module_fetched = false;
bool valid = true;
Copy link
Member

Choose a reason for hiding this comment

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

Should we initialize valid = false in case of a rogue PC jump over the actual valid assignment below?

This exists in hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp:92 as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in eedde55, however the default in validate_module_dependencies_full has to be left true.

@technobly technobly merged commit 9a93f40 into feature/photon/wiced-3.7.0-7 May 10, 2017
@technobly technobly deleted the feature/bootloader-dependency branch May 10, 2017 18:14
avtolstoy pushed a commit to avtolstoy/firmware that referenced this pull request May 10, 2017
…pendency

Adds bootloader as a secondary dependency to Photon/P1/Electron system-part2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants