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

Prepare linker scripts for ESP-IDF application descriptor #3124

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Feb 10, 2025

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

This adds config options to emit the app descriptor from information configured via config options.

Placing the app-descriptor where the bootloader expects it required some changed to the linker scripts. I tested a couple of scenarios and hopefully nothing broke

Testing

I tested with a custom made esp-idf bootloader. In theory you could just enable some security features which require the app-desc but for me that always resulted in a bootloader which is too big. In the end I just changed the bootloader code to load and parse the app-descriptor.

Alternatively, bootloaders from later esp-idf versions always get the app-descriptor (so just update esp-idf and try a recent bootloader)

esp-hal/build.rs Outdated
@@ -180,6 +180,26 @@ fn main() -> Result<(), Box<dyn Error>> {
copy_dir_all(&config_symbols, "ld/sections", &out)?;
copy_dir_all(&config_symbols, format!("ld/{device_name}"), &out)?;

// supply build time and date
// see https://reproducible-builds.org/docs/source-date-epoch/
let ts = match std::env::var("SOURCE_DATE_EPOCH") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is obviously not the time when some source files got touched for the last time but when the build script was run

I guess that is fine since production builds are usually clean builds

@bugadani
Copy link
Contributor

Since we get e.g. the package name and version we cannot just put this macro in esp_hal::init but the user needs to use it in the binary crate.

The app name/version doesn't have to be the real value. We could use esp-config to let the user configure these values independent of their Cargo.toml (which isn't ideal, but they would have the option to set it without the macro), or we could provide an esp-config switch to allow the user to call the macro themselves with data from their Cargo.toml.

My point is, we could default to a "something exists that we partially fake".

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Feb 10, 2025

I'm not even sure this should be a macro (it's more or less just the macro from esp-idf-hal)

We could do it all in the build.rs - which would work if we only honor config-values

We can also consider to get all the fields (including build time) via the config. (And we could change the esp-generate generated build.rs to set the fields automatically by default - in a follow up step) 🤔

@bjoernQ bjoernQ marked this pull request as ready for review February 10, 2025 15:36
@bjoernQ
Copy link
Contributor Author

bjoernQ commented Feb 10, 2025

Lets discuss how to proceed here - as said I'm not in favor of a macro at all

Copy link
Member

@MabezDev MabezDev 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 I'd prefer if we explored this in a separate crate. These configs feel too esp-idf specific to me (even if they're not named as such), and I'd rather we did this separately, at least for now, we can always add it directly to esp-hal later.

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Feb 20, 2025

I think I'd prefer if we explored this in a separate crate. These configs feel too esp-idf specific to me (even if they're not named as such), and I'd rather we did this separately, at least for now, we can always add it directly to esp-hal later.

I think that's a good idea - but we'd need the linker changes here so I will remove everything but the linker scripts from this and we can generate the actual content somewhere else (or just in the generated template maybe - behind a generator option)

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Feb 20, 2025

we can always add it directly to esp-hal later

Probably we shouldn't - I guess it would be good to have an esp-idf-boot-support and a mcu-boot-support crate (and whatever we'll support in future)

@MabezDev
Copy link
Member

Sounds good to me!

@bjoernQ bjoernQ changed the title Add application description for ESP-IDF image format Prepare linker scripts for ESP-IDF application descriptor Feb 20, 2025
@bjoernQ bjoernQ added the skip-changelog No changelog modification needed label Feb 20, 2025
@MabezDev
Copy link
Member

Thank you! The linker changes look fine to me, but just to be safe, let's merge this after we cut the beta.

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Mar 5, 2025

To test the app-descriptor work just add s.th. like this

 #[repr(C)]
    pub struct EspAppDesc {
        pub magic_word: u32,                       // Magic word ESP_APP_DESC_MAGIC_WORD
        pub secure_version: u32,                   // Secure version
        pub reserv1: [u32; 2],                     // reserv1
        pub version: [core::ffi::c_char; 32],      // Application version
        pub project_name: [core::ffi::c_char; 32], // Project name
        pub time: [core::ffi::c_char; 16],         // Compile time
        pub date: [core::ffi::c_char; 16],         // Compile date
        pub idf_ver: [core::ffi::c_char; 32],      // Version IDF
        pub app_elf_sha256: [u8; 32],              // sha256 of elf file
        pub min_efuse_blk_rev_full: u16, // Minimal eFuse block revision supported by image, in format: major * 100 + minor
        pub max_efuse_blk_rev_full: u16, // Maximal eFuse block revision supported by image, in format: major * 100 + minor
        pub mmu_page_size: u8,           // MMU page size in log base 2 format
        pub reserv3: [u8; 3],            // reserv3
        pub reserv2: [u32; 18],          // reserv2
    }

    const ESP_APP_DESC_MAGIC_WORD: u32 = 0xABCD5432;

    const fn str_to_cstr_array<const C: usize>(s: &str) -> [::core::ffi::c_char; C] {
        let bytes = s.as_bytes();
        if bytes.len() >= C {
            assert!(true, "String is too long for the C-string field");
        }

        let mut ret: [::core::ffi::c_char; C] = [0; C];
        let mut i = 0;
        loop {
            ret[i] = bytes[i] as _;
            i += 1;
            if i >= bytes.len() {
                break;
            }
        }
        ret
    }

    #[no_mangle]
    #[used]
    #[link_section = ".rodata_desc"]
    #[allow(non_upper_case_globals)]
    pub static esp_app_desc: EspAppDesc = EspAppDesc {
        magic_word: ESP_APP_DESC_MAGIC_WORD,
        secure_version: 0,
        reserv1: [0; 2],
        version: str_to_cstr_array(env!("CARGO_PKG_VERSION")),
        project_name: str_to_cstr_array(env!("CARGO_PKG_NAME")),
        time: str_to_cstr_array("15:15:15"),
        date: str_to_cstr_array("2025-03-05"),
        // just pretending some esp-idf version here
        idf_ver: str_to_cstr_array("5.3.1"),
        app_elf_sha256: [0; 32],
        min_efuse_blk_rev_full: 0,
        max_efuse_blk_rev_full: u16::MAX,
        mmu_page_size: 0,
        reserv3: [0; 3],
        reserv2: [0; 18],
    };

    println!("{:x}", esp_app_desc.magic_word);  

And use a recent bootloader

ESP-ROM:esp32h2-20221101
Build:Nov  1 2022
rst:0x1 (POWERON),boot:0xc (SPI_FAST_FLASH_BOOT)
SPIWP:0xee
mode:DIO, clock div:2
load:0x408460e0,len:0x1674
load:0x4083cad0,len:0xf04
load:0x4083efd0,len:0x2e98
entry 0x4083cada
I (23) boot: ESP-IDF v5.4 2nd stage bootloader
I (24) boot: compile time Mar  5 2025 17:48:31
I (25) boot: chip revision: v0.1
I (25) boot: efuse block revision: v0.0
I (27) boot.esp32h2: SPI Speed      : 32MHz
I (31) boot.esp32h2: SPI Mode       : DIO
I (35) boot.esp32h2: SPI Flash Size : 4MB
I (39) boot: Enabling RNG early entropy source...
I (43) boot: Partition Table:
I (46) boot: ## Label            Usage          Type ST Offset   Length
I (52) boot:  0 nvs              WiFi data        01 02 00009000 00006000
I (58) boot:  1 phy_init         RF data          01 01 0000f000 00001000
I (65) boot:  2 factory          factory app      00 00 00010000 003f0000
I (72) boot: End of partition table
I (75) esp_image: segment 0: paddr=00010020 vaddr=42000020 size=07cc0h ( 31936) map
I (92) esp_image: segment 1: paddr=00017ce8 vaddr=40800000 size=00014h (    20) load
I (93) esp_image: segment 2: paddr=00017d04 vaddr=42007d04 size=487b0h (296880) map
I (187) esp_image: segment 3: paddr=000604bc vaddr=40800014 size=056d0h ( 22224) load
I (195) esp_image: segment 4: paddr=00065b94 vaddr=40817b94 size=00030h (    48) load
I (198) boot: Loaded app from partition at offset 0x10000
I (198) boot: Disabling RNG early entropy source...

This since also contains a few general fixes would be good to get this merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog No changelog modification needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants