Watchpoint support

[fabalthazar]

Hi Uwe,

Here is a handful of commits that implement watchpoints, both with the usual 'hit' bit method when available, instruction decoding otherwise.
For that, I also revisited the tigger enumeration/discovery to strictly follow the specification and hopefully support forthcoming chips with a variety of possible unavailable triggers or unimplemented registers (like tinfo).

I still get the 32-bit word reading problem time to time. When it occurs, the instruction decoding algorithm gets of course lost. I didn't touch the memory management parts.
This makes me think about what @kdv-temp reported to me about target_mem_read() misbehaviour in his RTT context. Needs more investigation.

Tell me if that is of interest or if it needs rework.

Best,
Fabrice

[UweBonnes]

Thanks, for your input. I will try to have a look this weekend

[fabalthazar]

These last tiny commits should make us happy!
It fixes misaligned memory reading, particularly useful when reading a 32-bit word.
And watchpoint support with instruction decoding works now on addresses not multiple of 4, the case when using RVC Compressed instruction set.

Do you still have an ESP32-C3 at hand? I'd love to hear a feedback about the break/watchpoint experience on it, especially whether the 'hit' bit is implemented, to test the associated algorithm.

I usually start hosted whis DEBUG_INFO and DEBUG_TARGET verbosity:

$ blackmagic -j -v5

I am interested in seeing the triggers enumeration on ESP32-C3. On GD32VF103, I get this output:

Trigger #0, info = 0x003c
Trigger #1, info = 0x003c
Trigger #2, info = 0x003c
Trigger #3, info = 0x003c
Found 4 triggers

The info gives available trigger types.
Also when a watchpoint is hit, I look the output for this:

hit bit unimplemented
inst = 0xc43ec43e
offset = 8
base_reg = 2

Which means the algorithm has switched to instruction decoding.
And in GDB, the watchpoint is nicely signaled as this:

(gdb) 
Continuing.

Hardware watchpoint 2: b

Old value = 50
New value = 42
main () at src/main.c:95
95	            b = 42U;

In case the watchpoint is not correctly found, the generic SIGTRAP signal is sent forever on continue:

(gdb) 
Continuing.

Program received signal SIGTRAP, Trace/breakpoint trap.
0x08000466 in main () at src/main.c:87
87	        b++;
(gdb) 
Continuing.

Program received signal SIGTRAP, Trace/breakpoint trap.
0x08000466 in main () at src/main.c:87
87	        b++;

Have a nice weekend.

[UweBonnes]

Good catch. Sorry for the pain to find it...

[UweBonnes]

Nitpicking: Possible white space change...

Usage error

[UweBonnes]

Please check for whitespace here too or run "git rebase --whitespace=fix origin/ruabmbua"

[fabalthazar]

Here, CSR_MCONTROL_ACTION_DEBUG is shifted with spaces (not tabs) in order to make it aligned with the upper CSR_TDATA1_TYPE(2) in every editor, whatever the displayed width of the tab indentation by users. This is not an indentation actually.

[UweBonnes]

git log --check flags this red:
"src/arch/riscv/rvdbg013.c:1596: indent with spaces.

•                       CSR_MCONTROL_ACTION_DEBUG | CSR_MCONTROL_ENABLE_MASK;
  

"
Use as much tabs as possible and only fill the remainder with space.

[fabalthazar]

I didn't know of --check option, nice. But reading its help, it says a whitespace error is "a space character that is immediately followed by a tab character inside the initial indent of the line". Here, it is the opposite: tabs first for indentation, followed by a few space chars to align with upper line, as shown in this screenshot (tabs and spaces displayed with arrows and dots):

Also, git log --check doesn't report any error on my side:

$ git blame -L1502,+1 -- src/arch/riscv/rvdbg013.c
ffc9982b (Fabrice Prost-Boucle 2022-01-18 22:33:01 +0100 1502) ...
$ git log -n 1 --check ffc9982b
commit ffc9982b577121e91078c74b4f8cf3b38dee7c0e
Author: Fabrice Prost-Boucle <fabalthazar@falbalab.fr>
Date:   Tue Jan 18 22:33:01 2022 +0100

    Set triggers of type 2 explicitly and which have type 2 available.
    This supports triggers that do not have 'type' preset to 2.

git version 2.34.1

False positive?

[UweBonnes]

If you open the file in doubt in the review aspect on this page, you see solid darker green blocks for spaces where tab could be used. Just for your info, I hope not to annoy you

[fabalthazar]

Apologies, my answer referred to line 1502 instead of 1596.
But I still see none "indent with spaces" around 1596.
Can you see this screenshot?

The only spaces you see are before CSR_MCONTROL_ACTION_DEBUG, to align with above CSR_TDATA1_TYPE(2). It will stay aligned whatever editor is used and how width it displays tabs.
These spaces follow one single tab which is a real indentation level.

In other words, there are a lot of spaces here but seem legitimate as long as the indentation level is respected.

I feel I start to bother you about a point I cannot get. grrr :)

[fabalthazar]

Good catch. Sorry for the pain to find it...

The pain is largely rewarded by the working result. And I take it as an opportunity to read and learn a lot about RISC-V!

Please explain me more what you mean with whitespace changes and I'll rebase the branch.

[UweBonnes]

I tried to run on a ESP32-C3, but the jtag_scan() detects the chain wrong. Did you have success?

[fabalthazar]

I tried to run on a ESP32-C3, but the jtag_scan() detects the chain wrong. Did you have success?

I have no ESP32-C3, unfortunately.
Is your probe updated from riscv branch? Hosted + "master" firmware failed to scan on GD32 here. This is what I reported as "Issue 1" in upstream PR blackmagic-debug#924.
You told me that riscv jtag needs some idle cycles more, IIRC.
Could it be the same problem for you?

[UweBonnes]

Regarding ESP32-C3: The problem happens one step earlier, in the chain detection. I do not yet fully understand what is wrong and what is a good solution. Circumventing the problem I probably hit the Issue 1 you describe. I will recheck, when spare time allows. DX-Mon as one of the new project leads seem to not like the chain detection scheme and tend to revert, see blackmagic-debug#978

Regarding the idle cycles: The value announced by the device is used as a starting point, but eventually extended if the first read fails.

[UweBonnes]

Ah, the other ESP-C3 board is revision 3. As the ESP protocol is not (yet?) supported in BMP hosted, I needed to (permantently!) fuse the fuse to allow selection between internal JTAG and external:
./espefuse.py` --port /dev/ttyACM0 burn_efuse JTAG_SEL_ENABLE
External JTAG seems to be the default, I did not need to strap GPIO10.
With your branch and https://github.com/UweBonnes/blackmagic/tree/ftdi I get:```

./blackmagic -t
BMP hosted v1.7.1-415-g697aff0c
for ST-Link V2/3, CMSIS_DAP, JLINK and LIBFTDI/MPSSE
Using 0403:6010 0002 IKDA
FTDIJTAG
SWD with cable not possible, trying JTAG
Running in Test Mode
Target voltage: (null) Volt
Speed set to 3.7500 MHz for JTAG
1: IDCODE: 0x00005c25, IR len 5 RVDBG013
RISC-V DTM id 0x5c25 detected: RVDBG013
Scanning RISC-V target ...
dtmcs = 0x00001071
debug version = 0.13
abits = 7
idle = leave run/test immediately
RISC-V DMI op interrupted ret = 0x8101004000000000, idle now 2
RISC-V DMI op interrupted ret = 0x8101004000000000, idle now 3
RISC-V DMI op interrupted ret = 0x8101004000000000, idle now 4
Supports set/clear-resethaltreq
datacount = 2
RISC-V: Program buffer with available size 15 supported.
hartsellen = 20
num_harts = 1
MISA 40101104, XLEN 32 bits
Machine 612, 80000001, 1, 0
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #0, info = 0x0004
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #1, info = 0x0004
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #2, info = 0x0004
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #3, info = 0x0004
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #4, info = 0x0004
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #5, info = 0x0004
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #6, info = 0x0004
RISC-V abstract command 0x002207a4,err 3
RISC-V register 0x7a4
does not existTrigger #7, info = 0x0004
Found 8 triggers
*** 1 ESP32-C3 Generic RVDBG 0.13
RAM Start: 0x3c800000 length = 0x50000
RAM Start: 0x4037c000 length = 0x64000
RAM Start: 0x50000000 length = 0x2000```
I need to check what is wrong with the other board...

[fabalthazar]

I really like your test! The perfect example of a chip with tinfo unimplemented.
My code falling back to read the only available trigger type through tdata1 seems to work like a charm (0x0004 => type 2 = usual addr/data match).

If you enable TARGET debug messages (blackmagic -t -j -v5), I hope you see some "Trigger #x, tinfo unimplemented". Between a lot of DMI_Write/Read lines...

We also see that a new line at the end of "RISC-V register 0x7a4 does not exist" is welcome.

[UweBonnes]

Sorry, I was not exact enough. I meant this place:

The darker green area are the areas of (little ;-) concern.

[UweBonnes]

with -t -j -v 5. "Trigger #x, tinfo unimplemented" is seen as you expect.

[fabalthazar]

Sorry, I was not exact enough. I meant this place:

The darker green area are the areas of (little ;-) concern.

I see. The fourth dark green block is a mess by Github. It should be longer and start one tab earlier on the line. Please use your mouse or inspect the file locally to confirm.
The second "if" has correct one indent level more. Above, a few spaces for alignement in ( || || ) conditions.

[UweBonnes]

I will try to compile some platformio ESP32-C3 example, if there is one and will set breakpoint, debug and report asap.