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

drivers/entropy: stm32: non-compliant RNG configuration on some MCUs #45866

Closed
taltenbach opened this issue May 22, 2022 · 5 comments · Fixed by #45994
Closed

drivers/entropy: stm32: non-compliant RNG configuration on some MCUs #45866

taltenbach opened this issue May 22, 2022 · 5 comments · Fixed by #45994
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug

Comments

@taltenbach
Copy link
Contributor

taltenbach commented May 22, 2022

Describe the bug
On some STM32 MCUs, such as the STM32H723 or STM32L5, the RNG peripheral must be configured according to specific values provided in the reference manual to obtain best latency and to be compliant with NIST. This includes:

  • Writing appropriate values to CONFIG1, CONFIG2 and CONFIG3 fields in RNG_CR register.
  • Writing a proper value in RNG_HTCR register.

However, it seems that the RNG driver:

  • Does not write CONFIGx fields, although their reset value does not correspond to the recommended configuration.
  • Does not properly follow the process specified in the reference manual to write to RNG_HTCR. Indeed, CONDRST is not set before writing to RNG_HTCR, even though the RM0468 states (p. 1440):

Writing in RNG_HTCR is taken into account only if CONDRST bit is set, and CONFIGLOCK bit is cleared in RNG_CR. Writing to this register is ignored if CONFIGLOCK=1.

To Reproduce
N/A

Expected behavior
The STM32 RNG peripheral should be properly configured.

Impact
RNG configuration is not compliant with NIST and entropy generation latency might not be ideal.

Environment
Commit e9d3791

Additional context
Note that I unfortunately have no STM32 MCU with a RNG peripheral having such configuration fields at my disposal, so this issue is only based on my understanding of the reference manual and the driver implementation. In case the issue turns out to be real, I am willing to create a PR but I won't be able to test the fix on real hardware.

@taltenbach taltenbach added the bug The issue is a bug, or the PR is fixing a bug label May 22, 2022
@erwango erwango added the platform: STM32 ST Micro STM32 label May 23, 2022
@erwango erwango self-assigned this May 23, 2022
@erwango erwango added the priority: low Low impact/importance bug label May 23, 2022
@FRASTM
Copy link
Collaborator

FRASTM commented May 24, 2022

In the configure_rng(), during the Init sequence, health_test_config and health_test_magic are set by the LL_RNG_SetHealthConfig, once values are defined in the DTS rng node with

			health-test-magic = <0x17590abc>;
			health-test-config = <0xaa74>;

However missing before and after LL_RNG_SetHealthConfig :
LL_RNG_EnableCondReset(rng);
and
LL_RNG_DisableCondReset(rng);

@taltenbach
Copy link
Contributor Author

Precisely! I expect to be able to lay hands next week on a B-U85I-IOT01 board that includes an RNG peripheral with RNG_HTCR register and CONFIGx fields. Therefore, if that's alright with you I can fix the issue next week. Otherwise, if you prefer, I should be able to do the PR earlier but in that case it would be preferable that someone ensures that the entropy driver is still working properly on real hardware

@FRASTM
Copy link
Collaborator

FRASTM commented May 25, 2022

That's the purpose of the #45994
As a first step the result is correct.
There is still another change to apply to follow the sequence of the refMan:
"CONDRST: Conditioning soft reset : This bit must be set to 1 in the same access that set any configuration bits [29:4]. In other
words, when CONDRST bit is set to 1 correct configuration in bits [29:4] must also be written.
"
That means to write the configuration A into the RNG_CR bits in the configure_rng() function. (There is no LL function for that )

@FRASTM
Copy link
Collaborator

FRASTM commented May 25, 2022

In order to apply the RNG configuration for the NIST certification, the conditions are about the rng_clock source, the RNG_CR register to write and loop , then immediately writing the RNG_HTCR magic nb then immediately writing the RNG_HTCR config nb.
Because the RNG NIST config can depend on the mcu, a suggestion would be to introduce a new property in the RNG DTS, named nist-config
Then for stm32 mcu with this nist-config the sequence is applied in the driver configure_rng : based on the stm32 hal driver function HAL_RNGEx_SetConfig()

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants