stm32l4-multi: add stm32n6 cryp driver #591

winderdoot · 2025-09-29T08:48:41Z

Add simple driver for CRYP peripheral on STM32N6

Description

Add libcryp API similar to libaes for STM32L4 (AES peripheral).

Motivation and Context

Allows for hardware accelerated cryptography on this platform.

How Has This Been Tested?

Tested by hand on: armv8m55-stm32n6-nucleo (only relevant platform)

Checklist:

[ x ] All new and existing linter checks and tests passed.
[ x ] My changes generate no new compilation warnings for any of the targets.

Special treatment

[ x ] I will merge this PR by myself when appropriate.

gemini-code-assist · 2025-09-29T08:48:56Z

Summary of Changes

Hello @winderdoot, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates a new hardware cryptographic driver for the STM32N6 microcontroller, facilitating secure operations through dedicated hardware acceleration. The implementation offers a consistent interface with existing cryptographic libraries, simplifying its adoption within the system.

Highlights

New CRYP Driver: Introduces a driver for the Cryptographic Processor (CRYP) peripheral on STM32N6 microcontrollers.
Hardware Acceleration: Enables hardware-accelerated cryptography for the STM32N6 platform.
API Consistency: Provides a 'libcryp' API designed to be similar to the existing 'libaes' for STM32L4, ensuring a consistent interface.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

github-actions · 2025-09-29T08:49:02Z

multi/stm32l4-multi/libmulti/libcryp.c

+		storeVector(cryp_k0lr, key, 32);
+	}
+
+	/* Wait untill key writing sequence succeded */


[codespell] _{reported by reviewdog 🐶}
untill ==> until

github-actions · 2025-09-29T08:49:02Z

multi/stm32l4-multi/libmulti/libcryp.c

+		storeVector(cryp_k0lr, key, 32);
+	}
+
+	/* Wait untill key writing sequence succeded */


[codespell] _{reported by reviewdog 🐶}
succeded ==> succeeded

github-actions · 2025-09-29T08:49:02Z

multi/stm32l4-multi/libmulti/libcryp.c

+
+	dataBarier();
+
+	/* Wait untill input FIFO not full */


[codespell] _{reported by reviewdog 🐶}
untill ==> until

github-actions · 2025-09-29T08:49:02Z

multi/stm32l4-multi/libmulti/libcryp.c

+
+	dataBarier();
+
+	/* Wait untill output FIFO not empty */


[codespell] _{reported by reviewdog 🐶}
untill ==> until

gemini-code-assist

Code Review

This pull request introduces a new CRYP driver for the STM32N6 platform. The changes are well-structured, adding the necessary Makefile rule, a public header file, and the driver implementation. My review has identified a critical linker error due to a declared but undefined function. Additionally, there are several areas for improvement regarding robustness, such as adding timeouts to busy-wait loops to prevent CPU hangs. I've also provided suggestions to enhance maintainability by removing magic numbers, cleaning up unused includes, and clarifying enum definitions. An unimplemented function in the public API should also be addressed.

gemini-code-assist · 2025-09-29T08:51:27Z

multi/stm32l4-multi/libmulti/include/libmulti/libcryp.h

+};
+
+
+int libcryp_tmp(unsigned char *key, unsigned char *iv, const unsigned char *in, unsigned char *out);


The function libcryp_tmp is declared here but is not defined in libcryp.c. This will cause a linker error during compilation. If this function is temporary or unused, it should be removed from the public API.

gemini-code-assist · 2025-09-29T08:51:27Z

multi/stm32l4-multi/libmulti/libcryp.c

+static inline void waitBusy(void)
+{
+	while ((*(common.base + cryp_sr) & (0x1 << SR_BUSY)) != 0)
+		;
+}


This busy-wait loop does not have a timeout. If the hardware enters an unexpected state and the BUSY flag never clears, this loop will run forever, causing a CPU hang. It is crucial to add a timeout mechanism to prevent this.

gemini-code-assist · 2025-09-29T08:51:27Z

multi/stm32l4-multi/libmulti/libcryp.c

+	while ((*(common.base + cryp_sr) & (0x1 << SR_IFNF)) == 0)
+		;


This busy-wait loop for the input FIFO lacks a timeout. If the FIFO never becomes available, this will lock up the CPU. A timeout should be added to handle such error conditions gracefully.

gemini-code-assist · 2025-09-29T08:51:27Z

multi/stm32l4-multi/libmulti/libcryp.c

+	while ((*(common.base + cryp_sr) & (0x1 << SR_OFNE)) == 0)
+		;


This busy-wait loop for the output FIFO lacks a timeout. If the output data is never produced, this will lock up the CPU. A timeout should be added to handle such error conditions gracefully.

gemini-code-assist · 2025-09-29T08:51:28Z

multi/stm32l4-multi/libmulti/include/libmulti/libcryp.h

+enum {
+	aes_128 = 0,
+	aes_256 = 1,
+	aes_192 = 2
+};


The values for AES key sizes are a bit confusing, with aes_256 being 1 and aes_192 being 2. While the implementation in libcryp_setKey handles this correctly, it's counter-intuitive and error-prone for future maintenance. Consider reordering the enum values to be more logical and align with the hardware register values (00 for 128-bit, 01 for 192-bit, 10 for 256-bit). This would make the code more readable and robust, though it would require updating the logic in libcryp_setKey.

Suggested change

enum {

aes_128 = 0,

aes_256 = 1,

aes_192 = 2

};

enum {

aes_128 = 0, /* Corresponds to HW value 0 */

aes_192 = 1, /* Corresponds to HW value 1 */

aes_256 = 2 /* Corresponds to HW value 2 */

};

gemini-code-assist · 2025-09-29T08:51:28Z

multi/stm32l4-multi/libmulti/libcryp.c

+#define SR_IFNF     (1)
+#define SR_IFEM     (0)
+
+#define CRYP_BASE ((void *)0x54020800)


The base address for the CRYP peripheral is hardcoded. This is considered a 'magic number' and makes the code harder to maintain. It's better practice to define this constant in a shared header file, like stm32n6_base.h, along with other peripheral addresses.

gemini-code-assist · 2025-09-29T08:51:28Z

multi/stm32l4-multi/libmulti/libcryp.c

+
+
+/* Can only be enabled when KEYVALID is set */
+inline void libcryp_enable(void)


Using the inline keyword on a non-static function defined in a .c file is a GNU extension and may not behave as expected with all toolchains. For functions that are part of the public API, it's standard practice to declare them in the header without inline and define them in the source file without inline. If inlining is desired, the function should typically be defined as static inline in the header file.

Suggested change

inline void libcryp_enable(void)

void libcryp_enable(void)

gemini-code-assist · 2025-09-29T08:51:28Z

multi/stm32l4-multi/libmulti/libcryp.c

+}
+
+
+inline void libcryp_disable(void)


Using the inline keyword on a non-static function defined in a .c file is a GNU extension and may not behave as expected with all toolchains. For functions that are part of the public API, it's standard practice to declare them in the header without inline and define them in the source file without inline. If inlining is desired, the function should typically be defined as static inline in the header file.

Suggested change

inline void libcryp_disable(void)

void libcryp_disable(void)

gemini-code-assist · 2025-09-29T08:51:28Z

multi/stm32l4-multi/libmulti/libcryp.c

+	if (keylen == aes_128) {
+		*(common.base + cryp_cr) = t | (0x0 << CR_KEYSIZE);
+		storeVector(cryp_k2lr, key, 16);
+	}
+	else if (keylen == aes_192) {
+		*(common.base + cryp_cr) = t | (0x1 << CR_KEYSIZE);
+		storeVector(cryp_k1lr, key, 24);
+	}
+	else {
+		*(common.base + cryp_cr) = t | (0x2 << CR_KEYSIZE);
+		storeVector(cryp_k0lr, key, 32);
+	}


The if-else if-else structure for setting the key size is fragile. It implicitly assumes that any keylen that is not aes_128 or aes_192 must be aes_256. This could lead to incorrect behavior if other key sizes are added to the enum in the future. A more robust approach would be to use an explicit check for aes_256 or a switch statement, and handle invalid keylen values with an error.

gemini-code-assist · 2025-09-29T08:51:28Z

multi/stm32l4-multi/libmulti/libcryp.c

+void libcryp_getKey(unsigned char *key, int keylen)
+{
+	return;
+}


The function libcryp_getKey is an empty stub. This is misleading for an API function. If reading the key from the hardware is not supported (which is common for security reasons), the function should be removed from the public API in libcryp.h to avoid confusion.

github-actions · 2025-09-29T09:09:34Z

Unit Test Results

9 277 tests ±0 8 688 ✅ ±0 55m 20s ⏱️ +14s
545 suites ±0 589 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit fa000cf. ± Comparison against base commit ea6be14.

stm32l4-multi: add stm32n6 cryp driver

fa000cf

github-actions bot reviewed Sep 29, 2025

View reviewed changes

gemini-code-assist bot reviewed Sep 29, 2025

View reviewed changes

winderdoot mentioned this pull request Sep 29, 2025

libtinyaes: add stm32n6 hardware-assisted implementation phoenix-rtos/phoenix-rtos-corelibs#62

Draft

4 tasks

winderdoot marked this pull request as draft September 29, 2025 08:54

		};


		int libcryp_tmp(unsigned char key, unsigned char iv, const unsigned char in, unsigned char out);



		/* Can only be enabled when KEYVALID is set */
		inline void libcryp_enable(void)

stm32l4-multi: add stm32n6 cryp driver #591

Are you sure you want to change the base?

stm32l4-multi: add stm32n6 cryp driver #591

Uh oh!

Conversation

winderdoot commented Sep 29, 2025

Description

Motivation and Context

How Has This Been Tested?

Checklist:

Special treatment

Uh oh!

gemini-code-assist bot commented Sep 29, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

github-actions bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 29, 2025

Unit Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants