Skip to content

Commit

Permalink
target/riscv: improve error handling in write_memory_progbuf()
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
en-sc committed Dec 6, 2023
1 parent 6de536b commit 12a43ad
Show file tree
Hide file tree
Showing 5 changed files with 271 additions and 147 deletions.
10 changes: 10 additions & 0 deletions src/target/riscv/batch.c
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,13 @@ size_t riscv_batch_available_scans(struct riscv_batch *batch)
{
return batch->allocated_scans - batch->used_scans - 4;
}

bool riscv_batch_dmi_busy_encountered(const struct riscv_batch *batch)
{
if (!batch->used_scans)
return false;
assert(batch->last_scan == RISCV_SCAN_TYPE_NOP);
const struct scan_field *field = batch->fields + batch->used_scans - 1;
const uint64_t in = buf_get_u64(field->in_value, 0, field->num_bits);
return get_field(in, DTM_DMI_OP) == DTM_DMI_OP_BUSY;
}
3 changes: 3 additions & 0 deletions src/target/riscv/batch.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,7 @@ void riscv_batch_add_nop(struct riscv_batch *batch);
/* Returns the number of available scans. */
size_t riscv_batch_available_scans(struct riscv_batch *batch);

/* Return true iff the last scan in the batch returned DMI_OP_BUSY. */
bool riscv_batch_dmi_busy_encountered(const struct riscv_batch *batch);

#endif
17 changes: 17 additions & 0 deletions src/target/riscv/program.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,23 @@ int riscv_program_sbr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno
return riscv_program_insert(p, sb(d, b, offset));
}

int riscv_program_store(struct riscv_program *p, enum gdb_regno d, enum gdb_regno b, int offset,
unsigned int size)
{
switch (size) {
case 1:
return riscv_program_sbr(p, d, b, offset);
case 2:
return riscv_program_shr(p, d, b, offset);
case 4:
return riscv_program_swr(p, d, b, offset);
case 8:
return riscv_program_sdr(p, d, b, offset);
}
assert(false && "Unsupported size");
return ERROR_FAIL;
}

int riscv_program_ldr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno b, int offset)
{
return riscv_program_insert(p, ld(d, b, offset));
Expand Down
2 changes: 2 additions & 0 deletions src/target/riscv/program.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ int riscv_program_sdr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno
int riscv_program_swr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno a, int o);
int riscv_program_shr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno a, int o);
int riscv_program_sbr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno a, int o);
int riscv_program_store(struct riscv_program *p, enum gdb_regno d, enum gdb_regno b, int o,
unsigned int s);

int riscv_program_csrrsi(struct riscv_program *p, enum gdb_regno d, unsigned int z, enum gdb_regno csr);
int riscv_program_csrrci(struct riscv_program *p, enum gdb_regno d, unsigned int z, enum gdb_regno csr);
Expand Down
Loading

0 comments on commit 12a43ad

Please sign in to comment.