Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature

qemu-ppc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature

From:	Akihiko Odaki
Subject:	Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature
Date:	Wed, 16 Aug 2023 23:23:06 +0900
User-agent:	Mozilla Thunderbird

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 shouldalready have some knowledge of how registers are used. For example, aplugin that tracks stack frame for RISC-V should know sp is the stackpointer register. Similarly, a cycle simulator plugin should know howregisters 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 attributeemerges, 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;
  }

[Prev in Thread]

Current Thread

[Next in Thread]

Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature, Alex Bennée, 2023/08/14
- Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature, Akihiko Odaki <=
  - Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature, Alex Bennée, 2023/08/16
    - Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature, Akihiko Odaki, 2023/08/16

Prev by Date: Re: [PATCH 4/9] tests: remove test-gdbstub.py
Next by Date: Re: [RFC PATCH 21/24] plugins: Allow to read registers
Previous by thread: Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature
Next by thread: Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature
Index(es):
- Date
- Thread