From 86b430b6b43068bb32b9a98359186118112ab4e5 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 9 Nov 2023 13:48:51 -0800 Subject: [PATCH] target/riscv: Replace [sg]et_field macros with functions. Compilers are good at optimizing, and with functions it's abundantly clear what all the types involved are. This change means we don't have to be super careful about the type of values because of what the macro might do to them that might cause overflow. The only place where the return type matters is in printf-style functions, and I made get_value32() for those cases where a change was needed. This should set the stage for simply copying the latest debug_defines.h from the debug spec build again. Change-Id: I5fb19d0cfc1e20137832a7b344b05db215ce00e1 Signed-off-by: Tim Newsome --- src/target/riscv/batch.c | 4 +-- src/target/riscv/field_helpers.h | 47 ++++++++++++++++++++++++++++++++ src/target/riscv/riscv-011.c | 38 ++++++++++++-------------- src/target/riscv/riscv-013.c | 18 ++++-------- src/target/riscv/riscv.c | 5 +--- 5 files changed, 73 insertions(+), 39 deletions(-) create mode 100644 src/target/riscv/field_helpers.h diff --git a/src/target/riscv/batch.c b/src/target/riscv/batch.c index 2dbf851ba5..91b549f9d6 100644 --- a/src/target/riscv/batch.c +++ b/src/target/riscv/batch.c @@ -7,9 +7,7 @@ #include "batch.h" #include "debug_defines.h" #include "riscv.h" - -#define get_field(reg, mask) (((reg) & (mask)) / ((mask) & ~((mask) << 1))) -#define set_field(reg, mask, val) (((reg) & ~(mask)) | (((val) * ((mask) & ~((mask) << 1))) & (mask))) +#include "field_helpers.h" #define DTM_DMI_MAX_ADDRESS_LENGTH ((1< +#include + +static inline uint64_t get_field(uint64_t reg, uint64_t mask) +{ + return (reg & mask) / (mask & ~(mask << 1)); +} + +static inline uint32_t get_field32(uint64_t reg, uint64_t mask) +{ + uint64_t value = get_field(reg, mask); + assert(value <= UINT32_MAX); + return value; +} + +static inline uint64_t set_field(uint64_t reg, uint64_t mask, uint64_t val) +{ + /* Clear current value from field. */ + reg &= ~mask; + uint64_t low_field_bit = mask & ~(mask << 1); + /* Assert if the value doesn't fit in the field. */ + assert(((val * low_field_bit) & ~mask) == 0); + reg |= (val * low_field_bit) & mask; + return reg; +} + +static inline uint32_t set_field32(uint32_t reg, uint32_t mask, uint32_t val) +{ + return (uint32_t)set_field(reg, mask, val); +} + +static inline uint64_t field_value(uint64_t mask, uint64_t val) +{ + return set_field(0, mask, val); +} + +static inline uint32_t field_value32(uint32_t mask, uint32_t val) +{ + return set_field32(0, mask, val); +} + +#endif diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index c958c88026..e680bed923 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -24,6 +24,7 @@ #include "riscv.h" #include "asm.h" #include "gdb_regs.h" +#include "field_helpers.h" /** * Since almost everything can be accomplish by scanning the dbus register, all @@ -69,9 +70,6 @@ static int handle_halt(struct target *target, bool announce); -#define get_field(reg, mask) (((reg) & (mask)) / ((mask) & ~((mask) << 1))) -#define set_field(reg, mask, val) (((reg) & ~(mask)) | (((val) * ((mask) & ~((mask) << 1))) & (mask))) - /* Constants for legacy SiFive hardware breakpoints. */ #define CSR_BPCONTROL_X (1<<0) #define CSR_BPCONTROL_W (1<<1) @@ -1468,13 +1466,13 @@ static int examine(struct target *target) } LOG_DEBUG("dtmcontrol=0x%x", dtmcontrol); - LOG_DEBUG(" addrbits=%d", get_field(dtmcontrol, DTMCONTROL_ADDRBITS)); - LOG_DEBUG(" version=%d", get_field(dtmcontrol, DTMCONTROL_VERSION)); - LOG_DEBUG(" idle=%d", get_field(dtmcontrol, DTMCONTROL_IDLE)); + LOG_DEBUG(" addrbits=%d", get_field32(dtmcontrol, DTMCONTROL_ADDRBITS)); + LOG_DEBUG(" version=%d", get_field32(dtmcontrol, DTMCONTROL_VERSION)); + LOG_DEBUG(" idle=%d", get_field32(dtmcontrol, DTMCONTROL_IDLE)); if (get_field(dtmcontrol, DTMCONTROL_VERSION) != 0) { LOG_ERROR("Unsupported DTM version %d. (dtmcontrol=0x%x)", - get_field(dtmcontrol, DTMCONTROL_VERSION), dtmcontrol); + get_field32(dtmcontrol, DTMCONTROL_VERSION), dtmcontrol); return ERROR_FAIL; } @@ -1492,22 +1490,22 @@ static int examine(struct target *target) uint32_t dminfo = dbus_read(target, DMINFO); LOG_DEBUG("dminfo: 0x%08x", dminfo); - LOG_DEBUG(" abussize=0x%x", get_field(dminfo, DMINFO_ABUSSIZE)); - LOG_DEBUG(" serialcount=0x%x", get_field(dminfo, DMINFO_SERIALCOUNT)); - LOG_DEBUG(" access128=%d", get_field(dminfo, DMINFO_ACCESS128)); - LOG_DEBUG(" access64=%d", get_field(dminfo, DMINFO_ACCESS64)); - LOG_DEBUG(" access32=%d", get_field(dminfo, DMINFO_ACCESS32)); - LOG_DEBUG(" access16=%d", get_field(dminfo, DMINFO_ACCESS16)); - LOG_DEBUG(" access8=%d", get_field(dminfo, DMINFO_ACCESS8)); - LOG_DEBUG(" dramsize=0x%x", get_field(dminfo, DMINFO_DRAMSIZE)); - LOG_DEBUG(" authenticated=0x%x", get_field(dminfo, DMINFO_AUTHENTICATED)); - LOG_DEBUG(" authbusy=0x%x", get_field(dminfo, DMINFO_AUTHBUSY)); - LOG_DEBUG(" authtype=0x%x", get_field(dminfo, DMINFO_AUTHTYPE)); - LOG_DEBUG(" version=0x%x", get_field(dminfo, DMINFO_VERSION)); + LOG_DEBUG(" abussize=0x%x", get_field32(dminfo, DMINFO_ABUSSIZE)); + LOG_DEBUG(" serialcount=0x%x", get_field32(dminfo, DMINFO_SERIALCOUNT)); + LOG_DEBUG(" access128=%d", get_field32(dminfo, DMINFO_ACCESS128)); + LOG_DEBUG(" access64=%d", get_field32(dminfo, DMINFO_ACCESS64)); + LOG_DEBUG(" access32=%d", get_field32(dminfo, DMINFO_ACCESS32)); + LOG_DEBUG(" access16=%d", get_field32(dminfo, DMINFO_ACCESS16)); + LOG_DEBUG(" access8=%d", get_field32(dminfo, DMINFO_ACCESS8)); + LOG_DEBUG(" dramsize=0x%x", get_field32(dminfo, DMINFO_DRAMSIZE)); + LOG_DEBUG(" authenticated=0x%x", get_field32(dminfo, DMINFO_AUTHENTICATED)); + LOG_DEBUG(" authbusy=0x%x", get_field32(dminfo, DMINFO_AUTHBUSY)); + LOG_DEBUG(" authtype=0x%x", get_field32(dminfo, DMINFO_AUTHTYPE)); + LOG_DEBUG(" version=0x%x", get_field32(dminfo, DMINFO_VERSION)); if (get_field(dminfo, DMINFO_VERSION) != 1) { LOG_ERROR("OpenOCD only supports Debug Module version 1, not %d " - "(dminfo=0x%x)", get_field(dminfo, DMINFO_VERSION), dminfo); + "(dminfo=0x%x)", get_field32(dminfo, DMINFO_VERSION), dminfo); return ERROR_FAIL; } diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 5b84aadc8c..43226047e8 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -29,6 +29,7 @@ #include "asm.h" #include "batch.h" #include "debug_reg_printer.h" +#include "field_helpers.h" static int riscv013_on_step_or_resume(struct target *target, bool step); static int riscv013_step_or_resume_current_hart(struct target *target, @@ -85,9 +86,6 @@ static int set_group(struct target *target, bool *supported, unsigned int group, * currently in IR. They should set IR to dbus explicitly. */ -#define get_field(reg, mask) (((reg) & (mask)) / ((mask) & ~((mask) << 1))) -#define set_field(reg, mask, val) (((reg) & ~(mask)) | (((val) * ((mask) & ~((mask) << 1))) & (mask))) - #define RISCV013_INFO(r) riscv013_info_t *r = get_info(target) /*** JTAG registers. ***/ @@ -779,9 +777,9 @@ static int dmstatus_read_timeout(struct target *target, uint32_t *dmstatus, int dmstatus_version = get_field(*dmstatus, DM_DMSTATUS_VERSION); if (dmstatus_version != 2 && dmstatus_version != 3) { LOG_ERROR("OpenOCD only supports Debug Module version 2 (0.13) and 3 (1.0), not " - "%d (dmstatus=0x%x). This error might be caused by a JTAG " + "%" PRId32 " (dmstatus=0x%" PRIx32 "). This error might be caused by a JTAG " "signal issue. Try reducing the JTAG clock speed.", - get_field(*dmstatus, DM_DMSTATUS_VERSION), *dmstatus); + get_field32(*dmstatus, DM_DMSTATUS_VERSION), *dmstatus); } else if (authenticated && !get_field(*dmstatus, DM_DMSTATUS_AUTHENTICATED)) { LOG_ERROR("Debugger is not authenticated to target Debug Module. " "(dmstatus=0x%x). Use `riscv authdata_read` and " @@ -1859,15 +1857,11 @@ static int examine(struct target *target) } LOG_TARGET_DEBUG(target, "dtmcontrol=0x%x", dtmcontrol); - LOG_TARGET_DEBUG(target, " dmireset=%d", get_field(dtmcontrol, DTM_DTMCS_DMIRESET)); - LOG_TARGET_DEBUG(target, " idle=%d", get_field(dtmcontrol, DTM_DTMCS_IDLE)); - LOG_TARGET_DEBUG(target, " dmistat=%d", get_field(dtmcontrol, DTM_DTMCS_DMISTAT)); - LOG_TARGET_DEBUG(target, " abits=%d", get_field(dtmcontrol, DTM_DTMCS_ABITS)); - LOG_TARGET_DEBUG(target, " version=%d", get_field(dtmcontrol, DTM_DTMCS_VERSION)); + log_debug_reg(target, DTM_DTMCS_ORDINAL, dtmcontrol); if (get_field(dtmcontrol, DTM_DTMCS_VERSION) != 1) { - LOG_TARGET_ERROR(target, "Unsupported DTM version %d. (dtmcontrol=0x%x)", - get_field(dtmcontrol, DTM_DTMCS_VERSION), dtmcontrol); + LOG_TARGET_ERROR(target, "Unsupported DTM version %" PRIu32 ". (dtmcontrol=0x%" PRIx32 ")", + get_field32(dtmcontrol, DTM_DTMCS_VERSION), dtmcontrol); return ERROR_FAIL; } diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 57cc5c37b7..fcdf2f8b64 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -25,10 +25,7 @@ #include "rtos/rtos.h" #include "debug_defines.h" #include - -#define get_field(reg, mask) (((reg) & (mask)) / ((mask) & ~((mask) << 1))) -#define set_field(reg, mask, val) (((reg) & ~(mask)) | (((val) * ((mask) & ~((mask) << 1))) & (mask))) -#define field_value(mask, val) set_field((riscv_reg_t)0, mask, val) +#include "field_helpers.h" /*** JTAG registers. ***/