[Qemu-devel] [PATCH 2/3] target/ppc: Add GDB callbacks for SPRs

Fabiano Rosas posted 3 patches 7 years, 1 month ago
There is a newer version of this series
[Qemu-devel] [PATCH 2/3] target/ppc: Add GDB callbacks for SPRs
Posted by Fabiano Rosas 7 years, 1 month ago
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;
+}
+
+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) {
-- 
2.17.1


Re: [Qemu-devel] [PATCH 2/3] target/ppc: Add GDB callbacks for SPRs
Posted by David Gibson 7 years ago
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) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson