Skip to content
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

feat(PeriphDrivers): Add Basic MAX32657 PeriphDriver Support #1005

Merged
merged 43 commits into from
May 6, 2024

Conversation

Jake-Carter
Copy link
Contributor

@Jake-Carter Jake-Carter commented May 1, 2024

Description

This PR will add basic PeriphDriver support for the ME30. We are targeting a clean build. Functional testing/fixes are beyond the scope of this PR.

I've added a MAX32657 Hello World as an initial build point.

Status:

  • Basic system implementation and top-level mxc_sys.h
  • The SPI clock disable is defined in both PCLKDIS0 and PCLKDIS1. Resolve in register files
  • EXTCLK select is missing from gcr_regs.h. Resolve in register files
  • CLKCTRL sysclk_sel definitions seem incorrect. Resolve in register files
  • Decide on secure vs non-secure default for top-level utility macros. (feat(PeriphDrivers): Add Basic MAX32657 PeriphDriver Support #1005 (comment))
  • Linkerfiles
  • Register-level builds
  • PeriphDriver Build
    • mxc_assert
    • mxc_delay
    • mxc_lock
    • mxc_sys
    • mxc_pins (should be updated when we get pin defs)
    • AES
    • CRC
    • DMA
    • FLC
    • GPIO
    • ICC
    • LP (missing register file)
    • RTC
    • SPI
    • TMR
    • UART
    • WDT
    • WUT

@Jake-Carter Jake-Carter added the WIP work in progress label May 1, 2024
@Jake-Carter
Copy link
Contributor Author

@ozersa @ttmut @lorne-maxim @sihyung-maxim FYI. See current status above

- Target secure memory regions by default (CPU boots into secure mode by
  default)
- Update memory layout and sizes
- Remove dual-core/shared/mailbox (single-core device)
- Add MSECURITY_MODE build configuration variable
- Defines MSECURITY_MODE_SECURE/MSECURITY_MODE_NONSECURE at compile time
@Jake-Carter
Copy link
Contributor Author

Jake-Carter commented May 1, 2024

@sihyung-maxim @lorne-maxim @ozersa @ttmut some updates below.

5/1/2024

I've updated the ME30 linker script (1978725) and added a basic build mechanism for selecting between secure/non-secure builds (6f19ec6).

The M33 will boot/reset into secure mode, so I've made it the default. Additionally, access to the Flash controller is only possible in secure mode.

This allows the top-level max32657.h file to map the addresses to the correct aliases at build time (ex for UART: 9f7bbfe).

I think that this solves the issue of building completely secure vs non-secure binaries. Please comment on the approach before I apply it to the rest of the peripherals.

Security Mode Transitions

Conceptually it is also possible to switch between the two modes at run-time. However, if we link the entire application into the secure address mappings then I don't think it is possible to transition. Even if we call a BXNS, the rest of the code will be linked into the secure space. So we've effectively locked out the application from that point on.

image
(Arm v8-M Architecture Reference Manual)

Conceptually, I have the following questions now:

  • Is there a use-case/need for creating mixed security binaries?
  • Is a driver-layer API for manually transitioning between the two modes ever useful?

DMA Complications

After updating the DMA drivers to account for the ME30's dual instances (df0e566) there are some complications.

Many DMA drivers do not expose the DMA instance as an argument at the top-level header. The bottom-level RevA/RevB files call these variations directly. Ex attempting to build AES:

- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_revb.c
/home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_revb.c: In function 'MXC_AES_RevB_TXDMAConfig':
/home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_revb.c:243:5: error: too few arguments to function 'MXC_DMA_Init'
  243 |     MXC_DMA_Init();

On the ME30, DMA1 is a secure DMA with access to the secure space, and DMA0 is a non-secure instance. If a completely secure image is built, DMA0 will be useless. So - should we treat the ME30 as if it has a single DMA instance, and assign DMA1/DMA0 based on secure vs non-secure builds?

@sihyung-maxim
Copy link
Contributor

@sihyung-maxim @lorne-maxim @ozersa @ttmut some updates below.

5/1/2024

I've updated the ME30 linker script (1978725) and added a basic build mechanism for selecting between secure/non-secure builds (6f19ec6).

The M33 will boot/reset into secure mode, so I've made it the default. Additionally, access to the Flash controller is only possible in secure mode.

This allows the top-level max32657.h file to map the addresses to the correct aliases at build time (ex for UART: 9f7bbfe).

I think that this solves the issue of building completely secure vs non-secure binaries. Please comment on the approach before I apply it to the rest of the peripherals.

Security Mode Transitions

Conceptually it is also possible to switch between the two modes at run-time. However, if we link the entire application into the secure address mappings then I don't think it is possible to transition. Even if we call a BXNS, the rest of the code will be linked into the secure space. So we've effectively locked out the application from that point on.

image (Arm v8-M Architecture Reference Manual)

Conceptually, I have the following questions now:

  • Is there a use-case/need for creating mixed security binaries?
  • Is a driver-layer API for manually transitioning between the two modes ever useful?

DMA Complications

After updating the DMA drivers to account for the ME30's dual instances (df0e566) there are some complications.

Many DMA drivers do not expose the DMA instance as an argument at the top-level header. The bottom-level RevA/RevB files call these variations directly. Ex attempting to build AES:

- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_revb.c
/home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_revb.c: In function 'MXC_AES_RevB_TXDMAConfig':
/home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_revb.c:243:5: error: too few arguments to function 'MXC_DMA_Init'
  243 |     MXC_DMA_Init();

On the ME30, DMA1 is a secure DMA with access to the secure space, and DMA0 is a non-secure instance. If a completely secure image is built, DMA0 will be useless. So - should we treat the ME30 as if it has a single DMA instance, and assign DMA1/DMA0 based on secure vs non-secure builds?

I'm still working on my side of the updates, but to answer your DMA question:

Yeah, I think we should. We still need to define separate names for the DMAs in the registers: for example, GCR_PCLKDISn and _RSTn registers has fields representing each DMA instance. Maybe a better name for the two DMA instances are: DMA0 -> DMA_NS and DMA1 -> to DMA_S. Then as you said, define what the undecorated DMA name would be depending on the secure vs non-secure builds. So it would look something like this:

In max32657.h:

... Interrupt number table
    DMA_NS_CH0_IRQn,
    DMA_NS_CH1_IRQn,
    DMA_NS_CH2_IRQn,
    DMA_NS_CH3_IRQn,
    DMA_S_CH0_IRQn,
    DMA_S_CH1_IRQn,
    DMA_S_CH2_IRQn,
    DMA_S_CH3_IRQn,
...
... DMA section
    #define MXC_BASE_DMA_NS ((uint32_t)0x40028000UL)
    #define MXC_DMA_NS ((mxc_dma_regs_t *)MXC_BASE_DMA_NS)

    #define MXC_BASE_DMA_S ((uint32_t)0x50028000UL)
    #define MXC_DMA_S ((mxc_dma_regs_t *)MXC_BASE_DMA_S)

#if defined(MSECURITY_MODE_SECURE)
    #define MXC_BASE_DMA MXC_BASE_DMA_S
    #define MXC_DMA MXC_DMA_S
    #define DMA_CH0_IRQn DMA_S_CH0_IRQn
    #define DMA_CH1_IRQn DMA_S_CH1_IRQn
    #define DMA_CH2_IRQn DMA_S_CH2_IRQn
    #define DMA_CH3_IRQn DMA_S_CH3_IRQn
    #define DMA_CH0_IRQHandler DMA_S_CH0_IRQHandler
    #define DMA_CH1_IRQHandler DMA_S_CH1_IRQHandler
    #define DMA_CH2_IRQHandler DMA_S_CH2_IRQHandler
    #define DMA_CH3_IRQHandler DMA_S_CH3_IRQHandler
#else
    #define MXC_BASE_DMA MXC_BASE_DMA_NS
    #define MXC_DMA MXC_DMA_NS
    #define DMA_CH0_IRQn DMA_NS_CH0_IRQn
    #define DMA_CH1_IRQn DMA_NS_CH1_IRQn
    #define DMA_CH2_IRQn DMA_NS_CH2_IRQn
    #define DMA_CH3_IRQn DMA_NS_CH3_IRQn
    #define DMA_CH0_IRQHandler DMA_NS_CH0_IRQHandler
    #define DMA_CH1_IRQHandler DMA_NS_CH1_IRQHandler
    #define DMA_CH2_IRQHandler DMA_NS_CH2_IRQHandler
    #define DMA_CH3_IRQHandler DMA_NS_CH3_IRQHandler
#endif

Also, I've been meaning to implement this name change for newer parts, but we can rename the DMA vector names to something more accurate.

   DMA0_IRqn -> DMA_CH0_IRQn
   DMA1_IRqn -> DMA_CH1_IRQn
   DMA2_IRqn -> DMA_CH2_IRQn
   DMA3_IRqn -> DMA_CH3_IRQn

The channel number wouldn't be where you would expect the DMA instance number to be. What do we think?

@Jake-Carter
Copy link
Contributor Author

@sihyung-maxim I think this solution of mapping onto secure/non-secure base address based on the build value makes sense and would work well.

I'm reading more about TrustZone and the SAU/MPCs/PPCs and it sounds like we can partition the memory spaces and security attributes for each peripheral individually at startup.

The standard way to do things is to build secure applications separately (first), then compile and link in the non-secure image into the same binary afterwards. So I'm working on a mechanism now for that similar to what we've done with the dual-core micros. The secure image will be responsible for initializing the security attributes for each peripheral on startup. Then the non-secure image will be appended onto the secure image's SRAM/Flash using the non-secure address aliases.

This will turn the ME30 projects into dual-project workspaces for the most-part, but seems to be the standard approach.

So this works well.

Also, I've been meaning to implement this name change for newer parts, but we can rename the DMA vector names to something more accurate.

  DMA0_IRqn -> DMA_CH0_IRQn
  DMA1_IRqn -> DMA_CH1_IRQn
  DMA2_IRqn -> DMA_CH2_IRQn
  DMA3_IRqn -> DMA_CH3_IRQn

The channel number wouldn't be where you would expect the DMA instance number to be. What do we think?

Having the channel is good. I think having the instance in there is also needed. I went with this for the header file atm:

DMA0_CH0_IRQn,          /* 0x20  0x0080  32: DMA0 Channel 0 */
DMA0_CH1_IRQn,          /* 0x21  0x0084  33: DMA0 Channel 1 */
DMA0_CH2_IRQn,          /* 0x22  0x0088  34: DMA0 Channel 2 */
DMA0_CH3_IRQn,          /* 0x23  0x008C  35: DMA0 Channel 3 */
DMA1_CH0_IRQn,          /* 0x24  0x0090  36: DMA1 Channel 0 */
DMA1_CH1_IRQn,          /* 0x25  0x0094  37: DMA1 Channel 1 */
DMA1_CH2_IRQn,          /* 0x26  0x0098  38: DMA1 CHannel 2 */
DMA1_CH3_IRQn,          /* 0x27  0x009C  39: DMA1 Channel 3 */

@Jake-Carter
Copy link
Contributor Author

@sihyung-maxim there is also the concept of secure vs non-secure interrupts. We will need to add some additional NVIC entries with a secure/non-secure offset

- Add <arm_cmse.h> to max32657.h
- Define "IS_SECURE_ENVIRONMENT" that uses GCC-provided definitions
  instead of our custom build-time one.
  Usage:  #if IS_SECURE_ENVIRONMENT
          ...
          #endif
- SIR
- FCR
- WDT
- SVM
- BOOST
- TRIMSIR
- RTC
- WUT
- PWRSEQ
- MCR
- AES
- CRC
- TMR
- I3C
- SPI
- TRNG
- BTLE
@Jake-Carter
Copy link
Contributor Author

5/2/2024

I updated the system for building secure/non-secure images to use the built-in mcmse GCC flag (9644b4e) and added a compiler macro for checking the security state.

As long as max32657.h is included, the code can check the security state as follows:

#if IS_SECURE_ENVIRONMENT
// ...
#endif

I used the macro to update the peripheral base address aliases for all peripherals we currently have definitions for.

Initial PeriphDriver Build

Clean PeriphDriver/Hello World build! This build puts everything into the secure address mappings.

max32657.map.txt

~/repos/msdk/Examples/MAX32657/Hello_World (feat/ME30-PeriphDrivers*) » make                                          jhcarter@jhcarter-X670E
Loaded project.mk
****************************************************************************
* Analog Devices MSDK
* v2024_02-109-g4c9522a5ec
* - User Guide: https://analogdevicesinc.github.io/msdk/USERGUIDE/
* - Get Support: https://www.analog.com/support/technical-support.html
* - Report Issues: https://github.com/analogdevicesinc/msdk/issues
* - Contributing: https://analogdevicesinc.github.io/msdk/CONTRIBUTING/
****************************************************************************
- MKDIR /home/jhcarter/repos/msdk/Examples/MAX32657/Hello_World/build
make[1]: Entering directory '/home/jhcarter/repos/msdk/Examples/MAX32657/Hello_World'
- MKDIR /home/jhcarter/repos/msdk/Libraries/PeriphDrivers/bin/MAX32657/spi-v1_softfp
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/SYS/mxc_assert.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/SYS/mxc_delay.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/SYS/mxc_lock.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/SYS/sys_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/SYS/pins_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/AES/aes_revb.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/CRC/crc_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/CRC/crc_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/DMA/dma_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/DMA/dma_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/FLC/flc_common.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/FLC/flc_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/FLC/flc_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/GPIO/gpio_common.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/GPIO/gpio_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/GPIO/gpio_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/ICC/icc_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/ICC/icc_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/RTC/rtc_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/RTC/rtc_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/SPI/spi_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/SPI/spi_reva1.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/TMR/tmr_common.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/TMR/tmr_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/TMR/tmr_revb.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/TRNG/trng_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/TRNG/trng_revb.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/UART/uart_common.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/UART/uart_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/UART/uart_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/WDT/wdt_common.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/WDT/wdt_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/WDT/wdt_reva.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/WUT/wut_me30.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/../PeriphDrivers/Source/WUT/wut_reva.c
- AR /home/jhcarter/repos/msdk/Libraries/PeriphDrivers/bin/MAX32657/spi-v1_softfp/libPeriphDriver_spi-v1_softfp.a
make[1]: Leaving directory '/home/jhcarter/repos/msdk/Examples/MAX32657/Hello_World'
- CC main.c
- CC /home/jhcarter/repos/msdk/Libraries/Boards/MAX32657/EvKit_V1/Source/board.c
- CC /home/jhcarter/repos/msdk/Libraries/MiscDrivers/stdio.c
- CC /home/jhcarter/repos/msdk/Libraries/MiscDrivers/LED/led.c
- CC /home/jhcarter/repos/msdk/Libraries/MiscDrivers/PushButton/pb.c
- AS /home/jhcarter/repos/msdk/Libraries/CMSIS/Device/Maxim/MAX32657/Source/GCC/startup_max32657.S
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/Device/Maxim/MAX32657/Source/heap.c
- CC /home/jhcarter/repos/msdk/Libraries/CMSIS/Device/Maxim/MAX32657/Source/system_max32657.c
- LD /home/jhcarter/repos/msdk/Examples/MAX32657/Hello_World/build/max32657.elf
arm-none-eabi-size --format=berkeley /home/jhcarter/repos/msdk/Examples/MAX32657/Hello_World/build/max32657.elf
   text	  data	   bss	   dec	   hex	filename
  34156	  2500	   280	 36936	  9048	/home/jhcarter/repos/msdk/Examples/MAX32657/Hello_World/build/max32657.elf

I will start working on the dual-project build system and more advanced linkerfiles now.

@github-actions github-actions bot added MAX32655 Related to the MAX32655 (ME17) MAX32670 Related to the MAX32670 (ME15) MAX32672 Related to the MAX32672 (ME21) MAX32675 Related to the MAX32675 (ME16) MAX32680 Related to the MAX32680 (ME20) MAX78000 Related to the MAX78000 (AI85) MAX78002 Related to the MAX78002 (AI87) MAX32662 Related to the MAX32662 (ME12) labels May 6, 2024
@Jake-Carter Jake-Carter merged commit 26ac88e into feat/ME30 May 6, 2024
6 checks passed
@Jake-Carter
Copy link
Contributor Author

Merged.

The build system can specify secure (default) vs non-secure builds with the new MSECURITY_MODE build config variable. Acceptable values are:

  • SECURE
  • NONSECURE

I recommend setting this in project.mk so that any changes to its value trigger automatic full rebuilds.

Note we still need to check/update pin definitions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MAX32655 Related to the MAX32655 (ME17) MAX32662 Related to the MAX32662 (ME12) MAX32670 Related to the MAX32670 (ME15) MAX32672 Related to the MAX32672 (ME21) MAX32675 Related to the MAX32675 (ME16) MAX32680 Related to the MAX32680 (ME20) MAX78000 Related to the MAX78000 (AI85) MAX78002 Related to the MAX78002 (AI87) WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants