Skip to content

Commit 85654c1

Browse files
en-scen-sc
committed
target/riscv: access registers via reg->type
* `int riscv_reg_get()` and `int riscv_reg_set()` are implemented in terms of `reg->type->get/set` instead of the other way around. This makes it easier to support custom behavior for some registers. * Cacheability is determined by `reg->type` instead of `riscv_reg_impl_gdb_regno_cacheable()`. * Issues with redirection of `priv` -> `dcsr` and `pc` -> `dpc` are addressed at the "topmost" level. - `priv` and `pc` are always invalid. - Fixed some issues, e.g. the first `pc` write printed-out an uninitialized value: ``` > reg pc 0 pc (/64): 0x000075da6b33db20 ``` Change-Id: I514547f455d62b289fb5dee62753bf5d9aa3b8ae Signed-off-by: Evgeniy Naydanov <[email protected]>
1 parent 2c4dfd9 commit 85654c1

File tree

6 files changed

+509
-314
lines changed

6 files changed

+509
-314
lines changed

‎src/target/riscv/program.h

+4-5
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,10 @@ int riscv_program_init(struct riscv_program *p, struct target *t);
3939
/* Write the program to the program buffer. */
4040
int riscv_program_write(struct riscv_program *program);
4141

42-
/* Executes a program, returning 0 if the program successfully executed. Note
43-
* that this may cause registers to be saved or restored, which could result to
44-
* calls to things like riscv013_reg_save which itself could require a
45-
* program to execute. That's OK, just make sure this eventually terminates.
46-
* */
42+
/* Executes the RISC-V Program Buffer and returns ERROR_OK if the program
43+
* buffer got successfully executed. In case of failure, more detailed error reason
44+
* can be found in p->execution_result.
45+
*/
4746
int riscv_program_exec(struct riscv_program *p, struct target *t);
4847

4948
/* A lower level interface, you shouldn't use this unless you have a reason. */

‎src/target/riscv/riscv-013.c

+21-30
Original file line numberDiff line numberDiff line change
@@ -986,7 +986,7 @@ static int examine_progbuf(struct target *target)
986986
return ERROR_OK;
987987
}
988988

989-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
989+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
990990
return ERROR_FAIL;
991991

992992
struct riscv_program program;
@@ -1318,7 +1318,7 @@ static int fpr_read_progbuf(struct target *target, uint64_t *value,
13181318

13191319
const unsigned int freg = number - GDB_REGNO_FPR0;
13201320

1321-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
1321+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
13221322
return ERROR_FAIL;
13231323

13241324
struct riscv_program program;
@@ -1348,7 +1348,7 @@ static int csr_read_progbuf(struct target *target, uint64_t *value,
13481348
assert(target->state == TARGET_HALTED);
13491349
assert(number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095);
13501350

1351-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
1351+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
13521352
return ERROR_FAIL;
13531353

13541354
struct riscv_program program;
@@ -1416,7 +1416,7 @@ static int fpr_write_progbuf(struct target *target, enum gdb_regno number,
14161416
assert(number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31);
14171417
const unsigned int freg = number - GDB_REGNO_FPR0;
14181418

1419-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
1419+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
14201420
return ERROR_FAIL;
14211421

14221422
struct riscv_program program;
@@ -1446,11 +1446,11 @@ static int vtype_write_progbuf(struct target *target, riscv_reg_t value)
14461446
{
14471447
assert(target->state == TARGET_HALTED);
14481448

1449-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
1449+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
14501450
return ERROR_FAIL;
14511451
if (register_write_abstract(target, GDB_REGNO_S0, value) != ERROR_OK)
14521452
return ERROR_FAIL;
1453-
if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK)
1453+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK)
14541454
return ERROR_FAIL;
14551455

14561456
struct riscv_program program;
@@ -1467,11 +1467,11 @@ static int vl_write_progbuf(struct target *target, riscv_reg_t value)
14671467
{
14681468
assert(target->state == TARGET_HALTED);
14691469

1470-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
1470+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
14711471
return ERROR_FAIL;
14721472
if (register_write_abstract(target, GDB_REGNO_S0, value) != ERROR_OK)
14731473
return ERROR_FAIL;
1474-
if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK)
1474+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK)
14751475
return ERROR_FAIL;
14761476

14771477
struct riscv_program program;
@@ -1490,7 +1490,7 @@ static int csr_write_progbuf(struct target *target, enum gdb_regno number,
14901490
assert(target->state == TARGET_HALTED);
14911491
assert(number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095);
14921492

1493-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
1493+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
14941494
return ERROR_FAIL;
14951495
if (register_write_abstract(target, GDB_REGNO_S0, value) != ERROR_OK)
14961496
return ERROR_FAIL;
@@ -2296,7 +2296,7 @@ int riscv013_get_register_buf(struct target *target, uint8_t *value,
22962296
&debug_vl, &debug_vsew) != ERROR_OK)
22972297
return ERROR_FAIL;
22982298

2299-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
2299+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
23002300
return ERROR_FAIL;
23012301

23022302
unsigned int vnum = regno - GDB_REGNO_V0;
@@ -2351,7 +2351,7 @@ int riscv013_set_register_buf(struct target *target, enum gdb_regno regno,
23512351
&debug_vl, &debug_vsew) != ERROR_OK)
23522352
return ERROR_FAIL;
23532353

2354-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
2354+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
23552355
return ERROR_FAIL;
23562356

23572357
unsigned int vnum = regno - GDB_REGNO_V0;
@@ -4278,11 +4278,11 @@ static int read_memory_progbuf_inner_fill_progbuf(struct target *target,
42784278
{
42794279
const bool is_repeated_read = increment == 0;
42804280

4281-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
4281+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
42824282
return ERROR_FAIL;
4283-
if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK)
4283+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK)
42844284
return ERROR_FAIL;
4285-
if (is_repeated_read && riscv013_reg_save(target, GDB_REGNO_A0) != ERROR_OK)
4285+
if (is_repeated_read && riscv013_reg_save_gpr(target, GDB_REGNO_A0) != ERROR_OK)
42864286
return ERROR_FAIL;
42874287

42884288
struct riscv_program program;
@@ -4376,7 +4376,7 @@ read_memory_progbuf_inner_one(struct target *target, const riscv_mem_access_args
43764376
{
43774377
assert(riscv_mem_access_is_read(args));
43784378

4379-
if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK)
4379+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK)
43804380
return mem_access_result(MEM_ACCESS_SKIPPED_REG_SAVE_FAILED);
43814381

43824382
struct riscv_program program;
@@ -4951,9 +4951,9 @@ static int write_memory_progbuf_try_to_write(struct target *target,
49514951

49524952
static int write_memory_progbuf_fill_progbuf(struct target *target, uint32_t size)
49534953
{
4954-
if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK)
4954+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK)
49554955
return ERROR_FAIL;
4956-
if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK)
4956+
if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK)
49574957
return ERROR_FAIL;
49584958

49594959
struct riscv_program program;
@@ -5058,19 +5058,8 @@ struct target_type riscv013_target = {
50585058
int riscv013_get_register(struct target *target,
50595059
riscv_reg_t *value, enum gdb_regno rid)
50605060
{
5061-
/* It would be beneficial to move this redirection to the
5062-
* version-independent section, but there is a conflict:
5063-
* `dcsr[5]` is `dcsr.v` in current spec, but it is `dcsr.debugint` in 0.11.
5064-
*/
5065-
if (rid == GDB_REGNO_PRIV) {
5066-
uint64_t dcsr;
5067-
if (riscv_reg_get(target, &dcsr, GDB_REGNO_DCSR) != ERROR_OK)
5068-
return ERROR_FAIL;
5069-
*value = set_field(0, VIRT_PRIV_V, get_field(dcsr, CSR_DCSR_V));
5070-
*value = set_field(*value, VIRT_PRIV_PRV, get_field(dcsr, CSR_DCSR_PRV));
5071-
return ERROR_OK;
5072-
}
5073-
5061+
assert(rid != GDB_REGNO_PC && "'pc' should be read through 'dpc'");
5062+
assert(rid != GDB_REGNO_PRIV && "'priv' should be read through 'dcsr'");
50745063
LOG_TARGET_DEBUG(target, "reading register %s", riscv_reg_gdb_regno_name(target, rid));
50755064

50765065
if (dm013_select_target(target) != ERROR_OK)
@@ -5087,6 +5076,8 @@ int riscv013_get_register(struct target *target,
50875076
int riscv013_set_register(struct target *target, enum gdb_regno rid,
50885077
riscv_reg_t value)
50895078
{
5079+
assert(rid != GDB_REGNO_PC && "'pc' should be written through 'dpc'");
5080+
assert(rid != GDB_REGNO_PRIV && "'priv' should be written through 'dcsr'");
50905081
LOG_TARGET_DEBUG(target, "writing 0x%" PRIx64 " to register %s",
50915082
value, riscv_reg_gdb_regno_name(target, rid));
50925083

0 commit comments

Comments
 (0)