From bf6ce5e1cbe1ba88e5f09a3892bfe47dd576f724 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 27 Nov 2023 12:40:22 -0800 Subject: Add SBA read delay. This is helpful to test OpenOCD behavior when sbbusyerror is set. --- riscv/debug_module.cc | 118 +++++++++++++++++++++++++++++++++++++------------- riscv/debug_module.h | 13 ++++++ 2 files changed, 100 insertions(+), 31 deletions(-) diff --git a/riscv/debug_module.cc b/riscv/debug_module.cc index e9aef1a..2df7686 100644 --- a/riscv/debug_module.cc +++ b/riscv/debug_module.cc @@ -43,7 +43,8 @@ debug_module_t::debug_module_t(simif_t *sim, const debug_module_config_t &config // them because I'm too lazy to add the code to just ignore accesses. hart_state(1 << field_width(sim->get_cfg().max_hartid() + 1)), hart_array_mask(sim->get_cfg().max_hartid() + 1), - rti_remaining(0) + rti_remaining(0), + sb_read_wait(0) { D(fprintf(stderr, "debug_data_start=0x%x\n", debug_data_start)); D(fprintf(stderr, "debug_progbuf_start=0x%x\n", debug_progbuf_start)); @@ -297,6 +298,24 @@ void debug_module_t::sb_autoincrement() sbaddress[3] += carry; } +bool debug_module_t::sb_busy() const +{ + return sb_read_wait > 0; +} + +void debug_module_t::sb_read_start() +{ + if (sb_busy() || sbcs.sbbusyerror) { + if (!sbcs.sbbusyerror) + D(fprintf(stderr, "Set sbbusyerror because read start while busy\n")); + sbcs.sbbusyerror = true; + return; + } + /* Insert artificial delay, so debuggers can test how they handle that + * sbbusyerror being set. */ + sb_read_wait = 20; +} + void debug_module_t::sb_read() { reg_t address = ((uint64_t) sbaddress[1] << 32) | sbaddress[0]; @@ -314,6 +333,7 @@ void debug_module_t::sb_read() } else { sbcs.error = 3; } + D(fprintf(stderr, "sb_read() 0x%x @ 0x%lx\n", sbdata[0], address)); } catch (const mem_trap_t& ) { sbcs.error = 2; } @@ -494,6 +514,8 @@ bool debug_module_t::dmi_read(unsigned address, uint32_t *value) result = set_field(result, DM_SBCS_SBAUTOINCREMENT, sbcs.autoincrement); result = set_field(result, DM_SBCS_SBREADONDATA, sbcs.readondata); result = set_field(result, DM_SBCS_SBERROR, sbcs.error); + result = set_field(result, DM_SBCS_SBBUSY, sb_busy()); + result = set_field(result, DM_SBCS_SBBUSYERROR, sbcs.sbbusyerror); result = set_field(result, DM_SBCS_SBASIZE, sbcs.asize); result = set_field(result, DM_SBCS_SBACCESS128, sbcs.access128); result = set_field(result, DM_SBCS_SBACCESS64, sbcs.access64); @@ -515,23 +537,31 @@ bool debug_module_t::dmi_read(unsigned address, uint32_t *value) break; case DM_SBDATA0: result = sbdata[0]; - if (sbcs.error == 0) { + if (sb_busy()) { + sbcs.sbbusyerror = true; + } else if (sbcs.error == 0) { if (sbcs.readondata) { - sb_read(); - } - if (sbcs.error == 0) { - sb_autoincrement(); + sb_read_start(); } } break; case DM_SBDATA1: result = sbdata[1]; + if (sb_busy()) { + sbcs.sbbusyerror = true; + } break; case DM_SBDATA2: result = sbdata[2]; + if (sb_busy()) { + sbcs.sbbusyerror = true; + } break; case DM_SBDATA3: result = sbdata[3]; + if (sb_busy()) { + sbcs.sbbusyerror = true; + } break; case DM_AUTHDATA: result = challenge; @@ -563,6 +593,15 @@ void debug_module_t::run_test_idle() if (rti_remaining == 0 && abstractcs.busy && abstract_command_completed) { abstractcs.busy = false; } + if (sb_read_wait > 0) { + sb_read_wait--; + if (sb_read_wait == 0) { + sb_read(); + if (sbcs.error == 0) { + sb_autoincrement(); + } + } + } } static bool is_fpu_reg(unsigned regno) @@ -877,40 +916,57 @@ bool debug_module_t::dmi_write(unsigned address, uint32_t value) sbcs.autoincrement = get_field(value, DM_SBCS_SBAUTOINCREMENT); sbcs.readondata = get_field(value, DM_SBCS_SBREADONDATA); sbcs.error &= ~get_field(value, DM_SBCS_SBERROR); + if (get_field(value, DM_SBCS_SBBUSYERROR)) + sbcs.sbbusyerror = false; return true; case DM_SBADDRESS0: - sbaddress[0] = value; - if (sbcs.error == 0 && sbcs.readonaddr) { - sb_read(); - sb_autoincrement(); - } - return true; case DM_SBADDRESS1: - sbaddress[1] = value; - return true; case DM_SBADDRESS2: - sbaddress[2] = value; - return true; case DM_SBADDRESS3: - sbaddress[3] = value; - return true; case DM_SBDATA0: - sbdata[0] = value; - if (sbcs.error == 0) { - sb_write(); - if (sbcs.error == 0) { - sb_autoincrement(); - } - } - return true; case DM_SBDATA1: - sbdata[1] = value; - return true; case DM_SBDATA2: - sbdata[2] = value; - return true; case DM_SBDATA3: - sbdata[3] = value; + /* These all set busyerror if already busy. */ + if (sb_busy()) { + sbcs.sbbusyerror = true; + } else { + switch (address) { + case DM_SBADDRESS0: + sbaddress[0] = value; + if (sbcs.error == 0 && sbcs.readonaddr) { + sb_read_start(); + } + return true; + case DM_SBADDRESS1: + sbaddress[1] = value; + return true; + case DM_SBADDRESS2: + sbaddress[2] = value; + return true; + case DM_SBADDRESS3: + sbaddress[3] = value; + return true; + case DM_SBDATA0: + sbdata[0] = value; + if (sbcs.error == 0) { + sb_write(); + if (sbcs.error == 0) { + sb_autoincrement(); + } + } + return true; + case DM_SBDATA1: + sbdata[1] = value; + return true; + case DM_SBDATA2: + sbdata[2] = value; + return true; + case DM_SBDATA3: + sbdata[3] = value; + return true; + } + } return true; case DM_AUTHDATA: D(fprintf(stderr, "debug authentication: got 0x%x; 0x%x unlocks\n", value, diff --git a/riscv/debug_module.h b/riscv/debug_module.h index 73bb7fa..cf8781d 100644 --- a/riscv/debug_module.h +++ b/riscv/debug_module.h @@ -89,6 +89,7 @@ typedef struct { bool access32; bool access16; bool access8; + bool sbbusyerror; } sbcs_t; typedef struct { @@ -157,8 +158,18 @@ class debug_module_t : public abstract_device_t uint32_t read32(uint8_t *rom, unsigned int index); void sb_autoincrement(); + + /* Start a system bus read. (It could be instantaneous, but to help test + * OpenOCD a delay can be added.) */ + void sb_read_start(); + + /* Actually read/write. */ void sb_read(); void sb_write(); + + /* Return true iff a system bus access is in progress. */ + bool sb_busy() const; + unsigned sb_access_bits(); dmcontrol_t dmcontrol; @@ -191,6 +202,8 @@ class debug_module_t : public abstract_device_t * available. Otherwise it is unavailable. */ bool hart_available_state[2]; bool hart_available(unsigned hart_id) const; + + unsigned sb_read_wait; }; #endif -- cgit v1.1 From 650c15caf9a482154fe9dbf150b54b49e9849c63 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 27 Nov 2023 14:24:45 -0800 Subject: Add SBA write delay. This is helpful to test OpenOCD behavior when sbbusyerror is set. --- riscv/debug_module.cc | 31 +++++++++++++++++++++++++------ riscv/debug_module.h | 5 +++-- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/riscv/debug_module.cc b/riscv/debug_module.cc index 2df7686..07723c5 100644 --- a/riscv/debug_module.cc +++ b/riscv/debug_module.cc @@ -44,7 +44,7 @@ debug_module_t::debug_module_t(simif_t *sim, const debug_module_config_t &config hart_state(1 << field_width(sim->get_cfg().max_hartid() + 1)), hart_array_mask(sim->get_cfg().max_hartid() + 1), rti_remaining(0), - sb_read_wait(0) + sb_read_wait(0), sb_write_wait(0) { D(fprintf(stderr, "debug_data_start=0x%x\n", debug_data_start)); D(fprintf(stderr, "debug_progbuf_start=0x%x\n", debug_progbuf_start)); @@ -300,7 +300,7 @@ void debug_module_t::sb_autoincrement() bool debug_module_t::sb_busy() const { - return sb_read_wait > 0; + return sb_read_wait > 0 || sb_write_wait > 0; } void debug_module_t::sb_read_start() @@ -339,6 +339,19 @@ void debug_module_t::sb_read() } } +void debug_module_t::sb_write_start() +{ + if (sb_busy() || sbcs.sbbusyerror) { + if (!sbcs.sbbusyerror) + D(fprintf(stderr, "Set sbbusyerror because write start while busy\n")); + sbcs.sbbusyerror = true; + return; + } + /* Insert artificial delay, so debuggers can test how they handle that + * sbbusyerror being set. */ + sb_write_wait = 20; +} + void debug_module_t::sb_write() { reg_t address = ((uint64_t) sbaddress[1] << 32) | sbaddress[0]; @@ -602,6 +615,15 @@ void debug_module_t::run_test_idle() } } } + if (sb_write_wait > 0) { + sb_write_wait--; + if (sb_write_wait == 0) { + sb_write(); + if (sbcs.error == 0) { + sb_autoincrement(); + } + } + } } static bool is_fpu_reg(unsigned regno) @@ -950,10 +972,7 @@ bool debug_module_t::dmi_write(unsigned address, uint32_t value) case DM_SBDATA0: sbdata[0] = value; if (sbcs.error == 0) { - sb_write(); - if (sbcs.error == 0) { - sb_autoincrement(); - } + sb_write_start(); } return true; case DM_SBDATA1: diff --git a/riscv/debug_module.h b/riscv/debug_module.h index cf8781d..75109f6 100644 --- a/riscv/debug_module.h +++ b/riscv/debug_module.h @@ -159,9 +159,10 @@ class debug_module_t : public abstract_device_t void sb_autoincrement(); - /* Start a system bus read. (It could be instantaneous, but to help test + /* Start a system bus access. (It could be instantaneous, but to help test * OpenOCD a delay can be added.) */ void sb_read_start(); + void sb_write_start(); /* Actually read/write. */ void sb_read(); @@ -203,7 +204,7 @@ class debug_module_t : public abstract_device_t bool hart_available_state[2]; bool hart_available(unsigned hart_id) const; - unsigned sb_read_wait; + unsigned sb_read_wait, sb_write_wait; }; #endif -- cgit v1.1 From 0d85419fec49a8ddd41adc2c0eb468245da7f0fc Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 1 Dec 2023 12:31:18 -0800 Subject: Test OpenOCD that can deal with sbbusyerror. --- .github/workflows/debug-smoke.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/debug-smoke.yml b/.github/workflows/debug-smoke.yml index 7c24d55..1ae54ab 100644 --- a/.github/workflows/debug-smoke.yml +++ b/.github/workflows/debug-smoke.yml @@ -25,7 +25,7 @@ jobs: run: | git clone --recurse-submodules https://github.com/riscv/riscv-openocd.git cd riscv-openocd - git checkout a495dd854ce2e857a583125a31527a47320ec6b9 + git checkout d4c5d2657074613d429f57f60e939ca151ed4f32 - name: Build OpenOCD run: | -- cgit v1.1