STM32 clock security system for LSE

[maxgerhardt]

Description

• target: NUCLEO_L476RG
• tools: mbed-cli
• release: 5.13.3

Currently, the implementation of the 'Clock Security System' (for both LSE/LSI and HSE/HSI) for the STM32 MCUs is lacking in mbed-os. In fact the LSE/LSI CSS is even disabled in the MSI clock setup code and never re-enabled.

mbed-os/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L476xG/TARGET_NUCLEO_L476RG/system_clock.c

Lines 279 to 280 in daafb95

	/* Enable the CSS interrupt in case LSE signal is corrupted or not present */
	HAL_RCCEx_DisableLSECSS();

For hardening devices, the clock security system is important. However, the current mbed-os implementation makes it hard to implement CSS callbacks properly. As the reference manual for STM32L4x6 device says in chapter 6.2.11:

A Clock Security System on LSE can be activated by software writing the LSECSSON bit in
the Backup domain control register (RCC_BDCR). This bit can be disabled only by a
hardware reset or RTC software reset, or after a failure detection on LSE. LSECSSON must
be written after LSE and LSI are enabled (LSEON and LSION enabled) and ready (LSERDY
and LSIRDY set by hardware), and after the RTC clock has been selected by RTCSEL.
[..]
The software MUST then disable the LSECSSON bit, stop the defective 32 kHz oscillator
(disabling LSEON), and change the RTC clock source (no clock or LSI or HSE, with
RTCSEL), or take any required action to secure the application.

So even if I call HAL_RCCEx_EnableLSECSS() and implement the _WEAK callback HAL_RCCEx_LSECSS_Callback() in my app, I have to write my own RTC clock init routine because rtc_init() has hard-coded #ifdefs for whether the LSE is available on the system.

mbed-os/targets/TARGET_STM/rtc_api.c

Lines 61 to 86 in daafb95

	#if MBED_CONF_TARGET_LSE_AVAILABLE
	RCC_OscInitStruct.OscillatorType = RCC_OSCILLATORTYPE_LSE;
	RCC_OscInitStruct.PLL.PLLState = RCC_PLL_NONE;
	RCC_OscInitStruct.LSEState = RCC_LSE_ON;
	if (HAL_RCC_OscConfig(&RCC_OscInitStruct) != HAL_OK) {
	error("Cannot initialize RTC with LSE\n");
	}
	__HAL_RCC_RTC_CONFIG(RCC_RTCCLKSOURCE_LSE);
	PeriphClkInitStruct.PeriphClockSelection = RCC_PERIPHCLK_RTC;
	PeriphClkInitStruct.RTCClockSelection = RCC_RTCCLKSOURCE_LSE;
	if (HAL_RCCEx_PeriphCLKConfig(&PeriphClkInitStruct) != HAL_OK) {
	error("PeriphClkInitStruct RTC failed with LSE\n");
	}
	#else /* MBED_CONF_TARGET_LSE_AVAILABLE */
	#if TARGET_STM32WB
	RCC_OscInitStruct.OscillatorType = RCC_OSCILLATORTYPE_LSI1;
	#else
	RCC_OscInitStruct.OscillatorType = RCC_OSCILLATORTYPE_LSI;
	#endif
	RCC_OscInitStruct.PLL.PLLState = RCC_PLL_NONE;
	RCC_OscInitStruct.LSIState = RCC_LSI_ON;
	if (HAL_RCC_OscConfig(&RCC_OscInitStruct) != HAL_OK) {
	error("Cannot initialize RTC with LSI\n");
	}

mbed-os is hardcoded to assume either LSE or LSI will be there forever and that their frequencies never change (you made sure to hardcode RTC_CLOCK as well, which will mess up all timebases if this changes). It would be nice if mbed-os integrates the CSS system and reinitializes peripherals (here: RTC and LP ticker, which you hardcoded as well) on clock failure to make the firmwares more resilient by default.

Similar for high-speed clock sources. Though SetSysClock() is called on bootup and on every wakeup from deepsleep, but what if during the "run" section of a firmware, we suddenly lose the external high-speed clock source (HSE)?

Issue request type

[] Question
[X] Enhancement
[ ] Bug

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-1801

[jeromecoutant]

Hi
I agree we should enable LSE CSS should be enabled.
But I think, if we propose to change automatically RTC and LPTICKER configuration with LSI, this will increase code size ?

Note: #7787 correction to update

[maxgerhardt]

But I think, if we propose to change automatically RTC and LPTICKER configuration with LSI, this will increase code size ?

Definitely, since code excluded by the compile-time macro MBED_CONF_TARGET_LSE_AVAILABLE is now also enabled. I think functions like rtc_init() should get a parameter whether initialization is to be done with LSE or LSI, then all the #ifdef ..LSE_AVAILABLE become if(lse_available) via a parameter. Same for LPTICKER and the hardcoded frequency values noted above. However the code size increase shouldn't really be that much, since only a new branch exists with some extra code / parameter changes.

[0xc0170]

Note: #7787 correction to update

@jeromecoutant What is left for this one, all resolved?

[jeromecoutant]

What is left for this one, all resolved?

For this one, I agree we need to enable clock security system.
What is not decided yet, is the behavior the default mbed code should have in case of clock issue...

@maxgerhardt maybe you could propose in a PR what you are expecting?

@LMESTM

[ciarmcom]

Thank you for raising this issue. Please note we have updated our policies and
now only defects should be raised directly in GitHub. Going forward questions and
enhancements will be considered in our forums, https://forums.mbed.com/ . If this
issue is still relevant please re-raise it there.
This GitHub issue will now be closed.