This commit creates file structure for register cache related
functions.
Specifically:
* `riscv_reg.h` -- general interface to registers. Safe to use after
register cache initialization is successful.
* `riscv_reg_impl.h` -- helper functions to use while implementing
register cache initialization.
* `riscv_reg.c` -- definitions of functions from `riscv_reg.h` and
`riscv_reg_impl.h`.
* `riscv-011_reg.h` -- register cache interface specific to 0.11
targets.
* `riscv-013_reg.h` -- register cache interface specific to 0.13+
targets.
* `riscv-011/0.13.h` -- version-specific methods used to access
registers. Will be extended as needed once other functionality (not
related to register access) is separated (e.g. DM/DTM specific stuff).
Change-Id: I7918f78d0d79b97188c5703efd0296660e529f2a
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
Without the selection the TAP can be left in bypass.
Change-Id: I79c6bf74802dc9c9475947d1787a3d0b797f3952
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This allows to eliminate up to two DMI NOPs.
Change-Id: I09a18bd896fce2392d1b65d4efb38b53e334a358
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
Code cleanup: "slot_t" is unused in riscv013 - remove it.
Change-Id: I9d5a0cf8446a180b1d13a9ce2c86d904b946cf28
Signed-off-by: Jan Matyas <jan.matyas@codasip.com>
This would elliminate the need for an extra nop in-between the two reads
in case of a 64-bit register.
Change-Id: I2cddc14f7f78181bbda5f931c4e2289cfb7a6674
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
There was an error introduced by
8319eee9e1.
According to RISC-V Debug Spec 1.0.0-rc1 [3.14.2. Debug Module Contro]:
> 0 (inactive): The module’s state, including authentication mechanism,
takes its reset values (the dmactive bit is the only bit which can be
written to something other than its reset value).
`dmactive` was written together with `hartsel` and `hasel` in
8319eee9e1.
Change-Id: I11fba35cb87f8261c0a4a45e28b2813a5a086078
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This commit is related to testing how OpenOCD responds to `dmi.busy`.
Consider testing on Spike (e.g. `riscv-tests/debug` testsuite). Spike
returns `dmi.busy` if there were less then a given number of RTI cycles
(`required_rti_cycles`) between DR_UPDATE and DR_CAPTURE:
https://github.com/riscv-software-src/riscv-isa-sim/blob/master/riscv/jtag_dtm.cc#L145https://github.com/riscv-software-src/riscv-isa-sim/blob/master/riscv/jtag_dtm.cc#L202
`required_rti_cycles` gets it's value from `--dmi-rti` CLI argument and
is constant throughout the run.
OpenOCD learns this required number of RTI cycles by starting with zero
and increasing it if `dmi.busy` is encountered. So the required number
of RTI cycles is learned during the first DMI access in the `examine()`.
To induce `dmi.busy` on demand `riscv reset_delays <x>` command is
provided. This command initializes `riscv_info::reset_delays_wait`
counter to the provided `<x>` value. The counter is decreased before a
DMI access and when it reaches zero the learned value of RTI cycles
required is reset, so the DMI access results in `dmi.busy`.
Now consider running a batch of accesses. Before the change all the
accesses in the batch had the same number of RIT cycles in between them.
So either:
* Number of accesses in the batch was greater then the value of
`riscv_info::reset_delays_wait` counter and there was no `dmi.busy`
throughout the batch.
* Number of accesses in the batch was less or equal then the value of
`riscv_info::reset_delays_wait` counter and the first access of the
batch resulted in `dmi.busy`.
Therefore it was impossible to encounter `dmi.busy` on any scan of the
batch except the first one.
Change-Id: Ib0714ecaf7d2e11878140d16d9aa6152ff20f1e9
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This allows to merge the implementation in `batch.c` with the one in
`riscv-013.c`.
Change-Id: Ic3821a9ce2d75a7c6e618074679595ddefb14cfc
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
According to the RISC-V Debug Spec (1.0.0-rc1)[3.7 Abstract Commands]:
> While an abstract command is executing (busy in abstractcs is high), a
debugger must not change hartsel, and must not write 1 to haltreq,
resumereq, ackhavereset, setresethaltreq, or clrresethaltreq.
The patch ensures the rule is followed.
Change-Id: Id7d363d9fdeb365181b7058e0ceb0be0df39654f
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This allows to examine each DM ones (e.g. enumerating harts assigned to
the DM). Additionaly, it is guaranteed that the DM is reset before the
examination.
Change-Id: I2333d06ff1152bf51c647d59baa55cb402054cb9
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
According to the RISC-V Debug Spec (1.0.0-rc1)[3.7 Abstract Commands]:
> While an abstract command is executing (busy in abstractcs is high), a
debugger must not change hartsel, and must not write 1 to haltreq,
resumereq, ackhavereset, setresethaltreq, or clrresethaltreq.
Tracking `abstractcs.busy` allows to enforce this rule.
Change-Id: If5975b48cf9fd379033268145c79103c36fb8134
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
Fixes:
- Data types of address & data parameters in riscv_batch_add_*()
and riscv*_fill_dm*() changed to uint64_t and uint32_t.
- Corrected the comparison in riscv_batch_full().
- Corrected assertions in riscv_batch_get_dmi_read_op()
and riscv_batch_get_dmi_read_data().
Cleanup:
- Simplified calloc() fail handling in riscv_batch_alloc().
- Added explicit NULL assignments in riscv_batch_alloc()
for clarity and readability. Don't rely on calloc().
- Removed suffix `_u64` from riscv_*_fill_dm*() since it
does not have any meaning.
- Renamed *dmi_write_u64_bits() to *get_dmi_scan_length()
which better describes its purpose.
Change-Id: Id70e5968528d64b2ee5476f1c00e08459a1e291d
Signed-off-by: Jan Matyas <jan.matyas@codasip.com>
Cleanup, non-functional changes:
- Replaced one call of low-level function dm_op()
by high-level dm_read().
- Made sure that truncation of values passed to dm_*
is explicit. (Added explicit casts.)
Change-Id: I1d1b2f29a822b6841373f3313de2b1e96f514116
Signed-off-by: Jan Matyas <jan.matyas@codasip.com>
This reverts commits 2e920a212f and
8dbb1250f5.
The reason is, after `openocd_is_shutdown_pending()` becomes true,
arbitrary command may be executed:
* In `target_destroy()` and the corresponding
`target->type->deinit_target()`.
* In user-specifyed `pre_shutdown_commands` list.
Change-Id: Icd00d1d954cd45e255880a6f76c3a74c098d6a17
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
The reason for the change is a conflict: `dcsr[5]` is `dcsr.v` in
current spec, but it is `dcsr.debugint` in 0.11. This causes `priv`
register to be read incorrectly.
Change-Id: If2d8fdcd8536afa4c7149c453101b00ce0df1ce0
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
There was an error in `dm_read`/`dm_write`: DMI address was checked
against DM registers disregarding DM base address.
To solve the issue `dmi_address()` function was introduced.
Change-Id: Ia3be619417b5f5b53db5dfe302db05170d6787c9
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
The motivalion for the change:
* `riscv_hart_count()` is used only once to print the value into the log
during exmination.
* The returned value is a bit confusing: it's not the total number of
targets on the TAP. It is the number of targets accessable through the
same DM. So the name of the function is misleading.
* This value is already reported on `-d3` level.
So the function seems redundant and can be safely removed.
Change-Id: Iac9021af59ba8dba2cfb6b9dd15eebc98fe42a08
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
In loops that typically take longer time to complete, check if there is
a pending shutdown request. If so, terminate the loop.
This allows to respond to a signal requesting a shutdown during some
loops which do not return control to main OpenOCD loop.
Change-Id: Iace0b58eddde1237832d0f9333a7c7b930565674
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
I don't think there are any real bugs here, but at least this gives us a
clean slate moving forward.
Change-Id: I29c6c398c28dfe580f9a2deb3bdbcfc491a2ceb6
Signed-off-by: Tim Newsome <tim@sifive.com>
The goal of this commit is to provide more robust error handling in
`write_memory_progbuf()`. This is achieved by rewriting it in a fashion
similar to `read_memory_progbuf()`.
The motivation is: some instability in `load_image` was encountered. No
stable reproduction could be obtained, so the root cause was not
determined. Therefore, it was decided to clean-up the code, that may be
implicated in such failures.
Examples of unhanded errors in the code prior to this commit:
* Most of `dmi_write()` return values are discarded.
* If `dm_read()` on `abstractcs` failed (line 4546), `abstractauto` was
not cleared.
Furthermore, the structure of the code was quite complicated, which made
it hard to analyze and reason whether or not all possible failures are
handled properly.
Change-Id: I8a100b686e594855fbf34acf5ccf0e1550f18869
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
OpenOCD style guide(`doc/manual/style.txt`) prohibits use of VLA:
> - use malloc() to create dynamic arrays. Do @b not use @c alloca
> or variable length arrays on the stack. non-MMU hosts(uClinux) and
> pthreads require modest and predictable stack usage.
Change-Id: I12e4a5087fd056d69866137237af6deca27f5d33
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
`LOG_TARGET_DEBUG()` reports file, line and function name at the call
site. This information is not helpfull if it always points to the same
location inside `log_debug_reg()`.
Change-Id: Ib73be0344fb5c80c9ac8e5fdee1084d405522eb7
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
The existing code didn't seem to work right at all. I have spike
modifications that exercise these new cases. I'll merge those once this
has merged.
Change-Id: I89bd336f34f1b208a76f25b6b41fe3877800765b
Signed-off-by: Tim Newsome <tim@sifive.com>
Compilers are good at optimizing, and with functions it's abundantly
clear what all the types involved are. This change means we don't have
to be super careful about the type of values because of what the macro
might do to them that might cause overflow.
The only place where the return type matters is in printf-style
functions, and I made get_value32() for those cases where a change was
needed.
This should set the stage for simply copying the latest debug_defines.h
from the debug spec build again.
Change-Id: I5fb19d0cfc1e20137832a7b344b05db215ce00e1
Signed-off-by: Tim Newsome <tim@sifive.com>
The reasoning for the change:
* `__func__` is part of C99, `__PRETTY_FUNCTION__` is GNU extension.
* `__PRETTY_FUNCTION__` is defined to be the same as `__func__` for C
sources by GCC documentation but differ for C++ sources (full
signature instead of just a name).
* Currently Clang does support `__PRETTY_FUNCTION__`, though it uses
GCC's C++ variant across C and C++.
Therefore using `__PRETTY_FUNCTION__` creates confusion and does not
provide any valueble information in the logs.
Change-Id: Ie0db6d73f602784b6752a30911dcef3dd7ee4594
Sometimes, the value from of some DMI scans has no meaning (e.g. when
`op` is read). Such values should not be decoded. To make the dumps more
consistent, `<no decoding available>` is printed when there is no
decoding for a register.
Change-Id: I415f06a5a80f2fc8fb8ab3f79132bdf0602c8ad6
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
By definition in `target/target.h`, `coreid` is not a unique identifier
of a target -- it can be the same for targets on different TAPs.
Change-Id: Ifce78da55fffe28dd8b6b06ecae7d8c4e305c0a2
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
Added the ability to enter dimensionless registers
Change-Id: I1b781959ce4690ec65304142bd9a7c6f540b3e86
Signed-off-by: Anastasiya Chernikova <anastasiya.chernikova@syntacore.com>
Extend riscv set_ebreak* commands.
Now it can be called without args to print current value.
riscv_ebreak* flags are moved to riscv_info struct.
Change-Id: Ib46e6b6dfc0117599c7f6715c7aaf113e63bd7dc
Signed-off-by: Kirill Radkin <kirill.radkin@syntacore.com>
`riscv_debug_reg_to_s()` can be used to decode register value. If the
pointer to buffer is `NULL` it does not print anything, just returns the
length of the string.
The format is:
`<register_value> { <field_name>=<field_value_name or field_value>, ..., }`
e.g:
`0x400382 { version=2, ... ndmresetpending=false, }`
`0x321009 { regno=0x1009, ... cmdtype=0, }`
Change-Id: I63733d8d36385d89ca15de1a43139134bc488c4f
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This also fixes a bug when, after `examine` completion, the target still
has `unknown` status. To reproduce this one spike, it is enough to do
the following:
---
// make sure spike harts are halted
openocd ... -c init -c 'echo "[targets]"'
---
this behavior is quite dangerous and leads to segfaults in some cases
Change-Id: I13915f7038ad6d0251d56d2d519fbad9a2f13c18
Signed-off-by: Parshintsev Anatoly <anatoly.parshintsev@syntacore.com>
This patch improves the following issues:
1. Makes it compatible with targets with progbufsize == 1.
2. Although exceptions don’t update any registers, but do end execution
of the progbuf. This will make fence rw, rw impossible to execute.
Change-Id: I2208fd31ec6a7dae6e61c5952f90901568caada6
Signed-off-by: Xiang W <wxjstz@126.com>
The motivation for this refactor is to fixup error handling for some
corner cases. These functions attempt to cache S0 register and only then
perform a bunch of extra checks to figure out if the requested register
is valid one in this context. The problem is that there are few corner
cases when _*progbuf functions could receive a GPR as an input. For
example, an abstract read could fail (for whatever reason) leading to
infinite recursion:
````
save S0 -> read S0 -> save S0 -> read S0 -> ...
```
The case described above could be fixed by adding extra sanitity checks,
however I decided to make these functions more modular since I find
self-contained functions easier to read.
Change-Id: I01f57bf474ca45ebb67a30cd4d8fdef21f307c7d
Signed-off-by: Parshintsev Anatoly <anatoly.parshintsev@syntacore.com>
OpenOCD fails in the presence of inactive/unresponsive cores
I faced with case when inactive core returns 0 while reading dtmcontrol.
This leads to failure on assert: "addrbits != 0" in "dbus_scan".
Also change "read_bits","poll_target" funcs to avoid a lot lines in logs
Change-Id: If852126755317789602b7372c5c5732183fff6c5
Signed-off-by: Kirill Radkin <kirill.radkin@syntacore.com>
Support assign DMI address of the debug module by pass
-dbgbase to the target create command
Change-Id: I774c3746567f6e6d77c43a62dea5e9e67bb25770
Signed-off-by: Mark Zhuang <mark.zhuang@spacemit.com>
This patch adds target logging to logging instances where it makes sense.
This is especially useful when debugging multiple targets at once,
such as multicore systems.
Change-Id: Ia9861f3fa0e6e5908b683c2a8280659c3c264395
Signed-off-by: Marek Vrbka <marek.vrbka@codasip.com>
Besides checkpatch, now upstream codes are scanning with
Sparse semantic checker tool.
This commit addresses some Sparse and checkpatch warnings.
Change-Id: I0e3e9f15220d8829c5708897af27aa86a8f90c07
Signed-off-by: Erhan Kurubas <erhan.kurubas@espressif.com>
There were a couple of problems with previous implementation:
* Misalligned read would return ERROR_OK and print all zeroes.
* CMDERR_BUSY for abstract access was improperly handled:
According to the spec, no assumptions can be made about DM_DATA*
contents in such a case, but these were considered valid values from
memory.
* A fallback to one element read was implemented when DMI_STATUS_BUSY
occurred during batch reads, even though this can be accounted for.
Change-Id: I09174c61c951b2bb97a529b7f0aa5afaa995179b
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
With this change, failures to resume a hart due to it not being halted
are more explicitly logged or reported as an error.
Change-Id: Ia55d8df85a908363d0f2140637ce1e47c1ab6251
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This involves halting the target, which might have unintended side
effects, but when the debugger is connected software breakpoints must
trap to the debugger. Anything else is a terrible user experience.
Change-Id: I1f7bb610eeeb054cc3042dc6bcfc16589ce12a31
Signed-off-by: Tim Newsome <tim@sifive.com>
Will be used later when we want to do a quick halt/resume.
Change-Id: Ib80166234c4c277b7d9ce26b7566ac0f93017e64
Signed-off-by: Tim Newsome <tim@sifive.com>
* Only set ebreak bits that might be supported based on misa.
* Don't write dcsr if its value wouldn't change.
Change-Id: I7087af0b0df0fbdbf994373b5c887b9b389df872
Signed-off-by: Tim Newsome <tim@sifive.com>
Make it callable earlier, handle `supported` being NULL, and make enum
names more clear.
Change-Id: If4d286b54ccfc01eb5de5a57eb18f748c920e979
Signed-off-by: Tim Newsome <tim@sifive.com>
The riscv013_on_halt function was being called but its implementation was
empty, providing no additional functionality. Removed the function declaration,
calls to it, and its implementation since it is not required.
Change-Id: I425ea890deadeec945f0a47af247f3f99172e801
Signed-off-by: Tim Newsome <tim@sifive.com>
Otherwise we may end up modifying DCSR of a different hart than
intended.
Change-Id: I39bde21a1444623ed150f2b3d504b9318b9d6191
Signed-off-by: Tim Newsome <tim@sifive.com>
Register access on running target should fail if mstatus needs to be
modified.
Change-Id: Iec8e8d514ef2f5ca42606a5534cce55aaaa99180
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This way if you connect to a running target, before it's hit a breakpoint,
then when it does hit the breakpoint OpenOCD will catch it.
Change-Id: I6f1e5f169fa385f46759015786e664693c3872e4
Signed-off-by: Tim Newsome <tim@sifive.com>
Poll failure just means poll failed. It's safer to assume the target is
still running, because then if it is running and subsequently halts we can
relay this to gdb correctly. We can't do the other way around, because once
gdb thinks the target has halted, it can't deal with it spontaneously
running.
Change-Id: Idb56137f1d6baa9afc1b0e55e4a48f407b8ebe83
Signed-off-by: Tim Newsome <tim@sifive.com>