-
Notifications
You must be signed in to change notification settings - Fork 8k
Add support for configuring UICR.SECONDARY.PROCESSOR and UICR.PROTECTEDMEM #95733
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
samples/boards/nordic/nrf_ironside/secondary_boot_corruption/src/main.c
Outdated
Show resolved
Hide resolved
43e9550
to
5e71661
Compare
5e71661
to
1b1544f
Compare
soc/nordic/common/uicr/gen_uicr.py
Outdated
|
||
# Handle protected memory configuration | ||
if args.protectedmem: | ||
if args.protectedmem_size_bytes % 4096 != 0: |
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.
nit: Some variable named KB_4
would make this easier to read IMO.
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.
fixed
The sample uses UICR (User Information Configuration Registers) to configure | ||
secondary boot and protected memory regions, then intentionally corrupts the |
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 we state explicitly that we enable
the secondary boot? Not only configure it, some users might assume that it is enabled by default.
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.
fixed
************ | ||
|
||
* nRF54H20DK board | ||
* Zephyr SDK 0.17.0 or later |
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.
Do we need to state this?
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.
fixed
* ``CONFIG_ARM_MPU=n``: Disables the MPU to allow memory corruption | ||
* ``CONFIG_GEN_UICR_SECONDARY=y``: Enables secondary boot in UICR | ||
* ``CONFIG_GEN_UICR_PROTECTEDMEM=y``: Enables protected memory in UICR | ||
* ``CONFIG_FLASH_LOAD_OFFSET=0x1b0000``: Places secondary image at correct address |
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.
For which sample? Does both set all of these?
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.
Removed docs.
The config is most easily understood by reading the config files.
|
||
* **Primary Image**: Located at ``0xe030000`` (default slot0_partition) | ||
* **Secondary Image**: Located at ``0xe1b0000`` (secondary_partition) | ||
* **UICR Configuration**: Generated and placed at ``0xfff8000`` |
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 we have this here? Isn't that part of the nRF54H20 architecture and not set by this sample? Does it give any value to present it here?
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.
removed
/* Corrupt the protected memory (This is only possible because we | ||
* have disabled CONFIG_ARM_MPU). | ||
*/ | ||
*((volatile uint32_t *)0xe030000) = 0x5EB0; |
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.
Reading this its not clear that 0xe0300000
is covered by PROTECTEDMEM
.
The docs states that PROTECTEDMEM
starts from the end of RSLOT, so at the start of this image, but its not clear that this is 0xe0300000
.
would it be better to use the symbols in linker_defs.h
(or similar) and CONFIG_GEN_UICR_PROTECTEDMEM_SIZE_BYTES
directly?
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.
Using linker_defs.h
samples/boards/nordic/nrf_ironside/secondary_boot_corruption/secondary/CMakeLists.txt
Outdated
Show resolved
Hide resolved
*((volatile uint32_t *)0xe030008) = 0x5EB0; | ||
*((volatile uint32_t *)0xe030010) = 0x5EB0; | ||
|
||
printk("Rebooting\n"); |
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.
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.
fixded
63d2296
to
61040fd
Compare
c7110b2
to
6e740f7
Compare
6e740f7
to
2153f20
Compare
2153f20
to
57486a5
Compare
"GEN_UICR_SECONDARY", # Used in specialized build tool, not part of main Kconfig | ||
"GEN_UICR_SECONDARY_GENERATE_PERIPHCONF", # Used in specialized build tool, not part of main Kconfig | ||
"GEN_UICR_SECURESTORAGE", # Used in specialized build tool, not part of main Kconfig | ||
"GEN_UICR_SECONDARY_PROCESSOR_VALUE", # Used in specialized build tool, not part of main Kconfig |
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.
Need to fix the order to pass compliance check
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.
Thanks!
Fixed.
57486a5
to
058c94c
Compare
Add support for uicr.SECONDARY.PROCESSOR. Signed-off-by: Sebastian Bøe <[email protected]>
Add support for PROTECTEDMEM. Signed-off-by: Sebastian Bøe <[email protected]>
058c94c
to
556f2da
Compare
|
Add support for PROTECTEDMEM.
Add support for uicr.SECONDARY.PROCESSOR.