-
Notifications
You must be signed in to change notification settings - Fork 264
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
base: main
Are you sure you want to change the base?
Conversation
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") { |
There was a problem hiding this comment.
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
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". |
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) 🤔 |
Lets discuss how to proceed here - as said I'm not in favor of a macro at all |
There was a problem hiding this 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.
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) |
Probably we shouldn't - I guess it would be good to have an |
Sounds good to me! |
Thank you! The linker changes look fine to me, but just to be safe, let's merge this after we cut the beta. |
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
This since also contains a few general fixes would be good to get this merged |
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 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.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)