From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org, Alexander Graf <agraf@suse.de>
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v6] target-ppc: gdbstub allow byte swapping for reading/writing registers
Date: Mon, 10 Mar 2014 09:50:01 -0500 [thread overview]
Message-ID: <531DD119.4040107@linux.vnet.ibm.com> (raw)
In-Reply-To: <5315F1BD.5090700@linux.vnet.ibm.com>
On 03/04/2014 09:31 AM, Thomas Falcon wrote:
> On 02/28/2014 03:32 PM, Thomas Falcon wrote:
>
>> This patch allows registers to be properly read from and written to
>> when using the gdbstub to debug a ppc guest running in little
>> endian mode. It accomplishes this goal by byte swapping the values of
>> any registers if the MSR:LE value is set.
>>
>> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
>> ---
>> Differences from v5:
>>
>> ppc_cpu_gdb_swap_register uses built in byte swapping functions
>> ---
>> target-ppc/cpu-qom.h | 3 +
>> target-ppc/gdbstub.c | 139
>> +++++++++++++++++++++++++++++++++++---------
>> target-ppc/translate_init.c | 4 +-
>> 3 files changed, 115 insertions(+), 31 deletions(-)
>>
>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>> index 72b2232..b85133c 100644
>> --- a/target-ppc/cpu-qom.h
>> +++ b/target-ppc/cpu-qom.h
>> @@ -107,9 +107,12 @@ void ppc_cpu_dump_state(CPUState *cpu, FILE *f,
>> fprintf_function cpu_fprintf,
>> int flags);
>> void ppc_cpu_dump_statistics(CPUState *cpu, FILE *f,
>> fprintf_function cpu_fprintf, int flags);
>> +void ppc_cpu_gdb_swap_register(uint8_t *buf, int reg);
>> hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>> int ppc_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>> +int ppc_cpu_gdb_read_register_wrap(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_wrap(CPUState *cpu, uint8_t *buf, int
>> reg);
>> int ppc64_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
>> CPUState *cpu, void *opaque);
>> int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
>> diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
>> index 1c91090..92649ed 100644
>> --- a/target-ppc/gdbstub.c
>> +++ b/target-ppc/gdbstub.c
>> @@ -21,6 +21,82 @@
>> #include "qemu-common.h"
>> #include "exec/gdbstub.h"
>>
>> +static int ppc_cpu_gdb_register_len(int n)
>> +{
>> + switch (n) {
>> + case 0 ... 31:
>> + /* gprs */
>> + return sizeof(target_ulong);
>> + case 32 ... 63:
>> + /* fprs */
>> + if (gdb_has_xml) {
>> + return 0;
>> + }
>> + return 8;
>> + case 66:
>> + /* cr */
>> + return 4;
>> + case 64:
>> + /* nip */
>> + case 65:
>> + /* msr */
>> + case 67:
>> + /* lr */
>> + case 68:
>> + /* ctr */
>> + case 69:
>> + /* xer */
>> + return sizeof(target_ulong);
>> + case 70:
>> + /* fpscr */
>> + if (gdb_has_xml) {
>> + return 0;
>> + }
>> + return sizeof(target_ulong);
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +
>> +/* The following functions are used to ensure the correct
>> + * transfer of registers between a little endian ppc target
>> + * and a big endian host by checking the LE bit in the Machine State
>> Register
>> + */
>> +
>> +void ppc_cpu_gdb_swap_register(uint8_t *mem_buf, int n)
>> +{
>> + int len = ppc_cpu_gdb_register_len(n);
>> + if (len == 4) {
>> + bswap32s((uint32_t *)mem_buf);
>> + } else {
>> + bswap64s((uint64_t *)mem_buf);
>> + }
>> +}
>> +
>> +int ppc_cpu_gdb_read_register_wrap(CPUState *cs, uint8_t *mem_buf,
>> int n)
>> +{
>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>> + CPUPPCState *env = &cpu->env;
>> +
>> + int len = ppc_cpu_gdb_read_register(cs, mem_buf, n);
>> + if (msr_le) {
>> + ppc_cpu_gdb_swap_register(mem_buf, n);
>> + }
>> + return len;
>> +}
>> +
>> +int ppc_cpu_gdb_write_register_wrap(CPUState *cs, uint8_t *mem_buf,
>> int n)
>> +{
>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>> + CPUPPCState *env = &cpu->env;
>> +
>> + if (msr_le) {
>> + ppc_cpu_gdb_swap_register(mem_buf, n);
>> + }
>> + return ppc_cpu_gdb_write_register(cs, mem_buf, n);
>> +}
>> +
>> /* Old gdb always expects FP registers. Newer (xml-aware) gdb only
>> * expects whatever the target description contains. Due to a
>> * historical mishap the FP registers appear in between core integer
>> @@ -32,23 +108,26 @@ int ppc_cpu_gdb_read_register(CPUState *cs,
>> uint8_t *mem_buf, int n)
>> {
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>> CPUPPCState *env = &cpu->env;
>> + int r = ppc_cpu_gdb_register_len(n);
>> +
>> + if (!r) {
>> + return r;
>> + }
>>
>> if (n < 32) {
>> /* gprs */
>> - return gdb_get_regl(mem_buf, env->gpr[n]);
>> + gdb_get_regl(mem_buf, env->gpr[n]);
>> } else if (n < 64) {
>> /* fprs */
>> - if (gdb_has_xml) {
>> - return 0;
>> - }
>> stfq_p(mem_buf, env->fpr[n-32]);
>> - return 8;
>> } else {
>> switch (n) {
>> case 64:
>> - return gdb_get_regl(mem_buf, env->nip);
>> + gdb_get_regl(mem_buf, env->nip);
>> + break;
>> case 65:
>> - return gdb_get_regl(mem_buf, env->msr);
>> + gdb_get_regl(mem_buf, env->msr);
>> + break;
>> case 66:
>> {
>> uint32_t cr = 0;
>> @@ -56,50 +135,55 @@ int ppc_cpu_gdb_read_register(CPUState *cs,
>> uint8_t *mem_buf, int n)
>> for (i = 0; i < 8; i++) {
>> cr |= env->crf[i] << (32 - ((i + 1) * 4));
>> }
>> - return gdb_get_reg32(mem_buf, cr);
>> + gdb_get_reg32(mem_buf, cr);
>> + break;
>> }
>> case 67:
>> - return gdb_get_regl(mem_buf, env->lr);
>> + gdb_get_regl(mem_buf, env->lr);
>> + break;
>> case 68:
>> - return gdb_get_regl(mem_buf, env->ctr);
>> + gdb_get_regl(mem_buf, env->ctr);
>> + break;
>> case 69:
>> - return gdb_get_regl(mem_buf, env->xer);
>> + gdb_get_regl(mem_buf, env->xer);
>> + break;
>> case 70:
>> {
>> if (gdb_has_xml) {
>> return 0;
>> }
>> - return gdb_get_reg32(mem_buf, env->fpscr);
>> + gdb_get_reg32(mem_buf, env->fpscr);
>> + break;
>> }
>> }
>> }
>> - return 0;
>> + return r;
>> }
>>
>> int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>> {
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>> CPUPPCState *env = &cpu->env;
>> + int r = ppc_cpu_gdb_register_len(n);
>> +
>> + if (!r) {
>> + return r;
>> + }
>>
>> if (n < 32) {
>> /* gprs */
>> env->gpr[n] = ldtul_p(mem_buf);
>> - return sizeof(target_ulong);
>> } else if (n < 64) {
>> /* fprs */
>> - if (gdb_has_xml) {
>> - return 0;
>> - }
>> env->fpr[n-32] = ldfq_p(mem_buf);
>> - return 8;
>> } else {
>> switch (n) {
>> case 64:
>> env->nip = ldtul_p(mem_buf);
>> - return sizeof(target_ulong);
>> + break;
>> case 65:
>> ppc_store_msr(env, ldtul_p(mem_buf));
>> - return sizeof(target_ulong);
>> + break;
>> case 66:
>> {
>> uint32_t cr = ldl_p(mem_buf);
>> @@ -107,25 +191,22 @@ int ppc_cpu_gdb_write_register(CPUState *cs,
>> uint8_t *mem_buf, int n)
>> for (i = 0; i < 8; i++) {
>> env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF;
>> }
>> - return 4;
>> + break;
>> }
>> case 67:
>> env->lr = ldtul_p(mem_buf);
>> - return sizeof(target_ulong);
>> + break;
>> case 68:
>> env->ctr = ldtul_p(mem_buf);
>> - return sizeof(target_ulong);
>> + break;
>> case 69:
>> env->xer = ldtul_p(mem_buf);
>> - return sizeof(target_ulong);
>> + break;
>> case 70:
>> /* fpscr */
>> - if (gdb_has_xml) {
>> - return 0;
>> - }
>> store_fpscr(env, ldtul_p(mem_buf), 0xffffffff);
>> - return sizeof(target_ulong);
>> + break;
>> }
>> }
>> - return 0;
>> + return r;
>> }
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 445c360..cf12ba7 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -8655,8 +8655,8 @@ static void ppc_cpu_class_init(ObjectClass *oc,
>> void *data)
>> cc->dump_state = ppc_cpu_dump_state;
>> cc->dump_statistics = ppc_cpu_dump_statistics;
>> cc->set_pc = ppc_cpu_set_pc;
>> - cc->gdb_read_register = ppc_cpu_gdb_read_register;
>> - cc->gdb_write_register = ppc_cpu_gdb_write_register;
>> + cc->gdb_read_register = ppc_cpu_gdb_read_register_wrap;
>> + cc->gdb_write_register = ppc_cpu_gdb_write_register_wrap;
>> #ifndef CONFIG_USER_ONLY
>> cc->get_phys_page_debug = ppc_cpu_get_phys_page_debug;
>> cc->vmsd = &vmstate_ppc_cpu;
>
> http://patchwork.ozlabs.org/patch/325352/
>
> Sent this out last week, just pinging it again. Comments, suggestions?
>
> Thanks,
>
> Tom
>
>
Just pinging this patch again.
next prev parent reply other threads:[~2014-03-10 14:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-28 21:32 [Qemu-devel] [PATCH v6] target-ppc: gdbstub allow byte swapping for reading/writing registers Thomas Falcon
2014-03-04 15:31 ` [Qemu-devel] [Qemu-ppc] " Thomas Falcon
2014-03-10 14:50 ` Thomas Falcon [this message]
2014-03-20 15:25 ` Thomas Falcon
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=531DD119.4040107@linux.vnet.ibm.com \
--to=tlfalcon@linux.vnet.ibm.com \
--cc=agraf@suse.de \
--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).