12:02 <simon_> Hi. I might have a bug report. I want to flash 8 bytes from a hex file onto stm32f301 using an ST-link. The command looks like this: openocd -f interface/stlink.cfg -c "transport select hla_swd" -f target/stm32f3x.cfg -c "reset_config none separate" -c init -c halt -c "reset init" -c "flash write_image data.hex" -c exit. Also i'm using Ionescu's OpenOCD builds. This command works with OpenOCD version 0.11.0-4 (2022.03.25) but fails with 0.11.0-5 (2022.09.01)

12:03 <simon_> The error message is: Error: corrupted fifo read pointer 0x20000044

12:03 <simon_> Is this a known issue?

12:04 <karlp> sounds like you want to go and check wht ionescu what is the actually difference between those builds :)

12:04 <simon_> I also compiled OpenOCD from sources (0.12-rc) 2 days ago and the error was still there.

12:05 <simon_> I thought Ionescu does vanilla builds? Anyways my build was from original OOCD source. :) But i cat try it again just to be sure.

12:06 <karlp> it just sounds like a very small window, would be easys to see hwat the difference was?

12:07 <simon_> Additional note, the flashing works corretly with payloads of 20 bytes.

12:08 <karlp> right, ok, might just be that the flashing stub loader is using unitialized memory for the remainder then?

12:08 <karlp> and the -4 build just by hcance had different uninit memory.

12:09 <simon_> I tried to check the differences, but could not figure out much. I have no idea of internal workings of OpenOCD.

12:09 <karlp> you'd have to dig into the code then if that's the case, I'm not sure flashing 8 bytes is a very common use case to have run into it.

12:10 <simon_> I have an even older build 0.11.0-1 which also works correct.

12:10 <karlp> I wouldn't be surprised if that's just gcc builds of unitialized memory having changed,

12:10 <karlp> I would suspect a bug in the flash loader stub for handling small things like this.

12:11 <simon_> Flash loader stub is the code that is executed on the target side?

12:11 <karlp> yeah.

12:12 <karlp> either there or the code that loads the buffer into be flashed. should be a fairly small target area of code though if you have working 20bytes and failing 8 bytes.

12:14 <simon_> I tried to debug my build of the OpenOCD a little. And it looks like target_run_flash_async_algorithm is called with buffer_start = 0x20000044 and buffer_size = 8. This function then calculates fifo_start_addr and fifo_end_addr. And the error is raised because those 2 values are equal (rp = fifo_start_addr = fifo_end_addr)

14:42 <simon_> So it looks like target_run_flash_async_algorithm requires (and checks) a flash loader buffer of size of data size plus 2 words (8 bytes) for read and write pointers. stm32f1x.c was changed in that it tries to allocate only the bare minimum of RAM. I.e. if data size is 8 bytes then it allocates only 8 bytes for the flash loader buffer.

14:43 <simon_> In the stm32f1x.c i changed the line buffer_size = MIN(hwords_count * 2, MAX(buffer_size, 256)); to buffer_size = MIN(hwords_count * 2 + 8, MAX(buffer_size, 256)); and it seems to work now.

14:45 <simon_> I believe that flashing any payload of 8 or less bytes on stm32f1 targets will fail with the current code.

14:47 <simon_> Commit e3f4ea0b574d952672d57792895a91065aa7a569 has introduced this change. The old code tried to allocate the buffer as big as possible. Then the buffer was always big enough to hold data and read/write pointers.

14:48 <simon_> Would be cool if some knowing person looked at it and maybe confirmed.

14:50 <karlp> so, fwiw, you could get a zero code workaround for your specific problem by setting the workarea to 0 in your tcl config file

14:50 <karlp> it should fall back to word by word programming, but you're only programming 8 bytes anyway, so no big deal perf wise.

14:53 <karlp> I don't see how anything less than 20 works, it writes 5 words into the buffer to transfer?

14:53 <karlp> oh, but it takes words * 2 for some reason,

14:54 <karlp> yeah, looks bogus to me.

14:54 <karlp> just make a comment on gerrit on https://review.openocd.org/c/openocd/+/6708 and they'll see it.

14:54 <karlp> thomas is pretty responsive,

14:59 <simon_> It takes 2 * halfwords, the resulting size does match the size of the data. I get 10 halfwords for 20 byte payload and 4 halfword for 8 bytes.

15:02 <simon_> What i completely not understand is the size of the fifo buffer on target. Does it not have to be the size of the data? But then why does the allocation takes the size of the data as input parameter? Why not just always allocate like 256 bytes? Or even 16 bytes for stm since flashing works at most doubleword-wise.

15:07 <PaulFertser> simon_: I can't answer your question specifically but in general it makes sense to allocate plenty of RAM for the ring-buffer from which the target algorithm is taking the data so that it wouldn't be drained if the computer is busy with something else while the flashing process is ongoing.

15:08 <PaulFertser> simon_: the process is normally fully async lock-free.

15:08 <karlp> yeah, the change linked tries to go for the min of (flashing_data_size, 256) with the expectatoin that it would be 256 ....

15:09 <karlp> I don't think it was considered that it would be _less_ than 256, and that it had an actual hard lower limit of it's own.

15:13 <PaulFertser> simon_: why do you think reg_params are included in that buffer? It looks like they are allocated separately.

15:15 <PaulFertser> simon_: guess you're right, it needs 8 extra bytes first.

15:16 <simon_> It's not the reg_params. The target_run_flash_async_algorithm function at lines 1017-1020 sets wp_addr = buffer_start, rp_addr = buffer_start + 4, fifo_start_addr = buffer_start + 8.

15:16 <simon_> For me it looks like the function expects at least 8 bytes more in addition to the fifo buffer itself.

15:16 <PaulFertser> simon_: so if you can easily reproduce it, can you try a trivial fix: s/hwords_count * 2/hwords_count * 2 + 8/ ?

15:17 <simon_> This fix is exactly what i did and it did work for me.

15:17 <simon_> buffer_size = MIN(hwords_count * 2, MAX(buffer_size, 256));

15:17 <simon_> in stm32f1x.c

15:18 <PaulFertser> simon_: so feel free to send a patch to Gerrit. Since it's a fix it'll be fixed before final release.

15:18 <simon_> Just to explain the use-case. :) We flash multiple hex files with a single openOCD command. Some of these filea are just configuration values, hence the small size.

15:19 <simon_> @PaulFertser, do i need to register there?

15:21 <PaulFertser> simon_: not really, one can use existing bitbucket/github/openid account.

15:22 <PaulFertser> Or google account.

15:26 <simon_> PaulFertser: haha, if only i had one

15:26 <PaulFertser> simon_: how about a launchpad id?

15:26 <PaulFertser> simon_: how do you contribute to github projects?

15:27 <simon_> PaulFertser: sorry, i try to avoid all kinds of accounts.

15:27 <PaulFertser> simon_: if you really have a serious reason not to register on one of those, and you promise to answer mails if there're review comments, I can push the patch to Gerrit on your behalf.

15:28 <simon_> PaulFertser: That would be dope. Email i can share.

15:29 <PaulFertser> simon_: so please do git format-patch HEAD^, upload it to some pastebin, share the link with me, I'll push your patch.

15:29 <simon_> Also i don't really need my name mentioned anywhere

16:52 <PaulFertser> Tomas already approved the change, cool!

17:00 <simon_> That went quick. Thank you guys for your assistance.

17:03 <PaulFertser> Thank you for the patch :)