Moving set & getCSR from bThread header to implementation to prevent compiler errors #114
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
For my master thesis, I integrated the register-based configuration into my Coyote FPGA design. I then proceeded to try to write/read register values using the
getCSR
andsetCSR
methods offered bybThread
. However, doing so lead to a segmentation fault.After some hours of debugging, I found the following problem:
The
getCSR
andsetCSR
methods rely on a memory-mapped region. This mapping is performed inside thebThread::mmapFpga()
function where thectrl_reg
variable is set to point to the beginning of this memory region. Writing/Reading registers is then executed by reading from/writing to the address given byctrl_reg
. This writing/reading is implement in the header file of thebThread
class. However, with certain compiler settings/versions/kinds this can lead to problems. I compiled Coyote with gcc version 9.4.0 and clang version 10.0.0 that are available in the HACC cluster.Probably due to a compiler bug, the address of the
ctrl_reg
was NOT correct when this field is accessed via a method implemented inside the thebThread
header (e.g.getCSR
andsetCSR
) . To be precise, the address ofctrl_reg
was shifted by 8 bytes (not the pointer ctrl_reg holds, but the address of the field itself!). The memory this field was then referencing was uninitialized in my case. Therefore, de-referencing the pointerctrl_reg
was supposed to hold, lead to a segmentation fault (due toctrl_reg
being NULL).When using gcc, calling set/getCSR was still working fine inside the
bThread
header but broke as soon as the constructor finished (for example, when calling the same methods inside thecThread
constructor or viacThread
directly). Using clang, set/getCSR already broke inside thebThread
constructor right after thebThread::mmapFpga()
call finished.This issue could be mitigated by:
ctrl_reg
ctrl_reg
field by 8 before performing the de-referencing insidegetCSR
andsetCSR
bThread.hpp
header to its implementationbThread.cpp
As it is best practice to have implementations inside the implementation and not the header, this PR provides the changes for the third mitigation to prevent similar problems in the future. With this mitigation, all addresses were correct again and everything worked as expected.
Type of change
Tests & Results
I tested these changes on my Coyote project were they lead to registers now working properly!
Checklist
I did not run any tests besides the one mentioned above. Unfortunately, I would not find instructions on how to run further tests (maybe I am blind). If you could give me guidance on which additional tests to run I am happy to run them as well!