From: David Gibson <david@gibson.dropbear.id.au>
To: Fabiano Rosas <farosas@linux.ibm.com>
Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, aik@ozlabs.ru,
groug@kaod.org
Subject: Re: [Qemu-devel] [PATCH v5 1/1] target/ppc: Enable reporting of SPRs to GDB
Date: Fri, 8 Feb 2019 16:20:28 +1100 [thread overview]
Message-ID: <20190208052027.GA6434@umbus.fritz.box> (raw)
In-Reply-To: <20190206165133.12246-2-farosas@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 8836 bytes --]
On Wed, Feb 06, 2019 at 02:51:33PM -0200, Fabiano Rosas wrote:
> This allows reading and writing of SPRs via GDB:
>
> (gdb) p/x $srr1
> $1 = 0x8000000002803033
>
> (gdb) p/x $pvr
> $2 = 0x4b0201
> (gdb) set $pvr=0x4b0000
> (gdb) p/x $pvr
> $3 = 0x4b0000
>
> The `info` command can also be used:
> (gdb) info registers spr
>
> For this purpose, GDB needs to be provided with an XML description of
> the registers (see the gdb-xml directory for examples) and a set of
> callbacks for reading and writing the registers must be defined.
>
> The XML file in this case is created dynamically, based on the SPRs
> already defined in the machine. This way we avoid the need for several
> XML files to suit each possible ppc machine.
>
> The gdb_{get,set}_spr_reg callbacks take an index based on the order
> the registers appear in the XML file. 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.
>
> Note: GDB currently needs to know the guest endianness in order to
> properly print the registers values. This is done automatically by GDB
> when provided with the ELF file or explicitly with the `set endian
> <big|little>` command.
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
I've applied this to ppc-for-4.0 because it does something we don't
currently do. I'm still a bit baffled by the endian handling, but we
can fix that later if necessary.
> ---
> target/ppc/cpu-qom.h | 4 +++
> target/ppc/cpu.h | 5 +++
> target/ppc/gdbstub.c | 61 ++++++++++++++++++++++++++++++++
> target/ppc/translate_init.inc.c | 62 +++++++++++++++++++++++++++++++--
> 4 files changed, 130 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 4ea67692e2..3130802304 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -179,6 +179,10 @@ typedef struct PowerPCCPUClass {
> uint32_t flags;
> int bfd_mach;
> uint32_t l1_dcache_size, l1_icache_size;
> +#ifndef CONFIG_USER_ONLY
> + unsigned int gdb_num_sprs;
> + const char *gdb_spr_xml;
> +#endif
> const PPCHash64Options *hash64_opts;
> struct ppc_radix_page_info *radix_page_info;
> void (*init_proc)(CPUPPCState *env);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 2c22292e7f..78af7e4608 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -230,6 +230,7 @@ struct ppc_spr_t {
> void (*oea_write)(DisasContext *ctx, int spr_num, int gpr_num);
> void (*hea_read)(DisasContext *ctx, int gpr_num, int spr_num);
> void (*hea_write)(DisasContext *ctx, int spr_num, int gpr_num);
> + unsigned int gdb_id;
> #endif
> const char *name;
> target_ulong default_value;
> @@ -1263,6 +1264,10 @@ int ppc_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
> int ppc_cpu_gdb_read_register_apple(CPUState *cpu, uint8_t *buf, int reg);
> int ppc_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> int ppc_cpu_gdb_write_register_apple(CPUState *cpu, uint8_t *buf, int reg);
> +#ifndef CONFIG_USER_ONLY
> +void ppc_gdb_gen_spr_xml(PowerPCCPU *cpu);
> +const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name);
> +#endif
> int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> int cpuid, void *opaque);
> int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
> diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
> index 19565b584d..fbf3821f4b 100644
> --- a/target/ppc/gdbstub.c
> +++ b/target/ppc/gdbstub.c
> @@ -319,3 +319,64 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, uint8_t *mem_buf, int n)
> }
> return r;
> }
> +
> +#ifndef CONFIG_USER_ONLY
> +void ppc_gdb_gen_spr_xml(PowerPCCPU *cpu)
> +{
> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> + CPUPPCState *env = &cpu->env;
> + GString *xml;
> + char *spr_name;
> + unsigned int num_regs = 0;
> + int i;
> +
> + if (pcc->gdb_spr_xml) {
> + return;
> + }
> +
> + xml = g_string_new("<?xml version=\"1.0\"?>");
> + g_string_append(xml, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> + g_string_append(xml, "<feature name=\"org.qemu.power.spr\">");
> +
> + for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
> + ppc_spr_t *spr = &env->spr_cb[i];
> +
> + if (!spr->name) {
> + continue;
> + }
> +
> + spr_name = g_ascii_strdown(spr->name, -1);
> + g_string_append_printf(xml, "<reg name=\"%s\"", spr_name);
> + g_free(spr_name);
> +
> + g_string_append_printf(xml, " bitsize=\"%d\"", TARGET_LONG_BITS);
> + g_string_append(xml, " group=\"spr\"/>");
> +
> + /*
> + * GDB identifies registers based on the order they are
> + * presented in the XML. These ids will not match QEMU's
> + * representation (which follows the PowerISA).
> + *
> + * Store the position of the current register description so
> + * we can make the correspondence later.
> + */
> + spr->gdb_id = num_regs;
> + num_regs++;
> + }
> +
> + g_string_append(xml, "</feature>");
> +
> + pcc->gdb_num_sprs = num_regs;
> + pcc->gdb_spr_xml = g_string_free(xml, false);
> +}
> +
> +const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name)
> +{
> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> +
> + if (strcmp(xml_name, "power-spr.xml") == 0) {
> + return pcc->gdb_spr_xml;
> + }
> + return NULL;
> +}
> +#endif
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 59e0b86762..9295f78d5f 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -8979,6 +8979,10 @@ static void init_ppc_proc(PowerPCCPU *cpu)
> /* PowerPC implementation specific initialisations (SPRs, timers, ...) */
> (*pcc->init_proc)(env);
>
> +#if !defined(CONFIG_USER_ONLY)
> + ppc_gdb_gen_spr_xml(cpu);
> +#endif
> +
> /* MSR bits & flags consistency checks */
> if (env->msr_mask & (1 << 25)) {
> switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
> @@ -9475,6 +9479,55 @@ static bool avr_need_swap(CPUPPCState *env)
> #endif
> }
>
> +#if !defined(CONFIG_USER_ONLY)
> +static int gdb_find_spr_idx(CPUPPCState *env, int n)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
> + ppc_spr_t *spr = &env->spr_cb[i];
> +
> + if (spr->name && spr->gdb_id == n) {
> + return i;
> + }
> + }
> + return -1;
> +}
> +
> +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 < 0) {
> + 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 < 0) {
> + 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) {
> @@ -9704,7 +9757,10 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp)
> gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg,
> 32, "power-vsx.xml", 0);
> }
> -
> +#ifndef CONFIG_USER_ONLY
> + gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg,
> + pcc->gdb_num_sprs, "power-spr.xml", 0);
> +#endif
> qemu_init_vcpu(cs);
>
> pcc->parent_realize(dev, errp);
> @@ -10467,7 +10523,9 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
> #endif
>
> cc->gdb_num_core_regs = 71;
> -
> +#ifndef CONFIG_USER_ONLY
> + cc->gdb_get_dynamic_xml = ppc_gdb_get_dynamic_xml;
> +#endif
> #ifdef USE_APPLE_GDB
> cc->gdb_read_register = ppc_cpu_gdb_read_register_apple;
> cc->gdb_write_register = ppc_cpu_gdb_write_register_apple;
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
prev parent reply other threads:[~2019-02-08 6:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-06 16:51 [Qemu-devel] [PATCH v5 0/1] ppc/gdbstub: Expose SPRs to GDB Fabiano Rosas
2019-02-06 16:51 ` [Qemu-devel] [PATCH v5 1/1] target/ppc: Enable reporting of " Fabiano Rosas
2019-02-08 5:20 ` David Gibson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190208052027.GA6434@umbus.fritz.box \
--to=david@gibson.dropbear.id.au \
--cc=aik@ozlabs.ru \
--cc=farosas@linux.ibm.com \
--cc=groug@kaod.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).