necessary tests for progbufsize | Skip CSR read if insufficient progbuf in RISC-V targets #1300
Conversation
|
This code built and ran without issues ,In case of any needed modification of functionality ,please let me know ,Ill do it :) |
There was a problem hiding this comment.
@Biancaa-R, thank you for submitting the patch!
Please, take a look at Checkpatch failures.
The first ERROR:COMMIT_MESSAGE: can be quite cryptic -- the commit message is split into the title and the description with an empty line in-between.
Please see https://git-scm.com/docs/git-commit#_discussion for more.
The other errors seem descriptive enough.
You can run the script locally with:
> tools/scripts/checkpatch.pl -g HEAD
Also, what about vtype_write_progbuf()/vl_write_progbuf()?
src/target/riscv/riscv-013.c
Outdated
| assert(target->state == TARGET_HALTED); | ||
| assert(number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31); | ||
| if (!has_sufficient_progbuf(target, 2)) { | ||
| LOG_DEBUG("Skipping FPR read: insufficient progbuf (size=%d)", |
There was a problem hiding this comment.
[here and around] Please, use LOG_TARGET_DEBUG(...) to give a bit more context:
| LOG_DEBUG("Skipping FPR read: insufficient progbuf (size=%d)", | |
| LOG_TARGET_DEBUG(target, "Skipping FPR read: insufficient progbuf (size=%d)", |
Also progbufsize is unsigned, so the correct format specifier is %u (though this is undoubtedly a nitpick).
There was a problem hiding this comment.
Changed it in the updated version.
|
Also, if you have the time please check #1301 -- I believe it is a better approach to the issue. |
Sure I'll check for those errors. In the issue only csr_read_progbuf() Were mentioned, sure I'll add those 2 as well. |
Added additional debug logs to verify correct retrieval and printing of progbufsize in various code paths within riscv-013.c. This helps in debugging hardware configurations that expose custom program buffer sizes. Signed-off-by: Biancaa Ramesh <[email protected]>
…d-write Added a check for have_progbuf_size before attempting to execute. This prevents program buffer access on targets that do not support it, improving robustness and avoiding potential execution failures when the program buffer is not available or too small. Signed-off-by: Biancaa Ramesh <[email protected]>
Biancaa-R
force-pushed
the
riscv-progbufsize-check
branch
from
060a08b to
38450ad
Compare
| { | ||
| assert(target->state == TARGET_HALTED); | ||
| /* Ensure program buffer is large enough for 2 instructions */ | ||
| if (!has_sufficient_progbuf(target, 2)) { |
There was a problem hiding this comment.
| if (!has_sufficient_progbuf(target, 2)) { | |
| if (!has_sufficient_progbuf(target, 3)) { |
The sequence is:
csrr ...
vsetvl ...
ebreak
| { | ||
| assert(target->state == TARGET_HALTED); | ||
| /* Ensure program buffer is large enough for 2 instructions */ | ||
| if (!has_sufficient_progbuf(target, 2)) { |
There was a problem hiding this comment.
| if (!has_sufficient_progbuf(target, 2)) { | |
| if (!has_sufficient_progbuf(target, 3)) { |
(Same as for vl.)
There was a problem hiding this comment.
Oh as For vector register (v1) or vector type (vtype) operations we need to see 3 instructions ,right !
Ill fix it .Thanks @en-sc
2 participants
This PR resolves this issue #1264
These functions currently do not exit early if program the program buffer in the target is insufficient:
This pr resolves this issue using the pre existing function:
if (!has_sufficient_progbuf(target, 2))Add a check in CSR read functions to skip the operation when the target's
progbuf is smaller than required. This prevents errors on targets with
limited progbuf size and ensures safer debugging operations.
has_sufficient_progbufchecks to CSR read functions.Successful configuring:
Successful build ,after build the version was also tested to check working:
with
./src/openocd --version