I patched the code to fix this, and further found that: - The code also did not do an unlock sequence and did not set the FLASH_PG bit, so the write would also silently fail. - The code would allow protecting OTP sectors without requiring the otp enable command, as is required before doing regular writes to the main OTP memory. It would probably be better to require this, to minimize the chance of doing any unintentional OTP writes. - The STM32l4x code also supports OTP, but that OTP hardware does not have separate protection bytes (instead, every 64-bit block is automatically protected from further writes once any bit is written to 0). So this issue does not exist in that code.
--- ** [tickets:#393] STM32F2: Protecting OTP fails (code inverts lock value)** **Status:** new **Milestone:** 0.10.0 **Created:** Thu Apr 27, 2023 07:33 AM UTC by Matthijs Kooijman **Last Updated:** Thu Apr 27, 2023 07:33 AM UTC **Owner:** nobody **Attachments:** - [openocd.log.txt](https://sourceforge.net/p/openocd/tickets/393/attachment/openocd.log.txt) (317.0 kB; text/plain) The STM32F2x flash driver correctly reads protection bytes for the OTP memory, but fails to write them. Looking at the code, it seems the `stm32x_otp_protect` read protect function uses the correct meaning of the lock bytes (0x00 == locked, any other value == unlocked), while `stm32x_otp_protect` actually tries to write 0xff to enable protection, which is incorrect. See the code here: https://github.com/openocd-org/openocd/blob/91bd4313444c5a949ce49d88ab487608df7d6c37/src/flash/nor/stm32f2x.c#L510-L557 The documentation (PM0059: STM32F205/215, STM32F207/217 Flash programming manual) says: > The OTP area is divided into 16 OTP data blocks of 32 bytes and one lock OTP > block of 16 bytes. The OTP data and lock blocks cannot be erased. The lock block contains 16 bytes LOCKBi (0 ≤ i ≤ 15) to lock the corresponding OTP data block (blocks 0 to 15). Each OTP data block can be programmed until the value 0x00 is programmed in the corresponding OTP lock byte. The lock bytes must only contain 0x00 and 0xFF values, otherwise the OTP bytes might not be taken into account correctly. I've tested this on an F4, which uses the same driver (and has OTP that works the same way). Reading the lock bytes works correctly (this chip has one OTP sector locked already): ``` matthijs@dottie:~/docs/MKIT/3Devo/ChildbusBootloader$ openocd -f interface/stlink.cfg -f target/stm32f4x.cfg -c 'init; reset init; flash info 1' Open On-Chip Debugger 0.12.0 [Snip more output] #1 : stm32f2x at 0x1fff7800, size 0x00000200, buswidth 0, chipwidth 0 # 0: 0x00000000 (0x20 0kB) protected # 1: 0x00000020 (0x20 0kB) not protected # 2: 0x00000040 (0x20 0kB) not protected # 3: 0x00000060 (0x20 0kB) not protected [Snip more sectors] STM32F4xx (Low Power) - Rev: Z ``` However locking sector 1 fails: ``` matthijs@dottie:~/docs/MKIT/3Devo/ChildbusBootloader$ openocd -f interface/stlink.cfg -f target/stm32f4x.cfg -c 'init; reset init; flash protect 1 1 1 on' Open On-Chip Debugger 0.12.0 [Snip more output] Error: failed setting protection for blocks 1 to 1 ``` I've attached a complete log with `-d3` to show some more details. Looking at the [code for this](https://github.com/openocd-org/openocd/blob/91bd4313444c5a949ce49d88ab487608df7d6c37/src/flash/nor/stm32f2x.c#L530-L557): ``` for (i = first; first <= last; i++) { retval = target_read_u8(target, lock_base + i, &lock); if (retval != ERROR_OK) return retval; if (lock) continue; lock = 0xff; retval = target_write_u8(target, lock_base + i, lock); if (retval != ERROR_OK) return retval; } return ERROR_OK; } ``` You can see that the `if (lock)` is inverted (it is locked when it is 0, not non-zero), so it skips any unlocked blocks and will only try to lock blocks that are already locked. You would expect it to then just silently fail and not raise an error, but (thanks for gdb to help me here), it turns out the loop is actually an infinite loop, since it checks `first <= last` instead of `i <= last`, so that's another bug. Since `i` does increment, this means it reads all the lock bytes, then system memory and then tries to read past the end of system memory (0x1fff8000) and fails, breaking the loop and returning error. Looks like this code has been broken since when it was first introduced in [f21c12abec](https://github.com/openocd-org/openocd/commit/f21c12abec) and maybe was never tested (which is of course tricky with OTP...). I will see if I can provide a patch to fix this - I have some sacrificial boards that I can use for testing. --- Sent from sourceforge.net because openocd-devel@lists.sourceforge.net is subscribed to https://sourceforge.net/p/openocd/tickets/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/openocd/admin/tickets/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.
