Skip to content

Conversation

@duda-patryk
Copy link
Contributor

This PR introduces support for STM32F4 flash write protection (persistent across reboots) and readout protection (temporary or permanent).

The flash API was extended by functions responsible for checking and changing WP and RDP state. Enabling RDP permanently is guarded by FLASH_ALLOW_PERMANENT_READOUT_PROTECTION option. Disabling RDP (which triggers mass erase) is guarded by FLASH_ALLOW_DISABLING_READOUT_PROTECTION.

I guess that the most controversial part is API change, so any remarks are welcome!

Best regards,
Patryk

@zephyrbot zephyrbot added platform: STM32 ST Micro STM32 area: Flash area: Devicetree Binding PR modifies or adds a Device Tree binding labels Dec 12, 2022
@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch from 174df94 to 99a1a12 Compare December 12, 2022 16:20
@erwango erwango added the area: API Changes to public APIs label Dec 13, 2022
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some preliminary requests about API introduction.

@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch from 99a1a12 to 904cf55 Compare December 13, 2022 11:38
@duda-patryk
Copy link
Contributor Author

Make FLASH_WRITE_PROTECT_API and FLASH_READOUT_PROTECTION_API disabled by default. Add __unused to flash_stm32_option_bytes_lock.

@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch from 904cf55 to b90150e Compare December 13, 2022 11:51
@carlescufi carlescufi requested a review from erwango January 2, 2023 16:28
@erwango
Copy link
Member

erwango commented Jan 3, 2023

Since this PR introduces changes to Flash API (which I guess is a stable API), you need first to get the API change approved. This would be faster to do so by splitting API changes in a dedicated PR.

@duda-patryk
Copy link
Contributor Author

PR for flash API changes #53441

@duda-patryk
Copy link
Contributor Author

Modified implementation of WP and RDP to work with "extended operations" API (PR #53441).

@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch from 145d3e1 to 26a711b Compare February 2, 2023 15:11
@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch from 26a711b to 3bf0dfb Compare February 21, 2023 16:31
Copy link
Contributor

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty.
Some minor comments SMT32 maintainer may disagree with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note, no change required: the header may provide flash_stm32_ex_op_sector_wp_get() flash_stm32_ex_op_sector_wp_set() inline functions in this header that will be passing reduced set of parameters to flash_ex_op, users may appreciate.
This would probably work for all the others FLASH_STM32_EX commands provided.

@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch 3 times, most recently from bc5d7fa to 9bb767b Compare March 23, 2023 11:54
@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch from 9bb767b to 58ebef1 Compare March 23, 2023 13:57
@duda-patryk
Copy link
Contributor Author

Changed printf to TC_PRINT in tests. Increased main stack size.

de-nordic
de-nordic previously approved these changes Mar 23, 2023
Copy link
Contributor

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK. Thanks!

@duda-patryk
Copy link
Contributor Author

drivers.flash.common.default doesn't build on beagleconnect_freedom due to some unrelated issues. Previously, all tests were passing.

@de-nordic
Copy link
Contributor

drivers.flash.common.default doesn't build on beagleconnect_freedom due to some unrelated issues. Previously, all tests were passing.

Try rebasing, maybe main has been fixed.

erwango
erwango previously approved these changes Mar 24, 2023
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thanks

@duda-patryk
Copy link
Contributor Author

Created #56181 for drivers.flash.common.default unit test build failure.

This patch adds sector write protection support for STM32F4 devices
family. These devices can protect flash content with sector precision.

Write protection functionality was exposed as vendor extended operation.
To change write protection state, caller should provide mask of enabled
and disabled sectors.

Function responsible for locking/unlocking option bytes was implemented
for all STM32 devices supported by this driver.

Signed-off-by: Patryk Duda <[email protected]>
This patch adds flash readout protection support for STM32F4 devices
family. These devices can enable protection on entire flash content.

Readout protection functionality was exposed as vendor extended
operation. To change readout protection state, caller should provide a
structure which describes desired RDP state.

Enabling readout protection permanently or disabling readout protection
(changing from level 1 to level 0) is guarded by
CONFIG_FLASH_STM32_READOUT_PROTECTION_PERMANENT_ALLOW and
CONFIG_FLASH_STM32_READOUT_PROTECTION_DISABLE_ALLOW respectively.

Signed-off-by: Patryk Duda <[email protected]>
This patch makes possible to choose custom byte which should be used
to enable non-permanent readout protection (RDP1). Actually, any byte
except 0xAA and 0xCC (which are used by RDP0 and RDP2 respectively)
can be used to enable RDP1 but in multi-image environment, some other
image could check if RDP1 is enabled by comparing it to some hardcoded
value.

If property is not defined, 0x55 will be used to enable RDP1. The
default value comes from STM32 HAL.

Signed-off-by: Patryk Duda <[email protected]>
Introduce flash extended operations that can be used to disable access
to option and control registers until reset. Disabling access to these
registers improves system security, because flash content (or protection
settings) can't be changed even when exploit was found.

On STM32 devices, registers can be locked until reset by writing wrong
key during unlock procedure. It triggers a bus fault, so during the
procedure we need to ignore faults and clear bus fault pending bit.

Please note that option register disabling was implemented for devices
that have OPTCR register (F2, F4, F7 and H7). Implementation on other
devices requires more testing, since documentation is not precise
enough. Disabling control register was implemented for devices that
have CR register.

Signed-off-by: Patryk Duda <[email protected]>
After 'flash_ex_op' syscall was added we are able to expose vendor
specific features to the user.

This patch moves existing tests to 'tests/drivers/common'. New tests for
vendor specific operations should go to vendor directory under
'tests/drivers/flash'.

Signed-off-by: Patryk Duda <[email protected]>
Introduced tests covers following features:
- Write protection - enabling, disabling, confirming that it works.
- Readout protection - checking current status.

These features are implemented on STM32F4 boards, so we only allow these
boards.

Signed-off-by: Patryk Duda <[email protected]>
@duda-patryk duda-patryk dismissed stale reviews from erwango and de-nordic via 42cdf72 March 28, 2023 08:50
@duda-patryk duda-patryk force-pushed the pdk_stm32_flash_wp_rdp branch from 58ebef1 to 42cdf72 Compare March 28, 2023 08:50
@duda-patryk
Copy link
Contributor Author

Patches were rebased and the tests are passing now. @de-nordic, @erwango could you approve it once again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: API Changes to public APIs area: Devicetree Binding PR modifies or adds a Device Tree binding area: Flash platform: STM32 ST Micro STM32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants