feat(CMSIS): Update MAX32657 CMSIS with TrustZone support #1008
Conversation
| #define MXC_DMA_INSTANCES (1) | ||
| // ^ Note: We have 2 DMA instances in hardware, but they are secure vs non-secure | ||
| // instances. Therefore we treat the part as if there is only 1. | ||
| #define MXC_DMA_INSTANCES (2) |
There was a problem hiding this comment.
If you try to build with this you'll see an issue at the bottom-level DMA driver file with the handler definitions. Also RevA depends on MXC_DMA
There was a problem hiding this comment.
Yes, that is why I added a WIP for this PR before committing to this.
| // DMA0 can only access the non-secure mappings of the peripherals, | ||
| // but DMA0 can be accessed in both Non-secure and Secure code. | ||
| // DMA1 can access both secure and non-secure addresses of the peripherals, | ||
| // but DMA1 can Only be accessed in Secure code. |
There was a problem hiding this comment.
DMA0 can be accessed from secure mode but my understanding is that the controller cannot access secure memory/peripherals. I think we should still preserve the ability to access it at the register-level, but for this first pass at the drivers I think we should revert back to the previous setup
There was a problem hiding this comment.
Yes, you are correct - as mentioned in the PR description above:
Both DMA0 and DMA1 need to be defined, so we can't use the single
MXC_DMAinstance anymore.
- DMA0 has a secure and non-secure mapping - accessible for secure and non-secure builds. However, DMA0 can not access secure memory or secure peripherals.
- DMA1 only has a secure mapping - accessible for secure builds. But DMA1 can access both secure or non-secure memory and peripherals.
The diction for all the security can get confusing. If we do a secure build, there needs to be an option to access both DMA0 and DMA1.
There was a problem hiding this comment.
What if we define MXC_DMA as DMA0/DMA1 based on the security level, set MXC_DMA_INSTANCES to 1, but still expose both base definitions of MXC_DMA0/MXC_DMA1?
| #define MXC_DMA_CH_GET_IRQ(i) \ | ||
| #define MXC_BASE_DMA0 MXC_BASE_DMA0_NS | ||
| #define MXC_DMA0 MXC_DMA0_NS | ||
| // TODO(DMA1): Not entirely show how to handle access to MXC_DMA1 in non-secure mode. |
There was a problem hiding this comment.
The previous setup solves this problem as well
| *(.ARM.extab* .gnu.linkonce.armextab.*) | ||
| _sg_veneers = .; | ||
| KEEP(*(.gnu.sgstubs*)) | ||
| _esg_veneers = .; | ||
| } > FLASH_S |
There was a problem hiding this comment.
See 4e9156e
I updated the linkerfile to support secure/non-secure from the same file. I think we should be able to wrap this section in the same way so that it only gets generated for the secure builds.
Libraries/CMSIS/Device/Maxim/MAX32657/Include/partition_max32657.h
Outdated
Show resolved
Hide resolved
github-actions
bot
added
MAX32655
| @@ -500,7 +500,11 @@ int MXC_DMA_RevA_MemCpy(mxc_dma_reva_regs_t *dma, void *dest, void *src, int len | |||
| return retval; | |||
| } | |||
|
|
|||
| #if (TARGET_NUM == 32657) | |||
There was a problem hiding this comment.
Yep, agreed. I don't like this either lol. We can change this in another separate PR.
Description
Some updates to the CMSIS for Verification.
GET_S_andGET_NSdefinitions from max32657.hBoth DMA0 and DMA1 need to be defined, so we can't use the single
MXC_DMAinstance anymore.I'll have a separate PR for the remaining SYS register updates that I need to make.
Edit: Removing multiple linkers comment, just saw your changes @Jake-Carter. But I don't think we need multiple vector table definitions and startup_max32657.S startup code.
We can have one vector table defined that includes all secure and non-secure handlers, and we can dynamically switch between secure and non-secure vector tables during startup process using the
SCB->VTORvector table offset register. TheSCB->VTORregister is banked into two parts, and the appropriate offset for secure and non-secure is used depending on the security context. We can also explicitly label the secure-only interrupts with the_Snames, and include a build warning/error if the non-secure application references a secure handler.The secure vector table is set up in
SystemInit()called from Reset_Handler on startup. We'd have to figure out a mechanism for non-secure builds (non-secure reset handler) to set up the non-secure vector table (usingSCB->VTORagain). This is at least my current understanding of how we might go about this for our bare metal MSDK.There's a lot to think about - but given our schedule and priorities, we can focus on handling the secure mode for now so verification can begin. These changes are geared towards finishing the Secure build system changes that you've made, and to prep the non-secure builds for down the road.
Checklist Before Requesting Review