lowRISC
[dv/dpi] Fix TCP server hang when ring buffer fills up
These buffers are expected to be manipulated by multiple threads in these functions, so we need to mark them as volatile. GCC can and will optimize away things we don't want it to otherwise.
Closes #24616
I don't think using volatile here properly addresses the underlying issue.
The rptr, wptr, and buf need to have consistent values, for all threads. Your use of volatile, combined with x86's strong memory model, happens to make it work, but it's not guaranteed. To understand that, consider the scenario where thread A puts data in (mutates buf and wptr) and thread B sees the write to wptr but not yet to buf (due to allowed memory reorderings between threads).
Probably the easiest way to fix this is to use a lock to synchronize accesses to the overall struct tcp_buf. Using atomics and fences might provide higher performance but unless this is particularly performance-sensitive I'd avoid the extra complexity.
I don't think using
volatilehere properly addresses the underlying issue.The
rptr,wptr, andbufneed to have consistent values, for all threads. Your use ofvolatile, combined with x86's strong memory model, happens to make it work, but it's not guaranteed. To understand that, consider the scenario where thread A puts data in (mutatesbufandwptr) and thread B sees the write towptrbut not yet tobuf(due to allowed memory reorderings between threads).
You're right, this implementation as it stands is broken on any architecture with weak memory models. Using a blanket lock would probably not here since there's quite a bit of busy-looping going on. re: performance, the reason I found this to begin with was because the buffer was filled before it could be emptied, which stalled JTAG emulation completely at some point.
Some combination of mutexes and a condition flag would probably work, but I'm currently having issues compiling on a home machine due to (I think) missing vivado tags on some tests, namely hw/bitstream/universal