From 91552c79997f9baa859bc9324d5ebea790b1dcba Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 8 Feb 2023 11:02:20 -0800 Subject: [PATCH 1/4] Move yes_no_maybe_t into riscv.h. Change-Id: I5bbdc1af3147e05e25612bf496f409111248c979 --- src/target/riscv/riscv-013.c | 6 ------ src/target/riscv/riscv.h | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 661197ca4..7d3b54dcd 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -116,12 +116,6 @@ struct trigger { int unique_id; }; -typedef enum { - YNM_MAYBE, - YNM_YES, - YNM_NO -} yes_no_maybe_t; - #define HART_INDEX_MULTIPLE -1 #define HART_INDEX_UNKNOWN -2 diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 87ec13f82..5a9a06e77 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -41,6 +41,12 @@ typedef uint64_t riscv_reg_t; typedef uint32_t riscv_insn_t; typedef uint64_t riscv_addr_t; +typedef enum { + YNM_MAYBE, + YNM_YES, + YNM_NO +} yes_no_maybe_t; + enum riscv_mem_access_method { RISCV_MEM_ACCESS_UNSPECIFIED, RISCV_MEM_ACCESS_PROGBUF, From 344e8bd26388821512c931edbb8508264d193c04 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 8 Feb 2023 11:02:51 -0800 Subject: [PATCH 2/4] Print out debug value after the assignment is made. Change-Id: I6ba1064c09f48eba97d84ea9db5ff44d82b9d004 --- 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 7d3b54dcd..e2930abf4 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1732,8 +1732,8 @@ static int examine(struct target *target) LOG_TARGET_WARNING(target, "Couldn't read vlenb; vector register access won't work."); r->vlenb = 0; } else { - LOG_TARGET_INFO(target, "Vector support with vlenb=%d", r->vlenb); r->vlenb = vlenb; + LOG_TARGET_INFO(target, "Vector support with vlenb=%d", r->vlenb); } /* Now init registers based on what we discovered. */ From abb918685f2177ce08f8d491e511261ae59e1751 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 8 Feb 2023 11:03:33 -0800 Subject: [PATCH 3/4] If XLEN=64 and vsew=64 fails, fall back to vsew=32. This should make vector accesses work on 64-bit harts that implement Zve32*. There doesn't appear to be any way to easily determine what vsew values are allowed, so try and notice the failure. Change-Id: Ide0722d0d67da402a4fbe88163830094e46beb84 Signed-off-by: Tim Newsome --- src/target/riscv/riscv-013.c | 74 +++++++++++++++++++++++------------- src/target/riscv/riscv.c | 2 + src/target/riscv/riscv.h | 2 + 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index e2930abf4..cfc0f14ff 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1905,34 +1905,54 @@ COMMAND_HELPER(riscv013_print_info, struct target *target) return 0; } -static int prep_for_vector_access(struct target *target, uint64_t *vtype, - uint64_t *vl, unsigned *debug_vl) +static int prep_for_vector_access(struct target *target, uint64_t *saved_vtype, + uint64_t *saved_vl, unsigned *debug_vl, unsigned *debug_vsew) { RISCV_INFO(r); /* TODO: this continuous save/restore is terrible for performance. */ /* Write vtype and vl. */ - unsigned encoded_vsew; - switch (riscv_xlen(target)) { - case 32: - encoded_vsew = 2; - break; - case 64: - encoded_vsew = 3; - break; - default: - LOG_ERROR("Unsupported xlen: %d", riscv_xlen(target)); - return ERROR_FAIL; - } /* Save vtype and vl. */ - if (register_read_direct(target, vtype, GDB_REGNO_VTYPE) != ERROR_OK) + if (register_read_direct(target, saved_vtype, GDB_REGNO_VTYPE) != ERROR_OK) return ERROR_FAIL; - if (register_read_direct(target, vl, GDB_REGNO_VL) != ERROR_OK) + if (register_read_direct(target, saved_vl, GDB_REGNO_VL) != ERROR_OK) return ERROR_FAIL; - if (register_write_direct(target, GDB_REGNO_VTYPE, encoded_vsew << 3) != ERROR_OK) - return ERROR_FAIL; - *debug_vl = DIV_ROUND_UP(r->vlenb * 8, riscv_xlen(target)); + while (true) { + unsigned int encoded_vsew; + if (riscv_xlen(target) == 64 && r->vsew64_supported != YNM_NO) { + encoded_vsew = 3; + *debug_vsew = 64; + } else { + encoded_vsew = 2; + *debug_vsew = 32; + } + + /* Set standard element width to match XLEN, for vmv instruction to move + * the least significant bits into a GPR. */ + if (register_write_direct(target, GDB_REGNO_VTYPE, encoded_vsew << 3) != ERROR_OK) + return ERROR_FAIL; + + if (*debug_vsew == 64 && r->vsew64_supported == YNM_MAYBE) { + /* Check that it's supported. */ + uint64_t vtype; + if (register_read_direct(target, &vtype, GDB_REGNO_VTYPE) != ERROR_OK) + return ERROR_FAIL; + if (vtype >> (riscv_xlen(target) - 1)) { + r->vsew64_supported = YNM_NO; + /* Try again. */ + continue; + } else { + r->vsew64_supported = YNM_YES; + } + } + break; + } + + /* Set the number of elements to be updated with results from a vector + * instruction, for the vslide1down instruction. + * Set it so the entire V register is updated. */ + *debug_vl = DIV_ROUND_UP(r->vlenb * 8, *debug_vsew); if (register_write_direct(target, GDB_REGNO_VL, *debug_vl) != ERROR_OK) return ERROR_FAIL; @@ -1966,12 +1986,11 @@ static int riscv013_get_register_buf(struct target *target, return ERROR_FAIL; uint64_t vtype, vl; - unsigned debug_vl; - if (prep_for_vector_access(target, &vtype, &vl, &debug_vl) != ERROR_OK) + unsigned int debug_vl, debug_vsew; + if (prep_for_vector_access(target, &vtype, &vl, &debug_vl, &debug_vsew) != ERROR_OK) return ERROR_FAIL; unsigned vnum = regno - GDB_REGNO_V0; - unsigned xlen = riscv_xlen(target); struct riscv_program program; riscv_program_init(&program, target); @@ -1991,8 +2010,10 @@ static int riscv013_get_register_buf(struct target *target, uint64_t v; if (register_read_direct(target, &v, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; - buf_set_u64(value, xlen * i, xlen, v); + buf_set_u64(value, debug_vsew * i, debug_vsew, v); } else { + LOG_TARGET_ERROR(target, "Failed to execute vmv/vslide1down while reading %s", + gdb_regno_name(regno)); break; } } @@ -2022,12 +2043,11 @@ static int riscv013_set_register_buf(struct target *target, return ERROR_FAIL; uint64_t vtype, vl; - unsigned debug_vl; - if (prep_for_vector_access(target, &vtype, &vl, &debug_vl) != ERROR_OK) + unsigned int debug_vl, debug_vsew; + if (prep_for_vector_access(target, &vtype, &vl, &debug_vl, &debug_vsew) != ERROR_OK) return ERROR_FAIL; unsigned vnum = regno - GDB_REGNO_V0; - unsigned xlen = riscv_xlen(target); struct riscv_program program; riscv_program_init(&program, target); @@ -2035,7 +2055,7 @@ static int riscv013_set_register_buf(struct target *target, int result = ERROR_OK; for (unsigned i = 0; i < debug_vl; i++) { if (register_write_direct(target, GDB_REGNO_S0, - buf_get_u64(value, xlen * i, xlen)) != ERROR_OK) + buf_get_u64(value, debug_vsew * i, debug_vsew)) != ERROR_OK) return ERROR_FAIL; result = riscv_program_exec(&program, target); if (result != ERROR_OK) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 378704175..e5938e29c 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -3926,6 +3926,8 @@ void riscv_info_init(struct target *target, riscv_info_t *r) INIT_LIST_HEAD(&r->expose_csr); INIT_LIST_HEAD(&r->expose_custom); + + r->vsew64_supported = YNM_MAYBE; } static int riscv_resume_go_all_harts(struct target *target) diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 5a9a06e77..6a1d7a438 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -259,6 +259,8 @@ typedef struct { /* Track when we were last asked to do something substantial. */ int64_t last_activity; + + yes_no_maybe_t vsew64_supported; } riscv_info_t; COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key, From f4f3ce7db7a1f5a264a0a957e338e71c9d583ea2 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 9 Feb 2023 10:07:42 -0800 Subject: [PATCH 4/4] Don't reuse a single riscv_program. Because riscv_program_exec() tries to add an instruction every time through. This would cause an error accessing vector registers where VL > 14(?). Change-Id: Ie676ca8c9be786b46aa2a4b4028ac8b27f7a4b40 Signed-off-by: Tim Newsome --- src/target/riscv/riscv-013.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index cfc0f14ff..71cf2876a 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1992,13 +1992,15 @@ static int riscv013_get_register_buf(struct target *target, unsigned vnum = regno - GDB_REGNO_V0; - struct riscv_program program; - riscv_program_init(&program, target); - riscv_program_insert(&program, vmv_x_s(S0, vnum)); - riscv_program_insert(&program, vslide1down_vx(vnum, vnum, S0, true)); - int result = ERROR_OK; for (unsigned i = 0; i < debug_vl; i++) { + /* Can't reuse the same program because riscv_program_exec() adds + * ebreak to the end every time. */ + struct riscv_program program; + riscv_program_init(&program, target); + riscv_program_insert(&program, vmv_x_s(S0, vnum)); + riscv_program_insert(&program, vslide1down_vx(vnum, vnum, S0, true)); + /* Executing the program might result in an exception if there is some * issue with the vector implementation/instructions we're using. If that * happens, attempt to restore as usual. We may have clobbered the