Skip to content

Commit

Permalink
target/riscv: Replace [sg]et_field macros with functions.
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@timsifive
timsifive committed Nov 9, 2023
1 parent 6f02884 commit 1643d1b
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 37 deletions.
4 changes: 1 addition & 3 deletions src/target/riscv/batch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<<DTM_DTMCS_ABITS_LENGTH)-1)
#define DMI_SCAN_MAX_BIT_LENGTH (DTM_DMI_MAX_ADDRESS_LENGTH + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH)
Expand Down
34 changes: 34 additions & 0 deletions src/target/riscv/field_helpers.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */

#ifndef FIELD_HELPERS_H
#define FIELD_HELPERS_H

#include <stdint.h>
#include <assert.h>

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);
reg |= (val * low_field_bit) & mask;
return reg;
}

static inline uint64_t field_value(uint64_t mask, uint64_t val)
{
return set_field(0, mask, val);
}

#endif
38 changes: 18 additions & 20 deletions src/target/riscv/riscv-011.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
18 changes: 8 additions & 10 deletions src/target/riscv/riscv-013.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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. ***/
Expand Down Expand Up @@ -781,7 +779,7 @@ static int dmstatus_read_timeout(struct target *target, uint32_t *dmstatus,
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 "
"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 "
Expand Down Expand Up @@ -1859,15 +1857,15 @@ 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_TARGET_DEBUG(target, " dmireset=%d", get_field32(dtmcontrol, DTM_DTMCS_DMIRESET));
LOG_TARGET_DEBUG(target, " idle=%d", get_field32(dtmcontrol, DTM_DTMCS_IDLE));
LOG_TARGET_DEBUG(target, " dmistat=%d", get_field32(dtmcontrol, DTM_DTMCS_DMISTAT));
LOG_TARGET_DEBUG(target, " abits=%d", get_field32(dtmcontrol, DTM_DTMCS_ABITS));
LOG_TARGET_DEBUG(target, " version=%d", get_field32(dtmcontrol, DTM_DTMCS_VERSION));

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);
get_field32(dtmcontrol, DTM_DTMCS_VERSION), dtmcontrol);
return ERROR_FAIL;
}

Expand Down
5 changes: 1 addition & 4 deletions src/target/riscv/riscv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
#include "rtos/rtos.h"
#include "debug_defines.h"
#include <helper/bits.h>

#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. ***/

Expand Down

0 comments on commit 1643d1b

Please sign in to comment.