From 7a4948c126fe027587ef6e2a96a708fcd00ef86c Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Mon, 26 Jun 2017 20:28:03 -0700 Subject: riscv: initial checkin of a 'compliance test' command. --- src/target/riscv/riscv.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 7a40467..ca39d93 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -833,6 +833,9 @@ int riscv_openocd_deassert_reset(struct target *target) return ERROR_OK; } +// Declared below +const struct command_registration riscv_command_handlers[]; + struct target_type riscv_target = { .name = "riscv", @@ -868,6 +871,8 @@ struct target_type riscv_target = .arch_state = riscv_arch_state, .run_algorithm = riscv_run_algorithm, + + .commands = riscv_command_handlers }; /*** RISC-V Interface ***/ @@ -1236,3 +1241,46 @@ int riscv_dmi_write_u64_bits(struct target *target) RISCV_INFO(r); return r->dmi_write_u64_bits(target); } + +/* Command Handlers */ + +COMMAND_HANDLER(riscv_test_compliance) { + + //struct target *target = get_current_target(CMD_CTX); + //TODO: Create methods to check it's really RISC-V. + //struct riscv_target * riscv = (struct riscv_target*) target; + + if (CMD_ARGC > 0) { + if (strcmp(CMD_ARGV[0], "foo") == 0) + LOG_ERROR("FOO!"); + if (strcmp(CMD_ARGV[0], "bar") == 0) + LOG_DEBUG("BAR!"); + } else { + return ERROR_FAIL; + } + + return ERROR_OK; + +} + +static const struct command_registration riscv_exec_command_handlers[] = { + { + .name = "riscv_test_compliance", + .handler = riscv_test_compliance, + .mode = COMMAND_EXEC, + .usage = "['foo'|'bar']", + .help = "foos and bars" + }, + COMMAND_REGISTRATION_DONE +}; + +const struct command_registration riscv_command_handlers[] = { + { + .name = "riscv", + .mode = COMMAND_ANY, + .help = "RISC-V Command Group", + .usage = "", + .chain = riscv_exec_command_handlers + }, + COMMAND_REGISTRATION_DONE +}; -- cgit v1.1 From 95ee7975ea69191ac01bebbc853b07e9db87003a Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Tue, 27 Jun 2017 09:46:36 -0700 Subject: riscv: Add skeleton of RISC-V v013 compliance --- src/target/riscv/riscv-013.c | 19 ++++++++++++++++++- src/target/riscv/riscv.c | 28 ++++++++++++++-------------- src/target/riscv/riscv.h | 1 + 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 60846de..b50a5a4 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -64,6 +64,7 @@ static void riscv013_fill_dmi_read_u64(struct target *target, char *buf, int a); static int riscv013_dmi_write_u64_bits(struct target *target); static void riscv013_fill_dmi_nop_u64(struct target *target, char *buf); static void riscv013_reset_current_hart(struct target *target); +static int riscv013_test_compliance(struct target *target); /** * Since almost everything can be accomplish by scanning the dbus register, all @@ -728,7 +729,8 @@ static int init_target(struct command_context *cmd_ctx, generic_info->fill_dmi_nop_u64 = &riscv013_fill_dmi_nop_u64; generic_info->dmi_write_u64_bits = &riscv013_dmi_write_u64_bits; generic_info->reset_current_hart = &riscv013_reset_current_hart; - + generic_info->test_compliance = &riscv013_test_compliance; + generic_info->version_specific = calloc(1, sizeof(riscv013_info_t)); if (!generic_info->version_specific) return ERROR_FAIL; @@ -2108,3 +2110,18 @@ void riscv013_clear_abstract_error(struct target *target) uint32_t acs = dmi_read(target, DMI_ABSTRACTCS); dmi_write(target, DMI_ABSTRACTCS, acs); } + +int riscv013_test_compliance(struct target *target) { + LOG_INFO("Testing Compliance against RISC-V Debug Spec v0.13"); + + int total_tests = 0; + int passed_tests = 0; + + LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); + + if (total_tests == passed_tests) { + return ERROR_OK; + } else { + return ERROR_FAIL; + } +} diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index ca39d93..e5f4fe3 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1246,30 +1246,30 @@ int riscv_dmi_write_u64_bits(struct target *target) COMMAND_HANDLER(riscv_test_compliance) { - //struct target *target = get_current_target(CMD_CTX); - //TODO: Create methods to check it's really RISC-V. - //struct riscv_target * riscv = (struct riscv_target*) target; - + struct target *target = get_current_target(CMD_CTX); + + RISCV_INFO(r); + if (CMD_ARGC > 0) { - if (strcmp(CMD_ARGV[0], "foo") == 0) - LOG_ERROR("FOO!"); - if (strcmp(CMD_ARGV[0], "bar") == 0) - LOG_DEBUG("BAR!"); + LOG_ERROR("Command does not take any parameters."); + return ERROR_FAIL; + } + + if (r->test_compliance) { + return r->test_compliance(target); } else { + LOG_ERROR("This target does not support this command (may implement an older version of the spec)."); return ERROR_FAIL; } - - return ERROR_OK; - } static const struct command_registration riscv_exec_command_handlers[] = { { - .name = "riscv_test_compliance", + .name = "test_compliance", .handler = riscv_test_compliance, .mode = COMMAND_EXEC, - .usage = "['foo'|'bar']", - .help = "foos and bars" + .usage = "", + .help = "Runs a basic compliance test suite against the RISC-V Debug Spec." }, COMMAND_REGISTRATION_DONE }; diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index e23d49d..1c4b7ed 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -92,6 +92,7 @@ typedef struct { void (*fill_dmi_read_u64)(struct target *target, char *buf, int a); void (*fill_dmi_nop_u64)(struct target *target, char *buf); void (*reset_current_hart)(struct target *target); + int (*test_compliance)(struct target *target); } riscv_info_t; /* Everything needs the RISC-V specific info structure, so here's a nice macro -- cgit v1.1 From ccc605158a9412436e08b737ac8b699fcb587e7d Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Tue, 27 Jun 2017 15:01:42 -0700 Subject: riscv: Added several compliance test items --- src/target/riscv/riscv-013.c | 155 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index b50a5a4..66d31b4 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2111,12 +2111,167 @@ void riscv013_clear_abstract_error(struct target *target) dmi_write(target, DMI_ABSTRACTCS, acs); } +#define COMPLIANCE_TEST(b, message) { \ + int pass = 0; \ + if (b) { \ + pass = 1; \ + passed_tests ++; \ + } \ + LOG_INFO("%s test %d (%s)\n", (pass) ? "PASSED":"FAILED", total_tests, message); \ + total_tests ++; \ + } + int riscv013_test_compliance(struct target *target) { LOG_INFO("Testing Compliance against RISC-V Debug Spec v0.13"); + if (!riscv_rtos_enabled(target)) { + LOG_ERROR("Please run with -rtos riscv to run compliance test."); + return ERROR_FAIL; + } + int total_tests = 0; int passed_tests = 0; + uint32_t dmcontrol_orig = dmi_read(target, DMI_DMCONTROL); + uint32_t dmcontrol; + uint32_t testvar; + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSEL, RISCV_MAX_HARTS-1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmcontrol = dmi_read(target, DMI_DMCONTROL); + COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSEL) == (RISCV_MAX_HARTS-1), "DMCONTROL.hartsel should hold MAX_HARTS - 1"); + + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSEL, 0); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmcontrol = dmi_read(target, DMI_DMCONTROL); + COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSEL) == 0, "DMCONTROL.hartsel should hold Hart ID 0"); + + // hartreset + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmcontrol = dmi_read(target, DMI_DMCONTROL); + testvar = get_field(dmcontrol, DMI_DMCONTROL_HARTRESET); + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmcontrol = dmi_read(target, DMI_DMCONTROL); + COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HARTRESET)) == 0), "DMCONTROL.hartreset can be 0 or RW."); + + // hasel + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmcontrol = dmi_read(target, DMI_DMCONTROL); + testvar = get_field(dmcontrol, DMI_DMCONTROL_HASEL); + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmcontrol = dmi_read(target, DMI_DMCONTROL); + COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HASEL)) == 0), "DMCONTROL.hasel can be 0 or RW."); + //TODO: test that hamask registers exist if hasel does. + + // TODO: ndmreset + + // TODO: dmactive + + // haltreq + riscv_halt_all_harts(target); + // TODO: resumereq + + // HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. + for (uint32_t hartsel = 0; hartsel < riscv_count_harts(target); hartsel++){ + riscv_set_current_hartid(target, hartsel); + uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS); + if (get_field(dmstatus, DMI_DMSTATUS_ANYNONEXISTENT)) { + break; + } + + uint32_t hartinfo = dmi_read(target, DMI_HARTINFO); + dmi_write (target, DMI_HARTINFO, ~hartinfo); + COMPLIANCE_TEST((dmi_read(target, DMI_HARTINFO) == hartinfo), "DMHARTINFO should be Read-Only."); + + uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); + for (int d = 0; d < nscratch; d++) { + + for (testvar = 0x00112233; testvar != 0xDEAD ; testvar = testvar == 0x00112233 ? ~testvar : 0xDEAD ) { + + struct riscv_program program32; + riscv_program_init(&program32, target); + riscv_addr_t addr_in = riscv_program_alloc_x(&program32); + riscv_addr_t addr_out = riscv_program_alloc_x(&program32); + + riscv_program_write_ram(&program32, addr_in, testvar); + if (riscv_xlen(target) > 32) { + riscv_program_write_ram(&program32, addr_in + 4, testvar); + } + + riscv_program_lx(&program32, GDB_REGNO_S0, addr_in); + riscv_program_csrrw(&program32, GDB_REGNO_S0, GDB_REGNO_S0, GDB_REGNO_DSCRATCH); + riscv_program_csrrw(&program32, GDB_REGNO_S1, GDB_REGNO_S1, GDB_REGNO_DSCRATCH); + riscv_program_sx(&program32, GDB_REGNO_S1, addr_out); + riscv_program_fence(&program32); + COMPLIANCE_TEST(riscv_program_exec(&program32, target) == ERROR_OK, "Accessing DSCRATCH with program buffer should succeed."); + + COMPLIANCE_TEST(riscv_program_read_ram(&program32, addr_out) == testvar, "All DSCRATCH registers in HARTINFO must be R/W."); + if (riscv_xlen(target) > 32) { + COMPLIANCE_TEST(riscv_program_read_ram(&program32, addr_out + 4) == testvar, "All DSCRATCH registers in HARTINFO must be R/W."); + } + } + } + + // TODO: dataaccess + if (get_field(hartinfo, DMI_HARTINFO_DATAACCESS)) { + // TODO: Shadowed in memory map. + // TODO: datasize + // TODO: dataaddr + } else { + // TODO: Shadowed in CSRs. + // TODO: datasize + // TODO: dataaddr + } + + } + + // HALTSUM + uint32_t expected_haltsum = 0; + for (unsigned int i = 0; i < riscv_count_harts(target); i +=32){ + expected_haltsum |= (1 << (i / 32)); + } + COMPLIANCE_TEST(dmi_read(target, DMI_HALTSUM) == expected_haltsum, "HALTSUM should report all halted harts"); + + // TODO: HAWINDOWSEL + // TODO: HAWINDOW + + // ABSTRACTCS + + uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); + + // Check that all reported Data Words are really R/W + for (int invert = 0; invert < 2; invert++) { + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ + testvar = (i + 1) * 0x11111111; + if (invert) {testvar = ~testvar;} + dmi_write(target, DMI_DATA0 + i, testvar); + } + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ + testvar = (i + 1) * 0x11111111; + if (invert) {testvar = ~testvar;} + COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == testvar, "All reported DATA words must be R/W"); + } + } + + // Check that all reported ProgBuf words are really R/W + for (int invert = 0; invert < 2; invert++) { + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + testvar = (i + 1) * 0x11111111; + if (invert) {testvar = ~testvar;} + dmi_write(target, DMI_PROGBUF0 + i, testvar); + } + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + testvar = (i + 1) * 0x11111111; + if (invert) {testvar = ~testvar;} + COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == testvar, "All reported PROGBUF words must be R/W"); + } + } + + // TODO: Cause and clear all error types + LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); if (total_tests == passed_tests) { -- cgit v1.1 From 9e76ec177959906cfb8dd0adabf97182cb24e656 Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Tue, 27 Jun 2017 22:07:04 -0700 Subject: riscv: Compliance test for HALTREQ/RESUMEREQ R/W --- src/jtag/drivers/libjaylink | 2 +- src/target/riscv/riscv-013.c | 119 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 108 insertions(+), 13 deletions(-) diff --git a/src/jtag/drivers/libjaylink b/src/jtag/drivers/libjaylink index 699b700..d57dee6 160000 --- a/src/jtag/drivers/libjaylink +++ b/src/jtag/drivers/libjaylink @@ -1 +1 @@ -Subproject commit 699b7001d34a79c8e7064503dde1bede786fd7f0 +Subproject commit d57dee67bc756291b7d8b51d350d1c6213e514f0 diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 66d31b4..a68594c 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1807,7 +1807,7 @@ static void riscv013_halt_current_hart(struct target *target) /* Issue the halt command, and then wait for the current hart to halt. */ uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL); - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HALTREQ, 1); + dmcontrol |= DMI_DMCONTROL_HALTREQ; dmi_write(target, DMI_DMCONTROL, dmcontrol); for (size_t i = 0; i < 256; ++i) if (riscv_is_halted(target)) @@ -1823,7 +1823,7 @@ static void riscv013_halt_current_hart(struct target *target) abort(); } - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HALTREQ, 0); + dmcontrol &= ~DMI_DMCONTROL_HALTREQ; dmi_write(target, DMI_DMCONTROL, dmcontrol); } @@ -1963,7 +1963,7 @@ void riscv013_reset_current_hart(struct target *target) select_dmi(target); uint32_t control = dmi_read(target, DMI_DMCONTROL); control = set_field(control, DMI_DMCONTROL_NDMRESET, 1); - control = set_field(control, DMI_DMCONTROL_HALTREQ, 1); + control |= DMI_DMCONTROL_HALTREQ; dmi_write(target, DMI_DMCONTROL, control); control = set_field(control, DMI_DMCONTROL_NDMRESET, 0); @@ -1971,7 +1971,7 @@ void riscv013_reset_current_hart(struct target *target) while (get_field(dmi_read(target, DMI_DMSTATUS), DMI_DMSTATUS_ALLHALTED) == 0); - control = set_field(control, DMI_DMCONTROL_HALTREQ, 0); + control &= ~DMI_DMCONTROL_HALTREQ; dmi_write(target, DMI_DMCONTROL, control); } @@ -2118,6 +2118,7 @@ void riscv013_clear_abstract_error(struct target *target) passed_tests ++; \ } \ LOG_INFO("%s test %d (%s)\n", (pass) ? "PASSED":"FAILED", total_tests, message); \ + assert(pass); \ total_tests ++; \ } @@ -2172,22 +2173,46 @@ int riscv013_test_compliance(struct target *target) { // haltreq riscv_halt_all_harts(target); - // TODO: resumereq + // Writing haltreq should not cause any problems for a halted hart, but we + // should be able to read and write it. + dmcontrol = dmi_read(target, DMI_DMCONTROL); + dmcontrol |= DMI_DMCONTROL_HALTREQ; + dmi_write(target, DMI_DMCONTROL, dmcontrol); + COMPLIANCE_TEST(dmi_read(target, DMI_DMCONTROL) & DMI_DMCONTROL_HALTREQ, "DMCONTROL.haltreq should be R/W"); + uint32_t dmstatus; + do { + dmstatus = dmi_read(target, DMI_DMSTATUS); + } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); + + // resumereq. This will resume the hart but this test is destructive anyway. + dmcontrol &= ~DMI_DMCONTROL_HALTREQ; + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_DMCONTROL), DMI_DMCONTROL_RESUMEREQ) == 1, "DMCONTROL.resumereq should be R/W"); + + do { + dmstatus = dmi_read(target, DMI_DMSTATUS); + } while (get_field(dmstatus, DMI_DMSTATUS_ALLRESUMEACK) == 0); + + // Halt the hart again because the target isn't aware that we resumed it. + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0); + dmcontrol |= DMI_DMCONTROL_HALTREQ; + dmi_write(target, DMI_DMCONTROL, dmcontrol); + do { + dmstatus = dmi_read(target, DMI_DMSTATUS); + } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); + // HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. - for (uint32_t hartsel = 0; hartsel < riscv_count_harts(target); hartsel++){ + for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++){ riscv_set_current_hartid(target, hartsel); - uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS); - if (get_field(dmstatus, DMI_DMSTATUS_ANYNONEXISTENT)) { - break; - } uint32_t hartinfo = dmi_read(target, DMI_HARTINFO); dmi_write (target, DMI_HARTINFO, ~hartinfo); COMPLIANCE_TEST((dmi_read(target, DMI_HARTINFO) == hartinfo), "DMHARTINFO should be Read-Only."); uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); - for (int d = 0; d < nscratch; d++) { + for (unsigned int d = 0; d < nscratch; d++) { for (testvar = 0x00112233; testvar != 0xDEAD ; testvar = testvar == 0x00112233 ? ~testvar : 0xDEAD ) { @@ -2230,12 +2255,19 @@ int riscv013_test_compliance(struct target *target) { // HALTSUM uint32_t expected_haltsum = 0; - for (unsigned int i = 0; i < riscv_count_harts(target); i +=32){ + for (int i = 0; i < riscv_count_harts(target); i +=32){ expected_haltsum |= (1 << (i / 32)); } COMPLIANCE_TEST(dmi_read(target, DMI_HALTSUM) == expected_haltsum, "HALTSUM should report all halted harts"); + for (int i = 0; i < riscv_count_harts(target); i +=32){ + uint32_t haltstat = dmi_read(target, 0x40 + (i / 32)); + uint32_t haltstat_expected = (((i + 1) * 32) <= riscv_count_harts(target)) ? 0xFFFFFFFF : ((1 << (riscv_count_harts(target) % 32)) - 1); + COMPLIANCE_TEST(haltstat == haltstat_expected, "Halt Status Registers should report all halted harts."); + } + // TODO: HAWINDOWSEL + // TODO: HAWINDOW // ABSTRACTCS @@ -2271,6 +2303,69 @@ int riscv013_test_compliance(struct target *target) { } // TODO: Cause and clear all error types + + // COMMAND + // TODO: Unclear from the spec whether all these bits need to truly be R/W. + // But at any rate, this is not legal and should cause an error. + dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); + COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0xAAAAAAAA, "COMMAND register should be R/W"); + uint32_t result = dmi_read(target, DMI_ABSTRACTCS); + LOG_INFO("result is %x", result); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0x2, "Illegal COMMAND should result in UNSUPPORTED"); + dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + dmi_write(target, DMI_COMMAND, 0x55555555); + COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0x55555555, "COMMAND register should be R/W"); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0x2, "Illegal COMMAND should result in UNSUPPORTED"); + dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + + // ABSTRACTAUTO + // See which bits are actually writable + dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + uint32_t abstractauto = dmi_read(target, DMI_ABSTRACTAUTO); + if (abstractauto > 0) { + testvar = 0; + // TODO: This mechanism only works when you have a reasonable sized progbuf, which is not + // a true compliance requirement. + struct riscv_program program; + riscv_program_init(&program, target); + riscv_addr_t addr = riscv_program_alloc_x(&program); + riscv_program_lx(&program, GDB_REGNO_S0, addr); + riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); + riscv_program_sx(&program, GDB_REGNO_S0, addr); + riscv_program_write_ram(&program, addr + 4, 0); + if (riscv_xlen(target) > 32) { + riscv_program_write_ram(&program, addr, 0); + } + dmi_write(target, DMI_ABSTRACTAUTO, 0x0); + riscv_program_exec(&program, target); + testvar ++; + dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + abstractauto = dmi_read(target, DMI_ABSTRACTAUTO); + uint32_t autoexec_data = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECDATA); + uint32_t autoexec_progbuf = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECPROGBUF); + for (unsigned int i = 0; i < 12; i ++){ + dmi_read(target, DMI_DATA0 + i); + while(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY)); + if (autoexec_data & (1 << i)) { + COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT), "AUTOEXEC may be writable up to DATACOUNT bits."); + testvar ++; + } + } + for (unsigned int i = 0; i < 16; i ++){ + dmi_read(target, DMI_PROGBUF0 + i); + while(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY)); + if (autoexec_progbuf & (1 << i)) { + COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE), "AUTOEXEC may be writable up to PROGSIZE bits."); + testvar ++; + } + } + + dmi_write(target, DMI_ABSTRACTAUTO, 0); + int d_addr = (addr - riscv_debug_buffer_addr(target))/4 ; + + COMPLIANCE_TEST(testvar == riscv_read_debug_buffer_x(target, d_addr), \ + "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); + } LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); -- cgit v1.1 From 222850df55f10ae6e7821239c72492bf36bf3d6f Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Tue, 27 Jun 2017 22:29:35 -0700 Subject: debug: add a 'wfi' to compliance test. --- src/target/riscv/opcodes.h | 3 +++ src/target/riscv/riscv-013.c | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/target/riscv/opcodes.h b/src/target/riscv/opcodes.h index c4d9baf..a27a3dc 100644 --- a/src/target/riscv/opcodes.h +++ b/src/target/riscv/opcodes.h @@ -210,6 +210,9 @@ static uint32_t ebreak(void) { return MATCH_EBREAK; } static uint32_t ebreak_c(void) __attribute__ ((unused)); static uint32_t ebreak_c(void) { return MATCH_C_EBREAK; } +static uint32_t wfi(void) __attribute__ ((unused)); +static uint32_t wfi(void) { return MATCH_WFI; } + static uint32_t fence_i(void) __attribute__ ((unused)); static uint32_t fence_i(void) { diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index a68594c..506c3c6 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2330,6 +2330,8 @@ int riscv013_test_compliance(struct target *target) { riscv_program_init(&program, target); riscv_addr_t addr = riscv_program_alloc_x(&program); riscv_program_lx(&program, GDB_REGNO_S0, addr); + // Also testing that WFI() is a NOP during debug mode. + riscv_program_insert(&program, wfi()); riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); riscv_program_sx(&program, GDB_REGNO_S0, addr); riscv_program_write_ram(&program, addr + 4, 0); @@ -2366,6 +2368,8 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(testvar == riscv_read_debug_buffer_x(target, d_addr), \ "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); } + + // DCSR Tests LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); -- cgit v1.1 From e17ca3a31db6a9875d09aebd8fefac034d57d820 Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Tue, 27 Jun 2017 23:20:16 -0700 Subject: riscv: More compliance tests for core registers. --- src/target/riscv/riscv-013.c | 56 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 506c3c6..b0f73ea 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2310,14 +2310,44 @@ int riscv013_test_compliance(struct target *target) { dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0xAAAAAAAA, "COMMAND register should be R/W"); uint32_t result = dmi_read(target, DMI_ABSTRACTCS); - LOG_INFO("result is %x", result); - COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0x2, "Illegal COMMAND should result in UNSUPPORTED"); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ + "Illegal COMMAND should result in UNSUPPORTED"); dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); dmi_write(target, DMI_COMMAND, 0x55555555); COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0x55555555, "COMMAND register should be R/W"); - COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0x2, "Illegal COMMAND should result in UNSUPPORTED"); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ + "Illegal COMMAND should result in UNSUPPORTED"); dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + // Basic Abstract Commands + uint32_t command = 0; + uint32_t busy; + command = set_field(command, AC_ACCESS_REGISTER_SIZE, riscv_xlen(target) > 32 ? 3:2); + command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); + for (int i = 1 ; i < 32 ; i = i << 1) { + command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + GDB_REGNO_X0 + i); + command = set_field(command, AC_ACCESS_REGISTER_WRITE, 1); + dmi_write(target, DMI_DATA0, i); + if (riscv_xlen(target) > 32) { + dmi_write(target, DMI_DATA0 + 1, i + 1); + } + dmi_write(target, DMI_COMMAND, command); + do { busy = get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY);} while (busy); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0, "GPR Writes should be supported."); + dmi_write(target, DMI_DATA0, 0xDEADBEEF); + if (riscv_xlen(target) > 32) { + dmi_write(target, DMI_DATA0 + 1, 0xDEADBEEF); + } + command = set_field(command, AC_ACCESS_REGISTER_WRITE, 0); + dmi_write(target, DMI_COMMAND, command); + do { busy = get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY);} while (busy); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0, "GPR Reads should be supported."); + COMPLIANCE_TEST(dmi_read(target, DMI_DATA0) == i, "GPR Reads and writes should be supported."); + if (riscv_xlen(target) > 32) { + COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + 1) == (i + 1), "GPR Reads and writes should be supported."); + } + } + // ABSTRACTAUTO // See which bits are actually writable dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); @@ -2347,7 +2377,7 @@ int riscv013_test_compliance(struct target *target) { uint32_t autoexec_progbuf = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECPROGBUF); for (unsigned int i = 0; i < 12; i ++){ dmi_read(target, DMI_DATA0 + i); - while(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY)); + do { busy = get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY);} while (busy); if (autoexec_data & (1 << i)) { COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT), "AUTOEXEC may be writable up to DATACOUNT bits."); testvar ++; @@ -2355,7 +2385,7 @@ int riscv013_test_compliance(struct target *target) { } for (unsigned int i = 0; i < 16; i ++){ dmi_read(target, DMI_PROGBUF0 + i); - while(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY)); + do { busy = get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY);} while (busy); if (autoexec_progbuf & (1 << i)) { COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE), "AUTOEXEC may be writable up to PROGSIZE bits."); testvar ++; @@ -2369,7 +2399,21 @@ int riscv013_test_compliance(struct target *target) { "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); } - // DCSR Tests + // Core Register Tests + for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ + riscv_set_current_hartid(target, hartsel); + // DCSR Tests + riscv_set_register(target, GDB_REGNO_DCSR, 0x0); + COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DCSR) != 0, "Not all bits in DCSR are writable by Debugger"); + riscv_set_register(target, GDB_REGNO_DCSR, 0xFFFFFFFF); + COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DCSR) != 0, "At least some bits in DCSR must be 1"); + // DPC + uint64_t testvar64 = 0xAAAAAAAAAAAAAAAAUL; + riscv_set_register(target, GDB_REGNO_DPC, testvar64); + COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DPC) == testvar64, "DPC must be writable."); + riscv_set_register(target, GDB_REGNO_DPC, ~testvar64); + COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DPC) == ~testvar64, "DPC must be writable"); + } LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); -- cgit v1.1 From 41017409284f821b5a3d377b8fc7230c32ffe75d Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Wed, 28 Jun 2017 13:52:35 -0700 Subject: riscv: add compliance tests for DPC and DCSR --- src/target/riscv/riscv-013.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index b0f73ea..776bf56 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2402,17 +2402,23 @@ int riscv013_test_compliance(struct target *target) { // Core Register Tests for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ riscv_set_current_hartid(target, hartsel); + // DCSR Tests riscv_set_register(target, GDB_REGNO_DCSR, 0x0); COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DCSR) != 0, "Not all bits in DCSR are writable by Debugger"); riscv_set_register(target, GDB_REGNO_DCSR, 0xFFFFFFFF); COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DCSR) != 0, "At least some bits in DCSR must be 1"); + // DPC uint64_t testvar64 = 0xAAAAAAAAAAAAAAAAUL; + uint64_t dpcmask = 0xFFFFFFFFFFFFFFFFUL; + riscv_set_register(target, GDB_REGNO_DPC, dpcmask); + dpcmask = riscv_get_register(target, GDB_REGNO_PC); + COMPLIANCE_TEST(dpcmask >= 0xFFFFFFFF, "DPC must hold at least 32 bits (may hold more but hard to tell how many)"); riscv_set_register(target, GDB_REGNO_DPC, testvar64); - COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DPC) == testvar64, "DPC must be writable."); + COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == (testvar64 & dpcmask), "DPC must be writable."); riscv_set_register(target, GDB_REGNO_DPC, ~testvar64); - COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DPC) == ~testvar64, "DPC must be writable"); + COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == ((~testvar64) & dpcmask), "DPC must be writable"); } LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); -- cgit v1.1 From e32a8c911d8f90cc35b89869c82a238d0b9fff50 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 28 Jun 2017 18:58:28 -0700 Subject: riscv: Fix AUTOEXEC test for 32-bit cores --- src/target/riscv/riscv-013.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 776bf56..f82f14f 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2364,9 +2364,9 @@ int riscv013_test_compliance(struct target *target) { riscv_program_insert(&program, wfi()); riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); riscv_program_sx(&program, GDB_REGNO_S0, addr); - riscv_program_write_ram(&program, addr + 4, 0); + riscv_program_write_ram(&program, addr, 0); if (riscv_xlen(target) > 32) { - riscv_program_write_ram(&program, addr, 0); + riscv_program_write_ram(&program, addr+4, 0); } dmi_write(target, DMI_ABSTRACTAUTO, 0x0); riscv_program_exec(&program, target); -- cgit v1.1 From 434fb3708a2fef41a4107d4be68cd8e8a24c2f90 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 28 Jun 2017 19:17:48 -0700 Subject: riscv: Correct DPC masking in compliance test. --- src/target/riscv/riscv-013.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f82f14f..e5cdded 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2413,8 +2413,8 @@ int riscv013_test_compliance(struct target *target) { uint64_t testvar64 = 0xAAAAAAAAAAAAAAAAUL; uint64_t dpcmask = 0xFFFFFFFFFFFFFFFFUL; riscv_set_register(target, GDB_REGNO_DPC, dpcmask); - dpcmask = riscv_get_register(target, GDB_REGNO_PC); - COMPLIANCE_TEST(dpcmask >= 0xFFFFFFFF, "DPC must hold at least 32 bits (may hold more but hard to tell how many)"); + dpcmask = riscv_get_register(target, GDB_REGNO_DPC); + COMPLIANCE_TEST(dpcmask >= 0xFFFFFFFC, "DPC must hold the minimum for a virtual address (may tighten requirement later)."); riscv_set_register(target, GDB_REGNO_DPC, testvar64); COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == (testvar64 & dpcmask), "DPC must be writable."); riscv_set_register(target, GDB_REGNO_DPC, ~testvar64); -- cgit v1.1 From 7bc23c7776b58f0a1d2bedbe5a666633fbe514d8 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 28 Jun 2017 19:32:30 -0700 Subject: riscv: Add some comments on what else compliance test needs --- src/target/riscv/riscv-013.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index e5cdded..4b9e3e7 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2419,8 +2419,28 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == (testvar64 & dpcmask), "DPC must be writable."); riscv_set_register(target, GDB_REGNO_DPC, ~testvar64); COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == ((~testvar64) & dpcmask), "DPC must be writable"); + } + + //TODO: + // DMACTIVE + + // NDMRESET + // Asserting non-debug module reset should not reset Debug Module state. + // But it should reset Hart State, e.g. DPC should get a different value. + // Also make sure that DCSR reports cause of 'HALT' even though previously we single-stepped. + // DMCONTROL + // COMMAND + // PROGBUF + // ABSTRACTAUTO + // DATA + // TODO: HASEL, HAWINDOWSEL + + // Single-Step each hart. + + // Assert cause is SINGLESTEP + LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); if (total_tests == passed_tests) { -- cgit v1.1 From 8dc3c0a55cf5b855df215b5b3bf78aa0e98c4b00 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 28 Jun 2017 19:44:18 -0700 Subject: riscv: correct libjaylink version --- src/jtag/drivers/libjaylink | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jtag/drivers/libjaylink b/src/jtag/drivers/libjaylink index d57dee6..699b700 160000 --- a/src/jtag/drivers/libjaylink +++ b/src/jtag/drivers/libjaylink @@ -1 +1 @@ -Subproject commit d57dee67bc756291b7d8b51d350d1c6213e514f0 +Subproject commit 699b7001d34a79c8e7064503dde1bede786fd7f0 -- cgit v1.1 From 2b948881003ef0202fc2a60e7fd20606d9b18357 Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Wed, 5 Jul 2017 15:11:40 -0700 Subject: riscv: Add single-step, reset, and dmactive to the compliance test. --- src/target/riscv/riscv-013.c | 112 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 90 insertions(+), 22 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8672853..56255f9 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -447,8 +447,8 @@ static uint64_t dmi_read(struct target *target, uint16_t address) } if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("Failed read from 0x%x; value=0x%" PRIx64 ", status=%d", - address, value, status); + LOG_ERROR("Failed read from 0x%x" PRIx64 ", status=%d", + address, status); abort(); } @@ -1291,10 +1291,10 @@ static int assert_reset(struct target *target) control = set_field(control, DMI_DMCONTROL_NDMRESET, 1); if (target->reset_halt) { LOG_DEBUG("TARGET RESET HALT SET, ensuring halt is set during reset."); - control = set_field(control, DMI_DMCONTROL_HALTREQ, 1); + control |= DMI_DMCONTROL_HALTREQ; } else { LOG_DEBUG("TARGET RESET HALT NOT SET"); - control = set_field(control, DMI_DMCONTROL_HALTREQ, 0); + control &= ~DMI_DMCONTROL_HALTREQ; } dmi_write(target, DMI_DMCONTROL, control); @@ -1317,7 +1317,7 @@ static int deassert_reset(struct target *target) // Clear the reset, but make sure haltreq is still set if (target->reset_halt) { - control = set_field(control, DMI_DMCONTROL_HALTREQ, 1); + control |= DMI_DMCONTROL_HALTREQ; } control = set_field(control, DMI_DMCONTROL_NDMRESET, 0); @@ -2194,8 +2194,6 @@ int riscv013_test_compliance(struct target *target) { // TODO: ndmreset - // TODO: dmactive - // haltreq riscv_halt_all_harts(target); // Writing haltreq should not cause any problems for a halted hart, but we @@ -2423,8 +2421,17 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(testvar == riscv_read_debug_buffer_x(target, d_addr), \ "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); } - + + // Single-Step each hart. + for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ + riscv_set_current_hartid(target, hartsel); + riscv013_on_step(target); + riscv013_step_current_hart(target); + COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, "Single Step should result in SINGLESTEP"); + } + // Core Register Tests + uint64_t bogus_dpc; for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ riscv_set_current_hartid(target, hartsel); @@ -2443,28 +2450,89 @@ int riscv013_test_compliance(struct target *target) { riscv_set_register(target, GDB_REGNO_DPC, testvar64); COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == (testvar64 & dpcmask), "DPC must be writable."); riscv_set_register(target, GDB_REGNO_DPC, ~testvar64); - COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == ((~testvar64) & dpcmask), "DPC must be writable"); - + uint64_t dpc = riscv_get_register(target, GDB_REGNO_DPC); + COMPLIANCE_TEST((dpc & dpcmask) == ((~testvar64) & dpcmask), "DPC must be writable"); + if (hartsel == 0) {bogus_dpc = dpc;} // For a later test step } - - //TODO: - // DMACTIVE - + //NDMRESET // NDMRESET // Asserting non-debug module reset should not reset Debug Module state. // But it should reset Hart State, e.g. DPC should get a different value. // Also make sure that DCSR reports cause of 'HALT' even though previously we single-stepped. - // DMCONTROL - // COMMAND - // PROGBUF - // ABSTRACTAUTO - // DATA - // TODO: HASEL, HAWINDOWSEL + + // Write some registers. They should not be impacted by ndmreset. + dmi_write(target, DMI_COMMAND, 0xFFFFFFFF); + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + testvar = (i + 1) * 0x11111111; + dmi_write(target, DMI_PROGBUF0 + i, testvar); + } + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ + testvar = (i + 1) * 0x11111111; + dmi_write(target, DMI_DATA0 + i, testvar); + } - // Single-Step each hart. + dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + abstractauto = dmi_read(target, DMI_ABSTRACTAUTO); + + // Assert reset. + + target->reset_halt = true; + dmcontrol = dmi_read(target, DMI_DMCONTROL); + riscv_set_current_hartid(target, 0); + // Lie to OpenOCD and overwrite hartsel. + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HARTSEL, 0x5); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + assert_reset(target); + deassert_reset(target); + + + // Verify that most stuff is not affected by ndmreset. + COMPLIANCE_TEST(dmi_read(target, DMI_DMCONTROL) == dmcontrol, "NDMRESET should not affect DMI_DMSTATUS"); + COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0xFFFFFFFF, "NDMRESET should not affect DMI_COMMAND"); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0xFFFFFFFF, "NDMRESET should not affect DMI_ABSTRACTCS"); + COMPLIANCE_TEST(dmi_read(target, DMI_ABSTRACTAUTO) == abstractauto, "NDMRESET should not affect DMI_ABSTRACTAUTO"); + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + testvar = (i + 1) * 0x11111111; + COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == testvar, "PROGBUF words must not be affected by NDMRESET"); + } + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ + testvar = (i + 1) * 0x11111111; + COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == testvar, "DATA words must not be affected by NDMRESET"); + } + + // Stop lying to OpenOCD about which hart is selected. + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HARTSEL, 0); - // Assert cause is SINGLESTEP + // verify that DPC *is* affected by ndmreset. Since we don't know what it *should* be, + // just verify that at least it's not the bogus value anymore. + uint64_t dpc = riscv_get_register(target, GDB_REGNO_DPC); + COMPLIANCE_TEST(bogus_dpc != riscv_get_register(target, GDB_REGNO_DPC), "NDMRESET should move $DPC to reset value."); + COMPLIANCE_TEST(riscv_halt_reason(target, 0)==RISCV_HALT_INTERRUPT, "After NDMRESET halt, DCSR should report cause of halt"); + + // DMACTIVE -- deasserting DMACTIVE should reset all the above values. + + // Toggle dmactive + dmi_write(target, DMI_DMCONTROL, 0); + dmi_write(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); + COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0, "DMI_COMMAND should reset to 0"); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0xFFFFFFFF, "ABSTRACTCS.cmderr should reset to 0"); + COMPLIANCE_TEST(dmi_read(target, DMI_ABSTRACTAUTO) == 0, "ABSTRACTAUTO should reset to 0"); + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + testvar = (i + 1) * 0x11111111; + COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == 0, "PROGBUF words should reset to 0"); + } + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ + testvar = (i + 1) * 0x11111111; + COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == testvar, "DATA words should reset to 0"); + } + LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); -- cgit v1.1 From bdc38561c089810ce655c38fefd8ccfaebb561aa Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Wed, 5 Jul 2017 17:54:55 -0700 Subject: riscv: Clean up reset/dmactive/step compliance test --- src/target/riscv/riscv-013.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 56255f9..2a97dd6 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2224,7 +2224,10 @@ int riscv013_test_compliance(struct target *target) { do { dmstatus = dmi_read(target, DMI_DMSTATUS); } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); - + dmcontrol &= ~DMI_DMCONTROL_HALTREQ; + dmi_write(target, DMI_DMCONTROL, dmcontrol); + // Not clear that this read is required according to the spec. + dmi_read(target, DMI_DMSTATUS); // HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++){ @@ -2477,22 +2480,17 @@ int riscv013_test_compliance(struct target *target) { dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); abstractauto = dmi_read(target, DMI_ABSTRACTAUTO); - // Assert reset. + // Pulse reset. target->reset_halt = true; dmcontrol = dmi_read(target, DMI_DMCONTROL); riscv_set_current_hartid(target, 0); - // Lie to OpenOCD and overwrite hartsel. - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HARTSEL, 0x5); - dmi_write(target, DMI_DMCONTROL, dmcontrol); assert_reset(target); deassert_reset(target); - // Verify that most stuff is not affected by ndmreset. - COMPLIANCE_TEST(dmi_read(target, DMI_DMCONTROL) == dmcontrol, "NDMRESET should not affect DMI_DMSTATUS"); COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0xFFFFFFFF, "NDMRESET should not affect DMI_COMMAND"); - COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0xFFFFFFFF, "NDMRESET should not affect DMI_ABSTRACTCS"); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, "NDMRESET should not affect DMI_ABSTRACTCS"); COMPLIANCE_TEST(dmi_read(target, DMI_ABSTRACTAUTO) == abstractauto, "NDMRESET should not affect DMI_ABSTRACTAUTO"); for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ @@ -2505,14 +2503,11 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == testvar, "DATA words must not be affected by NDMRESET"); } - // Stop lying to OpenOCD about which hart is selected. - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HARTSEL, 0); - // verify that DPC *is* affected by ndmreset. Since we don't know what it *should* be, // just verify that at least it's not the bogus value anymore. uint64_t dpc = riscv_get_register(target, GDB_REGNO_DPC); COMPLIANCE_TEST(bogus_dpc != riscv_get_register(target, GDB_REGNO_DPC), "NDMRESET should move $DPC to reset value."); - COMPLIANCE_TEST(riscv_halt_reason(target, 0)==RISCV_HALT_INTERRUPT, "After NDMRESET halt, DCSR should report cause of halt"); + COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, "After NDMRESET halt, DCSR should report cause of halt"); // DMACTIVE -- deasserting DMACTIVE should reset all the above values. @@ -2520,7 +2515,7 @@ int riscv013_test_compliance(struct target *target) { dmi_write(target, DMI_DMCONTROL, 0); dmi_write(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0, "DMI_COMMAND should reset to 0"); - COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0xFFFFFFFF, "ABSTRACTCS.cmderr should reset to 0"); + COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0, "ABSTRACTCS.cmderr should reset to 0"); COMPLIANCE_TEST(dmi_read(target, DMI_ABSTRACTAUTO) == 0, "ABSTRACTAUTO should reset to 0"); for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ @@ -2530,7 +2525,7 @@ int riscv013_test_compliance(struct target *target) { for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ testvar = (i + 1) * 0x11111111; - COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == testvar, "DATA words should reset to 0"); + COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == 0, "DATA words should reset to 0"); } -- cgit v1.1 From 6842fd2c105fc83fc2a50cefbdbd3fda56c2b83e Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Wed, 5 Jul 2017 17:59:30 -0700 Subject: riscv: Add more TODO compliance comments --- src/target/riscv/riscv-013.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 2a97dd6..1c49b3f 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2527,7 +2527,13 @@ int riscv013_test_compliance(struct target *target) { testvar = (i + 1) * 0x11111111; COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == 0, "DATA words should reset to 0"); } - + + //TODO: + // DCSR.cause priorities + // DCSR.stoptime/stopcycle + // DCSR.stepie + // DCSR.ebreak + // DCSR.prv LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); -- cgit v1.1 From 66fa38add7864551b7b5f5c1f682e5675f7668a1 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 12 Jul 2017 18:44:41 -0700 Subject: riscv-compliance: Halt harts again at the end of the test. --- src/target/riscv/riscv-013.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 1c49b3f..336295e 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2535,6 +2535,9 @@ int riscv013_test_compliance(struct target *target) { // DCSR.ebreak // DCSR.prv + /* Halt every hart for any follow-up tests*/ + riscv_halt_all_harts(target); + LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); if (total_tests == passed_tests) { -- cgit v1.1 From c8015e8dc1efd588d89c8ea04ee814f108176551 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 13 Jul 2017 07:57:57 -0700 Subject: riscv compliance: More post-test cleanup --- src/target/riscv/riscv-013.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 1b8f15f..2ef4ac1 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2667,6 +2667,10 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0xFFFFFFFF, "NDMRESET should not affect DMI_COMMAND"); COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, "NDMRESET should not affect DMI_ABSTRACTCS"); COMPLIANCE_TEST(dmi_read(target, DMI_ABSTRACTAUTO) == abstractauto, "NDMRESET should not affect DMI_ABSTRACTAUTO"); + + // Clean up to avoid future test failures + dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + dmi_write(target, DMI_ABSTRACTAUTO, 0); for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ testvar = (i + 1) * 0x11111111; -- cgit v1.1 From 3ec1772c9673b92d402321b8fa91d4b6eadb5cec Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 15 Aug 2017 15:55:09 -0700 Subject: riscv: Add commands for setting timeouts --- src/target/riscv/riscv-011.c | 47 +++++++++++----------- src/target/riscv/riscv-013.c | 19 ++++----- src/target/riscv/riscv.c | 94 ++++++++++++++++++++++++++++++++++---------- src/target/riscv/riscv.h | 9 +++++ 4 files changed, 118 insertions(+), 51 deletions(-) diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index b126cf6..6b6666b 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -39,26 +39,26 @@ * * There are a few functions to just instantly shift a register and get its * value: - * dtmcontrol_scan - * idcode_scan - * dbus_scan + * dtmcontrol_scan + * idcode_scan + * dbus_scan * * Because doing one scan and waiting for the result is slow, most functions * batch up a bunch of dbus writes and then execute them all at once. They use * the scans "class" for this: - * scans_new - * scans_delete - * scans_execute - * scans_add_... + * scans_new + * scans_delete + * scans_execute + * scans_add_... * Usually you new(), call a bunch of add functions, then execute() and look * at the results by calling scans_get...() * * Optimized functions will directly use the scans class above, but slightly * lazier code will use the cache functions that in turn use the scans * functions: - * cache_get... - * cache_set... - * cache_write + * cache_get... + * cache_set... + * cache_write * cache_set... update a local structure, which is then synced to the target * with cache_write(). Only Debug RAM words that are actually changed are sent * to the target. Afterwards use cache_get... to read results. @@ -80,10 +80,10 @@ #define CSR_BPCONTROL_BPMATCH (0xf<<7) #define CSR_BPCONTROL_BPACTION (0xff<<11) -#define DEBUG_ROM_START 0x800 -#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) -#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) -#define DEBUG_RAM_START 0x400 +#define DEBUG_ROM_START 0x800 +#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) +#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) +#define DEBUG_RAM_START 0x400 #define SETHALTNOT 0x10c @@ -154,7 +154,6 @@ typedef enum slot { /*** Info about the core being debugged. ***/ #define DBUS_ADDRESS_UNKNOWN 0xffff -#define WALL_CLOCK_TIMEOUT 2 // gdb's register list is defined in riscv_gdb_reg_names gdb/riscv-tdep.c in // its source tree. We must interpret the numbers the same here. @@ -730,8 +729,9 @@ static int wait_for_debugint_clear(struct target *target, bool ignore_first) if (!bits.interrupt) { return ERROR_OK; } - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { - LOG_ERROR("Timed out waiting for debug int to clear."); + if (time(NULL) - start > riscv_command_timeout_sec) { + LOG_ERROR("Timed out waiting for debug int to clear." + "Increase timeout with riscv set_command_timeout_sec."); return ERROR_FAIL; } } @@ -864,7 +864,7 @@ static int cache_write(struct target *target, unsigned int address, bool run) if (last == info->dramsize) { // Nothing needs to be written to RAM. - dbus_write(target, DMCONTROL, DMCONTROL_HALTNOT | (run ? DMCONTROL_INTERRUPT : 0)); + dbus_write(target, DMCONTROL, DMCONTROL_HALTNOT | (run ? DMCONTROL_INTERRUPT : 0)); } else { for (unsigned int i = 0; i < info->dramsize; i++) { @@ -1016,8 +1016,9 @@ static int wait_for_state(struct target *target, enum target_state state) if (target->state == state) { return ERROR_OK; } - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { - LOG_ERROR("Timed out waiting for state %d.", state); + if (time(NULL) - start > riscv_command_timeout_sec) { + LOG_ERROR("Timed out waiting for state %d. " + "Increase timeout with riscv set_command_timeout_sec.", state); return ERROR_FAIL; } } @@ -1174,8 +1175,9 @@ static int full_step(struct target *target, bool announce) return result; if (target->state != TARGET_DEBUG_RUNNING) break; - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { - LOG_ERROR("Timed out waiting for step to complete."); + if (time(NULL) - start > riscv_command_timeout_sec) { + LOG_ERROR("Timed out waiting for step to complete." + "Increase timeout with riscv set_command_timeout_sec"); return ERROR_FAIL; } } @@ -1471,6 +1473,7 @@ static int init_target(struct command_context *cmd_ctx, riscv_info_t *generic_info = (riscv_info_t *) target->arch_info; generic_info->get_register = get_register; generic_info->set_register = set_register; + generic_info->version_specific = calloc(1, sizeof(riscv011_info_t)); if (!generic_info->version_specific) return ERROR_FAIL; diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index a45fcd3..30b4d8d 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -122,9 +122,6 @@ typedef enum slot { /*** Info about the core being debugged. ***/ -#define WALL_CLOCK_TIMEOUT 2 -#define WALL_CLOCK_RESET_TIMEOUT 30 - struct trigger { uint64_t address; uint32_t length; @@ -563,7 +560,7 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) return ERROR_OK; } - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { + if (time(NULL) - start > riscv_command_timeout_sec) { info->cmderr = get_field(*abstractcs, DMI_ABSTRACTCS_CMDERR); if (info->cmderr != CMDERR_NONE) { const char *errors[8] = { @@ -580,8 +577,10 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) errors[info->cmderr], *abstractcs); } - LOG_ERROR("Timed out waiting for busy to go low. (abstractcs=0x%x)", - *abstractcs); + LOG_ERROR("Timed out after %ds waiting for busy to go low. (abstractcs=0x%x)" + "Increase the timeout with riscv set_command_timeout_sec.", + riscv_command_timeout_sec, + *abstractcs); return ERROR_FAIL; } } @@ -911,7 +910,7 @@ static int init_target(struct command_context *cmd_ctx, generic_info->dmi_write_u64_bits = &riscv013_dmi_write_u64_bits; generic_info->reset_current_hart = &riscv013_reset_current_hart; generic_info->test_compliance = &riscv013_test_compliance; - + generic_info->version_specific = calloc(1, sizeof(riscv013_info_t)); if (!generic_info->version_specific) return ERROR_FAIL; @@ -1894,9 +1893,11 @@ void riscv013_reset_current_hart(struct target *target) if (get_field(dmstatus, DMI_DMSTATUS_ALLHALTED)) { break; } - if (time(NULL) - start > WALL_CLOCK_RESET_TIMEOUT) { + if (time(NULL) - start > riscv_reset_timeout_sec) { LOG_ERROR("Hart didn't halt coming out of reset in %ds; " - "dmstatus=0x%x", WALL_CLOCK_RESET_TIMEOUT, dmstatus); + "dmstatus=0x%x" + "Increase the timeout with riscv set_reset_timeout_sec.", + riscv_reset_timeout_sec, dmstatus); return; } } diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index d6c2bbd..fca2021 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -29,32 +29,32 @@ * Code structure * * At the bottom of the stack are the OpenOCD JTAG functions: - * jtag_add_[id]r_scan - * jtag_execute_query - * jtag_add_runtest + * jtag_add_[id]r_scan + * jtag_execute_query + * jtag_add_runtest * * There are a few functions to just instantly shift a register and get its * value: - * dtmcontrol_scan - * idcode_scan - * dbus_scan + * dtmcontrol_scan + * idcode_scan + * dbus_scan * * Because doing one scan and waiting for the result is slow, most functions * batch up a bunch of dbus writes and then execute them all at once. They use * the scans "class" for this: - * scans_new - * scans_delete - * scans_execute - * scans_add_... + * scans_new + * scans_delete + * scans_execute + * scans_add_... * Usually you new(), call a bunch of add functions, then execute() and look * at the results by calling scans_get...() * * Optimized functions will directly use the scans class above, but slightly * lazier code will use the cache functions that in turn use the scans * functions: - * cache_get... - * cache_set... - * cache_write + * cache_get... + * cache_set... + * cache_write * cache_set... update a local structure, which is then synced to the target * with cache_write(). Only Debug RAM words that are actually changed are sent * to the target. Afterwards use cache_get... to read results. @@ -77,8 +77,8 @@ #define CSR_BPCONTROL_BPACTION (0xff<<11) #define DEBUG_ROM_START 0x800 -#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) -#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) +#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) +#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) #define DEBUG_RAM_START 0x400 #define SETHALTNOT 0x10c @@ -150,7 +150,6 @@ typedef enum slot { /*** Info about the core being debugged. ***/ #define DBUS_ADDRESS_UNKNOWN 0xffff -#define WALL_CLOCK_TIMEOUT 2 // gdb's register list is defined in riscv_gdb_reg_names gdb/riscv-tdep.c in // its source tree. We must interpret the numbers the same here. @@ -195,6 +194,12 @@ struct trigger { int unique_id; }; +/* Wall-clock timeout for a command/access. Settable via RISC-V Target commands.*/ +int riscv_command_timeout_sec = DEFAULT_COMMAND_TIMEOUT_SEC; + +/* Wall-clock timeout after reset. Settable via RISC-V Target commands.*/ +int riscv_reset_timeout_sec = DEFAULT_RESET_TIMEOUT_SEC; + static uint32_t dtmcontrol_scan(struct target *target, uint32_t out) { struct scan_field field; @@ -997,7 +1002,7 @@ int riscv_openocd_poll(struct target *target) /* If we're here then at least one hart triggered. That means * we want to go and halt _every_ hart in the system, as that's - * the invariant we hold here. Some harts might have already + * the invariant we hold here. Some harts might have already * halted (as we're either in single-step mode or they also * triggered a breakpoint), so don't attempt to halt those * harts. */ @@ -1171,7 +1176,7 @@ struct target_type riscv_target = .run_algorithm = riscv_run_algorithm, - .commands = riscv_command_handlers + .commands = riscv_command_handlers }; /*** RISC-V Interface ***/ @@ -1617,7 +1622,7 @@ COMMAND_HANDLER(riscv_test_compliance) { if (CMD_ARGC > 0) { LOG_ERROR("Command does not take any parameters."); - return ERROR_FAIL; + return ERROR_COMMAND_SYNTAX_ERROR; } if (r->test_compliance) { @@ -1626,16 +1631,65 @@ COMMAND_HANDLER(riscv_test_compliance) { LOG_ERROR("This target does not support this command (may implement an older version of the spec)."); return ERROR_FAIL; } + +} + +COMMAND_HANDLER(riscv_set_command_timeout_sec) { + + if (CMD_ARGC != 1) { + LOG_ERROR("Command takes exactly 1 parameter"); + return ERROR_COMMAND_SYNTAX_ERROR; + } + int timeout = atoi(CMD_ARGV[0]); + if (timeout <= 0){ + LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); + return ERROR_FAIL; + } + + riscv_command_timeout_sec = timeout; + + return ERROR_OK; } +COMMAND_HANDLER(riscv_set_reset_timeout_sec) { + + if (CMD_ARGC != 1) { + LOG_ERROR("Command takes exactly 1 parameter"); + return ERROR_COMMAND_SYNTAX_ERROR; + } + int timeout = atoi(CMD_ARGV[0]); + if (timeout <= 0){ + LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); + return ERROR_FAIL; + } + + riscv_reset_timeout_sec = timeout; + return ERROR_OK; +} + + static const struct command_registration riscv_exec_command_handlers[] = { { .name = "test_compliance", .handler = riscv_test_compliance, .mode = COMMAND_EXEC, - .usage = "", + .usage = "riscv test_compliance", .help = "Runs a basic compliance test suite against the RISC-V Debug Spec." }, + { + .name = "set_command_timeout_sec", + .handler = riscv_set_command_timeout_sec, + .mode = COMMAND_ANY, + .usage = "riscv set_command_timeout_sec [sec]", + .help = "Set the wall-clock timeout (in seconds) for individual commands" + }, + { + .name = "set_reset_timeout_sec", + .handler = riscv_set_reset_timeout_sec, + .mode = COMMAND_ANY, + .usage = "riscv set_reset_timeout_sec [sec]", + .help = "Set the wall-clock timeout (in seconds) after reset is deasserted" + }, COMMAND_REGISTRATION_DONE }; diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index de0ebc5..e1110b1 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -13,6 +13,9 @@ struct riscv_program; #define RISCV_MAX_TRIGGERS 32 #define RISCV_MAX_HWBPS 16 +#define DEFAULT_COMMAND_TIMEOUT_SEC 2 +#define DEFAULT_RESET_TIMEOUT_SEC 30 + extern struct target_type riscv011_target; extern struct target_type riscv013_target; @@ -104,6 +107,12 @@ typedef struct { int (*test_compliance)(struct target *target); } riscv_info_t; +/* Wall-clock timeout for a command/access. Settable via RISC-V Target commands.*/ +extern int riscv_command_timeout_sec; + +/* Wall-clock timeout after reset. Settable via RISC-V Target commands.*/ +extern int riscv_reset_timeout_sec; + /* Everything needs the RISC-V specific info structure, so here's a nice macro * that provides that. */ static inline riscv_info_t *riscv_info(const struct target *target) __attribute__((unused)); -- cgit v1.1 From 9f56a9643dea11bb8e2ff5be6db88b5b8ca4aae8 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 2 Nov 2017 09:34:43 -0700 Subject: riscv-compliance: remove some compile warnings --- src/target/riscv/riscv-013.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 9d39acb..eeaacef 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2229,7 +2229,7 @@ int riscv013_test_compliance(struct target *target) { for (int i = 0; i < riscv_count_harts(target); i +=32){ uint32_t haltstat = dmi_read(target, 0x40 + (i / 32)); - uint32_t haltstat_expected = (((i + 1) * 32) <= riscv_count_harts(target)) ? 0xFFFFFFFF : ((1 << (riscv_count_harts(target) % 32)) - 1); + uint32_t haltstat_expected = (((i + 1) * 32) <= riscv_count_harts(target)) ? 0xFFFFFFFFU : ((1U << (riscv_count_harts(target) % 32)) - 1); COMPLIANCE_TEST(haltstat == haltstat_expected, "Halt Status Registers should report all halted harts."); } @@ -2276,7 +2276,6 @@ int riscv013_test_compliance(struct target *target) { // But at any rate, this is not legal and should cause an error. dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0xAAAAAAAA, "COMMAND register should be R/W"); - uint32_t result = dmi_read(target, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ "Illegal COMMAND should result in UNSUPPORTED"); dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); @@ -2291,7 +2290,7 @@ int riscv013_test_compliance(struct target *target) { uint32_t busy; command = set_field(command, AC_ACCESS_REGISTER_SIZE, riscv_xlen(target) > 32 ? 3:2); command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); - for (int i = 1 ; i < 32 ; i = i << 1) { + for (unsigned int i = 1 ; i < 32 ; i = i << 1) { command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + GDB_REGNO_X0 + i); command = set_field(command, AC_ACCESS_REGISTER_WRITE, 1); dmi_write(target, DMI_DATA0, i); @@ -2375,7 +2374,7 @@ int riscv013_test_compliance(struct target *target) { } // Core Register Tests - uint64_t bogus_dpc; + uint64_t bogus_dpc = 0xdeadbeef; for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ riscv_set_current_hartid(target, hartsel); @@ -2450,8 +2449,9 @@ int riscv013_test_compliance(struct target *target) { // verify that DPC *is* affected by ndmreset. Since we don't know what it *should* be, // just verify that at least it's not the bogus value anymore. - uint64_t dpc = riscv_get_register(target, GDB_REGNO_DPC); + COMPLIANCE_TEST(bogus_dpc != 0xdeadbeef, "BOGUS DPC should have been set somehow (bug in compliance test)"); COMPLIANCE_TEST(bogus_dpc != riscv_get_register(target, GDB_REGNO_DPC), "NDMRESET should move $DPC to reset value."); + COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, "After NDMRESET halt, DCSR should report cause of halt"); // DMACTIVE -- deasserting DMACTIVE should reset all the above values. -- cgit v1.1 From 41efb5b96480f23a9b81805117944d82f68ba710 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 13 Feb 2018 11:03:52 -0800 Subject: riscv-compliance: Fix libjaylink version --- src/jtag/drivers/libjaylink | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jtag/drivers/libjaylink b/src/jtag/drivers/libjaylink index 699b700..8645845 160000 --- a/src/jtag/drivers/libjaylink +++ b/src/jtag/drivers/libjaylink @@ -1 +1 @@ -Subproject commit 699b7001d34a79c8e7064503dde1bede786fd7f0 +Subproject commit 8645845c1abebd004e991ba9a7f808f4fd0c608b -- cgit v1.1 From 88370b398911d67602627b20c85ec039c9dc9ed9 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 13 Feb 2018 11:44:53 -0800 Subject: riscv-compliance: fix some macros which were renamed --- src/target/riscv/riscv-013.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index cd4254c..f6de5d8 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2222,15 +2222,15 @@ int riscv013_test_compliance(struct target *target) { uint32_t dmcontrol_orig = dmi_read(target, DMI_DMCONTROL); uint32_t dmcontrol; uint32_t testvar; - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSEL, RISCV_MAX_HARTS-1); + dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), RISCV_MAX_HARTS-1); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmcontrol = dmi_read(target, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSEL) == (RISCV_MAX_HARTS-1), "DMCONTROL.hartsel should hold MAX_HARTS - 1"); - - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSEL, 0); + COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == (RISCV_MAX_HARTS-1), "DMCONTROL.hartsel should hold all the harts allowed by HARTSELLEN."); + + dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmcontrol = dmi_read(target, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSEL) == 0, "DMCONTROL.hartsel should hold Hart ID 0"); + COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == 0, "DMCONTROL.hartsel should hold Hart ID 0"); // hartreset dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); @@ -2377,12 +2377,12 @@ int riscv013_test_compliance(struct target *target) { // Check that all reported ProgBuf words are really R/W for (int invert = 0; invert < 2; invert++) { - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; if (invert) {testvar = ~testvar;} dmi_write(target, DMI_PROGBUF0 + i, testvar); } - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; if (invert) {testvar = ~testvar;} COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == testvar, "All reported PROGBUF words must be R/W"); @@ -2473,7 +2473,7 @@ int riscv013_test_compliance(struct target *target) { dmi_read(target, DMI_PROGBUF0 + i); do { busy = get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_BUSY);} while (busy); if (autoexec_progbuf & (1 << i)) { - COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE), "AUTOEXEC may be writable up to PROGSIZE bits."); + COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE), "AUTOEXEC may be writable up to PROGBUFSIZE bits."); testvar ++; } } @@ -2527,7 +2527,7 @@ int riscv013_test_compliance(struct target *target) { // Write some registers. They should not be impacted by ndmreset. dmi_write(target, DMI_COMMAND, 0xFFFFFFFF); - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; dmi_write(target, DMI_PROGBUF0 + i, testvar); } @@ -2557,7 +2557,7 @@ int riscv013_test_compliance(struct target *target) { dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); dmi_write(target, DMI_ABSTRACTAUTO, 0); - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == testvar, "PROGBUF words must not be affected by NDMRESET"); } @@ -2583,7 +2583,7 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0, "ABSTRACTCS.cmderr should reset to 0"); COMPLIANCE_TEST(dmi_read(target, DMI_ABSTRACTAUTO) == 0, "ABSTRACTAUTO should reset to 0"); - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGSIZE); i ++){ + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == 0, "PROGBUF words should reset to 0"); } -- cgit v1.1 From 2e525e391fcb1fb838923a597d7a3493deae0794 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 13 Feb 2018 13:01:56 -0800 Subject: riscv-compliance: get it compiling against riscv branch again --- src/target/riscv/program.h | 4 +-- src/target/riscv/riscv-013.c | 71 ++++++++++++++++++++------------------------ 2 files changed, 35 insertions(+), 40 deletions(-) diff --git a/src/target/riscv/program.h b/src/target/riscv/program.h index d641be1..310460c 100644 --- a/src/target/riscv/program.h +++ b/src/target/riscv/program.h @@ -52,8 +52,8 @@ int riscv_program_insert(struct riscv_program *p, riscv_insn_t i); * memory. */ int riscv_program_save_to_dscratch(struct riscv_program *p, enum gdb_regno to_save); -/* Helpers to assembly various instructions. Return 0 on success. These might - * assembly into a multi-instruction sequence that overwrites some other +/* Helpers to assemble various instructions. Return 0 on success. These might + * assemble into a multi-instruction sequence that overwrites some other * register, but those will be properly saved and restored. */ int riscv_program_lwr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno a, int o); int riscv_program_lhr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno a, int o); diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f6de5d8..8564e75 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2222,6 +2222,8 @@ int riscv013_test_compliance(struct target *target) { uint32_t dmcontrol_orig = dmi_read(target, DMI_DMCONTROL); uint32_t dmcontrol; uint32_t testvar; + riscv_reg_t value; + dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), RISCV_MAX_HARTS-1); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmcontrol = dmi_read(target, DMI_DMCONTROL); @@ -2301,32 +2303,25 @@ int riscv013_test_compliance(struct target *target) { uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); for (unsigned int d = 0; d < nscratch; d++) { - for (testvar = 0x00112233; testvar != 0xDEAD ; testvar = testvar == 0x00112233 ? ~testvar : 0xDEAD ) { - - struct riscv_program program32; + //TODO: DSCRATCH CSRs should be 64-bit on 64-bit systems. + riscv_reg_t testval; + for (testval = 0x0011223300112233; testval != 0xDEAD ; testval = testval == 0x0011223300112233 ? ~testval : 0xDEAD ) { + COMPLIANCE_TEST(register_write_direct(target, GDB_REGNO_S0, testval) == ERROR_OK, "Need to be able to write S0 in order to test DSCRATCH."); + struct riscv_program program32; riscv_program_init(&program32, target); - riscv_addr_t addr_in = riscv_program_alloc_x(&program32); - riscv_addr_t addr_out = riscv_program_alloc_x(&program32); - - riscv_program_write_ram(&program32, addr_in, testvar); - if (riscv_xlen(target) > 32) { - riscv_program_write_ram(&program32, addr_in + 4, testvar); - } - - riscv_program_lx(&program32, GDB_REGNO_S0, addr_in); - riscv_program_csrrw(&program32, GDB_REGNO_S0, GDB_REGNO_S0, GDB_REGNO_DSCRATCH); - riscv_program_csrrw(&program32, GDB_REGNO_S1, GDB_REGNO_S1, GDB_REGNO_DSCRATCH); - riscv_program_sx(&program32, GDB_REGNO_S1, addr_out); + riscv_program_csrw(&program32, GDB_REGNO_S0, GDB_REGNO_DSCRATCH + d); + riscv_program_csrr(&program32, GDB_REGNO_S1, GDB_REGNO_DSCRATCH + d); riscv_program_fence(&program32); - COMPLIANCE_TEST(riscv_program_exec(&program32, target) == ERROR_OK, "Accessing DSCRATCH with program buffer should succeed."); - - COMPLIANCE_TEST(riscv_program_read_ram(&program32, addr_out) == testvar, "All DSCRATCH registers in HARTINFO must be R/W."); + riscv_program_ebreak(&program32); + COMPLIANCE_TEST(riscv_program_exec(&program32, target) == ERROR_OK, "Accessing DSCRATCH with program buffer should succeed."); + COMPLIANCE_TEST(register_read_direct(target, &value, GDB_REGNO_S1) == ERROR_OK, "Need to be able to read S1 in order to test DSCRATCH."); if (riscv_xlen(target) > 32) { - COMPLIANCE_TEST(riscv_program_read_ram(&program32, addr_out + 4) == testvar, "All DSCRATCH registers in HARTINFO must be R/W."); - } + COMPLIANCE_TEST(value == testval, "All DSCRATCH registers in HARTINFO must be R/W."); + } else { + COMPLIANCE_TEST(value == (testval & 0xFFFFFFFF), "All DSCRATCH registers in HARTINFO must be R/W."); + } } } - // TODO: dataaccess if (get_field(hartinfo, DMI_HARTINFO_DATAACCESS)) { // TODO: Shadowed in memory map. @@ -2411,7 +2406,7 @@ int riscv013_test_compliance(struct target *target) { command = set_field(command, AC_ACCESS_REGISTER_SIZE, riscv_xlen(target) > 32 ? 3:2); command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); for (unsigned int i = 1 ; i < 32 ; i = i << 1) { - command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + GDB_REGNO_X0 + i); + command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + GDB_REGNO_ZERO + i); command = set_field(command, AC_ACCESS_REGISTER_WRITE, 1); dmi_write(target, DMI_DATA0, i); if (riscv_xlen(target) > 32) { @@ -2442,18 +2437,13 @@ int riscv013_test_compliance(struct target *target) { testvar = 0; // TODO: This mechanism only works when you have a reasonable sized progbuf, which is not // a true compliance requirement. + COMPLIANCE_TEST(riscv_set_register(target, GDB_REGNO_S0, 0) == ERROR_OK, "Need to be able to write S0 to test ABSTRACTAUTO"); struct riscv_program program; riscv_program_init(&program, target); - riscv_addr_t addr = riscv_program_alloc_x(&program); - riscv_program_lx(&program, GDB_REGNO_S0, addr); // Also testing that WFI() is a NOP during debug mode. riscv_program_insert(&program, wfi()); riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); - riscv_program_sx(&program, GDB_REGNO_S0, addr); - riscv_program_write_ram(&program, addr, 0); - if (riscv_xlen(target) > 32) { - riscv_program_write_ram(&program, addr+4, 0); - } + riscv_program_ebreak(&program); dmi_write(target, DMI_ABSTRACTAUTO, 0x0); riscv_program_exec(&program, target); testvar ++; @@ -2479,9 +2469,9 @@ int riscv013_test_compliance(struct target *target) { } dmi_write(target, DMI_ABSTRACTAUTO, 0); - int d_addr = (addr - riscv_debug_buffer_addr(target))/4 ; + riscv_get_register(target, &value, GDB_REGNO_S0); - COMPLIANCE_TEST(testvar == riscv_read_debug_buffer_x(target, d_addr), \ + COMPLIANCE_TEST(testvar == value, \ "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); } @@ -2500,20 +2490,24 @@ int riscv013_test_compliance(struct target *target) { // DCSR Tests riscv_set_register(target, GDB_REGNO_DCSR, 0x0); - COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DCSR) != 0, "Not all bits in DCSR are writable by Debugger"); + riscv_get_register(target, &value, GDB_REGNO_DCSR); + COMPLIANCE_TEST(value != 0, "Not all bits in DCSR are writable by Debugger"); riscv_set_register(target, GDB_REGNO_DCSR, 0xFFFFFFFF); - COMPLIANCE_TEST(riscv_get_register(target, GDB_REGNO_DCSR) != 0, "At least some bits in DCSR must be 1"); + riscv_get_register(target, &value, GDB_REGNO_DCSR); + COMPLIANCE_TEST(value != 0, "At least some bits in DCSR must be 1"); // DPC uint64_t testvar64 = 0xAAAAAAAAAAAAAAAAUL; - uint64_t dpcmask = 0xFFFFFFFFFFFFFFFFUL; + riscv_reg_t dpcmask = 0xFFFFFFFFFFFFFFFFUL; riscv_set_register(target, GDB_REGNO_DPC, dpcmask); - dpcmask = riscv_get_register(target, GDB_REGNO_DPC); + riscv_get_register(target, &dpcmask, GDB_REGNO_DPC); COMPLIANCE_TEST(dpcmask >= 0xFFFFFFFC, "DPC must hold the minimum for a virtual address (may tighten requirement later)."); riscv_set_register(target, GDB_REGNO_DPC, testvar64); - COMPLIANCE_TEST((riscv_get_register(target, GDB_REGNO_DPC) & dpcmask) == (testvar64 & dpcmask), "DPC must be writable."); + riscv_get_register(target, &value, GDB_REGNO_DPC); + COMPLIANCE_TEST((value & dpcmask) == (testvar64 & dpcmask), "DPC must be writable."); riscv_set_register(target, GDB_REGNO_DPC, ~testvar64); - uint64_t dpc = riscv_get_register(target, GDB_REGNO_DPC); + riscv_reg_t dpc; + riscv_get_register(target, &dpc, GDB_REGNO_DPC); COMPLIANCE_TEST((dpc & dpcmask) == ((~testvar64) & dpcmask), "DPC must be writable"); if (hartsel == 0) {bogus_dpc = dpc;} // For a later test step } @@ -2570,7 +2564,8 @@ int riscv013_test_compliance(struct target *target) { // verify that DPC *is* affected by ndmreset. Since we don't know what it *should* be, // just verify that at least it's not the bogus value anymore. COMPLIANCE_TEST(bogus_dpc != 0xdeadbeef, "BOGUS DPC should have been set somehow (bug in compliance test)"); - COMPLIANCE_TEST(bogus_dpc != riscv_get_register(target, GDB_REGNO_DPC), "NDMRESET should move $DPC to reset value."); + riscv_get_register(target, &value, GDB_REGNO_DPC); + COMPLIANCE_TEST(bogus_dpc != value, "NDMRESET should move DPC to reset value."); COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, "After NDMRESET halt, DCSR should report cause of halt"); -- cgit v1.1 From 313885cb3bf3278b7a6600a942c7d13b984580c6 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 13 Feb 2018 13:06:44 -0800 Subject: riscv-compliance: whitespace fixes --- src/target/riscv/riscv-013.c | 64 +++++++++++++++++++++----------------------- src/target/riscv/riscv.c | 2 +- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8564e75..c6f60b3 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1975,7 +1975,7 @@ static int riscv013_halt_current_hart(struct target *target) /* Issue the halt command, and then wait for the current hart to halt. */ uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL); - dmcontrol |= DMI_DMCONTROL_HALTREQ; + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HALTREQ, 1); dmi_write(target, DMI_DMCONTROL, dmcontrol); for (size_t i = 0; i < 256; ++i) if (riscv_is_halted(target)) @@ -1991,7 +1991,7 @@ static int riscv013_halt_current_hart(struct target *target) return ERROR_FAIL; } - dmcontrol &= ~DMI_DMCONTROL_HALTREQ; + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HALTREQ, 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); return ERROR_OK; @@ -2207,7 +2207,7 @@ void riscv013_clear_abstract_error(struct target *target) assert(pass); \ total_tests ++; \ } - + int riscv013_test_compliance(struct target *target) { LOG_INFO("Testing Compliance against RISC-V Debug Spec v0.13"); @@ -2223,12 +2223,12 @@ int riscv013_test_compliance(struct target *target) { uint32_t dmcontrol; uint32_t testvar; riscv_reg_t value; - + dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), RISCV_MAX_HARTS-1); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmcontrol = dmi_read(target, DMI_DMCONTROL); COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == (RISCV_MAX_HARTS-1), "DMCONTROL.hartsel should hold all the harts allowed by HARTSELLEN."); - + dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmcontrol = dmi_read(target, DMI_DMCONTROL); @@ -2254,9 +2254,7 @@ int riscv013_test_compliance(struct target *target) { dmcontrol = dmi_read(target, DMI_DMCONTROL); COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HASEL)) == 0), "DMCONTROL.hasel can be 0 or RW."); //TODO: test that hamask registers exist if hasel does. - - // TODO: ndmreset - + // haltreq riscv_halt_all_harts(target); // Writing haltreq should not cause any problems for a halted hart, but we @@ -2279,7 +2277,7 @@ int riscv013_test_compliance(struct target *target) { do { dmstatus = dmi_read(target, DMI_DMSTATUS); } while (get_field(dmstatus, DMI_DMSTATUS_ALLRESUMEACK) == 0); - + // Halt the hart again because the target isn't aware that we resumed it. dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0); dmcontrol |= DMI_DMCONTROL_HALTREQ; @@ -2291,15 +2289,15 @@ int riscv013_test_compliance(struct target *target) { dmi_write(target, DMI_DMCONTROL, dmcontrol); // Not clear that this read is required according to the spec. dmi_read(target, DMI_DMSTATUS); - + // HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++){ riscv_set_current_hartid(target, hartsel); - + uint32_t hartinfo = dmi_read(target, DMI_HARTINFO); dmi_write (target, DMI_HARTINFO, ~hartinfo); COMPLIANCE_TEST((dmi_read(target, DMI_HARTINFO) == hartinfo), "DMHARTINFO should be Read-Only."); - + uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); for (unsigned int d = 0; d < nscratch; d++) { @@ -2332,7 +2330,7 @@ int riscv013_test_compliance(struct target *target) { // TODO: datasize // TODO: dataaddr } - + } // HALTSUM @@ -2349,7 +2347,7 @@ int riscv013_test_compliance(struct target *target) { } // TODO: HAWINDOWSEL - + // TODO: HAWINDOW // ABSTRACTCS @@ -2369,7 +2367,7 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == testvar, "All reported DATA words must be R/W"); } } - + // Check that all reported ProgBuf words are really R/W for (int invert = 0; invert < 2; invert++) { for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ @@ -2387,7 +2385,7 @@ int riscv013_test_compliance(struct target *target) { // TODO: Cause and clear all error types // COMMAND - // TODO: Unclear from the spec whether all these bits need to truly be R/W. + // TODO: Unclear from the spec whether all these bits need to truly be R/W. // But at any rate, this is not legal and should cause an error. dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0xAAAAAAAA, "COMMAND register should be R/W"); @@ -2428,14 +2426,14 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + 1) == (i + 1), "GPR Reads and writes should be supported."); } } - + // ABSTRACTAUTO // See which bits are actually writable dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); uint32_t abstractauto = dmi_read(target, DMI_ABSTRACTAUTO); if (abstractauto > 0) { testvar = 0; - // TODO: This mechanism only works when you have a reasonable sized progbuf, which is not + // TODO: This mechanism only works when you have a reasonable sized progbuf, which is not // a true compliance requirement. COMPLIANCE_TEST(riscv_set_register(target, GDB_REGNO_S0, 0) == ERROR_OK, "Need to be able to write S0 to test ABSTRACTAUTO"); struct riscv_program program; @@ -2470,11 +2468,11 @@ int riscv013_test_compliance(struct target *target) { dmi_write(target, DMI_ABSTRACTAUTO, 0); riscv_get_register(target, &value, GDB_REGNO_S0); - + COMPLIANCE_TEST(testvar == value, \ "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); } - + // Single-Step each hart. for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ riscv_set_current_hartid(target, hartsel); @@ -2482,7 +2480,7 @@ int riscv013_test_compliance(struct target *target) { riscv013_step_current_hart(target); COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, "Single Step should result in SINGLESTEP"); } - + // Core Register Tests uint64_t bogus_dpc = 0xdeadbeef; for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ @@ -2511,21 +2509,21 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST((dpc & dpcmask) == ((~testvar64) & dpcmask), "DPC must be writable"); if (hartsel == 0) {bogus_dpc = dpc;} // For a later test step } - + //NDMRESET // NDMRESET // Asserting non-debug module reset should not reset Debug Module state. // But it should reset Hart State, e.g. DPC should get a different value. // Also make sure that DCSR reports cause of 'HALT' even though previously we single-stepped. - + // Write some registers. They should not be impacted by ndmreset. dmi_write(target, DMI_COMMAND, 0xFFFFFFFF); - + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; dmi_write(target, DMI_PROGBUF0 + i, testvar); } - + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ testvar = (i + 1) * 0x11111111; dmi_write(target, DMI_DATA0 + i, testvar); @@ -2535,7 +2533,7 @@ int riscv013_test_compliance(struct target *target) { abstractauto = dmi_read(target, DMI_ABSTRACTAUTO); // Pulse reset. - + target->reset_halt = true; dmcontrol = dmi_read(target, DMI_DMCONTROL); riscv_set_current_hartid(target, 0); @@ -2550,12 +2548,12 @@ int riscv013_test_compliance(struct target *target) { // Clean up to avoid future test failures dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); dmi_write(target, DMI_ABSTRACTAUTO, 0); - + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == testvar, "PROGBUF words must not be affected by NDMRESET"); } - + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ testvar = (i + 1) * 0x11111111; COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == testvar, "DATA words must not be affected by NDMRESET"); @@ -2566,23 +2564,23 @@ int riscv013_test_compliance(struct target *target) { COMPLIANCE_TEST(bogus_dpc != 0xdeadbeef, "BOGUS DPC should have been set somehow (bug in compliance test)"); riscv_get_register(target, &value, GDB_REGNO_DPC); COMPLIANCE_TEST(bogus_dpc != value, "NDMRESET should move DPC to reset value."); - + COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, "After NDMRESET halt, DCSR should report cause of halt"); - + // DMACTIVE -- deasserting DMACTIVE should reset all the above values. - + // Toggle dmactive dmi_write(target, DMI_DMCONTROL, 0); dmi_write(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); COMPLIANCE_TEST(dmi_read(target, DMI_COMMAND) == 0, "DMI_COMMAND should reset to 0"); COMPLIANCE_TEST(get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0, "ABSTRACTCS.cmderr should reset to 0"); COMPLIANCE_TEST(dmi_read(target, DMI_ABSTRACTAUTO) == 0, "ABSTRACTAUTO should reset to 0"); - + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ testvar = (i + 1) * 0x11111111; COMPLIANCE_TEST(dmi_read(target, DMI_PROGBUF0 + i) == 0, "PROGBUF words should reset to 0"); } - + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ testvar = (i + 1) * 0x11111111; COMPLIANCE_TEST(dmi_read(target, DMI_DATA0 + i) == 0, "DATA words should reset to 0"); diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index f6ad20e..2b5e89f 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1286,7 +1286,7 @@ static const struct command_registration riscv_exec_command_handlers[] = { { .name = "test_compliance", .handler = riscv_test_compliance, - .mode = COMMAND_EXEC, + .mode = COMMAND_EXEC, .usage = "riscv test_compliance", .help = "Runs a basic compliance test suite against the RISC-V Debug Spec." }, -- cgit v1.1 From 8f7195af76dd5ba048e6d82758ff21a24b660d01 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 13 Feb 2018 13:47:14 -0800 Subject: riscv-compliance: Turn off ABSTRACTAUTO until the appropriate time --- src/target/riscv/riscv-013.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index c6f60b3..74abfc5 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2431,6 +2431,7 @@ int riscv013_test_compliance(struct target *target) { // See which bits are actually writable dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); uint32_t abstractauto = dmi_read(target, DMI_ABSTRACTAUTO); + dmi_write(target, DMI_ABSTRACTAUTO, 0x0); if (abstractauto > 0) { testvar = 0; // TODO: This mechanism only works when you have a reasonable sized progbuf, which is not -- cgit v1.1 From 1b37f60969fb6a0df99fc16c7370a3499a9fa31d Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 13 Feb 2018 15:02:31 -0800 Subject: riscv-compliance: Check that DPC is sign extended properly. --- src/target/riscv/riscv-013.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 74abfc5..68f6b3b 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2495,19 +2495,23 @@ int riscv013_test_compliance(struct target *target) { riscv_get_register(target, &value, GDB_REGNO_DCSR); COMPLIANCE_TEST(value != 0, "At least some bits in DCSR must be 1"); - // DPC - uint64_t testvar64 = 0xAAAAAAAAAAAAAAAAUL; - riscv_reg_t dpcmask = 0xFFFFFFFFFFFFFFFFUL; - riscv_set_register(target, GDB_REGNO_DPC, dpcmask); - riscv_get_register(target, &dpcmask, GDB_REGNO_DPC); - COMPLIANCE_TEST(dpcmask >= 0xFFFFFFFC, "DPC must hold the minimum for a virtual address (may tighten requirement later)."); - riscv_set_register(target, GDB_REGNO_DPC, testvar64); - riscv_get_register(target, &value, GDB_REGNO_DPC); - COMPLIANCE_TEST((value & dpcmask) == (testvar64 & dpcmask), "DPC must be writable."); - riscv_set_register(target, GDB_REGNO_DPC, ~testvar64); + // DPC. Note that DPC is sign-extended. + riscv_reg_t dpcmask = 0xFFFFFFFCUL; riscv_reg_t dpc; + + if (riscv_xlen(target) > 32) { + dpcmask |= (0xFFFFFFFFUL << 32); + } + if (riscv_supports_extension(target, 'C')){ + dpcmask |= 0x2; + } + + riscv_set_register(target, GDB_REGNO_DPC, dpcmask); + riscv_get_register(target, &dpc, GDB_REGNO_DPC); + COMPLIANCE_TEST(dpcmask == dpc, "DPC must be sign-extended to XLEN and writable to all-1s (except the least significant bits)"); + riscv_set_register(target, GDB_REGNO_DPC, 0); riscv_get_register(target, &dpc, GDB_REGNO_DPC); - COMPLIANCE_TEST((dpc & dpcmask) == ((~testvar64) & dpcmask), "DPC must be writable"); + COMPLIANCE_TEST(dpc == 0, "DPC must be writable to 0."); if (hartsel == 0) {bogus_dpc = dpc;} // For a later test step } -- cgit v1.1 From 7eca2dfe5de5ddfb0230a424ab8e86c8f70c460d Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 12 Apr 2018 15:02:04 -0700 Subject: Squashed commit of the following: commit fb7009fc38eff2f169385589e0f71ca4f9626cb2 Author: Gleb Gagarin Date: Fri Feb 23 16:41:14 2018 -0800 Make some error messages to be printed once commit e09dd62229bd2c152bf403e601aa098e29aaa850 Author: Gleb Gagarin Date: Fri Feb 23 15:30:10 2018 -0800 Reduce severity of the error messages that are polluting the log commit 73b6ea55ebefd004bef71359d611d3a887b89983 Author: Gleb Gagarin Date: Fri Feb 23 13:32:54 2018 -0800 removed unused variable commit c3bdcb0c4ae63bb5afad628c58eb29fe43c9646d Author: Gleb Gagarin Date: Thu Feb 22 18:32:08 2018 -0800 more R/O checks commit 353cf212bd892bbb065e6a57fb9412cc8fea0548 Author: Gleb Gagarin Date: Thu Feb 22 14:27:25 2018 -0800 write progbuf via DMI commit e73d82e3d6cf064d0323e3c686f3b7710f42a51f Author: Gleb Gagarin Date: Wed Feb 21 18:47:36 2018 -0800 add writes to progbuf commit f97e4b53e4064ae4e7807a1aac74c5d92aea7666 Author: Gleb Gagarin Date: Wed Feb 21 16:20:12 2018 -0800 Try to zero out ROM --- src/target/riscv/riscv-013.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 68f6b3b..0879d51 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -383,7 +383,11 @@ static dmi_status_t dmi_scan(struct target *target, uint16_t *address_in, int retval = jtag_execute_queue(); if (retval != ERROR_OK) { - LOG_ERROR("dmi_scan failed jtag scan"); + static int once = 1; + if (once) { + LOG_ERROR("dmi_scan failed jtag scan"); + once = 0; + } return DMI_STATUS_FAILED; } @@ -418,13 +422,22 @@ static uint32_t dmi_read(struct target *target, uint16_t address) } else if (status == DMI_STATUS_SUCCESS) { break; } else { - LOG_ERROR("failed read from 0x%x, status=%d", address, status); + static int once = 1; + if (once) { + LOG_ERROR("failed read from 0x%x, status=%d", address, status); + once = 0; + } break; } + usleep(100000); } if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("Failed read from 0x%x; status=%d", address, status); + static int once = 1; + if (once) { + LOG_INFO("Failed read from 0x%x; status=%d", address, status); + once = 0; + } return ~0; } @@ -2268,6 +2281,9 @@ int riscv013_test_compliance(struct target *target) { dmstatus = dmi_read(target, DMI_DMSTATUS); } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); + dmi_write(target, DMI_DMSTATUS, 0xffffffff); + COMPLIANCE_TEST(dmi_read(target, DMI_DMSTATUS) == dmstatus, "DMSTATUS is R/O"); + // resumereq. This will resume the hart but this test is destructive anyway. dmcontrol &= ~DMI_DMCONTROL_HALTREQ; dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 1); @@ -2340,6 +2356,12 @@ int riscv013_test_compliance(struct target *target) { } COMPLIANCE_TEST(dmi_read(target, DMI_HALTSUM) == expected_haltsum, "HALTSUM should report all halted harts"); + dmi_write(target, DMI_HALTSUM, 0xffffffff); + COMPLIANCE_TEST(dmi_read(target, DMI_HALTSUM) == expected_haltsum, "HALTSUM is R/O"); + + dmi_write(target, DMI_HALTSUM, 0x0); + COMPLIANCE_TEST(dmi_read(target, DMI_HALTSUM) == expected_haltsum, "HALTSUM is R/O"); + for (int i = 0; i < riscv_count_harts(target); i +=32){ uint32_t haltstat = dmi_read(target, 0x40 + (i / 32)); uint32_t haltstat_expected = (((i + 1) * 32) <= riscv_count_harts(target)) ? 0xFFFFFFFFU : ((1U << (riscv_count_harts(target) % 32)) - 1); -- cgit v1.1 From 415da7ed4e7d55a03c076d0170f6fecfe5845811 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 12 Apr 2018 16:06:30 -0700 Subject: riscv: update definitions to meet current version of spec --- src/target/riscv/debug_defines.h | 193 +++++++++++++++------------------------ 1 file changed, 75 insertions(+), 118 deletions(-) diff --git a/src/target/riscv/debug_defines.h b/src/target/riscv/debug_defines.h index 04500e5..17b1444 100644 --- a/src/target/riscv/debug_defines.h +++ b/src/target/riscv/debug_defines.h @@ -343,6 +343,16 @@ #define CSR_MCONTROL_MASKMAX_LENGTH 6 #define CSR_MCONTROL_MASKMAX (0x3fULL << CSR_MCONTROL_MASKMAX_OFFSET) /* +* If this optional bit is implemented, the hardware sets it when this +* trigger matches. The trigger's user can set or clear it at any +* time. The trigger's user can use this bit to determine which +* trigger(s) matched. If the bit is not implemented, it is always 0 +* and writing it has no effect. + */ +#define CSR_MCONTROL_HIT_OFFSET 20 +#define CSR_MCONTROL_HIT_LENGTH 1 +#define CSR_MCONTROL_HIT (0x1ULL << CSR_MCONTROL_HIT_OFFSET) +/* * 0: Perform a match on the virtual address. * * 1: Perform a match on the data value loaded/stored, or the @@ -368,7 +378,7 @@ * which case the debugger has a little more control. * * Data load triggers with \Ftiming of 0 will result in the same load -* happening again when the debugger lets the core run. For data load +* happening again when the debugger lets the hart run. For data load * triggers, debuggers must first attempt to set the breakpoint with * \Ftiming of 1. * @@ -479,6 +489,16 @@ #define CSR_ICOUNT_DMODE_LENGTH 1 #define CSR_ICOUNT_DMODE (0x1ULL << CSR_ICOUNT_DMODE_OFFSET) /* +* If this optional bit is implemented, the hardware sets it when this +* trigger matches. The trigger's user can set or clear it at any +* time. The trigger's user can use this bit to determine which +* trigger(s) matched. If the bit is not implemented, it is always 0 +* and writing it has no effect. + */ +#define CSR_ICOUNT_HIT_OFFSET 24 +#define CSR_ICOUNT_HIT_LENGTH 1 +#define CSR_ICOUNT_HIT (0x1ULL << CSR_ICOUNT_HIT_OFFSET) +/* * When count is decremented to 0, the trigger fires. Instead of * changing \Fcount from 1 to 0, it is also acceptable for hardware to * clear \Fm, \Fs, and \Fu. This allows \Fcount to be hard-wired @@ -674,7 +694,7 @@ */ #define DMI_DMCONTROL_HALTREQ_OFFSET 31 #define DMI_DMCONTROL_HALTREQ_LENGTH 1 -#define DMI_DMCONTROL_HALTREQ (0x1ULL << DMI_DMCONTROL_HALTREQ_OFFSET) +#define DMI_DMCONTROL_HALTREQ (0x1U << DMI_DMCONTROL_HALTREQ_OFFSET) /* * Writes the resume request bit for all currently selected harts. * When set to 1, each selected hart will resume if it is currently @@ -687,7 +707,7 @@ */ #define DMI_DMCONTROL_RESUMEREQ_OFFSET 30 #define DMI_DMCONTROL_RESUMEREQ_LENGTH 1 -#define DMI_DMCONTROL_RESUMEREQ (0x1ULL << DMI_DMCONTROL_RESUMEREQ_OFFSET) +#define DMI_DMCONTROL_RESUMEREQ (0x1U << DMI_DMCONTROL_RESUMEREQ_OFFSET) /* * This optional field writes the reset bit for all the currently * selected harts. To perform a reset the debugger writes 1, and then @@ -701,7 +721,7 @@ */ #define DMI_DMCONTROL_HARTRESET_OFFSET 29 #define DMI_DMCONTROL_HARTRESET_LENGTH 1 -#define DMI_DMCONTROL_HARTRESET (0x1ULL << DMI_DMCONTROL_HARTRESET_OFFSET) +#define DMI_DMCONTROL_HARTRESET (0x1U << DMI_DMCONTROL_HARTRESET_OFFSET) /* * Writing 1 to this bit clears the {\tt havereset} bits for * any selected harts. @@ -710,7 +730,7 @@ */ #define DMI_DMCONTROL_ACKHAVERESET_OFFSET 28 #define DMI_DMCONTROL_ACKHAVERESET_LENGTH 1 -#define DMI_DMCONTROL_ACKHAVERESET (0x1ULL << DMI_DMCONTROL_ACKHAVERESET_OFFSET) +#define DMI_DMCONTROL_ACKHAVERESET (0x1U << DMI_DMCONTROL_ACKHAVERESET_OFFSET) /* * Selects the definition of currently selected harts. * @@ -720,20 +740,27 @@ * plus those selected by the hart array mask register. * * An implementation which does not implement the hart array mask register -* should tie this field to 0. A debugger which wishes to use the hart array +* must tie this field to 0. A debugger which wishes to use the hart array * mask register feature should set this bit and read back to see if the functionality * is supported. */ #define DMI_DMCONTROL_HASEL_OFFSET 26 #define DMI_DMCONTROL_HASEL_LENGTH 1 -#define DMI_DMCONTROL_HASEL (0x1ULL << DMI_DMCONTROL_HASEL_OFFSET) +#define DMI_DMCONTROL_HASEL (0x1U << DMI_DMCONTROL_HASEL_OFFSET) /* -* The DM-specific index of the hart to select. This hart is always part of the -* currently selected harts. +* The low 10 bits of \Fhartsel: the DM-specific index of the hart to +* select. This hart is always part of the currently selected harts. */ -#define DMI_DMCONTROL_HARTSEL_OFFSET 16 -#define DMI_DMCONTROL_HARTSEL_LENGTH HARTSELLEN -#define DMI_DMCONTROL_HARTSEL (((1L< Date: Thu, 12 Apr 2018 16:10:45 -0700 Subject: riscv: hartsel-> hartsello (not supporting hartselhi yet) --- src/target/riscv/riscv-013.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8b16a4e..684ea0e 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -264,7 +264,7 @@ static dm013_info_t *get_dm(struct target *target) static uint32_t hartsel_mask(const struct target *target) { RISCV013_INFO(info); - return ((1L<hartsellen)-1) << DMI_DMCONTROL_HARTSEL_OFFSET; + return ((1L<hartsellen)-1) << DMI_DMCONTROL_HARTSELLO_OFFSET; } static void decode_dmi(char *text, unsigned address, unsigned data) @@ -278,7 +278,7 @@ static void decode_dmi(char *text, unsigned address, unsigned data) { DMI_DMCONTROL, DMI_DMCONTROL_RESUMEREQ, "resumereq" }, { DMI_DMCONTROL, DMI_DMCONTROL_HARTRESET, "hartreset" }, { DMI_DMCONTROL, DMI_DMCONTROL_HASEL, "hasel" }, - { DMI_DMCONTROL, ((1L<<10)-1) << DMI_DMCONTROL_HARTSEL_OFFSET, "hartsel" }, + { DMI_DMCONTROL, ((1L<<10)-1) << DMI_DMCONTROL_HARTSELLO_OFFSET, "hartsel" }, { DMI_DMCONTROL, DMI_DMCONTROL_NDMRESET, "ndmreset" }, { DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE, "dmactive" }, @@ -1290,7 +1290,7 @@ static int examine(struct target *target) dm->was_reset = true; } - uint32_t max_hartsel_mask = ((1L<<10)-1) << DMI_DMCONTROL_HARTSEL_OFFSET; + uint32_t max_hartsel_mask = ((1L<<10)-1) << DMI_DMCONTROL_HARTSELLO_OFFSET; dmi_write(target, DMI_DMCONTROL, max_hartsel_mask | DMI_DMCONTROL_DMACTIVE); uint32_t dmcontrol; if (dmi_read(target, &dmcontrol, DMI_DMCONTROL) != ERROR_OK) -- cgit v1.1 From ff365173a0808477a879ae37464a1592c48a204b Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 12 Apr 2018 17:31:23 -0700 Subject: riscv-compliance: fix too-narrow constant --- src/target/riscv/riscv-013.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 684ea0e..8b94e77 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3294,7 +3294,7 @@ int riscv013_test_compliance(struct target *target) { riscv_reg_t dpc; if (riscv_xlen(target) > 32) { - dpcmask |= (0xFFFFFFFFUL << 32); + dpcmask |= (0xFFFFFFFFULL << 32); } if (riscv_supports_extension(target, riscv_current_hartid(target), 'C')){ dpcmask |= 0x2; -- cgit v1.1 From 4c39ed9d7f6fda344a944d5e91fb7b12af912ae1 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 13 Apr 2018 17:02:34 -0700 Subject: Enforce OpenOCD style guide. Change-Id: I579a9f54ed22a774bf52f6aa5bc13bcbd2e82cd8 --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 37a5993..aed8bd2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -50,5 +50,6 @@ matrix: - binutils-mingw-w64-i686 gcc-mingw-w64-i686 g++-mingw-w64-i686 script: + - git diff `git merge-base master HEAD` | ./tools/scripts/checkpatch.pl - - ./bootstrap && ./configure --enable-remote-bitbang --enable-jtag_vpi $CONFIGURE_ARGS && make - file src/$EXECUTABLE -- cgit v1.1 From 9cb2c56dc10528c1d44658b34459f93d0a375da0 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 16 Apr 2018 13:17:09 -0700 Subject: Fail if `git diff` fails Change-Id: I57256b0a24247f6123cb0e25a89c1b59867cb3f9 --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index aed8bd2..a8b919c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -50,6 +50,7 @@ matrix: - binutils-mingw-w64-i686 gcc-mingw-w64-i686 g++-mingw-w64-i686 script: + - set -o pipefail - git diff `git merge-base master HEAD` | ./tools/scripts/checkpatch.pl - - ./bootstrap && ./configure --enable-remote-bitbang --enable-jtag_vpi $CONFIGURE_ARGS && make - file src/$EXECUTABLE -- cgit v1.1 From bc32aaafa4590bc2f0753b3dcab6b53f57915ddf Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Mon, 16 Apr 2018 17:49:16 -0700 Subject: riscv-compliance: whitespace cleanup --- src/target/riscv/riscv.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 4d0a0b0..3b91fad 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1208,22 +1208,21 @@ COMMAND_HANDLER(riscv_set_reset_timeout_sec) COMMAND_HANDLER(riscv_test_compliance) { - struct target *target = get_current_target(CMD_CTX); - - RISCV_INFO(r); + struct target *target = get_current_target(CMD_CTX); - if (CMD_ARGC > 0) { - LOG_ERROR("Command does not take any parameters."); - return ERROR_COMMAND_SYNTAX_ERROR; - } + RISCV_INFO(r); - if (r->test_compliance) { - return r->test_compliance(target); - } else { - LOG_ERROR("This target does not support this command (may implement an older version of the spec)."); - return ERROR_FAIL; - } + if (CMD_ARGC > 0) { + LOG_ERROR("Command does not take any parameters."); + return ERROR_COMMAND_SYNTAX_ERROR; + } + if (r->test_compliance) { + return r->test_compliance(target); + } else { + LOG_ERROR("This target does not support this command (may implement an older version of the spec)."); + return ERROR_FAIL; + } } COMMAND_HANDLER(riscv_set_scratch_ram) @@ -1237,7 +1236,7 @@ COMMAND_HANDLER(riscv_set_scratch_ram) return ERROR_OK; } - // TODO: use COMMAND_PARSE_NUMBER + /** TODO: use COMMAND_PARSE_NUMBER **/ long long unsigned int address; int result = sscanf(CMD_ARGV[0], "%llx", &address); if (result != (int) strlen(CMD_ARGV[0])) { -- cgit v1.1 From fa99b8e3b125e1c47af9185fd6cdb9fde3d2b199 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 07:49:06 -0700 Subject: riscv-compliance: Fix OpenOCD lint checks --- src/target/riscv/riscv-013.c | 986 ++++++++++++++++++++++--------------------- 1 file changed, 507 insertions(+), 479 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8b94e77..8199750 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1501,7 +1501,7 @@ static int init_target(struct command_context *cmd_ctx, generic_info->dmi_read = &dmi_read; generic_info->dmi_write = &dmi_write; generic_info->test_compliance = &riscv013_test_compliance; - generic_info->version_specific = calloc(1, sizeof(riscv013_info_t)); + generic_info->version_specific = calloc(1, sizeof(riscv013_info_t)); if (!generic_info->version_specific) return ERROR_FAIL; riscv013_info_t *info = get_info(target); @@ -2934,482 +2934,510 @@ void riscv013_clear_abstract_error(struct target *target) dmi_write(target, DMI_ABSTRACTCS, abstractcs & DMI_ABSTRACTCS_CMDERR); } -#define COMPLIANCE_TEST(b, message) { \ - int pass = 0; \ - if (b) { \ - pass = 1; \ - passed_tests ++; \ - } \ - LOG_INFO("%s test %d (%s)\n", (pass) ? "PASSED":"FAILED", total_tests, message); \ - assert(pass); \ - total_tests ++; \ - } - -int riscv013_test_compliance(struct target *target) { - LOG_INFO("Testing Compliance against RISC-V Debug Spec v0.13"); - - if (!riscv_rtos_enabled(target)) { - LOG_ERROR("Please run with -rtos riscv to run compliance test."); - return ERROR_FAIL; - } - - int total_tests = 0; - int passed_tests = 0; - - uint32_t dmcontrol_orig; - dmi_read(target, &dmcontrol_orig, DMI_DMCONTROL); - uint32_t dmcontrol; - uint32_t testvar; - uint32_t testvar_read; - riscv_reg_t value; - - dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), RISCV_MAX_HARTS-1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == (RISCV_MAX_HARTS-1), "DMCONTROL.hartsel should hold all the harts allowed by HARTSELLEN."); - - dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), 0); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == 0, "DMCONTROL.hartsel should hold Hart ID 0"); - - // hartreset - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - testvar = get_field(dmcontrol, DMI_DMCONTROL_HARTRESET); - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HARTRESET)) == 0), "DMCONTROL.hartreset can be 0 or RW."); - - // hasel - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - testvar = get_field(dmcontrol, DMI_DMCONTROL_HASEL); - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HASEL)) == 0), "DMCONTROL.hasel can be 0 or RW."); - //TODO: test that hamask registers exist if hasel does. - - // haltreq - riscv_halt_all_harts(target); - // Writing haltreq should not cause any problems for a halted hart, but we - // should be able to read and write it. - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - dmcontrol |= DMI_DMCONTROL_HALTREQ; - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(dmcontrol & DMI_DMCONTROL_HALTREQ, "DMCONTROL.haltreq should be R/W"); - uint32_t dmstatus, dmstatus_read; - do { - dmi_read(target, &dmstatus, DMI_DMSTATUS); - } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); - - dmi_write(target, DMI_DMSTATUS, 0xffffffff); - dmi_read(target, &dmstatus_read, DMI_DMSTATUS); - COMPLIANCE_TEST(dmstatus_read == dmstatus, "DMSTATUS is R/O"); - - // resumereq. This will resume the hart but this test is destructive anyway. - dmcontrol &= ~DMI_DMCONTROL_HALTREQ; - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ) == 1, "DMCONTROL.resumereq should be R/W"); - - do { - dmi_read(target, &dmstatus, DMI_DMSTATUS); - } while (get_field(dmstatus, DMI_DMSTATUS_ALLRESUMEACK) == 0); - - // Halt the hart again because the target isn't aware that we resumed it. - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0); - dmcontrol |= DMI_DMCONTROL_HALTREQ; - dmi_write(target, DMI_DMCONTROL, dmcontrol); - do { - dmi_read(target, &dmstatus, DMI_DMSTATUS); - } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); - dmcontrol &= ~DMI_DMCONTROL_HALTREQ; - dmi_write(target, DMI_DMCONTROL, dmcontrol); - // Not clear that this read is required according to the spec. - dmi_read(target, &dmstatus, DMI_DMSTATUS); - - // HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. - for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++){ - riscv_set_current_hartid(target, hartsel); - - uint32_t hartinfo, hartinfo_read; - dmi_read(target, &hartinfo, DMI_HARTINFO); - dmi_write(target, DMI_HARTINFO, ~hartinfo); - dmi_read(target, &hartinfo_read, DMI_HARTINFO); - COMPLIANCE_TEST((hartinfo_read == hartinfo), "DMHARTINFO should be Read-Only."); - - uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); - for (unsigned int d = 0; d < nscratch; d++) { - - //TODO: DSCRATCH CSRs should be 64-bit on 64-bit systems. - riscv_reg_t testval; - for (testval = 0x0011223300112233; testval != 0xDEAD ; testval = testval == 0x0011223300112233 ? ~testval : 0xDEAD ) { - COMPLIANCE_TEST(register_write_direct(target, GDB_REGNO_S0, testval) == ERROR_OK, "Need to be able to write S0 in order to test DSCRATCH."); - struct riscv_program program32; - riscv_program_init(&program32, target); - riscv_program_csrw(&program32, GDB_REGNO_S0, GDB_REGNO_DSCRATCH + d); - riscv_program_csrr(&program32, GDB_REGNO_S1, GDB_REGNO_DSCRATCH + d); - riscv_program_fence(&program32); - riscv_program_ebreak(&program32); - COMPLIANCE_TEST(riscv_program_exec(&program32, target) == ERROR_OK, "Accessing DSCRATCH with program buffer should succeed."); - COMPLIANCE_TEST(register_read_direct(target, &value, GDB_REGNO_S1) == ERROR_OK, "Need to be able to read S1 in order to test DSCRATCH."); - if (riscv_xlen(target) > 32) { - COMPLIANCE_TEST(value == testval, "All DSCRATCH registers in HARTINFO must be R/W."); - } else { - COMPLIANCE_TEST(value == (testval & 0xFFFFFFFF), "All DSCRATCH registers in HARTINFO must be R/W."); - } - } - } - // TODO: dataaccess - if (get_field(hartinfo, DMI_HARTINFO_DATAACCESS)) { - // TODO: Shadowed in memory map. - // TODO: datasize - // TODO: dataaddr - } else { - // TODO: Shadowed in CSRs. - // TODO: datasize - // TODO: dataaddr - } - - } - - // HALTSUM -- TODO: More than 32 harts - // TODO: HALTSUM2, HALTSUM3 - uint32_t expected_haltsum0 = 0; - for (int i = 0; i < riscv_count_harts(target); i +=32){ - expected_haltsum0 |= (1 << (i / 32)); - } - dmi_read(target, &testvar_read, DMI_HALTSUM0); - COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should report summary of 32 halted harts"); - - dmi_write(target, DMI_HALTSUM0, 0xffffffff); - dmi_read(target, &testvar_read, DMI_HALTSUM0); - COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); - - dmi_write(target, DMI_HALTSUM0, 0x0); - dmi_read(target, &testvar_read, DMI_HALTSUM0); - COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); - - for (int i = 0; i < 32/*TODO: riscv_count_harts(target)*/; i +=32){ - //TODO: Set hartsel for i > 32 harts. - dmi_read(target, &testvar_read, DMI_HALTSUM1); - uint32_t haltsum1_expected = (((i + 1) * 32) <= riscv_count_harts(target)) ? 0xFFFFFFFFU : ((1U << (riscv_count_harts(target) % 32)) - 1); - COMPLIANCE_TEST(testvar_read == haltsum1_expected, "HALTSUM1 should report summary of 1024 halted harts"); - - // Just have to check this once - if (i == 0) { - dmi_write(target, DMI_HALTSUM1, 0xffffffff); - dmi_read(target, &testvar_read, DMI_HALTSUM1); - COMPLIANCE_TEST(testvar_read == haltsum1_expected, "HALTSUM1 should be R/O"); - - dmi_write(target, DMI_HALTSUM1, 0x0); - dmi_read(target, &testvar_read, DMI_HALTSUM1); - COMPLIANCE_TEST(testvar_read == haltsum1_expected, "HALTSUM1 should be R/O"); - } - } - - // TODO: HAWINDOWSEL - - // TODO: HAWINDOW - - // ABSTRACTCS - - uint32_t abstractcs; - dmi_read(target, &abstractcs, DMI_ABSTRACTCS); - - // Check that all reported Data Words are really R/W - for (int invert = 0; invert < 2; invert++) { - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ - testvar = (i + 1) * 0x11111111; - if (invert) {testvar = ~testvar;} - dmi_write(target, DMI_DATA0 + i, testvar); - } - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ - testvar = (i + 1) * 0x11111111; - if (invert) {testvar = ~testvar;} - dmi_read(target, &testvar_read, DMI_DATA0 + i); - COMPLIANCE_TEST(testvar_read == testvar, "All reported DATA words must be R/W"); - } - } - - // Check that all reported ProgBuf words are really R/W - for (int invert = 0; invert < 2; invert++) { - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ - testvar = (i + 1) * 0x11111111; - if (invert) {testvar = ~testvar;} - dmi_write(target, DMI_PROGBUF0 + i, testvar); - } - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ - testvar = (i + 1) * 0x11111111; - if (invert) {testvar = ~testvar;} - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); - COMPLIANCE_TEST(testvar_read == testvar, "All reported PROGBUF words must be R/W"); - } - } - - // TODO: Cause and clear all error types - - // COMMAND - // TODO: Unclear from the spec whether all these bits need to truly be R/W. - // But at any rate, this is not legal and should cause an error. - dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); - dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0xAAAAAAAA, "COMMAND register should be R/W"); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ - "Illegal COMMAND should result in UNSUPPORTED"); - dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); - dmi_write(target, DMI_COMMAND, 0x55555555); - dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0x55555555, "COMMAND register should be R/W"); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ - "Illegal COMMAND should result in UNSUPPORTED"); - dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); - - // Basic Abstract Commands - uint32_t command = 0; - uint32_t busy; - command = set_field(command, AC_ACCESS_REGISTER_SIZE, riscv_xlen(target) > 32 ? 3:2); - command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); - for (unsigned int i = 1 ; i < 32 ; i = i << 1) { - command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + GDB_REGNO_ZERO + i); - command = set_field(command, AC_ACCESS_REGISTER_WRITE, 1); - dmi_write(target, DMI_DATA0, i); - if (riscv_xlen(target) > 32) { - dmi_write(target, DMI_DATA0 + 1, i + 1); - } - dmi_write(target, DMI_COMMAND, command); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, "GPR Writes should be supported."); - dmi_write(target, DMI_DATA0, 0xDEADBEEF); - if (riscv_xlen(target) > 32) { - dmi_write(target, DMI_DATA0 + 1, 0xDEADBEEF); - } - command = set_field(command, AC_ACCESS_REGISTER_WRITE, 0); - dmi_write(target, DMI_COMMAND, command); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, "GPR Reads should be supported."); - dmi_read(target, &testvar_read, DMI_DATA0); - COMPLIANCE_TEST(testvar_read == i, "GPR Reads and writes should be supported."); - if (riscv_xlen(target) > 32) { - dmi_read(target, &testvar_read, DMI_DATA0 + 1); - COMPLIANCE_TEST(testvar_read == (i + 1), "GPR Reads and writes should be supported."); - } - } - - // ABSTRACTAUTO - // See which bits are actually writable - dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); - uint32_t abstractauto; - dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); - dmi_write(target, DMI_ABSTRACTAUTO, 0x0); - if (abstractauto > 0) { - testvar = 0; - // TODO: This mechanism only works when you have a reasonable sized progbuf, which is not - // a true compliance requirement. - uint32_t result = riscv_set_register(target, GDB_REGNO_S0, 0); - COMPLIANCE_TEST(result == ERROR_OK, "Need to be able to write S0 to test ABSTRACTAUTO"); - struct riscv_program program; - riscv_program_init(&program, target); - // Also testing that WFI() is a NOP during debug mode. - riscv_program_insert(&program, wfi()); - riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); - riscv_program_ebreak(&program); - dmi_write(target, DMI_ABSTRACTAUTO, 0x0); - riscv_program_exec(&program, target); - testvar ++; - dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); - dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); - uint32_t autoexec_data = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECDATA); - uint32_t autoexec_progbuf = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECPROGBUF); - for (unsigned int i = 0; i < 12; i ++){ - dmi_read(target, &testvar_read, DMI_DATA0 + i); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - if (autoexec_data & (1 << i)) { - COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT), "AUTOEXEC may be writable up to DATACOUNT bits."); - testvar ++; - } - } - for (unsigned int i = 0; i < 16; i ++){ - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - if (autoexec_progbuf & (1 << i)) { - COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE), "AUTOEXEC may be writable up to PROGBUFSIZE bits."); - testvar ++; - } - } - - dmi_write(target, DMI_ABSTRACTAUTO, 0); - riscv_get_register(target, &value, GDB_REGNO_S0); - - COMPLIANCE_TEST(testvar == value, \ - "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); - } - - // Single-Step each hart. - for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ - riscv_set_current_hartid(target, hartsel); - riscv013_on_step(target); - riscv013_step_current_hart(target); - COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, "Single Step should result in SINGLESTEP"); - } - - // Core Register Tests - uint64_t bogus_dpc = 0xdeadbeef; - for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel ++){ - riscv_set_current_hartid(target, hartsel); - - // DCSR Tests - riscv_set_register(target, GDB_REGNO_DCSR, 0x0); - riscv_get_register(target, &value, GDB_REGNO_DCSR); - COMPLIANCE_TEST(value != 0, "Not all bits in DCSR are writable by Debugger"); - riscv_set_register(target, GDB_REGNO_DCSR, 0xFFFFFFFF); - riscv_get_register(target, &value, GDB_REGNO_DCSR); - COMPLIANCE_TEST(value != 0, "At least some bits in DCSR must be 1"); - - // DPC. Note that DPC is sign-extended. - riscv_reg_t dpcmask = 0xFFFFFFFCUL; - riscv_reg_t dpc; - - if (riscv_xlen(target) > 32) { - dpcmask |= (0xFFFFFFFFULL << 32); - } - if (riscv_supports_extension(target, riscv_current_hartid(target), 'C')){ - dpcmask |= 0x2; - } - - riscv_set_register(target, GDB_REGNO_DPC, dpcmask); - riscv_get_register(target, &dpc, GDB_REGNO_DPC); - COMPLIANCE_TEST(dpcmask == dpc, "DPC must be sign-extended to XLEN and writable to all-1s (except the least significant bits)"); - riscv_set_register(target, GDB_REGNO_DPC, 0); - riscv_get_register(target, &dpc, GDB_REGNO_DPC); - COMPLIANCE_TEST(dpc == 0, "DPC must be writable to 0."); - if (hartsel == 0) {bogus_dpc = dpc;} // For a later test step - } - - //NDMRESET - // NDMRESET - // Asserting non-debug module reset should not reset Debug Module state. - // But it should reset Hart State, e.g. DPC should get a different value. - // Also make sure that DCSR reports cause of 'HALT' even though previously we single-stepped. - - // Write some registers. They should not be impacted by ndmreset. - dmi_write(target, DMI_COMMAND, 0xFFFFFFFF); - - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ - testvar = (i + 1) * 0x11111111; - dmi_write(target, DMI_PROGBUF0 + i, testvar); - } - - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ - testvar = (i + 1) * 0x11111111; - dmi_write(target, DMI_DATA0 + i, testvar); - } - - dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); - dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); - - // Pulse reset. - - target->reset_halt = true; - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - riscv_set_current_hartid(target, 0); - assert_reset(target); - deassert_reset(target); - - // Verify that most stuff is not affected by ndmreset. - dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0xFFFFFFFF, "NDMRESET should not affect DMI_COMMAND"); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, "NDMRESET should not affect DMI_ABSTRACTCS"); - dmi_read(target, &testvar_read, DMI_ABSTRACTAUTO); - COMPLIANCE_TEST(testvar_read == abstractauto, "NDMRESET should not affect DMI_ABSTRACTAUTO"); - - // Clean up to avoid future test failures - dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); - dmi_write(target, DMI_ABSTRACTAUTO, 0); - - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ - testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); - COMPLIANCE_TEST(testvar_read == testvar, "PROGBUF words must not be affected by NDMRESET"); - } - - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ - testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_DATA0 + i); - COMPLIANCE_TEST(testvar_read == testvar, "DATA words must not be affected by NDMRESET"); - } - - // verify that DPC *is* affected by ndmreset. Since we don't know what it *should* be, - // just verify that at least it's not the bogus value anymore. - COMPLIANCE_TEST(bogus_dpc != 0xdeadbeef, "BOGUS DPC should have been set somehow (bug in compliance test)"); - riscv_get_register(target, &value, GDB_REGNO_DPC); - COMPLIANCE_TEST(bogus_dpc != value, "NDMRESET should move DPC to reset value."); - - COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, "After NDMRESET halt, DCSR should report cause of halt"); - - // DMACTIVE -- deasserting DMACTIVE should reset all the above values. - - // Toggle dmactive - dmi_write(target, DMI_DMCONTROL, 0); - dmi_write(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); - dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0, "DMI_COMMAND should reset to 0"); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, "ABSTRACTCS.cmderr should reset to 0"); - dmi_read(target, &testvar_read, DMI_ABSTRACTAUTO); - COMPLIANCE_TEST(testvar_read == 0, "ABSTRACTAUTO should reset to 0"); - - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i ++){ - testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); - COMPLIANCE_TEST(testvar_read == 0, "PROGBUF words should reset to 0"); - } - - for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i ++){ - testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_DATA0 + i); - COMPLIANCE_TEST(testvar_read == 0, "DATA words should reset to 0"); - } - - //TODO: - // DCSR.cause priorities - // DCSR.stoptime/stopcycle - // DCSR.stepie - // DCSR.ebreak - // DCSR.prv - - /* Halt every hart for any follow-up tests*/ - riscv_halt_all_harts(target); - - LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); - - if (total_tests == passed_tests) { - return ERROR_OK; - } else { - return ERROR_FAIL; - } +#define COMPLIANCE_TEST(b, message) \ +{ \ + int pass = 0; \ + if (b) { \ + pass = 1; \ + passed_tests++; \ + } \ + LOG_INFO("%s test %d (%s)\n", (pass) ? "PASSED" : "FAILED", total_tests, message); \ + assert(pass); \ + total_tests++; \ +} + +int riscv013_test_compliance(struct target *target) +{ + LOG_INFO("Testing Compliance against RISC-V Debug Spec v0.13"); + + if (!riscv_rtos_enabled(target)) { + LOG_ERROR("Please run with -rtos riscv to run compliance test."); + return ERROR_FAIL; + } + + int total_tests = 0; + int passed_tests = 0; + + uint32_t dmcontrol_orig; + dmi_read(target, &dmcontrol_orig, DMI_DMCONTROL); + uint32_t dmcontrol; + uint32_t testvar; + uint32_t testvar_read; + riscv_reg_t value; + + dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), RISCV_MAX_HARTS-1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == (RISCV_MAX_HARTS-1), + "DMCONTROL.hartsel should hold all the harts allowed by HARTSELLEN."); + + dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), 0); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == 0, "DMCONTROL.hartsel should hold Hart ID 0"); + + /* hartreset */ + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + testvar = get_field(dmcontrol, DMI_DMCONTROL_HARTRESET); + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HARTRESET)) == 0), + "DMCONTROL.hartreset can be 0 or RW."); + + /* hasel */ + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + testvar = get_field(dmcontrol, DMI_DMCONTROL_HASEL); + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HASEL)) == 0), + "DMCONTROL.hasel can be 0 or RW."); + /* TODO: test that hamask registers exist if hasel does. */ + + /* haltreq */ + riscv_halt_all_harts(target); + /* Writing haltreq should not cause any problems for a halted hart, but we + should be able to read and write it. */ + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + dmcontrol |= DMI_DMCONTROL_HALTREQ; + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST(dmcontrol & DMI_DMCONTROL_HALTREQ, "DMCONTROL.haltreq should be R/W"); + uint32_t dmstatus, dmstatus_read; + do { + dmi_read(target, &dmstatus, DMI_DMSTATUS); + } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); + + dmi_write(target, DMI_DMSTATUS, 0xffffffff); + dmi_read(target, &dmstatus_read, DMI_DMSTATUS); + COMPLIANCE_TEST(dmstatus_read == dmstatus, "DMSTATUS is R/O"); + + /* resumereq. This will resume the hart but this test is destructive anyway. */ + dmcontrol &= ~DMI_DMCONTROL_HALTREQ; + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 1); + dmi_write(target, DMI_DMCONTROL, dmcontrol); + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ) == 1, + "DMCONTROL.resumereq should be R/W"); + + do { + dmi_read(target, &dmstatus, DMI_DMSTATUS); + } while (get_field(dmstatus, DMI_DMSTATUS_ALLRESUMEACK) == 0); + + /* Halt the hart again because the target isn't aware that we resumed it. */ + dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0); + dmcontrol |= DMI_DMCONTROL_HALTREQ; + dmi_write(target, DMI_DMCONTROL, dmcontrol); + do { + dmi_read(target, &dmstatus, DMI_DMSTATUS); + } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); + dmcontrol &= ~DMI_DMCONTROL_HALTREQ; + dmi_write(target, DMI_DMCONTROL, dmcontrol); + /* Not clear that this read is required according to the spec. */ + dmi_read(target, &dmstatus, DMI_DMSTATUS); + + /* HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. */ + for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { + riscv_set_current_hartid(target, hartsel); + + uint32_t hartinfo, hartinfo_read; + dmi_read(target, &hartinfo, DMI_HARTINFO); + dmi_write(target, DMI_HARTINFO, ~hartinfo); + dmi_read(target, &hartinfo_read, DMI_HARTINFO); + COMPLIANCE_TEST((hartinfo_read == hartinfo), "DMHARTINFO should be Read-Only."); + + uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); + for (unsigned int d = 0; d < nscratch; d++) { + /* TODO: DSCRATCH CSRs should be 64-bit on 64-bit systems. */ + riscv_reg_t testval; + for (testval = 0x0011223300112233; + testval != 0xDEAD; + testval = testval == 0x0011223300112233 ? ~testval : 0xDEAD) { + COMPLIANCE_TEST(register_write_direct(target, GDB_REGNO_S0, testval) == ERROR_OK, + "Need to be able to write S0 in order to test DSCRATCH."); + struct riscv_program program32; + riscv_program_init(&program32, target); + riscv_program_csrw(&program32, GDB_REGNO_S0, GDB_REGNO_DSCRATCH + d); + riscv_program_csrr(&program32, GDB_REGNO_S1, GDB_REGNO_DSCRATCH + d); + riscv_program_fence(&program32); + riscv_program_ebreak(&program32); + COMPLIANCE_TEST(riscv_program_exec(&program32, target) == ERROR_OK, + "Accessing DSCRATCH with program buffer should succeed."); + COMPLIANCE_TEST(register_read_direct(target, &value, GDB_REGNO_S1) == ERROR_OK, + "Need to be able to read S1 in order to test DSCRATCH."); + if (riscv_xlen(target) > 32) + COMPLIANCE_TEST(value == testval, + "All DSCRATCH registers in HARTINFO must be R/W."); + else + COMPLIANCE_TEST(value == (testval & 0xFFFFFFFF), + "All DSCRATCH registers in HARTINFO must be R/W."); + } + } + /* TODO: dataaccess */ + if (get_field(hartinfo, DMI_HARTINFO_DATAACCESS)) { + /* TODO: Shadowed in memory map. */ + /* TODO: datasize */ + /* TODO: dataaddr */ + } else { + /* TODO: Shadowed in CSRs. */ + /* TODO: datasize */ + /* TODO: dataaddr */ + } + + } + + /* HALTSUM -- TODO: More than 32 harts */ + /* TODO: HALTSUM2, HALTSUM3 */ + uint32_t expected_haltsum0 = 0; + for (int i = 0; i < riscv_count_harts(target); i += 32) + expected_haltsum0 |= (1 << (i / 32)); + + dmi_read(target, &testvar_read, DMI_HALTSUM0); + COMPLIANCE_TEST(testvar_read == expected_haltsum0, + "HALTSUM0 should report summary of 32 halted harts"); + + dmi_write(target, DMI_HALTSUM0, 0xffffffff); + dmi_read(target, &testvar_read, DMI_HALTSUM0); + COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); + + dmi_write(target, DMI_HALTSUM0, 0x0); + dmi_read(target, &testvar_read, DMI_HALTSUM0); + COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); + + for (int i = 0; i < 32/*TODO: riscv_count_harts(target)*/; i += 32) { + /* TODO: Set hartsel for i > 32 harts. */ + dmi_read(target, &testvar_read, DMI_HALTSUM1); + uint32_t haltsum1_expected = (((i + 1) * 32) <= riscv_count_harts(target)) ? + 0xFFFFFFFFU : + ((1U << (riscv_count_harts(target) % 32)) - 1); + COMPLIANCE_TEST(testvar_read == haltsum1_expected, + "HALTSUM1 should report summary of 1024 halted harts"); + + /* Just have to check this once */ + if (i == 0) { + dmi_write(target, DMI_HALTSUM1, 0xffffffff); + dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_TEST(testvar_read == haltsum1_expected, "HALTSUM1 should be R/O"); + + dmi_write(target, DMI_HALTSUM1, 0x0); + dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_TEST(testvar_read == haltsum1_expected, "HALTSUM1 should be R/O"); + } + } + + /* TODO: HAWINDOWSEL */ + + /* TODO: HAWINDOW */ + + /* ABSTRACTCS */ + + uint32_t abstractcs; + dmi_read(target, &abstractcs, DMI_ABSTRACTCS); + + /* Check that all reported Data Words are really R/W */ + for (int invert = 0; invert < 2; invert++) { + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { + testvar = (i + 1) * 0x11111111; + if (invert) + testvar = ~testvar; + dmi_write(target, DMI_DATA0 + i, testvar); + } + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { + testvar = (i + 1) * 0x11111111; + if (invert) + testvar = ~testvar; + dmi_read(target, &testvar_read, DMI_DATA0 + i); + COMPLIANCE_TEST(testvar_read == testvar, "All reported DATA words must be R/W"); + } + } + + /*Check that all reported ProgBuf words are really R/W */ + for (int invert = 0; invert < 2; invert++) { + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { + testvar = (i + 1) * 0x11111111; + if (invert) + testvar = ~testvar; + dmi_write(target, DMI_PROGBUF0 + i, testvar); + } + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { + testvar = (i + 1) * 0x11111111; + if (invert) + testvar = ~testvar; + dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + COMPLIANCE_TEST(testvar_read == testvar, "All reported PROGBUF words must be R/W"); + } + } + + /* TODO: Cause and clear all error types */ + + /* COMMAND + TODO: Unclear from the spec whether all these bits need to truly be R/W. + But at any rate, this is not legal and should cause an error. */ + dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); + dmi_read(target, &testvar_read, DMI_COMMAND); + COMPLIANCE_TEST(testvar_read == 0xAAAAAAAA, "COMMAND register should be R/W"); + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ + "Illegal COMMAND should result in UNSUPPORTED"); + dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + dmi_write(target, DMI_COMMAND, 0x55555555); + dmi_read(target, &testvar_read, DMI_COMMAND); + COMPLIANCE_TEST(testvar_read == 0x55555555, "COMMAND register should be R/W"); + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ + "Illegal COMMAND should result in UNSUPPORTED"); + dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + + /* Basic Abstract Commands */ + uint32_t command = 0; + uint32_t busy; + command = set_field(command, AC_ACCESS_REGISTER_SIZE, riscv_xlen(target) > 32 ? 3 : 2); + command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); + for (unsigned int i = 1; i < 32; i = i << 1) { + command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + GDB_REGNO_ZERO + i); + command = set_field(command, AC_ACCESS_REGISTER_WRITE, 1); + dmi_write(target, DMI_DATA0, i); + if (riscv_xlen(target) > 32) + dmi_write(target, DMI_DATA0 + 1, i + 1); + dmi_write(target, DMI_COMMAND, command); + do { + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); + } while (busy); + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, + "GPR Writes should be supported."); + dmi_write(target, DMI_DATA0, 0xDEADBEEF); + if (riscv_xlen(target) > 32) + dmi_write(target, DMI_DATA0 + 1, 0xDEADBEEF); + command = set_field(command, AC_ACCESS_REGISTER_WRITE, 0); + dmi_write(target, DMI_COMMAND, command); + do { + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); + } while (busy); + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, + "GPR Reads should be supported."); + dmi_read(target, &testvar_read, DMI_DATA0); + COMPLIANCE_TEST(testvar_read == i, "GPR Reads and writes should be supported."); + if (riscv_xlen(target) > 32) { + dmi_read(target, &testvar_read, DMI_DATA0 + 1); + COMPLIANCE_TEST(testvar_read == (i + 1), + "GPR Reads and writes should be supported."); + } + } + + /* ABSTRACTAUTO + See which bits are actually writable */ + dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + uint32_t abstractauto; + dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); + dmi_write(target, DMI_ABSTRACTAUTO, 0x0); + if (abstractauto > 0) { + testvar = 0; + /* TODO: This mechanism only works when you have a reasonable sized progbuf, which is not + a true compliance requirement. */ + uint32_t result = riscv_set_register(target, GDB_REGNO_S0, 0); + COMPLIANCE_TEST(result == ERROR_OK, "Need to be able to write S0 to test ABSTRACTAUTO"); + struct riscv_program program; + riscv_program_init(&program, target); + /* Also testing that WFI() is a NOP during debug mode. */ + riscv_program_insert(&program, wfi()); + riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); + riscv_program_ebreak(&program); + dmi_write(target, DMI_ABSTRACTAUTO, 0x0); + riscv_program_exec(&program, target); + testvar++; + dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); + uint32_t autoexec_data = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECDATA); + uint32_t autoexec_progbuf = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECPROGBUF); + for (unsigned int i = 0; i < 12; i++) { + dmi_read(target, &testvar_read, DMI_DATA0 + i); + do { + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); + } while (busy); + if (autoexec_data & (1 << i)) { + COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT), + "AUTOEXEC may be writable up to DATACOUNT bits."); + testvar++; + } + } + for (unsigned int i = 0; i < 16; i++) { + dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + do { + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); + } while (busy); + if (autoexec_progbuf & (1 << i)) { + COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE), + "AUTOEXEC may be writable up to PROGBUFSIZE bits."); + testvar++; + } + } + + dmi_write(target, DMI_ABSTRACTAUTO, 0); + riscv_get_register(target, &value, GDB_REGNO_S0); + + COMPLIANCE_TEST(testvar == value, \ + "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); + } + + /* Single-Step each hart. */ + for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { + riscv_set_current_hartid(target, hartsel); + riscv013_on_step(target); + riscv013_step_current_hart(target); + COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, + "Single Step should result in SINGLESTEP"); + } + + /* Core Register Tests */ + uint64_t bogus_dpc = 0xdeadbeef; + for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { + riscv_set_current_hartid(target, hartsel); + + /* DCSR Tests */ + riscv_set_register(target, GDB_REGNO_DCSR, 0x0); + riscv_get_register(target, &value, GDB_REGNO_DCSR); + COMPLIANCE_TEST(value != 0, "Not all bits in DCSR are writable by Debugger"); + riscv_set_register(target, GDB_REGNO_DCSR, 0xFFFFFFFF); + riscv_get_register(target, &value, GDB_REGNO_DCSR); + COMPLIANCE_TEST(value != 0, "At least some bits in DCSR must be 1"); + + /* DPC. Note that DPC is sign-extended. */ + riscv_reg_t dpcmask = 0xFFFFFFFCUL; + riscv_reg_t dpc; + + if (riscv_xlen(target) > 32) + dpcmask |= (0xFFFFFFFFULL << 32); + + if (riscv_supports_extension(target, riscv_current_hartid(target), 'C')) + dpcmask |= 0x2; + + riscv_set_register(target, GDB_REGNO_DPC, dpcmask); + riscv_get_register(target, &dpc, GDB_REGNO_DPC); + COMPLIANCE_TEST(dpcmask == dpc, + "DPC must be sign-extended to XLEN and writable to all-1s (except the least significant bits)"); + riscv_set_register(target, GDB_REGNO_DPC, 0); + riscv_get_register(target, &dpc, GDB_REGNO_DPC); + COMPLIANCE_TEST(dpc == 0, "DPC must be writable to 0."); + if (hartsel == 0) + bogus_dpc = dpc; /* For a later test step */ + } + + /* NDMRESET + Asserting non-debug module reset should not reset Debug Module state. + But it should reset Hart State, e.g. DPC should get a different value. + Also make sure that DCSR reports cause of 'HALT' even though previously we single-stepped. + */ + + /* Write some registers. They should not be impacted by ndmreset. */ + dmi_write(target, DMI_COMMAND, 0xFFFFFFFF); + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { + testvar = (i + 1) * 0x11111111; + dmi_write(target, DMI_PROGBUF0 + i, testvar); + } + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { + testvar = (i + 1) * 0x11111111; + dmi_write(target, DMI_DATA0 + i, testvar); + } + + dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); + + /* Pulse reset. */ + + target->reset_halt = true; + dmi_read(target, &dmcontrol, DMI_DMCONTROL); + riscv_set_current_hartid(target, 0); + assert_reset(target); + deassert_reset(target); + + /* Verify that most stuff is not affected by ndmreset. */ + dmi_read(target, &testvar_read, DMI_COMMAND); + COMPLIANCE_TEST(testvar_read == 0xFFFFFFFF, "NDMRESET should not affect DMI_COMMAND"); + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, + "NDMRESET should not affect DMI_ABSTRACTCS"); + dmi_read(target, &testvar_read, DMI_ABSTRACTAUTO); + COMPLIANCE_TEST(testvar_read == abstractauto, "NDMRESET should not affect DMI_ABSTRACTAUTO"); + + /* Clean up to avoid future test failures */ + dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + dmi_write(target, DMI_ABSTRACTAUTO, 0); + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { + testvar = (i + 1) * 0x11111111; + dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + COMPLIANCE_TEST(testvar_read == testvar, "PROGBUF words must not be affected by NDMRESET"); + } + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { + testvar = (i + 1) * 0x11111111; + dmi_read(target, &testvar_read, DMI_DATA0 + i); + COMPLIANCE_TEST(testvar_read == testvar, "DATA words must not be affected by NDMRESET"); + } + + /* Verify that DPC *is* affected by ndmreset. Since we don't know what it *should* be, + just verify that at least it's not the bogus value anymore. */ + + COMPLIANCE_TEST(bogus_dpc != 0xdeadbeef, "BOGUS DPC should have been set somehow (bug in compliance test)"); + riscv_get_register(target, &value, GDB_REGNO_DPC); + COMPLIANCE_TEST(bogus_dpc != value, "NDMRESET should move DPC to reset value."); + + COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, + "After NDMRESET halt, DCSR should report cause of halt"); + + /* DMACTIVE -- deasserting DMACTIVE should reset all the above values. */ + + /* Toggle dmactive */ + dmi_write(target, DMI_DMCONTROL, 0); + dmi_write(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); + dmi_read(target, &testvar_read, DMI_COMMAND); + COMPLIANCE_TEST(testvar_read == 0, "DMI_COMMAND should reset to 0"); + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, "ABSTRACTCS.cmderr should reset to 0"); + dmi_read(target, &testvar_read, DMI_ABSTRACTAUTO); + COMPLIANCE_TEST(testvar_read == 0, "ABSTRACTAUTO should reset to 0"); + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { + testvar = (i + 1) * 0x11111111; + dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + COMPLIANCE_TEST(testvar_read == 0, "PROGBUF words should reset to 0"); + } + + for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { + testvar = (i + 1) * 0x11111111; + dmi_read(target, &testvar_read, DMI_DATA0 + i); + COMPLIANCE_TEST(testvar_read == 0, "DATA words should reset to 0"); + } + + /* + * TODO: + * DCSR.cause priorities + * DCSR.stoptime/stopcycle + * DCSR.stepie + * DCSR.ebreak + * DCSR.prv + */ + + /* Halt every hart for any follow-up tests*/ + riscv_halt_all_harts(target); + + LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); + + if (total_tests == passed_tests) + return ERROR_OK; + else + return ERROR_FAIL; } -- cgit v1.1 From 4c6c4cb0782f93115acd40826bc061fc6b4ea93b Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 07:53:34 -0700 Subject: riscv: Add a TODO note we need to handle hartselhi --- src/target/riscv/riscv-013.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8199750..e349a43 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -278,8 +278,9 @@ static void decode_dmi(char *text, unsigned address, unsigned data) { DMI_DMCONTROL, DMI_DMCONTROL_RESUMEREQ, "resumereq" }, { DMI_DMCONTROL, DMI_DMCONTROL_HARTRESET, "hartreset" }, { DMI_DMCONTROL, DMI_DMCONTROL_HASEL, "hasel" }, - { DMI_DMCONTROL, ((1L<<10)-1) << DMI_DMCONTROL_HARTSELLO_OFFSET, "hartsel" }, - { DMI_DMCONTROL, DMI_DMCONTROL_NDMRESET, "ndmreset" }, + { DMI_DMCONTROL, ((1L<<10)-1) << DMI_DMCONTROL_HARTSELLO_OFFSET, "hartsello" }, + /* TODO: hartsellhi */ + { DMI_DMCONTROL, DMI_DMCONTROL_NDMRESET, "ndmreset" }, { DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE, "dmactive" }, { DMI_DMSTATUS, DMI_DMSTATUS_IMPEBREAK, "impebreak" }, -- cgit v1.1 From 716c12bcaf96a44d952e35b4e8658e80179e2996 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 07:57:32 -0700 Subject: riscv: don't supporess errors --- src/target/riscv/riscv-013.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index e349a43..7f4c7e9 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -466,11 +466,7 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in, int retval = jtag_execute_queue(); if (retval != ERROR_OK) { - static int once = 1; - if (once) { - LOG_ERROR("dmi_scan failed jtag scan"); - once = 0; - } + LOG_ERROR("dmi_scan failed jtag scan"); return DMI_STATUS_FAILED; } -- cgit v1.1 From ef684c2e68e646ba05da2c34130b2e1dd67fa02b Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 10:28:13 -0700 Subject: riscv-compliance: Incorporate feedback to make tests make fewer assumptions about hte implementation and properly use OpenOCD functions --- src/target/riscv/riscv-013.c | 287 +++++++++++++++++++------------------------ 1 file changed, 127 insertions(+), 160 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 7f4c7e9..c250c7e 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -264,6 +264,7 @@ static dm013_info_t *get_dm(struct target *target) static uint32_t hartsel_mask(const struct target *target) { RISCV013_INFO(info); + /* TODO: Properly handle hartselhi as well*/ return ((1L<hartsellen)-1) << DMI_DMCONTROL_HARTSELLO_OFFSET; } @@ -2955,30 +2956,34 @@ int riscv013_test_compliance(struct target *target) int total_tests = 0; int passed_tests = 0; - uint32_t dmcontrol_orig; - dmi_read(target, &dmcontrol_orig, DMI_DMCONTROL); + uint32_t dmcontrol_orig = 0; uint32_t dmcontrol; uint32_t testvar; uint32_t testvar_read; riscv_reg_t value; + RISCV013_INFO(info); - dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), RISCV_MAX_HARTS-1); + /* TODO: Support HARTSELLHI as well */ + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSELLO, RISCV_MAX_HARTS-1); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == (RISCV_MAX_HARTS-1), - "DMCONTROL.hartsel should hold all the harts allowed by HARTSELLEN."); + COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSELLO) == (uint32_t) ((1 << info->hartsellen) - 1), + "DMCONTROL.hartsello should hold all the harts allowed by its hartsellen."); - dmcontrol = set_field(dmcontrol_orig, hartsel_mask(target), 0); + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSELLO, 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, hartsel_mask(target)) == 0, "DMCONTROL.hartsel should hold Hart ID 0"); + COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSELLO) == 0, + "DMCONTROL.hartsello should hold Hart ID 0"); /* hartreset */ + /* This field is optional. Either we can read and write it to 1/0, + or it is tied to 0. */ dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_read(target, &dmcontrol, DMI_DMCONTROL); testvar = get_field(dmcontrol, DMI_DMCONTROL_HARTRESET); - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_read(target, &dmcontrol, DMI_DMCONTROL); COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HARTRESET)) == 0), @@ -2989,7 +2994,7 @@ int riscv013_test_compliance(struct target *target) dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_read(target, &dmcontrol, DMI_DMCONTROL); testvar = get_field(dmcontrol, DMI_DMCONTROL_HASEL); - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); + dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_read(target, &dmcontrol, DMI_DMCONTROL); COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HASEL)) == 0), @@ -2998,45 +3003,21 @@ int riscv013_test_compliance(struct target *target) /* haltreq */ riscv_halt_all_harts(target); - /* Writing haltreq should not cause any problems for a halted hart, but we - should be able to read and write it. */ - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - dmcontrol |= DMI_DMCONTROL_HALTREQ; - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(dmcontrol & DMI_DMCONTROL_HALTREQ, "DMCONTROL.haltreq should be R/W"); - uint32_t dmstatus, dmstatus_read; - do { - dmi_read(target, &dmstatus, DMI_DMSTATUS); - } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); + /* This bit is not actually readable according to the spec, so nothing to check.*/ - dmi_write(target, DMI_DMSTATUS, 0xffffffff); + /* DMSTATUS */ + uint32_t dmstatus, dmstatus_read; + dmi_read(target, &dmstatus, DMI_DMSTATUS); + dmi_write(target, DMI_DMSTATUS, ~dmstatus); dmi_read(target, &dmstatus_read, DMI_DMSTATUS); COMPLIANCE_TEST(dmstatus_read == dmstatus, "DMSTATUS is R/O"); - /* resumereq. This will resume the hart but this test is destructive anyway. */ - dmcontrol &= ~DMI_DMCONTROL_HALTREQ; - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ) == 1, - "DMCONTROL.resumereq should be R/W"); - - do { - dmi_read(target, &dmstatus, DMI_DMSTATUS); - } while (get_field(dmstatus, DMI_DMSTATUS_ALLRESUMEACK) == 0); + /* resumereq */ + /* This bit is not actually readable according to the spec, so nothing to check.*/ + riscv_resume_all_harts(target); - /* Halt the hart again because the target isn't aware that we resumed it. */ - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0); - dmcontrol |= DMI_DMCONTROL_HALTREQ; - dmi_write(target, DMI_DMCONTROL, dmcontrol); - do { - dmi_read(target, &dmstatus, DMI_DMSTATUS); - } while ((dmstatus & DMI_DMSTATUS_ALLHALTED) == 0); - dmcontrol &= ~DMI_DMCONTROL_HALTREQ; - dmi_write(target, DMI_DMCONTROL, dmcontrol); - /* Not clear that this read is required according to the spec. */ - dmi_read(target, &dmstatus, DMI_DMSTATUS); + /* Halt all harts again so the test can continue.*/ + riscv_halt_all_harts(target); /* HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. */ for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { @@ -3048,32 +3029,37 @@ int riscv013_test_compliance(struct target *target) dmi_read(target, &hartinfo_read, DMI_HARTINFO); COMPLIANCE_TEST((hartinfo_read == hartinfo), "DMHARTINFO should be Read-Only."); + /* $dscratch CSRs */ uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); for (unsigned int d = 0; d < nscratch; d++) { - /* TODO: DSCRATCH CSRs should be 64-bit on 64-bit systems. */ - riscv_reg_t testval; - for (testval = 0x0011223300112233; - testval != 0xDEAD; - testval = testval == 0x0011223300112233 ? ~testval : 0xDEAD) { - COMPLIANCE_TEST(register_write_direct(target, GDB_REGNO_S0, testval) == ERROR_OK, - "Need to be able to write S0 in order to test DSCRATCH."); - struct riscv_program program32; - riscv_program_init(&program32, target); - riscv_program_csrw(&program32, GDB_REGNO_S0, GDB_REGNO_DSCRATCH + d); - riscv_program_csrr(&program32, GDB_REGNO_S1, GDB_REGNO_DSCRATCH + d); - riscv_program_fence(&program32); - riscv_program_ebreak(&program32); - COMPLIANCE_TEST(riscv_program_exec(&program32, target) == ERROR_OK, - "Accessing DSCRATCH with program buffer should succeed."); - COMPLIANCE_TEST(register_read_direct(target, &value, GDB_REGNO_S1) == ERROR_OK, - "Need to be able to read S1 in order to test DSCRATCH."); - if (riscv_xlen(target) > 32) - COMPLIANCE_TEST(value == testval, - "All DSCRATCH registers in HARTINFO must be R/W."); - else - COMPLIANCE_TEST(value == (testval & 0xFFFFFFFF), - "All DSCRATCH registers in HARTINFO must be R/W."); - } + riscv_reg_t testval, testval_read; + /* Because DSCRATCH is not guaranteed to last across PB executions, need to put + this all into one PB execution. Which may not be possible on all implementations.*/ + if (info->progbufsize >= 5) { + for (testval = 0x0011223300112233; + testval != 0xDEAD; + testval = testval == 0x0011223300112233 ? ~testval : 0xDEAD) { + COMPLIANCE_TEST(register_write_direct(target, GDB_REGNO_S0, testval) == ERROR_OK, + "Need to be able to write S0 in order to test DSCRATCH."); + struct riscv_program program32; + riscv_program_init(&program32, target); + riscv_program_csrw(&program32, GDB_REGNO_S0, GDB_REGNO_DSCRATCH + d); + riscv_program_csrr(&program32, GDB_REGNO_S1, GDB_REGNO_DSCRATCH + d); + riscv_program_fence(&program32); + riscv_program_ebreak(&program32); + COMPLIANCE_TEST(riscv_program_exec(&program32, target) == ERROR_OK, + "Accessing DSCRATCH with program buffer should succeed."); + COMPLIANCE_TEST(register_read_direct(target, &testval_read, GDB_REGNO_S1) == ERROR_OK, + "Need to be able to read S1 in order to test DSCRATCH."); + if (riscv_xlen(target) > 32) { + COMPLIANCE_TEST(testval == testval_read, + "All DSCRATCH registers in HARTINFO must be R/W."); + } else { + COMPLIANCE_TEST(testval_read == (testval & 0xFFFFFFFF), + "All DSCRATCH registers in HARTINFO must be R/W."); + } + } + } } /* TODO: dataaccess */ if (get_field(hartinfo, DMI_HARTINFO_DATAACCESS)) { @@ -3153,7 +3139,7 @@ int riscv013_test_compliance(struct target *target) } } - /*Check that all reported ProgBuf words are really R/W */ + /* Check that all reported ProgBuf words are really R/W */ for (int invert = 0; invert < 2; invert++) { for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { testvar = (i + 1) * 0x11111111; @@ -3173,60 +3159,35 @@ int riscv013_test_compliance(struct target *target) /* TODO: Cause and clear all error types */ /* COMMAND - TODO: Unclear from the spec whether all these bits need to truly be R/W. + According to the spec, this register is only W, so can't really check the read result. But at any rate, this is not legal and should cause an error. */ dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0xAAAAAAAA, "COMMAND register should be R/W"); dmi_read(target, &testvar_read, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ "Illegal COMMAND should result in UNSUPPORTED"); dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); dmi_write(target, DMI_COMMAND, 0x55555555); dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0x55555555, "COMMAND register should be R/W"); dmi_read(target, &testvar_read, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ "Illegal COMMAND should result in UNSUPPORTED"); dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); /* Basic Abstract Commands */ - uint32_t command = 0; - uint32_t busy; - command = set_field(command, AC_ACCESS_REGISTER_SIZE, riscv_xlen(target) > 32 ? 3 : 2); - command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); for (unsigned int i = 1; i < 32; i = i << 1) { - command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + GDB_REGNO_ZERO + i); - command = set_field(command, AC_ACCESS_REGISTER_WRITE, 1); - dmi_write(target, DMI_DATA0, i); - if (riscv_xlen(target) > 32) - dmi_write(target, DMI_DATA0 + 1, i + 1); - dmi_write(target, DMI_COMMAND, command); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, + riscv_reg_t testval = i | ((i + 1ULL) << 32); + riscv_reg_t testval_read; + COMPLIANCE_TEST(ERROR_OK == register_write_direct(target, GDB_REGNO_ZERO + i, testval), "GPR Writes should be supported."); - dmi_write(target, DMI_DATA0, 0xDEADBEEF); - if (riscv_xlen(target) > 32) - dmi_write(target, DMI_DATA0 + 1, 0xDEADBEEF); - command = set_field(command, AC_ACCESS_REGISTER_WRITE, 0); - dmi_write(target, DMI_COMMAND, command); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, + write_abstract_arg(target, 0, 0xDEADBEEFDEADBEEF, 64); + COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &testval_read, GDB_REGNO_ZERO + i), "GPR Reads should be supported."); - dmi_read(target, &testvar_read, DMI_DATA0); - COMPLIANCE_TEST(testvar_read == i, "GPR Reads and writes should be supported."); if (riscv_xlen(target) > 32) { - dmi_read(target, &testvar_read, DMI_DATA0 + 1); - COMPLIANCE_TEST(testvar_read == (i + 1), - "GPR Reads and writes should be supported."); + COMPLIANCE_TEST(testval == testval_read, "GPR Reads and writes should be supported."); + } else { + COMPLIANCE_TEST((testval & 0xFFFFFFFF) == testval_read, "GPR Reads and writes should be supported."); + } } @@ -3234,58 +3195,64 @@ int riscv013_test_compliance(struct target *target) See which bits are actually writable */ dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); uint32_t abstractauto; + uint32_t busy; dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); dmi_write(target, DMI_ABSTRACTAUTO, 0x0); if (abstractauto > 0) { - testvar = 0; - /* TODO: This mechanism only works when you have a reasonable sized progbuf, which is not + /* This mechanism only works when you have a reasonable sized progbuf, which is not a true compliance requirement. */ - uint32_t result = riscv_set_register(target, GDB_REGNO_S0, 0); - COMPLIANCE_TEST(result == ERROR_OK, "Need to be able to write S0 to test ABSTRACTAUTO"); - struct riscv_program program; - riscv_program_init(&program, target); - /* Also testing that WFI() is a NOP during debug mode. */ - riscv_program_insert(&program, wfi()); - riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); - riscv_program_ebreak(&program); - dmi_write(target, DMI_ABSTRACTAUTO, 0x0); - riscv_program_exec(&program, target); - testvar++; - dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); - dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); - uint32_t autoexec_data = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECDATA); - uint32_t autoexec_progbuf = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECPROGBUF); - for (unsigned int i = 0; i < 12; i++) { - dmi_read(target, &testvar_read, DMI_DATA0 + i); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - if (autoexec_data & (1 << i)) { - COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT), - "AUTOEXEC may be writable up to DATACOUNT bits."); - testvar++; + if (info->progbufsize >= 3) { + + testvar = 0; + COMPLIANCE_TEST(ERROR_OK == register_write_direct(target, GDB_REGNO_S0, 0), + "Need to be able to write S0 to test ABSTRACTAUTO"); + struct riscv_program program; + riscv_program_init(&program, target); + /* This is also testing that WFI() is a NOP during debug mode. */ + riscv_program_insert(&program, wfi()); + riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); + riscv_program_ebreak(&program); + dmi_write(target, DMI_ABSTRACTAUTO, 0x0); + riscv_program_exec(&program, target); + testvar++; + dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); + uint32_t autoexec_data = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECDATA); + uint32_t autoexec_progbuf = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECPROGBUF); + for (unsigned int i = 0; i < 12; i++) { + dmi_read(target, &testvar_read, DMI_DATA0 + i); + do { + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); + } while (busy); + if (autoexec_data & (1 << i)) { + COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT), + "AUTOEXEC may be writable up to DATACOUNT bits."); + testvar++; + } } - } - for (unsigned int i = 0; i < 16; i++) { - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); - do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); - busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); - } while (busy); - if (autoexec_progbuf & (1 << i)) { - COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE), - "AUTOEXEC may be writable up to PROGBUFSIZE bits."); - testvar++; + for (unsigned int i = 0; i < 16; i++) { + dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + do { + dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); + } while (busy); + if (autoexec_progbuf & (1 << i)) { + COMPLIANCE_TEST(i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE), + "AUTOEXEC may be writable up to PROGBUFSIZE bits."); + testvar++; + } } - } - - dmi_write(target, DMI_ABSTRACTAUTO, 0); - riscv_get_register(target, &value, GDB_REGNO_S0); - - COMPLIANCE_TEST(testvar == value, \ - "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); - } + + dmi_write(target, DMI_ABSTRACTAUTO, 0); + COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &value, GDB_REGNO_S0), + "Need to be able to read S0 to test ABSTRACTAUTO"); + + COMPLIANCE_TEST(testvar == value, + "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); + } + + } /* Single-Step each hart. */ for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { @@ -3293,7 +3260,7 @@ int riscv013_test_compliance(struct target *target) riscv013_on_step(target); riscv013_step_current_hart(target); COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, - "Single Step should result in SINGLESTEP"); + "Single Step should result in SINGLESTEP"); } /* Core Register Tests */ @@ -3302,11 +3269,11 @@ int riscv013_test_compliance(struct target *target) riscv_set_current_hartid(target, hartsel); /* DCSR Tests */ - riscv_set_register(target, GDB_REGNO_DCSR, 0x0); - riscv_get_register(target, &value, GDB_REGNO_DCSR); + register_write_direct(target, GDB_REGNO_DCSR, 0x0); + register_read_direct(target, &value, GDB_REGNO_DCSR); COMPLIANCE_TEST(value != 0, "Not all bits in DCSR are writable by Debugger"); - riscv_set_register(target, GDB_REGNO_DCSR, 0xFFFFFFFF); - riscv_get_register(target, &value, GDB_REGNO_DCSR); + register_write_direct(target, GDB_REGNO_DCSR, 0xFFFFFFFF); + register_read_direct(target, &value, GDB_REGNO_DCSR); COMPLIANCE_TEST(value != 0, "At least some bits in DCSR must be 1"); /* DPC. Note that DPC is sign-extended. */ @@ -3319,12 +3286,12 @@ int riscv013_test_compliance(struct target *target) if (riscv_supports_extension(target, riscv_current_hartid(target), 'C')) dpcmask |= 0x2; - riscv_set_register(target, GDB_REGNO_DPC, dpcmask); - riscv_get_register(target, &dpc, GDB_REGNO_DPC); + register_write_direct(target, GDB_REGNO_DPC, dpcmask); + register_read_direct(target, &dpc, GDB_REGNO_DPC); COMPLIANCE_TEST(dpcmask == dpc, - "DPC must be sign-extended to XLEN and writable to all-1s (except the least significant bits)"); - riscv_set_register(target, GDB_REGNO_DPC, 0); - riscv_get_register(target, &dpc, GDB_REGNO_DPC); + "DPC must be sign-extended to XLEN and writable to all-1s (except the least significant bits)"); + register_write_direct(target, GDB_REGNO_DPC, 0); + register_read_direct(target, &dpc, GDB_REGNO_DPC); COMPLIANCE_TEST(dpc == 0, "DPC must be writable to 0."); if (hartsel == 0) bogus_dpc = dpc; /* For a later test step */ @@ -3389,7 +3356,7 @@ int riscv013_test_compliance(struct target *target) just verify that at least it's not the bogus value anymore. */ COMPLIANCE_TEST(bogus_dpc != 0xdeadbeef, "BOGUS DPC should have been set somehow (bug in compliance test)"); - riscv_get_register(target, &value, GDB_REGNO_DPC); + register_read_direct(target, &value, GDB_REGNO_DPC); COMPLIANCE_TEST(bogus_dpc != value, "NDMRESET should move DPC to reset value."); COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, -- cgit v1.1 From 30e1dbdc6bc08b276b7ec23f8edb81f89d91780e Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 10:43:36 -0700 Subject: riscv-compliance: fix compile errors and whitespace --- src/target/riscv/riscv-013.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index c250c7e..2024088 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -281,7 +281,7 @@ static void decode_dmi(char *text, unsigned address, unsigned data) { DMI_DMCONTROL, DMI_DMCONTROL_HASEL, "hasel" }, { DMI_DMCONTROL, ((1L<<10)-1) << DMI_DMCONTROL_HARTSELLO_OFFSET, "hartsello" }, /* TODO: hartsellhi */ - { DMI_DMCONTROL, DMI_DMCONTROL_NDMRESET, "ndmreset" }, + { DMI_DMCONTROL, DMI_DMCONTROL_NDMRESET, "ndmreset" }, { DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE, "dmactive" }, { DMI_DMSTATUS, DMI_DMSTATUS_IMPEBREAK, "impebreak" }, @@ -2977,7 +2977,7 @@ int riscv013_test_compliance(struct target *target) "DMCONTROL.hartsello should hold Hart ID 0"); /* hartreset */ - /* This field is optional. Either we can read and write it to 1/0, + /* This field is optional. Either we can read and write it to 1/0, or it is tied to 0. */ dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); dmi_write(target, DMI_DMCONTROL, dmcontrol); @@ -3006,7 +3006,7 @@ int riscv013_test_compliance(struct target *target) /* This bit is not actually readable according to the spec, so nothing to check.*/ /* DMSTATUS */ - uint32_t dmstatus, dmstatus_read; + uint32_t dmstatus, dmstatus_read; dmi_read(target, &dmstatus, DMI_DMSTATUS); dmi_write(target, DMI_DMSTATUS, ~dmstatus); dmi_read(target, &dmstatus_read, DMI_DMSTATUS); @@ -3032,11 +3032,11 @@ int riscv013_test_compliance(struct target *target) /* $dscratch CSRs */ uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); for (unsigned int d = 0; d < nscratch; d++) { - riscv_reg_t testval, testval_read; + riscv_reg_t testval, testval_read; /* Because DSCRATCH is not guaranteed to last across PB executions, need to put this all into one PB execution. Which may not be possible on all implementations.*/ - if (info->progbufsize >= 5) { - for (testval = 0x0011223300112233; + if (info->progbufsize >= 5) { + for (testval = 0x0011223300112233; testval != 0xDEAD; testval = testval == 0x0011223300112233 ? ~testval : 0xDEAD) { COMPLIANCE_TEST(register_write_direct(target, GDB_REGNO_S0, testval) == ERROR_OK, @@ -3059,7 +3059,7 @@ int riscv013_test_compliance(struct target *target) "All DSCRATCH registers in HARTINFO must be R/W."); } } - } + } } /* TODO: dataaccess */ if (get_field(hartinfo, DMI_HARTINFO_DATAACCESS)) { @@ -3177,17 +3177,16 @@ int riscv013_test_compliance(struct target *target) /* Basic Abstract Commands */ for (unsigned int i = 1; i < 32; i = i << 1) { riscv_reg_t testval = i | ((i + 1ULL) << 32); - riscv_reg_t testval_read; + riscv_reg_t testval_read; COMPLIANCE_TEST(ERROR_OK == register_write_direct(target, GDB_REGNO_ZERO + i, testval), "GPR Writes should be supported."); write_abstract_arg(target, 0, 0xDEADBEEFDEADBEEF, 64); COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &testval_read, GDB_REGNO_ZERO + i), "GPR Reads should be supported."); if (riscv_xlen(target) > 32) { - COMPLIANCE_TEST(testval == testval_read, "GPR Reads and writes should be supported."); + COMPLIANCE_TEST(testval == testval_read, "GPR Reads and writes should be supported."); } else { - COMPLIANCE_TEST((testval & 0xFFFFFFFF) == testval_read, "GPR Reads and writes should be supported."); - + COMPLIANCE_TEST((testval & 0xFFFFFFFF) == testval_read, "GPR Reads and writes should be supported."); } } @@ -3203,7 +3202,7 @@ int riscv013_test_compliance(struct target *target) a true compliance requirement. */ if (info->progbufsize >= 3) { - testvar = 0; + testvar = 0; COMPLIANCE_TEST(ERROR_OK == register_write_direct(target, GDB_REGNO_S0, 0), "Need to be able to write S0 to test ABSTRACTAUTO"); struct riscv_program program; @@ -3243,16 +3242,15 @@ int riscv013_test_compliance(struct target *target) testvar++; } } - + dmi_write(target, DMI_ABSTRACTAUTO, 0); COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &value, GDB_REGNO_S0), "Need to be able to read S0 to test ABSTRACTAUTO"); - + COMPLIANCE_TEST(testvar == value, "ABSTRACTAUTO should cause COMMAND to run the expected number of times."); - } - - } + } + } /* Single-Step each hart. */ for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { -- cgit v1.1 From aef488824917caa31d8b611192e816d2fc28f54f Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 11:55:50 -0700 Subject: riscv-compliance: Fix writing hartsello --- src/target/riscv/riscv-013.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 2024088..c062de1 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2964,7 +2964,7 @@ int riscv013_test_compliance(struct target *target) RISCV013_INFO(info); /* TODO: Support HARTSELLHI as well */ - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSELLO, RISCV_MAX_HARTS-1); + dmcontrol = dmcontrol_orig | DMI_DMCONTROL_HARTSELLO; dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_read(target, &dmcontrol, DMI_DMCONTROL); COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSELLO) == (uint32_t) ((1 << info->hartsellen) - 1), -- cgit v1.1 From f516825079b8965e9c0bb6bea5146e7045807ca0 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 14:30:37 -0700 Subject: riscv-compliance: make sure not to clear DMACTIVE --- src/target/riscv/riscv-013.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index c062de1..9d7bb95 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2956,7 +2956,7 @@ int riscv013_test_compliance(struct target *target) int total_tests = 0; int passed_tests = 0; - uint32_t dmcontrol_orig = 0; + uint32_t dmcontrol_orig = DMI_DMCONTROL_DMACTIVE; uint32_t dmcontrol; uint32_t testvar; uint32_t testvar_read; @@ -3320,7 +3320,6 @@ int riscv013_test_compliance(struct target *target) /* Pulse reset. */ target->reset_halt = true; - dmi_read(target, &dmcontrol, DMI_DMCONTROL); riscv_set_current_hartid(target, 0); assert_reset(target); deassert_reset(target); -- cgit v1.1 From 401dcf7a06c707f01aca0f0ad147fcc65a847a1d Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 15:47:15 -0700 Subject: riscv-compliance: make sure reset assertion and deassertion actually worked. --- src/target/riscv/riscv-013.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 9d7bb95..3af52f9 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3318,11 +3318,10 @@ int riscv013_test_compliance(struct target *target) dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); /* Pulse reset. */ - target->reset_halt = true; riscv_set_current_hartid(target, 0); - assert_reset(target); - deassert_reset(target); + COMPLIANCE_TEST(ERROR_OK == assert_reset(target), "Must be able to assert NDMRESET"); + COMPLIANCE_TEST(ERROR_OK == deassert_reset(target), "Must be able to deassert NDMRESET"); /* Verify that most stuff is not affected by ndmreset. */ dmi_read(target, &testvar_read, DMI_COMMAND); -- cgit v1.1 From 8ce4f787ca429cedac4da9c8931ac702d270ebb4 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 16:05:15 -0700 Subject: riscv-compliance: whitespace cleanup --- src/target/riscv/riscv-013.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 6e560e2..c7d5068 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1598,9 +1598,15 @@ static int deassert_reset(struct target *target) if (target->reset_halt) { LOG_DEBUG("Waiting for hart %d to halt out of reset.", index); + /* set this temporarily because this read could take the entire + * time the hart takes to come out of reset. */ + int saved_riscv_command_timeout_sec = riscv_command_timeout_sec; + riscv_command_timeout_sec = MAX(riscv_reset_timeout_sec, riscv_command_timeout_sec); do { if (dmstatus_read(target, &dmstatus, true) != ERROR_OK) return ERROR_FAIL; + if (get_field(dmstatus, DMI_DMSTATUS_ALLHALTED) == 1) + break; if (time(NULL) - start > riscv_reset_timeout_sec) { LOG_ERROR("Hart %d didn't halt coming out of reset in %ds; " "dmstatus=0x%x; " @@ -1610,9 +1616,11 @@ static int deassert_reset(struct target *target) } } while (get_field(dmstatus, DMI_DMSTATUS_ALLHALTED) == 0); target->state = TARGET_HALTED; - + riscv_command_timeout_sec = saved_riscv_command_timeout_sec; } else { LOG_DEBUG("Waiting for hart %d to run out of reset.", index); + int saved_riscv_command_timeout_sec = riscv_command_timeout_sec; + riscv_command_timeout_sec = MAX(riscv_reset_timeout_sec, riscv_command_timeout_sec); while (get_field(dmstatus, DMI_DMSTATUS_ALLRUNNING) == 0) { if (dmstatus_read(target, &dmstatus, true) != ERROR_OK) return ERROR_FAIL; @@ -1622,6 +1630,8 @@ static int deassert_reset(struct target *target) index, dmstatus); return ERROR_FAIL; } + if (get_field(dmstatus, DMI_DMSTATUS_ALLRUNNING) == 1) + break; if (time(NULL) - start > riscv_reset_timeout_sec) { LOG_ERROR("Hart %d didn't run coming out of reset in %ds; " "dmstatus=0x%x; " @@ -1631,6 +1641,7 @@ static int deassert_reset(struct target *target) } } target->state = TARGET_RUNNING; + riscv_command_timeout_sec = saved_riscv_command_timeout_sec; } if (get_field(dmstatus, DMI_DMSTATUS_ALLHAVERESET)) { @@ -3202,11 +3213,10 @@ int riscv013_test_compliance(struct target *target) write_abstract_arg(target, 0, 0xDEADBEEFDEADBEEF, 64); COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &testval_read, GDB_REGNO_ZERO + i), "GPR Reads should be supported."); - if (riscv_xlen(target) > 32) { + if (riscv_xlen(target) > 32) COMPLIANCE_TEST(testval == testval_read, "GPR Reads and writes should be supported."); - } else { + else COMPLIANCE_TEST((testval & 0xFFFFFFFF) == testval_read, "GPR Reads and writes should be supported."); - } } /* ABSTRACTAUTO @@ -3221,7 +3231,7 @@ int riscv013_test_compliance(struct target *target) a true compliance requirement. */ if (info->progbufsize >= 3) { - testvar = 0; + testvar = 0; COMPLIANCE_TEST(ERROR_OK == register_write_direct(target, GDB_REGNO_S0, 0), "Need to be able to write S0 to test ABSTRACTAUTO"); struct riscv_program program; -- cgit v1.1 From 8fa81c1f979e19e313b8a83e5f23dd19feca1e46 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 17 Apr 2018 16:11:03 -0700 Subject: riscv-compliance... code that compiles > code that makes linter happy --- src/target/riscv/riscv-013.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index c7d5068..85d0ca9 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3213,10 +3213,11 @@ int riscv013_test_compliance(struct target *target) write_abstract_arg(target, 0, 0xDEADBEEFDEADBEEF, 64); COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &testval_read, GDB_REGNO_ZERO + i), "GPR Reads should be supported."); - if (riscv_xlen(target) > 32) + if (riscv_xlen(target) > 32) { COMPLIANCE_TEST(testval == testval_read, "GPR Reads and writes should be supported."); - else + } else { COMPLIANCE_TEST((testval & 0xFFFFFFFF) == testval_read, "GPR Reads and writes should be supported."); + } } /* ABSTRACTAUTO -- cgit v1.1 From 1616a710734fb9a0278412b0c0e947f6499d054e Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 18 Apr 2018 15:44:57 -0700 Subject: riscv: attempt something else for travis (probably won't work otherwise Tim would have already done it this way...) --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2aeed08..452bcab 100644 --- a/.travis.yml +++ b/.travis.yml @@ -55,6 +55,6 @@ script: # 50 changes any case. Most merges won't consist of more than 40 changes, # so this should work fine most of the time, and be a lot better than not # checking at all. - - git diff HEAD~40 | ./tools/scripts/checkpatch.pl --no-signoff - + - git diff origin/riscv | ./tools/scripts/checkpatch.pl --no-signoff - - ./bootstrap && ./configure --enable-remote-bitbang --enable-jtag_vpi $CONFIGURE_ARGS && make - file src/$EXECUTABLE -- cgit v1.1 From 06fc61f464dfcf7e61f5efb93903803977a74536 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 18 Apr 2018 16:10:41 -0700 Subject: riscv-compliance: whitespace --- src/target/riscv/riscv-013.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f39aa70..1de3b92 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3213,7 +3213,7 @@ int riscv013_test_compliance(struct target *target) COMPLIANCE_TEST(testval == testval_read, "GPR Reads and writes should be supported."); } else { COMPLIANCE_TEST((testval & 0xFFFFFFFF) == testval_read, "GPR Reads and writes should be supported."); - } + } } /* ABSTRACTAUTO -- cgit v1.1 From ac953c71c0bcf7a2fcc17080ae199ad91c677353 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Wed, 18 Apr 2018 16:15:07 -0700 Subject: riscv-compliance: add dummy comments to appease the linter --- src/target/riscv/riscv-013.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 1de3b92..92f290b 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3210,8 +3210,10 @@ int riscv013_test_compliance(struct target *target) COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &testval_read, GDB_REGNO_ZERO + i), "GPR Reads should be supported."); if (riscv_xlen(target) > 32) { + /* Dummy comment to satisfy linter, since removing the brances here doesn't actually compile. */ COMPLIANCE_TEST(testval == testval_read, "GPR Reads and writes should be supported."); } else { + /* Dummy comment to satisfy linter, since removing the brances here doesn't actually compile. */ COMPLIANCE_TEST((testval & 0xFFFFFFFF) == testval_read, "GPR Reads and writes should be supported."); } } -- cgit v1.1 From debf2b040a98202105aeae733e7dfe4d43c0f8eb Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 19 Apr 2018 10:36:52 -0700 Subject: riscv-compliance: correct the HALTSUM0/HALTSUM1 checks --- src/target/riscv/riscv-013.c | 45 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 92f290b..f1b14c7 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3100,15 +3100,16 @@ int riscv013_test_compliance(struct target *target) } - /* HALTSUM -- TODO: More than 32 harts */ + /* HALTSUM -- TODO: More than 32 harts. Would need to loop over this to set hartsel */ /* TODO: HALTSUM2, HALTSUM3 */ + /* HALTSUM0 */ uint32_t expected_haltsum0 = 0; - for (int i = 0; i < riscv_count_harts(target); i += 32) - expected_haltsum0 |= (1 << (i / 32)); + for (int i = 0; i < MIN(riscv_count_harts(target), 32); i ++) + expected_haltsum0 |= (1 << i); dmi_read(target, &testvar_read, DMI_HALTSUM0); COMPLIANCE_TEST(testvar_read == expected_haltsum0, - "HALTSUM0 should report summary of 32 halted harts"); + "HALTSUM0 should report summary of up to 32 halted harts"); dmi_write(target, DMI_HALTSUM0, 0xffffffff); dmi_read(target, &testvar_read, DMI_HALTSUM0); @@ -3118,26 +3119,22 @@ int riscv013_test_compliance(struct target *target) dmi_read(target, &testvar_read, DMI_HALTSUM0); COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); - for (int i = 0; i < 32/*TODO: riscv_count_harts(target)*/; i += 32) { - /* TODO: Set hartsel for i > 32 harts. */ - dmi_read(target, &testvar_read, DMI_HALTSUM1); - uint32_t haltsum1_expected = (((i + 1) * 32) <= riscv_count_harts(target)) ? - 0xFFFFFFFFU : - ((1U << (riscv_count_harts(target) % 32)) - 1); - COMPLIANCE_TEST(testvar_read == haltsum1_expected, - "HALTSUM1 should report summary of 1024 halted harts"); - - /* Just have to check this once */ - if (i == 0) { - dmi_write(target, DMI_HALTSUM1, 0xffffffff); - dmi_read(target, &testvar_read, DMI_HALTSUM1); - COMPLIANCE_TEST(testvar_read == haltsum1_expected, "HALTSUM1 should be R/O"); - - dmi_write(target, DMI_HALTSUM1, 0x0); - dmi_read(target, &testvar_read, DMI_HALTSUM1); - COMPLIANCE_TEST(testvar_read == haltsum1_expected, "HALTSUM1 should be R/O"); - } - } + /* HALTSUM1 */ + uint32_t expected_haltsum1 = 0; + for (int i = 0; i < MIN(riscv_count_harts(target), 1024); i +=32) + expected_haltsum1 |= (1 << (i/32)); + + dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_TEST(testvar_read == expected_haltsum1, + "HALTSUM1 should report summary of up to 1024 halted harts"); + + dmi_write(target, DMI_HALTSUM1, 0xffffffff); + dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should be R/O"); + + dmi_write(target, DMI_HALTSUM1, 0x0); + dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should be R/O"); /* TODO: HAWINDOWSEL */ -- cgit v1.1 From eeac4f7fd4a607d2acffd8bfb9ac2e2d5ed45bcb Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 19 Apr 2018 10:49:45 -0700 Subject: riscv-compliance: remove whitespace --- src/target/riscv/riscv-013.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f1b14c7..632a76e 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3102,9 +3102,9 @@ int riscv013_test_compliance(struct target *target) /* HALTSUM -- TODO: More than 32 harts. Would need to loop over this to set hartsel */ /* TODO: HALTSUM2, HALTSUM3 */ - /* HALTSUM0 */ + /* HALTSUM0 */ uint32_t expected_haltsum0 = 0; - for (int i = 0; i < MIN(riscv_count_harts(target), 32); i ++) + for (int i = 0; i < MIN(riscv_count_harts(target), 32); i++) expected_haltsum0 |= (1 << i); dmi_read(target, &testvar_read, DMI_HALTSUM0); @@ -3120,18 +3120,18 @@ int riscv013_test_compliance(struct target *target) COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); /* HALTSUM1 */ - uint32_t expected_haltsum1 = 0; - for (int i = 0; i < MIN(riscv_count_harts(target), 1024); i +=32) + uint32_t expected_haltsum1 = 0; + for (int i = 0; i < MIN(riscv_count_harts(target), 1024); i += 32) expected_haltsum1 |= (1 << (i/32)); - dmi_read(target, &testvar_read, DMI_HALTSUM1); + dmi_read(target, &testvar_read, DMI_HALTSUM1); COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should report summary of up to 1024 halted harts"); dmi_write(target, DMI_HALTSUM1, 0xffffffff); dmi_read(target, &testvar_read, DMI_HALTSUM1); COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should be R/O"); - + dmi_write(target, DMI_HALTSUM1, 0x0); dmi_read(target, &testvar_read, DMI_HALTSUM1); COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should be R/O"); -- cgit v1.1 From 934440b80ea665ba503b4429e6b056765cc2694b Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 30 Aug 2018 11:26:05 -0700 Subject: riscv-compliance: incorporate review feedback --- src/target/riscv/riscv-013.c | 231 ++++++++++++++++++++----------------------- 1 file changed, 108 insertions(+), 123 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index cce5f47..ec7b2f1 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3058,6 +3058,21 @@ void riscv013_clear_abstract_error(struct target *target) total_tests++; \ } +#define COMPLIANCE_MUST_PASS(b) COMPLIANCE_TEST(ERROR_OK == (b), "Regular calls must return ERROR_OK") + +#define COMPLIANCE_READ(target, addr, value) COMPLIANCE_MUST_PASS(dmi_read(target, addr, value)) +#define COMPLIANCE_WRITE(target, addr, value) COMPLIANCE_MUST_PASS(dmi_write(target, addr, value)) + +#define COMPLIANCE_CHECK_RO(target, addr) \ +{ \ + uint32_t orig; \ + uint32_t inverse; \ + COMPLIANCE_READ(target, &orig, addr); \ + COMPLIANCE_WRITE(target, addr, ~orig); \ + COMPLIANCE_READ(target, &inverse, addr); \ + COMPLIANCE_TEST(orig == inverse, "Register must be read-only"); \ +} + int riscv013_test_compliance(struct target *target) { LOG_INFO("Testing Compliance against RISC-V Debug Spec v0.13"); @@ -3077,71 +3092,48 @@ int riscv013_test_compliance(struct target *target) riscv_reg_t value; RISCV013_INFO(info); - /* TODO: Support HARTSELLHI as well */ - dmcontrol = dmcontrol_orig | DMI_DMCONTROL_HARTSELLO; - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSELLO) == (uint32_t) ((1 << info->hartsellen) - 1), - "DMCONTROL.hartsello should hold all the harts allowed by its hartsellen."); - - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTSELLO, 0); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(get_field(dmcontrol, DMI_DMCONTROL_HARTSELLO) == 0, - "DMCONTROL.hartsello should hold Hart ID 0"); + /* All the bits of HARTSEL is covered by the examine sequence. */ /* hartreset */ /* This field is optional. Either we can read and write it to 1/0, - or it is tied to 0. */ - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - testvar = get_field(dmcontrol, DMI_DMCONTROL_HARTRESET); - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 0); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HARTRESET)) == 0), - "DMCONTROL.hartreset can be 0 or RW."); + or it is tied to 0. This check doesn't really do anything, but + it does attempt to set the bit to 1 and then back to 0, which needs to + work if its implemented. */ + COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1)); + COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 0)); + COMPLIANCE_READ(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST((get_field(dmcontrol, DMI_DMCONTROL_HARTRESET) == 0), + "DMCONTROL.hartreset can be 0 or RW."); /* hasel */ - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - testvar = get_field(dmcontrol, DMI_DMCONTROL_HASEL); - dmcontrol = set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 0); - dmi_write(target, DMI_DMCONTROL, dmcontrol); - dmi_read(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST(((testvar == 0) || (get_field(dmcontrol, DMI_DMCONTROL_HASEL)) == 0), + COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1)); + COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 0)); + COMPLIANCE_READ(target, &dmcontrol, DMI_DMCONTROL); + COMPLIANCE_TEST((get_field(dmcontrol, DMI_DMCONTROL_HASEL) == 0), "DMCONTROL.hasel can be 0 or RW."); /* TODO: test that hamask registers exist if hasel does. */ /* haltreq */ - riscv_halt_all_harts(target); + COMPLIANCE_MUST_PASS(riscv_halt_all_harts(target)); /* This bit is not actually readable according to the spec, so nothing to check.*/ /* DMSTATUS */ - uint32_t dmstatus, dmstatus_read; - dmi_read(target, &dmstatus, DMI_DMSTATUS); - dmi_write(target, DMI_DMSTATUS, ~dmstatus); - dmi_read(target, &dmstatus_read, DMI_DMSTATUS); - COMPLIANCE_TEST(dmstatus_read == dmstatus, "DMSTATUS is R/O"); + COMPLIANCE_CHECK_RO(target, DMI_DMSTATUS); /* resumereq */ /* This bit is not actually readable according to the spec, so nothing to check.*/ - riscv_resume_all_harts(target); + COMPLIANCE_MUST_PASS(riscv_resume_all_harts(target)); /* Halt all harts again so the test can continue.*/ - riscv_halt_all_harts(target); + COMPLIANCE_MUST_PASS(riscv_halt_all_harts(target)); /* HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. */ + uint32_t hartinfo; + COMPLIANCE_READ(target, &hartinfo, DMI_HARTINFO); for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { - riscv_set_current_hartid(target, hartsel); + COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, hartsel)); - uint32_t hartinfo, hartinfo_read; - dmi_read(target, &hartinfo, DMI_HARTINFO); - dmi_write(target, DMI_HARTINFO, ~hartinfo); - dmi_read(target, &hartinfo_read, DMI_HARTINFO); - COMPLIANCE_TEST((hartinfo_read == hartinfo), "DMHARTINFO should be Read-Only."); + COMPLIANCE_CHECK_RO(target, DMI_HARTINFO); /* $dscratch CSRs */ uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); @@ -3195,16 +3187,16 @@ int riscv013_test_compliance(struct target *target) for (int i = 0; i < MIN(riscv_count_harts(target), 32); i++) expected_haltsum0 |= (1 << i); - dmi_read(target, &testvar_read, DMI_HALTSUM0); + COMPLIANCE_READ(target, &testvar_read, DMI_HALTSUM0); COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should report summary of up to 32 halted harts"); - dmi_write(target, DMI_HALTSUM0, 0xffffffff); - dmi_read(target, &testvar_read, DMI_HALTSUM0); + COMPLIANCE_WRITE(target, DMI_HALTSUM0, 0xffffffff); + COMPLIANCE_READ(target, &testvar_read, DMI_HALTSUM0); COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); - dmi_write(target, DMI_HALTSUM0, 0x0); - dmi_read(target, &testvar_read, DMI_HALTSUM0); + COMPLIANCE_WRITE(target, DMI_HALTSUM0, 0x0); + COMPLIANCE_READ(target, &testvar_read, DMI_HALTSUM0); COMPLIANCE_TEST(testvar_read == expected_haltsum0, "HALTSUM0 should be R/O"); /* HALTSUM1 */ @@ -3212,16 +3204,16 @@ int riscv013_test_compliance(struct target *target) for (int i = 0; i < MIN(riscv_count_harts(target), 1024); i += 32) expected_haltsum1 |= (1 << (i/32)); - dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_READ(target, &testvar_read, DMI_HALTSUM1); COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should report summary of up to 1024 halted harts"); - dmi_write(target, DMI_HALTSUM1, 0xffffffff); - dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_WRITE(target, DMI_HALTSUM1, 0xffffffff); + COMPLIANCE_READ(target, &testvar_read, DMI_HALTSUM1); COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should be R/O"); - dmi_write(target, DMI_HALTSUM1, 0x0); - dmi_read(target, &testvar_read, DMI_HALTSUM1); + COMPLIANCE_WRITE(target, DMI_HALTSUM1, 0x0); + COMPLIANCE_READ(target, &testvar_read, DMI_HALTSUM1); COMPLIANCE_TEST(testvar_read == expected_haltsum1, "HALTSUM1 should be R/O"); /* TODO: HAWINDOWSEL */ @@ -3231,7 +3223,7 @@ int riscv013_test_compliance(struct target *target) /* ABSTRACTCS */ uint32_t abstractcs; - dmi_read(target, &abstractcs, DMI_ABSTRACTCS); + COMPLIANCE_READ(target, &abstractcs, DMI_ABSTRACTCS); /* Check that all reported Data Words are really R/W */ for (int invert = 0; invert < 2; invert++) { @@ -3239,13 +3231,13 @@ int riscv013_test_compliance(struct target *target) testvar = (i + 1) * 0x11111111; if (invert) testvar = ~testvar; - dmi_write(target, DMI_DATA0 + i, testvar); + COMPLIANCE_WRITE(target, DMI_DATA0 + i, testvar); } for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { testvar = (i + 1) * 0x11111111; if (invert) testvar = ~testvar; - dmi_read(target, &testvar_read, DMI_DATA0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_DATA0 + i); COMPLIANCE_TEST(testvar_read == testvar, "All reported DATA words must be R/W"); } } @@ -3256,13 +3248,13 @@ int riscv013_test_compliance(struct target *target) testvar = (i + 1) * 0x11111111; if (invert) testvar = ~testvar; - dmi_write(target, DMI_PROGBUF0 + i, testvar); + COMPLIANCE_WRITE(target, DMI_PROGBUF0 + i, testvar); } for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { testvar = (i + 1) * 0x11111111; if (invert) testvar = ~testvar; - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_PROGBUF0 + i); COMPLIANCE_TEST(testvar_read == testvar, "All reported PROGBUF words must be R/W"); } } @@ -3272,18 +3264,17 @@ int riscv013_test_compliance(struct target *target) /* COMMAND According to the spec, this register is only W, so can't really check the read result. But at any rate, this is not legal and should cause an error. */ - dmi_write(target, DMI_COMMAND, 0xAAAAAAAA); - dmi_read(target, &testvar_read, DMI_COMMAND); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_WRITE(target, DMI_COMMAND, 0xAAAAAAAA); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ "Illegal COMMAND should result in UNSUPPORTED"); - dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); - dmi_write(target, DMI_COMMAND, 0x55555555); - dmi_read(target, &testvar_read, DMI_COMMAND); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_WRITE(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + + COMPLIANCE_WRITE(target, DMI_COMMAND, 0x55555555); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ "Illegal COMMAND should result in UNSUPPORTED"); - dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + COMPLIANCE_WRITE(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); /* Basic Abstract Commands */ for (unsigned int i = 1; i < 32; i = i << 1) { @@ -3291,7 +3282,7 @@ int riscv013_test_compliance(struct target *target) riscv_reg_t testval_read; COMPLIANCE_TEST(ERROR_OK == register_write_direct(target, GDB_REGNO_ZERO + i, testval), "GPR Writes should be supported."); - write_abstract_arg(target, 0, 0xDEADBEEFDEADBEEF, 64); + COMPLIANCE_MUST_PASS(write_abstract_arg(target, 0, 0xDEADBEEFDEADBEEF, 64)); COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &testval_read, GDB_REGNO_ZERO + i), "GPR Reads should be supported."); if (riscv_xlen(target) > 32) { @@ -3305,11 +3296,11 @@ int riscv013_test_compliance(struct target *target) /* ABSTRACTAUTO See which bits are actually writable */ - dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + COMPLIANCE_WRITE(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); uint32_t abstractauto; uint32_t busy; - dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); - dmi_write(target, DMI_ABSTRACTAUTO, 0x0); + COMPLIANCE_READ(target, &abstractauto, DMI_ABSTRACTAUTO); + COMPLIANCE_WRITE(target, DMI_ABSTRACTAUTO, 0x0); if (abstractauto > 0) { /* This mechanism only works when you have a reasonable sized progbuf, which is not a true compliance requirement. */ @@ -3319,22 +3310,22 @@ int riscv013_test_compliance(struct target *target) COMPLIANCE_TEST(ERROR_OK == register_write_direct(target, GDB_REGNO_S0, 0), "Need to be able to write S0 to test ABSTRACTAUTO"); struct riscv_program program; - riscv_program_init(&program, target); + COMPLIANCE_MUST_PASS(riscv_program_init(&program, target)); /* This is also testing that WFI() is a NOP during debug mode. */ - riscv_program_insert(&program, wfi()); - riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1); - riscv_program_ebreak(&program); - dmi_write(target, DMI_ABSTRACTAUTO, 0x0); - riscv_program_exec(&program, target); + COMPLIANCE_MUST_PASS(riscv_program_insert(&program, wfi())); + COMPLIANCE_MUST_PASS(riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, 1)); + COMPLIANCE_MUST_PASS(riscv_program_ebreak(&program)); + COMPLIANCE_WRITE(target, DMI_ABSTRACTAUTO, 0x0); + COMPLIANCE_MUST_PASS(riscv_program_exec(&program, target)); testvar++; - dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); - dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); + COMPLIANCE_WRITE(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + COMPLIANCE_READ(target, &abstractauto, DMI_ABSTRACTAUTO); uint32_t autoexec_data = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECDATA); uint32_t autoexec_progbuf = get_field(abstractauto, DMI_ABSTRACTAUTO_AUTOEXECPROGBUF); for (unsigned int i = 0; i < 12; i++) { - dmi_read(target, &testvar_read, DMI_DATA0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_DATA0 + i); do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTCS); busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); } while (busy); if (autoexec_data & (1 << i)) { @@ -3344,9 +3335,9 @@ int riscv013_test_compliance(struct target *target) } } for (unsigned int i = 0; i < 16; i++) { - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_PROGBUF0 + i); do { - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTCS); busy = get_field(testvar_read, DMI_ABSTRACTCS_BUSY); } while (busy); if (autoexec_progbuf & (1 << i)) { @@ -3356,7 +3347,7 @@ int riscv013_test_compliance(struct target *target) } } - dmi_write(target, DMI_ABSTRACTAUTO, 0); + COMPLIANCE_WRITE(target, DMI_ABSTRACTAUTO, 0); COMPLIANCE_TEST(ERROR_OK == register_read_direct(target, &value, GDB_REGNO_S0), "Need to be able to read S0 to test ABSTRACTAUTO"); @@ -3367,24 +3358,24 @@ int riscv013_test_compliance(struct target *target) /* Single-Step each hart. */ for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { - riscv_set_current_hartid(target, hartsel); - riscv013_on_step(target); - riscv013_step_current_hart(target); - COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, + COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, hartsel)); + COMPLIANCE_MUST_PASS(riscv013_on_step(target)); + COMPLIANCE_MUST_PASS(riscv013_step_current_hart(target)); + COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, "Single Step should result in SINGLESTEP"); } /* Core Register Tests */ uint64_t bogus_dpc = 0xdeadbeef; for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { - riscv_set_current_hartid(target, hartsel); + COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, hartsel)); /* DCSR Tests */ - register_write_direct(target, GDB_REGNO_DCSR, 0x0); - register_read_direct(target, &value, GDB_REGNO_DCSR); + COMPLIANCE_MUST_PASS(register_write_direct(target, GDB_REGNO_DCSR, 0x0)); + COMPLIANCE_MUST_PASS(register_read_direct(target, &value, GDB_REGNO_DCSR)); COMPLIANCE_TEST(value != 0, "Not all bits in DCSR are writable by Debugger"); - register_write_direct(target, GDB_REGNO_DCSR, 0xFFFFFFFF); - register_read_direct(target, &value, GDB_REGNO_DCSR); + COMPLIANCE_MUST_PASS(register_write_direct(target, GDB_REGNO_DCSR, 0xFFFFFFFF)); + COMPLIANCE_MUST_PASS(register_read_direct(target, &value, GDB_REGNO_DCSR)); COMPLIANCE_TEST(value != 0, "At least some bits in DCSR must be 1"); /* DPC. Note that DPC is sign-extended. */ @@ -3397,12 +3388,12 @@ int riscv013_test_compliance(struct target *target) if (riscv_supports_extension(target, riscv_current_hartid(target), 'C')) dpcmask |= 0x2; - register_write_direct(target, GDB_REGNO_DPC, dpcmask); - register_read_direct(target, &dpc, GDB_REGNO_DPC); + COMPLIANCE_MUST_PASS(register_write_direct(target, GDB_REGNO_DPC, dpcmask)); + COMPLIANCE_MUST_PASS(register_read_direct(target, &dpc, GDB_REGNO_DPC)); COMPLIANCE_TEST(dpcmask == dpc, "DPC must be sign-extended to XLEN and writable to all-1s (except the least significant bits)"); - register_write_direct(target, GDB_REGNO_DPC, 0); - register_read_direct(target, &dpc, GDB_REGNO_DPC); + COMPLIANCE_MUST_PASS(register_write_direct(target, GDB_REGNO_DPC, 0)); + COMPLIANCE_MUST_PASS(register_read_direct(target, &dpc, GDB_REGNO_DPC)); COMPLIANCE_TEST(dpc == 0, "DPC must be writable to 0."); if (hartsel == 0) bogus_dpc = dpc; /* For a later test step */ @@ -3415,49 +3406,47 @@ int riscv013_test_compliance(struct target *target) */ /* Write some registers. They should not be impacted by ndmreset. */ - dmi_write(target, DMI_COMMAND, 0xFFFFFFFF); + COMPLIANCE_WRITE(target, DMI_COMMAND, 0xFFFFFFFF); for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { testvar = (i + 1) * 0x11111111; - dmi_write(target, DMI_PROGBUF0 + i, testvar); + COMPLIANCE_WRITE(target, DMI_PROGBUF0 + i, testvar); } for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { testvar = (i + 1) * 0x11111111; - dmi_write(target, DMI_DATA0 + i, testvar); + COMPLIANCE_WRITE(target, DMI_DATA0 + i, testvar); } - dmi_write(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); - dmi_read(target, &abstractauto, DMI_ABSTRACTAUTO); + COMPLIANCE_WRITE(target, DMI_ABSTRACTAUTO, 0xFFFFFFFF); + COMPLIANCE_READ(target, &abstractauto, DMI_ABSTRACTAUTO); /* Pulse reset. */ target->reset_halt = true; - riscv_set_current_hartid(target, 0); + COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, 0)); COMPLIANCE_TEST(ERROR_OK == assert_reset(target), "Must be able to assert NDMRESET"); COMPLIANCE_TEST(ERROR_OK == deassert_reset(target), "Must be able to deassert NDMRESET"); /* Verify that most stuff is not affected by ndmreset. */ - dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0xFFFFFFFF, "NDMRESET should not affect DMI_COMMAND"); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, "NDMRESET should not affect DMI_ABSTRACTCS"); - dmi_read(target, &testvar_read, DMI_ABSTRACTAUTO); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTAUTO); COMPLIANCE_TEST(testvar_read == abstractauto, "NDMRESET should not affect DMI_ABSTRACTAUTO"); /* Clean up to avoid future test failures */ - dmi_write(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); - dmi_write(target, DMI_ABSTRACTAUTO, 0); + COMPLIANCE_WRITE(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); + COMPLIANCE_WRITE(target, DMI_ABSTRACTAUTO, 0); for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_PROGBUF0 + i); COMPLIANCE_TEST(testvar_read == testvar, "PROGBUF words must not be affected by NDMRESET"); } for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_DATA0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_DATA0 + i); COMPLIANCE_TEST(testvar_read == testvar, "DATA words must not be affected by NDMRESET"); } @@ -3465,7 +3454,7 @@ int riscv013_test_compliance(struct target *target) just verify that at least it's not the bogus value anymore. */ COMPLIANCE_TEST(bogus_dpc != 0xdeadbeef, "BOGUS DPC should have been set somehow (bug in compliance test)"); - register_read_direct(target, &value, GDB_REGNO_DPC); + COMPLIANCE_MUST_PASS(register_read_direct(target, &value, GDB_REGNO_DPC)); COMPLIANCE_TEST(bogus_dpc != value, "NDMRESET should move DPC to reset value."); COMPLIANCE_TEST(riscv_halt_reason(target, 0) == RISCV_HALT_INTERRUPT, @@ -3474,24 +3463,20 @@ int riscv013_test_compliance(struct target *target) /* DMACTIVE -- deasserting DMACTIVE should reset all the above values. */ /* Toggle dmactive */ - dmi_write(target, DMI_DMCONTROL, 0); - dmi_write(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); - dmi_read(target, &testvar_read, DMI_COMMAND); - COMPLIANCE_TEST(testvar_read == 0, "DMI_COMMAND should reset to 0"); - dmi_read(target, &testvar_read, DMI_ABSTRACTCS); + COMPLIANCE_WRITE(target, DMI_DMCONTROL, 0); + COMPLIANCE_WRITE(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == 0, "ABSTRACTCS.cmderr should reset to 0"); - dmi_read(target, &testvar_read, DMI_ABSTRACTAUTO); + COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTAUTO); COMPLIANCE_TEST(testvar_read == 0, "ABSTRACTAUTO should reset to 0"); for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); i++) { - testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_PROGBUF0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_PROGBUF0 + i); COMPLIANCE_TEST(testvar_read == 0, "PROGBUF words should reset to 0"); } for (unsigned int i = 0; i < get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); i++) { - testvar = (i + 1) * 0x11111111; - dmi_read(target, &testvar_read, DMI_DATA0 + i); + COMPLIANCE_READ(target, &testvar_read, DMI_DATA0 + i); COMPLIANCE_TEST(testvar_read == 0, "DATA words should reset to 0"); } @@ -3505,7 +3490,7 @@ int riscv013_test_compliance(struct target *target) */ /* Halt every hart for any follow-up tests*/ - riscv_halt_all_harts(target); + COMPLIANCE_MUST_PASS(riscv_halt_all_harts(target)); LOG_INFO("PASSED %d of %d TESTS\n", passed_tests, total_tests); -- cgit v1.1 From 7448f8780a901015ab8f1eec9d935775e204b5e2 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 30 Aug 2018 11:30:14 -0700 Subject: riscv-compliance: fix whitespace --- src/target/riscv/riscv-013.c | 50 ++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index ec7b2f1..d612e7a 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3065,12 +3065,12 @@ void riscv013_clear_abstract_error(struct target *target) #define COMPLIANCE_CHECK_RO(target, addr) \ { \ - uint32_t orig; \ - uint32_t inverse; \ - COMPLIANCE_READ(target, &orig, addr); \ - COMPLIANCE_WRITE(target, addr, ~orig); \ - COMPLIANCE_READ(target, &inverse, addr); \ - COMPLIANCE_TEST(orig == inverse, "Register must be read-only"); \ + uint32_t orig; \ + uint32_t inverse; \ + COMPLIANCE_READ(target, &orig, addr); \ + COMPLIANCE_WRITE(target, addr, ~orig); \ + COMPLIANCE_READ(target, &inverse, addr); \ + COMPLIANCE_TEST(orig == inverse, "Register must be read-only"); \ } int riscv013_test_compliance(struct target *target) @@ -3096,20 +3096,20 @@ int riscv013_test_compliance(struct target *target) /* hartreset */ /* This field is optional. Either we can read and write it to 1/0, - or it is tied to 0. This check doesn't really do anything, but - it does attempt to set the bit to 1 and then back to 0, which needs to - work if its implemented. */ + or it is tied to 0. This check doesn't really do anything, but + it does attempt to set the bit to 1 and then back to 0, which needs to + work if its implemented. */ COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 1)); COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HARTRESET, 0)); COMPLIANCE_READ(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST((get_field(dmcontrol, DMI_DMCONTROL_HARTRESET) == 0), - "DMCONTROL.hartreset can be 0 or RW."); + COMPLIANCE_TEST((get_field(dmcontrol, DMI_DMCONTROL_HARTRESET) == 0), + "DMCONTROL.hartreset can be 0 or RW."); /* hasel */ - COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1)); - COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 0)); + COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 1)); + COMPLIANCE_WRITE(target, DMI_DMCONTROL, set_field(dmcontrol_orig, DMI_DMCONTROL_HASEL, 0)); COMPLIANCE_READ(target, &dmcontrol, DMI_DMCONTROL); - COMPLIANCE_TEST((get_field(dmcontrol, DMI_DMCONTROL_HASEL) == 0), + COMPLIANCE_TEST((get_field(dmcontrol, DMI_DMCONTROL_HASEL) == 0), "DMCONTROL.hasel can be 0 or RW."); /* TODO: test that hamask registers exist if hasel does. */ @@ -3118,7 +3118,7 @@ int riscv013_test_compliance(struct target *target) /* This bit is not actually readable according to the spec, so nothing to check.*/ /* DMSTATUS */ - COMPLIANCE_CHECK_RO(target, DMI_DMSTATUS); + COMPLIANCE_CHECK_RO(target, DMI_DMSTATUS); /* resumereq */ /* This bit is not actually readable according to the spec, so nothing to check.*/ @@ -3128,12 +3128,12 @@ int riscv013_test_compliance(struct target *target) COMPLIANCE_MUST_PASS(riscv_halt_all_harts(target)); /* HARTINFO: Read-Only. This is per-hart, so need to adjust hartsel. */ - uint32_t hartinfo; - COMPLIANCE_READ(target, &hartinfo, DMI_HARTINFO); + uint32_t hartinfo; + COMPLIANCE_READ(target, &hartinfo, DMI_HARTINFO); for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { - COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, hartsel)); + COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, hartsel)); - COMPLIANCE_CHECK_RO(target, DMI_HARTINFO); + COMPLIANCE_CHECK_RO(target, DMI_HARTINFO); /* $dscratch CSRs */ uint32_t nscratch = get_field(hartinfo, DMI_HARTINFO_NSCRATCH); @@ -3269,7 +3269,7 @@ int riscv013_test_compliance(struct target *target) COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ "Illegal COMMAND should result in UNSUPPORTED"); COMPLIANCE_WRITE(target, DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR); - + COMPLIANCE_WRITE(target, DMI_COMMAND, 0x55555555); COMPLIANCE_READ(target, &testvar_read, DMI_ABSTRACTCS); COMPLIANCE_TEST(get_field(testvar_read, DMI_ABSTRACTCS_CMDERR) == CMDERR_NOT_SUPPORTED, \ @@ -3358,10 +3358,10 @@ int riscv013_test_compliance(struct target *target) /* Single-Step each hart. */ for (int hartsel = 0; hartsel < riscv_count_harts(target); hartsel++) { - COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, hartsel)); - COMPLIANCE_MUST_PASS(riscv013_on_step(target)); - COMPLIANCE_MUST_PASS(riscv013_step_current_hart(target)); - COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, + COMPLIANCE_MUST_PASS(riscv_set_current_hartid(target, hartsel)); + COMPLIANCE_MUST_PASS(riscv013_on_step(target)); + COMPLIANCE_MUST_PASS(riscv013_step_current_hart(target)); + COMPLIANCE_TEST(riscv_halt_reason(target, hartsel) == RISCV_HALT_SINGLESTEP, "Single Step should result in SINGLESTEP"); } @@ -3389,7 +3389,7 @@ int riscv013_test_compliance(struct target *target) dpcmask |= 0x2; COMPLIANCE_MUST_PASS(register_write_direct(target, GDB_REGNO_DPC, dpcmask)); - COMPLIANCE_MUST_PASS(register_read_direct(target, &dpc, GDB_REGNO_DPC)); + COMPLIANCE_MUST_PASS(register_read_direct(target, &dpc, GDB_REGNO_DPC)); COMPLIANCE_TEST(dpcmask == dpc, "DPC must be sign-extended to XLEN and writable to all-1s (except the least significant bits)"); COMPLIANCE_MUST_PASS(register_write_direct(target, GDB_REGNO_DPC, 0)); -- cgit v1.1 From 24513fe51f01cbbdb9518f184ea18b67f319df7a Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 30 Aug 2018 15:37:15 -0700 Subject: riscv-compliance: fix comment typo --- src/target/riscv/riscv-013.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index d612e7a..4e3deab 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3092,7 +3092,7 @@ int riscv013_test_compliance(struct target *target) riscv_reg_t value; RISCV013_INFO(info); - /* All the bits of HARTSEL is covered by the examine sequence. */ + /* All the bits of HARTSEL are covered by the examine sequence. */ /* hartreset */ /* This field is optional. Either we can read and write it to 1/0, -- cgit v1.1