Skip to content

Commit

Permalink
gdbstub: Simplify XML lookup
Browse files Browse the repository at this point in the history 
Now we know all instances of GDBFeature that is used in CPU so we can
traverse them to find XML. This removes the need for a CPU-specific
lookup function for dynamic XMLs.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231213-gdb-v17-7-777047380591@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20240227144335.1196131-12-alex.bennee@linaro.org>
  • Loading branch information
@akihikodaki @stsquad
akihikodaki authored and stsquad committed Feb 28, 2024
1 parent 6626015 commit ee59fa1
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 60 deletions.
118 changes: 60 additions & 58 deletions gdbstub/gdbstub.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ static const char *get_feature_xml(const char *p, const char **newp,
{
CPUState *cpu = gdb_get_first_cpu_in_process(process);
CPUClass *cc = CPU_GET_CLASS(cpu);
GDBRegisterState *r;
size_t len;

/*
Expand All @@ -365,7 +366,6 @@ static const char *get_feature_xml(const char *p, const char **newp,
/* Is it the main target xml? */
if (strncmp(p, "target.xml", len) == 0) {
if (!process->target_xml) {
GDBRegisterState *r;
g_autoptr(GPtrArray) xml = g_ptr_array_new_with_free_func(g_free);

g_ptr_array_add(
Expand All @@ -380,18 +380,12 @@ static const char *get_feature_xml(const char *p, const char **newp,
g_markup_printf_escaped("<architecture>%s</architecture>",
cc->gdb_arch_name(cpu)));
}
g_ptr_array_add(
xml,
g_markup_printf_escaped("<xi:include href=\"%s\"/>",
cc->gdb_core_xml_file));
if (cpu->gdb_regs) {
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
g_ptr_array_add(
xml,
g_markup_printf_escaped("<xi:include href=\"%s\"/>",
r->feature->xmlname));
}
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
g_ptr_array_add(
xml,
g_markup_printf_escaped("<xi:include href=\"%s\"/>",
r->feature->xmlname));
}
g_ptr_array_add(xml, g_strdup("</target>"));
g_ptr_array_add(xml, NULL);
Expand All @@ -400,20 +394,11 @@ static const char *get_feature_xml(const char *p, const char **newp,
}
return process->target_xml;
}
/* Is it dynamically generated by the target? */
if (cc->gdb_get_dynamic_xml) {
g_autofree char *xmlname = g_strndup(p, len);
const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
if (xml) {
return xml;
}
}
/* Is it one of the encoded gdb-xml/ files? */
for (int i = 0; gdb_static_features[i].xmlname; i++) {
const char *name = gdb_static_features[i].xmlname;
if ((strncmp(name, p, len) == 0) &&
strlen(name) == len) {
return gdb_static_features[i].xml;
/* Is it one of the features? */
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (strncmp(p, r->feature->xmlname, len) == 0) {
return r->feature->xml;
}
}

Expand Down Expand Up @@ -508,12 +493,10 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
return cc->gdb_read_register(cpu, buf, reg);
}

if (cpu->gdb_regs) {
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
return r->get_reg(cpu, buf, reg - r->base_reg);
}
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
return r->get_reg(cpu, buf, reg - r->base_reg);
}
}
return 0;
Expand All @@ -528,51 +511,70 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
return cc->gdb_write_register(cpu, mem_buf, reg);
}

if (cpu->gdb_regs) {
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
return r->set_reg(cpu, mem_buf, reg - r->base_reg);
}
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
return r->set_reg(cpu, mem_buf, reg - r->base_reg);
}
}
return 0;
}

static void gdb_register_feature(CPUState *cpu, int base_reg,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
const GDBFeature *feature)
{
GDBRegisterState s = {
.base_reg = base_reg,
.get_reg = get_reg,
.set_reg = set_reg,
.feature = feature
};

g_array_append_val(cpu->gdb_regs, s);
}

void gdb_init_cpu(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
const GDBFeature *feature;

cpu->gdb_regs = g_array_new(false, false, sizeof(GDBRegisterState));

if (cc->gdb_core_xml_file) {
feature = gdb_find_static_feature(cc->gdb_core_xml_file);
gdb_register_feature(cpu, 0,
cc->gdb_read_register, cc->gdb_write_register,
feature);
}

cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
}

void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
const GDBFeature *feature, int g_pos)
{
GDBRegisterState *s;
guint i;
int base_reg = cpu->gdb_num_regs;

if (cpu->gdb_regs) {
for (i = 0; i < cpu->gdb_regs->len; i++) {
/* Check for duplicates. */
s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (s->feature == feature) {
return;
}
for (i = 0; i < cpu->gdb_regs->len; i++) {
/* Check for duplicates. */
s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (s->feature == feature) {
return;
}
} else {
cpu->gdb_regs = g_array_new(false, false, sizeof(GDBRegisterState));
i = 0;
}

g_array_set_size(cpu->gdb_regs, i + 1);
s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
s->base_reg = cpu->gdb_num_regs;
s->get_reg = get_reg;
s->set_reg = set_reg;
s->feature = feature;
gdb_register_feature(cpu, base_reg, get_reg, set_reg, feature);

/* Add to end of list. */
cpu->gdb_num_regs += feature->num_regs;
if (g_pos) {
if (g_pos != s->base_reg) {
if (g_pos != base_reg) {
error_report("Error: Bad gdb register numbering for '%s', "
"expected %d got %d", feature->xml,
g_pos, s->base_reg);
"expected %d got %d", feature->xml, g_pos, base_reg);
} else {
cpu->gdb_num_g_regs = cpu->gdb_num_regs;
}
Expand Down
5 changes: 3 additions & 2 deletions hw/core/cpu-common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "qemu/main-loop.h"
#include "exec/log.h"
#include "exec/cpu-common.h"
#include "exec/gdbstub.h"
#include "qemu/error-report.h"
#include "qemu/qemu-print.h"
#include "sysemu/tcg.h"
Expand Down Expand Up @@ -240,11 +241,10 @@ static void cpu_common_unrealizefn(DeviceState *dev)
static void cpu_common_initfn(Object *obj)
{
CPUState *cpu = CPU(obj);
CPUClass *cc = CPU_GET_CLASS(obj);

gdb_init_cpu(cpu);
cpu->cpu_index = UNASSIGNED_CPU_INDEX;
cpu->cluster_index = UNASSIGNED_CLUSTER_INDEX;
cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
/* user-mode doesn't have configurable SMP topology */
/* the default value is changed by qemu_init_vcpu() for system-mode */
cpu->nr_cores = 1;
Expand All @@ -264,6 +264,7 @@ static void cpu_common_finalize(Object *obj)
{
CPUState *cpu = CPU(obj);

g_array_free(cpu->gdb_regs, TRUE);
qemu_lockcnt_destroy(&cpu->in_ioctl_lock);
qemu_mutex_destroy(&cpu->work_mutex);
}
Expand Down
6 changes: 6 additions & 0 deletions include/exec/gdbstub.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ typedef struct GDBFeatureBuilder {
typedef int (*gdb_get_reg_cb)(CPUState *cpu, GByteArray *buf, int reg);
typedef int (*gdb_set_reg_cb)(CPUState *cpu, uint8_t *buf, int reg);

/**
* gdb_init_cpu(): Initialize the CPU for gdbstub.
* @cpu: The CPU to be initialized.
*/
void gdb_init_cpu(CPUState *cpu);

/**
* gdb_register_coprocessor() - register a supplemental set of registers
* @cpu - the CPU associated with registers
Expand Down

0 comments on commit ee59fa1

Please sign in to comment.