STM32 TrustZone improvements (when tested with wolfIP)#775
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates STM32H5 TrustZone configuration to better support running a non-secure wolfIP application by loosening GPIO and RAM security/privilege settings where needed.
Changes:
- Clear GPIO secure-configuration (SECCFGR) for additional ports used by the non-secure app and enable GPIOG clocking.
- Rework GTZC MPCBB configuration: keep SRAM1 secure while making SRAM2/SRAM3 non-secure and unprivileged.
- Expand the SAU non-secure RAM region to include SRAM2 in addition to SRAM3.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| hal/stm32h5.c | Clears GPIO SECCFGR across ports used by wolfIP NS app and enables GPIOG clock to allow NS GPIO configuration. |
| hal/stm32_tz.c | Adjusts GTZC MPCBB and SAU configuration so SRAM2+SRAM3 are NS/unpriv, while SRAM1 remains secure for wolfBoot. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Clear SECCFGR for every pin on the ports the wolfIP NS app uses | ||
| * (RMII + USART3 + LED), then enable GPIOG's clock (the existing | ||
| * code only covered A/B/C/D). PD8 (USART3 TX) is already cleared | ||
| * above, but covering all of GPIOD is harmless. */ | ||
| GPIO_SECCFGR(GPIOA_BASE) = 0u; | ||
| GPIO_SECCFGR(GPIOB_BASE) = 0u; | ||
| GPIO_SECCFGR(GPIOC_BASE) = 0u; | ||
| GPIO_SECCFGR(GPIOD_BASE) = 0u; | ||
| GPIO_SECCFGR(GPIOG_BASE) = 0u; |
|
|
||
| #define SET_GTZC1_MPCBBx_SECCFGR_VCTR(bank,n,val) \ | ||
| (*((volatile uint32_t *)(GTZC1_MPCBB##bank##_SECCFGR) + n )) = val | ||
| /* PRIVCFGR_VCTR sits 0x100 after SECCFGR_VCTR in each MPCBB block. */ | ||
| #define SET_GTZC1_MPCBBx_PRIVCFGR_VCTR(bank,n,val) \ | ||
| (*((volatile uint32_t *)(GTZC1_MPCBB##bank##_SECCFGR) + 64 + n )) = val |
| for (i = 0; i < 4; i++) { | ||
| SET_GTZC1_MPCBBx_SECCFGR_VCTR(2, i, 0xFFFFFFFF); | ||
| SET_GTZC1_MPCBBx_SECCFGR_VCTR(2, i, 0x0); | ||
| SET_GTZC1_MPCBBx_PRIVCFGR_VCTR(2, i, 0x0); | ||
| } | ||
|
|
||
| /* Configure SRAM3 as non-secure (320 KB) */ | ||
| /* Configure SRAM3 as non-secure (320 KB) and unprivileged. */ | ||
| for (i = 0; i < 20; i++) { | ||
| SET_GTZC1_MPCBBx_SECCFGR_VCTR(3, i, 0x0); | ||
| SET_GTZC1_MPCBBx_PRIVCFGR_VCTR(3, i, 0x0); | ||
| } |
danielinux
left a comment
danielinux
left a comment
There was a problem hiding this comment.
I'm OK about setting all GPIO banks to non-secure for the purpose of the test application. The non-privilege Copilot "medium" comments make sense to me, I'd keep non-secure+privileged SRAM2 and SRAM3 as default for the OS to decide boundaries later. Please check again and consider Copilot's "medium" comment in stm32_tz.c line 234 to 243
No description provided.