zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.52k stars 6.44k forks source link

drivers: stm32: st,stm32-aes support for stm32l462 soc #78482

Open lucasdietrich opened 2 days ago

lucasdietrich commented 2 days ago

Describe the bug I am building a custom board based on the STM32L462RE and compilation of the st,stm32-aes device driver fails.

To Reproduce To reproduce the issue, you will need to create a custom board based on the STM32L462, as no existing board uses this SoC in the current tree (as far as I know).

Then enable support for the st,stm32-aes device.

&aes {
    status = "okay";
};

And

CONFIG_CRYPTO=y

I provided the following minimal example to reproduce the issue: demo app, build with west build -b teal_caniot

Expected behavior If st,stm32-aes compatible device is present in the soc devicetree, the driver should compile successfully.

Impact It is currently impossible to use AES on the STM32L462.

Logs and console output

The build fails with the following message

In file included from /home/lucas/zephyr-caniot-device-workspace/zephyr/drivers/crypto/crypto_stm32.c:17:
/home/lucas/zephyr-caniot-device-workspace/zephyr/drivers/crypto/crypto_stm32_priv.h:25:9: error: unknown type name 'CRYP_ConfigTypeDef'
   25 |         CRYP_ConfigTypeDef config;
      |         ^~~~~~~~~~~~~~~~~~
/home/lucas/zephyr-caniot-device-workspace/zephyr/drivers/crypto/crypto_stm32.c: In function 'do_encrypt':
/home/lucas/zephyr-caniot-device-workspace/zephyr/drivers/crypto/crypto_stm32.c:86:18: warning: implicit declaration of function 'HAL_CRYP_SetConfig'; did you mean 'UART_SetConfig'? [-Wimplicit-function-declaration]
   86 |         status = HAL_CRYP_SetConfig(&data->hcryp, &session->config);
      |                  ^~~~~~~~~~~~~~~~~~
      |                  UART_SetConfig
/home/lucas/zephyr-caniot-device-workspace/zephyr/drivers/crypto/crypto_stm32.c:93:18: warning: implicit declaration of function 'HAL_CRYP_Encrypt'; did you mean 'HAL_CRYP_Init'? [-Wimplicit-function-declaration]
   93 |         status = HAL_CRYP_Encrypt(&data->hcryp, (uint32_t *)in_buf, in_len,
      |                  ^~~~~~~~~~~~~~~~
      |                  HAL_CRYP_Init
/home/lucas/zephyr-caniot-device-workspace/zephyr/drivers/crypto/crypto_stm32.c: In function 'do_decrypt':
/home/lucas/zephyr-caniot-device-workspace/zephyr/drivers/crypto/crypto_stm32.c:123:18: warning: implicit declaration of function 'HAL_CRYP_Decrypt'; did you mean 'HAL_CRYP_DeInit'? [-Wimplicit-function-declaration]
  123 |         status = HAL_CRYP_Decrypt(&data->hcryp, (uint32_t *)in_buf, in_len,
      |                  ^~~~~~~~~~~~~~~~
      |                  HAL_CRYP_DeInit

Environment (please complete the following information):

Additional context After a quick investigation, it seems that the HAL API for the AES device in the L0, L1, and L4 series differs from other series like the F4. As a result, the driver crypto_stm32.c cannot compile without modifications. It is likely that the driver needs to be adapted for the L4 series.

erwango commented 2 days ago

@lucasdietrich Thanks for raising this point.

Following investigation, it appears that AES support for STM32L4 series was never implemented. Though, it seems that this PR, aes node has been added to L4 device tree description but was never enabled on a board, hence we failed to see that STM32 crypto driver was actually not compatible with L4 series.

To conclude, given the state of the driver, the fair thing to do would be to remove aes nodes from L4 device tree files. Then, unfortunately, after a quick look, L4 AES HAL (which is by the way same as L0 and L1) differs from currently supported cryto/aes HAL, while the actual hardware block is actually the same as WB for instance for which the HAL is compatible with the driver. Which means that supporting AES on L4 involves a complete new implementation of the stm32 crypto driver, which is not currently in the plans...

I understand that this answer doesn't solve your issue. If you don't want to implement a L4 compatible AES driver, what I can propose is to overwrite current L4 HAL crypto files with WB AES HAL. The hardware IP looks the same according to the reference manual and CMSIS description. I didn't had a try but it should work with minor tweaks.

lucasdietrich commented 19 hours ago

@erwango Ok, thank you for your answer.

I managed to partially implement an AES driver for the STM32L462RE by adapting crypto_stm32.c (ECB, CTR, and CBC only). The sample samples/drivers/crypto works fine with it on my board, but I don't know if it is mature enough for integration into Zephyr. Draft PR: #78669

I still have concerns on the following points:

erwango commented 1 hour ago

Thanks for working on this. Adding the actual driver support would indeed be a great addition.

On your concerns, I agree that a dedicated crypto_stm32l4.c would indeed be welcome. I'll also try to provide direct feedback on the PR.