These members will be used to help plugins to identify registers.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
target/arm/gdbstub.c | 46 +++++++++++++++++++++++++++---------------
target/arm/gdbstub64.c | 42 +++++++++++++++++++++++++-------------
2 files changed, 58 insertions(+), 30 deletions(-)
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 100a6eed15..56d24028f6 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -270,6 +270,7 @@ static void arm_gen_one_feature_sysreg(GString *s,
g_string_append_printf(s, " regnum=\"%d\"", regnum);
g_string_append_printf(s, " group=\"cp_regs\"/>");
dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key;
+ ((const char **)dyn_feature->desc.regs)[dyn_feature->desc.num_regs] = ri->name;
dyn_feature->desc.num_regs++;
}
@@ -316,6 +317,8 @@ static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
gsize num_regs = g_hash_table_size(cpu->cp_regs);
+ dyn_feature->desc.name = "org.qemu.gdb.arm.sys.regs";
+ dyn_feature->desc.regs = g_new(const char *, num_regs);
dyn_feature->desc.num_regs = 0;
dyn_feature->data.cpregs.keys = g_new(uint32_t, num_regs);
g_string_printf(s, "<?xml version=\"1.0\"?>");
@@ -418,30 +421,34 @@ static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
}
static GDBFeature *arm_gen_dynamic_m_systemreg_feature(CPUState *cs,
- int orig_base_reg)
+ int base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
GString *s = g_string_new(NULL);
- int base_reg = orig_base_reg;
- int i;
+ const char **regs = g_new(const char *, ARRAY_SIZE(m_sysreg_def));
+ int i = 0;
+ int j;
g_string_printf(s, "<?xml version=\"1.0\"?>");
g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.m-system\">\n");
- for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
- if (arm_feature(env, m_sysreg_def[i].feature)) {
+ for (j = 0; j < ARRAY_SIZE(m_sysreg_def); j++) {
+ if (arm_feature(env, m_sysreg_def[j].feature)) {
+ regs[i] = m_sysreg_def[j].name;
g_string_append_printf(s,
"<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
- m_sysreg_def[i].name, base_reg++);
+ m_sysreg_def[j].name, base_reg + i++);
}
}
g_string_append_printf(s, "</feature>");
+ cpu->dyn_m_systemreg_feature.desc.name = "org.gnu.gdb.arm.m-system";
cpu->dyn_m_systemreg_feature.desc.xmlname = "arm-m-system.xml";
cpu->dyn_m_systemreg_feature.desc.xml = g_string_free(s, false);
- cpu->dyn_m_systemreg_feature.desc.num_regs = base_reg - orig_base_reg;
+ cpu->dyn_m_systemreg_feature.desc.regs = regs;
+ cpu->dyn_m_systemreg_feature.desc.num_regs = i;
return &cpu->dyn_m_systemreg_feature.desc;
}
@@ -462,30 +469,37 @@ static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
}
static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
- int orig_base_reg)
+ int base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
- int base_reg = orig_base_reg;
- int i;
+ const char **regs = g_new(const char *, ARRAY_SIZE(m_sysreg_def) * 2);
+ int i = 0;
+ int j;
g_string_printf(s, "<?xml version=\"1.0\"?>");
g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.secext\">\n");
- for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
+ for (j = 0; j < ARRAY_SIZE(m_sysreg_def); j++) {
+ regs[i] = g_strconcat(m_sysreg_def[j].name, "_ns", NULL);
g_string_append_printf(s,
- "<reg name=\"%s_ns\" bitsize=\"32\" regnum=\"%d\"/>\n",
- m_sysreg_def[i].name, base_reg++);
+ "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
+ regs[i], base_reg + i);
+ i++;
+ regs[i] = g_strconcat(m_sysreg_def[j].name, "_s", NULL);
g_string_append_printf(s,
- "<reg name=\"%s_s\" bitsize=\"32\" regnum=\"%d\"/>\n",
- m_sysreg_def[i].name, base_reg++);
+ "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
+ regs[i], base_reg + i);
+ i++;
}
g_string_append_printf(s, "</feature>");
+ cpu->dyn_m_secextreg_feature.desc.name = "org.gnu.gdb.arm.secext";
cpu->dyn_m_secextreg_feature.desc.xmlname = "arm-m-secext.xml";
cpu->dyn_m_secextreg_feature.desc.xml = g_string_free(s, false);
- cpu->dyn_m_secextreg_feature.desc.num_regs = base_reg - orig_base_reg;
+ cpu->dyn_m_secextreg_feature.desc.regs = regs;
+ cpu->dyn_m_secextreg_feature.desc.num_regs = i;
return &cpu->dyn_m_secextreg_feature.desc;
}
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 20483ef9bc..c5ed7c0aa3 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -316,15 +316,21 @@ static void output_vector_union_type(GString *s, int reg_width,
g_string_append(s, "</union>");
}
-GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
+GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
DynamicGDBFeatureInfo *info = &cpu->dyn_svereg_feature;
+ const char **regs;
int reg_width = cpu->sve_max_vq * 128;
int pred_width = cpu->sve_max_vq * 16;
- int base_reg = orig_base_reg;
- int i;
+ int i = 0;
+ int j;
+
+ info->desc.name = "org.gnu.gdb.aarch64.sve";
+ info->desc.num_regs = 32 + 16 + 4;
+ regs = g_new(const char *, info->desc.num_regs);
+ info->desc.regs = regs;
g_string_printf(s, "<?xml version=\"1.0\"?>");
g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
@@ -339,44 +345,52 @@ GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
pred_width / 8);
/* Define the vector registers. */
- for (i = 0; i < 32; i++) {
+ for (j = 0; j < 32; j++) {
+ regs[i] = g_strdup_printf("z%d", j);
g_string_append_printf(s,
- "<reg name=\"z%d\" bitsize=\"%d\""
+ "<reg name=\"%s\" bitsize=\"%d\""
" regnum=\"%d\" type=\"svev\"/>",
- i, reg_width, base_reg++);
+ regs[i], reg_width, base_reg + i);
+ i++;
}
/* fpscr & status registers */
+ regs[i] = "fpsr";
g_string_append_printf(s, "<reg name=\"fpsr\" bitsize=\"32\""
" regnum=\"%d\" group=\"float\""
- " type=\"int\"/>", base_reg++);
+ " type=\"int\"/>", base_reg + i++);
+ regs[i] = "fpcr";
g_string_append_printf(s, "<reg name=\"fpcr\" bitsize=\"32\""
" regnum=\"%d\" group=\"float\""
- " type=\"int\"/>", base_reg++);
+ " type=\"int\"/>", base_reg + i++);
/* Define the predicate registers. */
- for (i = 0; i < 16; i++) {
+ for (j = 0; j < 16; j++) {
+ regs[i] = g_strdup_printf("p%d", j);
g_string_append_printf(s,
- "<reg name=\"p%d\" bitsize=\"%d\""
+ "<reg name=\"%s\" bitsize=\"%d\""
" regnum=\"%d\" type=\"svep\"/>",
- i, pred_width, base_reg++);
+ regs[i], pred_width, base_reg + i);
+ i++;
}
+ regs[i] = "ffr";
g_string_append_printf(s,
"<reg name=\"ffr\" bitsize=\"%d\""
" regnum=\"%d\" group=\"vector\""
" type=\"svep\"/>",
- pred_width, base_reg++);
+ pred_width, base_reg + i++);
/* Define the vector length pseudo-register. */
+ regs[i] = "vg";
g_string_append_printf(s,
"<reg name=\"vg\" bitsize=\"64\""
" regnum=\"%d\" type=\"int\"/>",
- base_reg++);
+ base_reg + i++);
g_string_append_printf(s, "</feature>");
info->desc.xmlname = "sve-registers.xml";
info->desc.xml = g_string_free(s, false);
- info->desc.num_regs = base_reg - orig_base_reg;
+ assert(info->desc.num_regs == i);
return &info->desc;
}
--
2.41.0
Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> These members will be used to help plugins to identify registers.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> target/arm/gdbstub.c | 46 +++++++++++++++++++++++++++---------------
> target/arm/gdbstub64.c | 42 +++++++++++++++++++++++++-------------
> 2 files changed, 58 insertions(+), 30 deletions(-)
>
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index 100a6eed15..56d24028f6 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -270,6 +270,7 @@ static void arm_gen_one_feature_sysreg(GString *s,
> g_string_append_printf(s, " regnum=\"%d\"", regnum);
> g_string_append_printf(s, " group=\"cp_regs\"/>");
> dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key;
> + ((const char **)dyn_feature->desc.regs)[dyn_feature->desc.num_regs] = ri->name;
> dyn_feature->desc.num_regs++;
> }
>
> @@ -316,6 +317,8 @@ static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
> DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
> gsize num_regs = g_hash_table_size(cpu->cp_regs);
>
> + dyn_feature->desc.name = "org.qemu.gdb.arm.sys.regs";
> + dyn_feature->desc.regs = g_new(const char *, num_regs);
AIUI this means we now have an array of register names which mirrors the
names embedded in the XML. This smells like a few steps away from just
abstracting the whole XML away from the targets and generating them
inside gdbstub when we need them. As per my stalled attempt I referenced
earlier.
> dyn_feature->desc.num_regs = 0;
> dyn_feature->data.cpregs.keys = g_new(uint32_t, num_regs);
> g_string_printf(s, "<?xml version=\"1.0\"?>");
> @@ -418,30 +421,34 @@ static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
> }
>
> static GDBFeature *arm_gen_dynamic_m_systemreg_feature(CPUState *cs,
> - int orig_base_reg)
> + int base_reg)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> GString *s = g_string_new(NULL);
> - int base_reg = orig_base_reg;
> - int i;
> + const char **regs = g_new(const char *, ARRAY_SIZE(m_sysreg_def));
> + int i = 0;
> + int j;
>
> g_string_printf(s, "<?xml version=\"1.0\"?>");
> g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.m-system\">\n");
>
> - for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
> - if (arm_feature(env, m_sysreg_def[i].feature)) {
> + for (j = 0; j < ARRAY_SIZE(m_sysreg_def); j++) {
> + if (arm_feature(env, m_sysreg_def[j].feature)) {
> + regs[i] = m_sysreg_def[j].name;
> g_string_append_printf(s,
> "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
> - m_sysreg_def[i].name, base_reg++);
> + m_sysreg_def[j].name, base_reg + i++);
> }
> }
>
> g_string_append_printf(s, "</feature>");
> + cpu->dyn_m_systemreg_feature.desc.name = "org.gnu.gdb.arm.m-system";
> cpu->dyn_m_systemreg_feature.desc.xmlname = "arm-m-system.xml";
> cpu->dyn_m_systemreg_feature.desc.xml = g_string_free(s, false);
> - cpu->dyn_m_systemreg_feature.desc.num_regs = base_reg - orig_base_reg;
> + cpu->dyn_m_systemreg_feature.desc.regs = regs;
> + cpu->dyn_m_systemreg_feature.desc.num_regs = i;
>
> return &cpu->dyn_m_systemreg_feature.desc;
> }
> @@ -462,30 +469,37 @@ static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
> }
>
> static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
> - int orig_base_reg)
> + int base_reg)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> GString *s = g_string_new(NULL);
> - int base_reg = orig_base_reg;
> - int i;
> + const char **regs = g_new(const char *, ARRAY_SIZE(m_sysreg_def) * 2);
> + int i = 0;
> + int j;
>
> g_string_printf(s, "<?xml version=\"1.0\"?>");
> g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.secext\">\n");
>
> - for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
> + for (j = 0; j < ARRAY_SIZE(m_sysreg_def); j++) {
> + regs[i] = g_strconcat(m_sysreg_def[j].name, "_ns", NULL);
> g_string_append_printf(s,
> - "<reg name=\"%s_ns\" bitsize=\"32\" regnum=\"%d\"/>\n",
> - m_sysreg_def[i].name, base_reg++);
> + "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
> + regs[i], base_reg + i);
> + i++;
> + regs[i] = g_strconcat(m_sysreg_def[j].name, "_s", NULL);
> g_string_append_printf(s,
> - "<reg name=\"%s_s\" bitsize=\"32\" regnum=\"%d\"/>\n",
> - m_sysreg_def[i].name, base_reg++);
> + "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
> + regs[i], base_reg + i);
> + i++;
> }
>
> g_string_append_printf(s, "</feature>");
> + cpu->dyn_m_secextreg_feature.desc.name = "org.gnu.gdb.arm.secext";
> cpu->dyn_m_secextreg_feature.desc.xmlname = "arm-m-secext.xml";
> cpu->dyn_m_secextreg_feature.desc.xml = g_string_free(s, false);
> - cpu->dyn_m_secextreg_feature.desc.num_regs = base_reg - orig_base_reg;
> + cpu->dyn_m_secextreg_feature.desc.regs = regs;
> + cpu->dyn_m_secextreg_feature.desc.num_regs = i;
>
> return &cpu->dyn_m_secextreg_feature.desc;
> }
> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> index 20483ef9bc..c5ed7c0aa3 100644
> --- a/target/arm/gdbstub64.c
> +++ b/target/arm/gdbstub64.c
> @@ -316,15 +316,21 @@ static void output_vector_union_type(GString *s, int reg_width,
> g_string_append(s, "</union>");
> }
>
> -GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
> +GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> GString *s = g_string_new(NULL);
> DynamicGDBFeatureInfo *info = &cpu->dyn_svereg_feature;
> + const char **regs;
> int reg_width = cpu->sve_max_vq * 128;
> int pred_width = cpu->sve_max_vq * 16;
> - int base_reg = orig_base_reg;
> - int i;
> + int i = 0;
> + int j;
> +
> + info->desc.name = "org.gnu.gdb.aarch64.sve";
> + info->desc.num_regs = 32 + 16 + 4;
> + regs = g_new(const char *, info->desc.num_regs);
> + info->desc.regs = regs;
>
> g_string_printf(s, "<?xml version=\"1.0\"?>");
> g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> @@ -339,44 +345,52 @@ GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
> pred_width / 8);
>
> /* Define the vector registers. */
> - for (i = 0; i < 32; i++) {
> + for (j = 0; j < 32; j++) {
> + regs[i] = g_strdup_printf("z%d", j);
> g_string_append_printf(s,
> - "<reg name=\"z%d\" bitsize=\"%d\""
> + "<reg name=\"%s\" bitsize=\"%d\""
> " regnum=\"%d\" type=\"svev\"/>",
> - i, reg_width, base_reg++);
> + regs[i], reg_width, base_reg + i);
> + i++;
> }
>
> /* fpscr & status registers */
> + regs[i] = "fpsr";
> g_string_append_printf(s, "<reg name=\"fpsr\" bitsize=\"32\""
> " regnum=\"%d\" group=\"float\""
> - " type=\"int\"/>", base_reg++);
> + " type=\"int\"/>", base_reg + i++);
> + regs[i] = "fpcr";
> g_string_append_printf(s, "<reg name=\"fpcr\" bitsize=\"32\""
> " regnum=\"%d\" group=\"float\""
> - " type=\"int\"/>", base_reg++);
> + " type=\"int\"/>", base_reg + i++);
>
> /* Define the predicate registers. */
> - for (i = 0; i < 16; i++) {
> + for (j = 0; j < 16; j++) {
> + regs[i] = g_strdup_printf("p%d", j);
> g_string_append_printf(s,
> - "<reg name=\"p%d\" bitsize=\"%d\""
> + "<reg name=\"%s\" bitsize=\"%d\""
> " regnum=\"%d\" type=\"svep\"/>",
> - i, pred_width, base_reg++);
> + regs[i], pred_width, base_reg + i);
> + i++;
> }
> + regs[i] = "ffr";
> g_string_append_printf(s,
> "<reg name=\"ffr\" bitsize=\"%d\""
> " regnum=\"%d\" group=\"vector\""
> " type=\"svep\"/>",
> - pred_width, base_reg++);
> + pred_width, base_reg + i++);
>
> /* Define the vector length pseudo-register. */
> + regs[i] = "vg";
> g_string_append_printf(s,
> "<reg name=\"vg\" bitsize=\"64\""
> " regnum=\"%d\" type=\"int\"/>",
> - base_reg++);
> + base_reg + i++);
>
> g_string_append_printf(s, "</feature>");
>
> info->desc.xmlname = "sve-registers.xml";
> info->desc.xml = g_string_free(s, false);
> - info->desc.num_regs = base_reg - orig_base_reg;
> + assert(info->desc.num_regs == i);
> return &info->desc;
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
On 2023/08/14 23:56, Alex Bennée wrote:
>
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>
>> These members will be used to help plugins to identify registers.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>> target/arm/gdbstub.c | 46 +++++++++++++++++++++++++++---------------
>> target/arm/gdbstub64.c | 42 +++++++++++++++++++++++++-------------
>> 2 files changed, 58 insertions(+), 30 deletions(-)
>>
>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>> index 100a6eed15..56d24028f6 100644
>> --- a/target/arm/gdbstub.c
>> +++ b/target/arm/gdbstub.c
>> @@ -270,6 +270,7 @@ static void arm_gen_one_feature_sysreg(GString *s,
>> g_string_append_printf(s, " regnum=\"%d\"", regnum);
>> g_string_append_printf(s, " group=\"cp_regs\"/>");
>> dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key;
>> + ((const char **)dyn_feature->desc.regs)[dyn_feature->desc.num_regs] = ri->name;
>> dyn_feature->desc.num_regs++;
>> }
>>
>> @@ -316,6 +317,8 @@ static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
>> DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
>> gsize num_regs = g_hash_table_size(cpu->cp_regs);
>>
>> + dyn_feature->desc.name = "org.qemu.gdb.arm.sys.regs";
>> + dyn_feature->desc.regs = g_new(const char *, num_regs);
>
> AIUI this means we now have an array of register names which mirrors the
> names embedded in the XML. This smells like a few steps away from just
> abstracting the whole XML away from the targets and generating them
> inside gdbstub when we need them. As per my stalled attempt I referenced
> earlier.
The abstraction is strictly limited for identifiers. Most plugin should
already have some knowledge of how registers are used. For example, a
plugin that tracks stack frame for RISC-V should know sp is the stack
pointer register. Similarly, a cycle simulator plugin should know how
registers are used in a program. Only identifiers matter in such cases.
I'm definitely *not* in favor of abstracting the whole XML for plugins.
It will be too hard to maintain ABI compatibility when a new attribute
emerges, for example.
>
>
>> dyn_feature->desc.num_regs = 0;
>> dyn_feature->data.cpregs.keys = g_new(uint32_t, num_regs);
>> g_string_printf(s, "<?xml version=\"1.0\"?>");
>> @@ -418,30 +421,34 @@ static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
>> }
>>
>> static GDBFeature *arm_gen_dynamic_m_systemreg_feature(CPUState *cs,
>> - int orig_base_reg)
>> + int base_reg)
>> {
>> ARMCPU *cpu = ARM_CPU(cs);
>> CPUARMState *env = &cpu->env;
>> GString *s = g_string_new(NULL);
>> - int base_reg = orig_base_reg;
>> - int i;
>> + const char **regs = g_new(const char *, ARRAY_SIZE(m_sysreg_def));
>> + int i = 0;
>> + int j;
>>
>> g_string_printf(s, "<?xml version=\"1.0\"?>");
>> g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
>> g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.m-system\">\n");
>>
>> - for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
>> - if (arm_feature(env, m_sysreg_def[i].feature)) {
>> + for (j = 0; j < ARRAY_SIZE(m_sysreg_def); j++) {
>> + if (arm_feature(env, m_sysreg_def[j].feature)) {
>> + regs[i] = m_sysreg_def[j].name;
>> g_string_append_printf(s,
>> "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
>> - m_sysreg_def[i].name, base_reg++);
>> + m_sysreg_def[j].name, base_reg + i++);
>> }
>> }
>>
>> g_string_append_printf(s, "</feature>");
>> + cpu->dyn_m_systemreg_feature.desc.name = "org.gnu.gdb.arm.m-system";
>> cpu->dyn_m_systemreg_feature.desc.xmlname = "arm-m-system.xml";
>> cpu->dyn_m_systemreg_feature.desc.xml = g_string_free(s, false);
>> - cpu->dyn_m_systemreg_feature.desc.num_regs = base_reg - orig_base_reg;
>> + cpu->dyn_m_systemreg_feature.desc.regs = regs;
>> + cpu->dyn_m_systemreg_feature.desc.num_regs = i;
>>
>> return &cpu->dyn_m_systemreg_feature.desc;
>> }
>> @@ -462,30 +469,37 @@ static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
>> }
>>
>> static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
>> - int orig_base_reg)
>> + int base_reg)
>> {
>> ARMCPU *cpu = ARM_CPU(cs);
>> GString *s = g_string_new(NULL);
>> - int base_reg = orig_base_reg;
>> - int i;
>> + const char **regs = g_new(const char *, ARRAY_SIZE(m_sysreg_def) * 2);
>> + int i = 0;
>> + int j;
>>
>> g_string_printf(s, "<?xml version=\"1.0\"?>");
>> g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
>> g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.secext\">\n");
>>
>> - for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
>> + for (j = 0; j < ARRAY_SIZE(m_sysreg_def); j++) {
>> + regs[i] = g_strconcat(m_sysreg_def[j].name, "_ns", NULL);
>> g_string_append_printf(s,
>> - "<reg name=\"%s_ns\" bitsize=\"32\" regnum=\"%d\"/>\n",
>> - m_sysreg_def[i].name, base_reg++);
>> + "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
>> + regs[i], base_reg + i);
>> + i++;
>> + regs[i] = g_strconcat(m_sysreg_def[j].name, "_s", NULL);
>> g_string_append_printf(s,
>> - "<reg name=\"%s_s\" bitsize=\"32\" regnum=\"%d\"/>\n",
>> - m_sysreg_def[i].name, base_reg++);
>> + "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
>> + regs[i], base_reg + i);
>> + i++;
>> }
>>
>> g_string_append_printf(s, "</feature>");
>> + cpu->dyn_m_secextreg_feature.desc.name = "org.gnu.gdb.arm.secext";
>> cpu->dyn_m_secextreg_feature.desc.xmlname = "arm-m-secext.xml";
>> cpu->dyn_m_secextreg_feature.desc.xml = g_string_free(s, false);
>> - cpu->dyn_m_secextreg_feature.desc.num_regs = base_reg - orig_base_reg;
>> + cpu->dyn_m_secextreg_feature.desc.regs = regs;
>> + cpu->dyn_m_secextreg_feature.desc.num_regs = i;
>>
>> return &cpu->dyn_m_secextreg_feature.desc;
>> }
>> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
>> index 20483ef9bc..c5ed7c0aa3 100644
>> --- a/target/arm/gdbstub64.c
>> +++ b/target/arm/gdbstub64.c
>> @@ -316,15 +316,21 @@ static void output_vector_union_type(GString *s, int reg_width,
>> g_string_append(s, "</union>");
>> }
>>
>> -GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
>> +GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
>> {
>> ARMCPU *cpu = ARM_CPU(cs);
>> GString *s = g_string_new(NULL);
>> DynamicGDBFeatureInfo *info = &cpu->dyn_svereg_feature;
>> + const char **regs;
>> int reg_width = cpu->sve_max_vq * 128;
>> int pred_width = cpu->sve_max_vq * 16;
>> - int base_reg = orig_base_reg;
>> - int i;
>> + int i = 0;
>> + int j;
>> +
>> + info->desc.name = "org.gnu.gdb.aarch64.sve";
>> + info->desc.num_regs = 32 + 16 + 4;
>> + regs = g_new(const char *, info->desc.num_regs);
>> + info->desc.regs = regs;
>>
>> g_string_printf(s, "<?xml version=\"1.0\"?>");
>> g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
>> @@ -339,44 +345,52 @@ GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
>> pred_width / 8);
>>
>> /* Define the vector registers. */
>> - for (i = 0; i < 32; i++) {
>> + for (j = 0; j < 32; j++) {
>> + regs[i] = g_strdup_printf("z%d", j);
>> g_string_append_printf(s,
>> - "<reg name=\"z%d\" bitsize=\"%d\""
>> + "<reg name=\"%s\" bitsize=\"%d\""
>> " regnum=\"%d\" type=\"svev\"/>",
>> - i, reg_width, base_reg++);
>> + regs[i], reg_width, base_reg + i);
>> + i++;
>> }
>>
>> /* fpscr & status registers */
>> + regs[i] = "fpsr";
>> g_string_append_printf(s, "<reg name=\"fpsr\" bitsize=\"32\""
>> " regnum=\"%d\" group=\"float\""
>> - " type=\"int\"/>", base_reg++);
>> + " type=\"int\"/>", base_reg + i++);
>> + regs[i] = "fpcr";
>> g_string_append_printf(s, "<reg name=\"fpcr\" bitsize=\"32\""
>> " regnum=\"%d\" group=\"float\""
>> - " type=\"int\"/>", base_reg++);
>> + " type=\"int\"/>", base_reg + i++);
>>
>> /* Define the predicate registers. */
>> - for (i = 0; i < 16; i++) {
>> + for (j = 0; j < 16; j++) {
>> + regs[i] = g_strdup_printf("p%d", j);
>> g_string_append_printf(s,
>> - "<reg name=\"p%d\" bitsize=\"%d\""
>> + "<reg name=\"%s\" bitsize=\"%d\""
>> " regnum=\"%d\" type=\"svep\"/>",
>> - i, pred_width, base_reg++);
>> + regs[i], pred_width, base_reg + i);
>> + i++;
>> }
>> + regs[i] = "ffr";
>> g_string_append_printf(s,
>> "<reg name=\"ffr\" bitsize=\"%d\""
>> " regnum=\"%d\" group=\"vector\""
>> " type=\"svep\"/>",
>> - pred_width, base_reg++);
>> + pred_width, base_reg + i++);
>>
>> /* Define the vector length pseudo-register. */
>> + regs[i] = "vg";
>> g_string_append_printf(s,
>> "<reg name=\"vg\" bitsize=\"64\""
>> " regnum=\"%d\" type=\"int\"/>",
>> - base_reg++);
>> + base_reg + i++);
>>
>> g_string_append_printf(s, "</feature>");
>>
>> info->desc.xmlname = "sve-registers.xml";
>> info->desc.xml = g_string_free(s, false);
>> - info->desc.num_regs = base_reg - orig_base_reg;
>> + assert(info->desc.num_regs == i);
>> return &info->desc;
>> }
>
>
Akihiko Odaki <akihiko.odaki@daynix.com> writes: > On 2023/08/14 23:56, Alex Bennée wrote: >> Akihiko Odaki <akihiko.odaki@daynix.com> writes: >> >>> These members will be used to help plugins to identify registers. >>> >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>> --- >>> target/arm/gdbstub.c | 46 +++++++++++++++++++++++++++--------------- >>> target/arm/gdbstub64.c | 42 +++++++++++++++++++++++++------------- >>> 2 files changed, 58 insertions(+), 30 deletions(-) >>> >>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c >>> index 100a6eed15..56d24028f6 100644 >>> --- a/target/arm/gdbstub.c >>> +++ b/target/arm/gdbstub.c >>> @@ -270,6 +270,7 @@ static void arm_gen_one_feature_sysreg(GString *s, >>> g_string_append_printf(s, " regnum=\"%d\"", regnum); >>> g_string_append_printf(s, " group=\"cp_regs\"/>"); >>> dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key; >>> + ((const char **)dyn_feature->desc.regs)[dyn_feature->desc.num_regs] = ri->name; >>> dyn_feature->desc.num_regs++; >>> } >>> @@ -316,6 +317,8 @@ static GDBFeature >>> *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg) >>> DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature; >>> gsize num_regs = g_hash_table_size(cpu->cp_regs); >>> + dyn_feature->desc.name = "org.qemu.gdb.arm.sys.regs"; >>> + dyn_feature->desc.regs = g_new(const char *, num_regs); >> AIUI this means we now have an array of register names which mirrors >> the >> names embedded in the XML. This smells like a few steps away from just >> abstracting the whole XML away from the targets and generating them >> inside gdbstub when we need them. As per my stalled attempt I referenced >> earlier. > > The abstraction is strictly limited for identifiers. Most plugin > should already have some knowledge of how registers are used. For > example, a plugin that tracks stack frame for RISC-V should know sp is > the stack pointer register. Similarly, a cycle simulator plugin should > know how registers are used in a program. Only identifiers matter in > such cases. > > I'm definitely *not* in favor of abstracting the whole XML for > plugins. It will be too hard to maintain ABI compatibility when a new > attribute emerges, for example. No I agree the XML shouldn't go near the plugins. I was just looking to avoid having an XML builder for every target. -- Alex Bennée Virtualisation Tech Lead @ Linaro
On 2023/08/17 0:03, Alex Bennée wrote: > > Akihiko Odaki <akihiko.odaki@daynix.com> writes: > >> On 2023/08/14 23:56, Alex Bennée wrote: >>> Akihiko Odaki <akihiko.odaki@daynix.com> writes: >>> >>>> These members will be used to help plugins to identify registers. >>>> >>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>>> --- >>>> target/arm/gdbstub.c | 46 +++++++++++++++++++++++++++--------------- >>>> target/arm/gdbstub64.c | 42 +++++++++++++++++++++++++------------- >>>> 2 files changed, 58 insertions(+), 30 deletions(-) >>>> >>>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c >>>> index 100a6eed15..56d24028f6 100644 >>>> --- a/target/arm/gdbstub.c >>>> +++ b/target/arm/gdbstub.c >>>> @@ -270,6 +270,7 @@ static void arm_gen_one_feature_sysreg(GString *s, >>>> g_string_append_printf(s, " regnum=\"%d\"", regnum); >>>> g_string_append_printf(s, " group=\"cp_regs\"/>"); >>>> dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key; >>>> + ((const char **)dyn_feature->desc.regs)[dyn_feature->desc.num_regs] = ri->name; >>>> dyn_feature->desc.num_regs++; >>>> } >>>> @@ -316,6 +317,8 @@ static GDBFeature >>>> *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg) >>>> DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature; >>>> gsize num_regs = g_hash_table_size(cpu->cp_regs); >>>> + dyn_feature->desc.name = "org.qemu.gdb.arm.sys.regs"; >>>> + dyn_feature->desc.regs = g_new(const char *, num_regs); >>> AIUI this means we now have an array of register names which mirrors >>> the >>> names embedded in the XML. This smells like a few steps away from just >>> abstracting the whole XML away from the targets and generating them >>> inside gdbstub when we need them. As per my stalled attempt I referenced >>> earlier. >> >> The abstraction is strictly limited for identifiers. Most plugin >> should already have some knowledge of how registers are used. For >> example, a plugin that tracks stack frame for RISC-V should know sp is >> the stack pointer register. Similarly, a cycle simulator plugin should >> know how registers are used in a program. Only identifiers matter in >> such cases. >> >> I'm definitely *not* in favor of abstracting the whole XML for >> plugins. It will be too hard to maintain ABI compatibility when a new >> attribute emerges, for example. > > No I agree the XML shouldn't go near the plugins. I was just looking to > avoid having an XML builder for every target. Oh, I see. It's done in v4 with patch "gdbstub: Introduce GDBFeatureBuilder". >
© 2016 - 2026 Red Hat, Inc.