-
Notifications
You must be signed in to change notification settings - Fork 668
[nrf fromlist] soc: add ironside boot report #2752
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
Conversation
|
||
cpuflpr_code_partition: partition@f4000 { | ||
reg = <0xf4000 DT_SIZE_K(48)>; | ||
}; |
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.
FYI, I intend to introduce a "memory map" DTS file that both APP and RAD use soon.
This is so that the app can send the correct vector table address to IRONside SE.
We need to make sure we don't conflict with each other more than necessary somehow.
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 suggest your change goes in first, then I adapt to that. I need this for the random numbers anyway.
soc/nordic/nrf54h/ironside_boot_report/include/nrf/ironside_boot_report.h
Outdated
Show resolved
Hide resolved
soc/nordic/nrf54h/ironside_boot_report/include/nrf/ironside_boot_report.h
Outdated
Show resolved
Hide resolved
233d4c6
to
ba5eedb
Compare
8c81397
to
5cf2744
Compare
/** See @ref ironside_se_boot_report_uicr_error */ | ||
struct ironside_se_boot_report_uicr_error uicr_error_description; | ||
/** Data passed from booting local domain to local domain being booted */ | ||
uint8_t local_domain_context[IRONSIDE_SE_BOOT_REPORT_LOCAL_DOMAIN_CONTEXT_SIZE]; |
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 see that it's optional for this to be populated.
So I assume we need a local_domain_context_len to communicate how many bytes were written?
(0 when cpusec boots cpuapp for instance)
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.
Alternatively we could make this mandatory.
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.
IMO there is a contract between the image being booted and the image that boots in this situation. For instance, for sec -> app we know that there is no local domain context.
For app -> rad there would be a struct that would communicate within in whether or not there is data of interest within the local domain context.
soc/nordic/nrf54h/ironside_se/include/nrf/ironside_se_boot_report.h
Outdated
Show resolved
Hide resolved
soc/nordic/nrf54h/ironside_se/include/nrf/ironside_se_boot_report.h
Outdated
Show resolved
Hide resolved
config SOC_NRF54H20_IRONSIDE_SE_BOOT_REPORT | ||
bool "Nordic IRONside SE boot report" | ||
default y if SOC_NRF54H20_CPUAPP | ||
depends on SOC_NRF54H20_IRON |
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.
Right now we depend on building for cpuapp as well - I guess there is no other core that sets the IRON config until cpurad gets added, but should we add a depends on regardless for correctness? Better to see it fail at build time than run time if/when cpurad gets added
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.
We use this for CPSEC, I will see if there is some other way to get around that.
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.
did we decide on ironside_se vs. ironside/se for the directory layout?
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.
Should be ironside/se IIRC
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 see the point for consistency. But we are under a 'nrf54h' folder here, which dictates the "ironside" variant being used. I will update it anyway
91ce46f
to
c83c88c
Compare
c83c88c
to
14dc342
Compare
14dc342
to
1fae501
Compare
Also update memory map to leverage unused MRAM and move sysctrl IPC to RAM20 to free global RAM. Upstream PR #: 88932 Signed-off-by: Håkon Amundsen <[email protected]>
1fae501
to
3228627
Compare
|
non-empty PR description