diff mbox series

[2/3] target/ppc: Add GDB callbacks for SPRs

Message ID 20190104195654.19976-3-farosas@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series ppc/gdbstub: Expose SPRs to GDB | expand

Commit Message

Fabiano Rosas Jan. 4, 2019, 7:56 p.m. UTC
These will be used to let GDB know about PPC's Special Purpose
Registers (SPR).

They take an index based on the order the registers appear in the XML
file sent by QEMU to GDB. This index does not match the actual
location of the registers in the env->spr array so the
gdb_find_spr_idx function does that conversion.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/translate_init.inc.c | 50 +++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Comments

David Gibson Jan. 11, 2019, 12:50 a.m. UTC | #1
On Fri, Jan 04, 2019 at 05:56:53PM -0200, Fabiano Rosas wrote:
> These will be used to let GDB know about PPC's Special Purpose
> Registers (SPR).
> 
> They take an index based on the order the registers appear in the XML
> file sent by QEMU to GDB. This index does not match the actual
> location of the registers in the env->spr array so the
> gdb_find_spr_idx function does that conversion.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/translate_init.inc.c | 50 +++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 03f1d34a97..f10a3637d9 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -9483,6 +9483,56 @@ static bool avr_need_swap(CPUPPCState *env)
>  #endif
>  }
>  
> +#if !defined(CONFIG_USER_ONLY)
> +static int gdb_find_spr_idx(CPUPPCState *env, int n)
> +{
> +    int idx = -1;
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
> +        ppc_spr_t *spr = &env->spr_cb[i];
> +
> +        if (spr->name && ++idx == n) {
> +            break;
> +        }
> +    }
> +    return i;
> +}

This is very subtle - it relies on the fact that you also generate the
XML in sequence, which makes for a very non-obvious dependency between
different parts of the code.  At the very least this needs a big fat
comment explaining how the gdb ids are allocated.

I think better would be to explicitly put a gdb_id into the spr_cb
structure - that would be filled in at the same time you generate the
XML, then referenced here.

> +static int gdb_get_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
> +{
> +    int reg;
> +    int len;
> +
> +    reg = gdb_find_spr_idx(env, n);
> +    if (!reg) {
> +        return 0;
> +    }
> +
> +    len = TARGET_LONG_SIZE;
> +    stn_p(mem_buf, len, env->spr[reg]);
> +    ppc_maybe_bswap_register(env, mem_buf, len);
> +    return len;
> +}
> +
> +static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
> +{
> +    int reg;
> +    int len;
> +
> +    reg = gdb_find_spr_idx(env, n);
> +    if (!reg) {
> +        return 0;
> +    }
> +
> +    len = TARGET_LONG_SIZE;
> +    ppc_maybe_bswap_register(env, mem_buf, len);
> +    env->spr[reg] = ldn_p(mem_buf, len);
> +
> +    return len;
> +}
> +#endif
> +
>  static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
>  {
>      if (n < 32) {
diff mbox series

Patch

diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 03f1d34a97..f10a3637d9 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -9483,6 +9483,56 @@  static bool avr_need_swap(CPUPPCState *env)
 #endif
 }
 
+#if !defined(CONFIG_USER_ONLY)
+static int gdb_find_spr_idx(CPUPPCState *env, int n)
+{
+    int idx = -1;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
+        ppc_spr_t *spr = &env->spr_cb[i];
+
+        if (spr->name && ++idx == n) {
+            break;
+        }
+    }
+    return i;
+}
+
+static int gdb_get_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+{
+    int reg;
+    int len;
+
+    reg = gdb_find_spr_idx(env, n);
+    if (!reg) {
+        return 0;
+    }
+
+    len = TARGET_LONG_SIZE;
+    stn_p(mem_buf, len, env->spr[reg]);
+    ppc_maybe_bswap_register(env, mem_buf, len);
+    return len;
+}
+
+static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+{
+    int reg;
+    int len;
+
+    reg = gdb_find_spr_idx(env, n);
+    if (!reg) {
+        return 0;
+    }
+
+    len = TARGET_LONG_SIZE;
+    ppc_maybe_bswap_register(env, mem_buf, len);
+    env->spr[reg] = ldn_p(mem_buf, len);
+
+    return len;
+}
+#endif
+
 static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
 {
     if (n < 32) {