* [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
@ 2014-01-14 21:47 Thomas Falcon
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Falcon @ 2014-01-14 21:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Falcon, qemu-ppc, agraf
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 only if the MSR:LE value is set and if the host machine
is big endian.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
target-ppc/gdbstub.c | 50 ++++++++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
index 1c91090..eba501a 100644
--- a/target-ppc/gdbstub.c
+++ b/target-ppc/gdbstub.c
@@ -21,6 +21,19 @@
#include "qemu-common.h"
#include "exec/gdbstub.h"
+/* The following macros 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
+ */
+
+#define end_swap64(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap64(x) : x
+#define end_swap32(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap32(x) : x
+#if TARGET_LONG_BITS == 64
+#define end_swapl(x) end_swap64(x)
+#else
+#define end_swapl(x) end_swap32(x)
+#endif
+
/* 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
@@ -35,20 +48,20 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
if (n < 32) {
/* gprs */
- return gdb_get_regl(mem_buf, env->gpr[n]);
+ return gdb_get_regl(mem_buf, end_swapl(env->gpr[n]));
} else if (n < 64) {
/* fprs */
if (gdb_has_xml) {
return 0;
}
- stfq_p(mem_buf, env->fpr[n-32]);
+ stfq_p(mem_buf, end_swapl(env->fpr[n-32]));
return 8;
} else {
switch (n) {
case 64:
- return gdb_get_regl(mem_buf, env->nip);
+ return gdb_get_regl(mem_buf, end_swapl(env->nip));
case 65:
- return gdb_get_regl(mem_buf, env->msr);
+ return gdb_get_regl(mem_buf, end_swapl(env->msr));
case 66:
{
uint32_t cr = 0;
@@ -56,20 +69,20 @@ 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);
+ return gdb_get_reg32(mem_buf, end_swap32(cr));
}
case 67:
- return gdb_get_regl(mem_buf, env->lr);
+ return gdb_get_regl(mem_buf, end_swapl(env->lr));
case 68:
- return gdb_get_regl(mem_buf, env->ctr);
+ return gdb_get_regl(mem_buf, end_swapl(env->ctr));
case 69:
- return gdb_get_regl(mem_buf, env->xer);
+ return gdb_get_regl(mem_buf, end_swapl(env->xer));
case 70:
{
if (gdb_has_xml) {
return 0;
}
- return gdb_get_reg32(mem_buf, env->fpscr);
+ return gdb_get_reg32(mem_buf, end_swap32(env->fpscr));
}
}
}
@@ -83,47 +96,48 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
if (n < 32) {
/* gprs */
- env->gpr[n] = ldtul_p(mem_buf);
+ env->gpr[n] = end_swapl(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);
+ env->fpr[n-32] = end_swapl(ldfq_p(mem_buf));
return 8;
} else {
switch (n) {
case 64:
- env->nip = ldtul_p(mem_buf);
+ env->nip = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 65:
- ppc_store_msr(env, ldtul_p(mem_buf));
+ ppc_store_msr(env, end_swapl(ldtul_p(mem_buf)));
return sizeof(target_ulong);
case 66:
{
uint32_t cr = ldl_p(mem_buf);
int i;
for (i = 0; i < 8; i++) {
- env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF;
+ env->crf[i] = end_swap32((cr >> (32 -
+ ((i + 1) * 4))) & 0xF);
}
return 4;
}
case 67:
- env->lr = ldtul_p(mem_buf);
+ env->lr = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 68:
- env->ctr = ldtul_p(mem_buf);
+ env->ctr = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 69:
- env->xer = ldtul_p(mem_buf);
+ env->xer = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 70:
/* fpscr */
if (gdb_has_xml) {
return 0;
}
- store_fpscr(env, ldtul_p(mem_buf), 0xffffffff);
+ store_fpscr(env, end_swapl(ldtul_p(mem_buf)), 0xffffffff);
return sizeof(target_ulong);
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
@ 2014-01-14 22:06 Thomas Falcon
2014-01-14 22:20 ` Peter Maydell
2014-01-14 22:40 ` Alexander Graf
0 siblings, 2 replies; 9+ messages in thread
From: Thomas Falcon @ 2014-01-14 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, agraf
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 only if the MSR:LE value is set and if the host machine
is big endian.
Signed-off-by: Thomas Falcon<tlfalcon@linux.vnet.ibm.com>
---
target-ppc/gdbstub.c | 50 ++++++++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
index 1c91090..eba501a 100644
--- a/target-ppc/gdbstub.c
+++ b/target-ppc/gdbstub.c
@@ -21,6 +21,19 @@
#include "qemu-common.h"
#include "exec/gdbstub.h"
+/* The following macros 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
+ */
+
+#define end_swap64(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap64(x) : x
+#define end_swap32(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap32(x) : x
+#if TARGET_LONG_BITS == 64
+#define end_swapl(x) end_swap64(x)
+#else
+#define end_swapl(x) end_swap32(x)
+#endif
+
/* 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
@@ -35,20 +48,20 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
if (n < 32) {
/* gprs */
- return gdb_get_regl(mem_buf, env->gpr[n]);
+ return gdb_get_regl(mem_buf, end_swapl(env->gpr[n]));
} else if (n < 64) {
/* fprs */
if (gdb_has_xml) {
return 0;
}
- stfq_p(mem_buf, env->fpr[n-32]);
+ stfq_p(mem_buf, end_swapl(env->fpr[n-32]));
return 8;
} else {
switch (n) {
case 64:
- return gdb_get_regl(mem_buf, env->nip);
+ return gdb_get_regl(mem_buf, end_swapl(env->nip));
case 65:
- return gdb_get_regl(mem_buf, env->msr);
+ return gdb_get_regl(mem_buf, end_swapl(env->msr));
case 66:
{
uint32_t cr = 0;
@@ -56,20 +69,20 @@ 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);
+ return gdb_get_reg32(mem_buf, end_swap32(cr));
}
case 67:
- return gdb_get_regl(mem_buf, env->lr);
+ return gdb_get_regl(mem_buf, end_swapl(env->lr));
case 68:
- return gdb_get_regl(mem_buf, env->ctr);
+ return gdb_get_regl(mem_buf, end_swapl(env->ctr));
case 69:
- return gdb_get_regl(mem_buf, env->xer);
+ return gdb_get_regl(mem_buf, end_swapl(env->xer));
case 70:
{
if (gdb_has_xml) {
return 0;
}
- return gdb_get_reg32(mem_buf, env->fpscr);
+ return gdb_get_reg32(mem_buf, end_swap32(env->fpscr));
}
}
}
@@ -83,47 +96,48 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
if (n < 32) {
/* gprs */
- env->gpr[n] = ldtul_p(mem_buf);
+ env->gpr[n] = end_swapl(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);
+ env->fpr[n-32] = end_swapl(ldfq_p(mem_buf));
return 8;
} else {
switch (n) {
case 64:
- env->nip = ldtul_p(mem_buf);
+ env->nip = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 65:
- ppc_store_msr(env, ldtul_p(mem_buf));
+ ppc_store_msr(env, end_swapl(ldtul_p(mem_buf)));
return sizeof(target_ulong);
case 66:
{
uint32_t cr = ldl_p(mem_buf);
int i;
for (i = 0; i < 8; i++) {
- env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF;
+ env->crf[i] = end_swap32((cr >> (32 -
+ ((i + 1) * 4))) & 0xF);
}
return 4;
}
case 67:
- env->lr = ldtul_p(mem_buf);
+ env->lr = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 68:
- env->ctr = ldtul_p(mem_buf);
+ env->ctr = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 69:
- env->xer = ldtul_p(mem_buf);
+ env->xer = end_swapl(ldtul_p(mem_buf));
return sizeof(target_ulong);
case 70:
/* fpscr */
if (gdb_has_xml) {
return 0;
}
- store_fpscr(env, ldtul_p(mem_buf), 0xffffffff);
+ store_fpscr(env, end_swapl(ldtul_p(mem_buf)), 0xffffffff);
return sizeof(target_ulong);
}
}
-- 1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
2014-01-14 22:06 Thomas Falcon
@ 2014-01-14 22:20 ` Peter Maydell
2014-01-14 22:40 ` Alexander Graf
1 sibling, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2014-01-14 22:20 UTC (permalink / raw)
To: Thomas Falcon; +Cc: qemu-ppc@nongnu.org, QEMU Developers, Alexander Graf
On 14 January 2014 22:06, Thomas Falcon <tlfalcon@linux.vnet.ibm.com> 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 only if the MSR:LE value is set and if the host machine
> is big endian.
Since this patch only affects ppc targets it would be helpful if the
subject line had an appropriate prefix indicating that.
>
> Signed-off-by: Thomas Falcon<tlfalcon@linux.vnet.ibm.com>
> ---
> target-ppc/gdbstub.c | 50
> ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
> index 1c91090..eba501a 100644
> --- a/target-ppc/gdbstub.c
> +++ b/target-ppc/gdbstub.c
> @@ -21,6 +21,19 @@
> #include "qemu-common.h"
> #include "exec/gdbstub.h"
>
> +/* The following macros 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
> + */
> +
> +#define end_swap64(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap64(x) : x
> +#define end_swap32(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap32(x) : x
Surely we need to swap if the host is little endian and the target
is bigendian, as well as if the host is bigendian and the target
little endian?
Also, it seems a bit dubious to switch the endianness of words
based on a runtime flag in the guest CPU -- I'm pretty sure a
connected gdb won't be able to cope with that. On the other
hand, gdb's pretty bad at dealing with the kind of thing real
CPUs can do with switching endianness or word size at run
time, so maybe this is just better than the alternatives...
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
2014-01-14 22:06 Thomas Falcon
2014-01-14 22:20 ` Peter Maydell
@ 2014-01-14 22:40 ` Alexander Graf
2014-01-14 22:55 ` Peter Maydell
2014-01-15 17:29 ` Ulrich Weigand
1 sibling, 2 replies; 9+ messages in thread
From: Alexander Graf @ 2014-01-14 22:40 UTC (permalink / raw)
To: Thomas Falcon; +Cc: Ulrich Weigand, qemu-ppc, QEMU Developers
On 14.01.2014, at 23:06, Thomas Falcon <tlfalcon@linux.vnet.ibm.com> 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 only if the MSR:LE value is set and if the host machine
> is big endian.
>
> Signed-off-by: Thomas Falcon<tlfalcon@linux.vnet.ibm.com>
Uli, I thought ppc64le gdb wasn't finalized yet? What does the gdbstub layout look like? Are all fields the same as ppc64(be) but simply byte swapped - including FPR ones?
Alex
> ---
> target-ppc/gdbstub.c | 50 ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
> index 1c91090..eba501a 100644
> --- a/target-ppc/gdbstub.c
> +++ b/target-ppc/gdbstub.c
> @@ -21,6 +21,19 @@
> #include "qemu-common.h"
> #include "exec/gdbstub.h"
>
> +/* The following macros 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
> + */
> +
> +#define end_swap64(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap64(x) : x
> +#define end_swap32(x) (msr_le && HOST_WORDS_BIGENDIAN) ? bswap32(x) : x
> +#if TARGET_LONG_BITS == 64
> +#define end_swapl(x) end_swap64(x)
> +#else
> +#define end_swapl(x) end_swap32(x)
> +#endif
> +
> /* 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
> @@ -35,20 +48,20 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
>
> if (n < 32) {
> /* gprs */
> - return gdb_get_regl(mem_buf, env->gpr[n]);
> + return gdb_get_regl(mem_buf, end_swapl(env->gpr[n]));
This is quite invasive (and prone to get wrong). If we really just have to swap every single register by its size (which we yet have to confirm with Uli) why don't we just wrap this function by another one that takes the return value of ppc_cpu_gdb_read_register (the integer size) and swaps it in-place in mem_buf? At least we're 100% consistent that way.
Unless of course we only need to swap half of the registers, then it makes more sense the way you do it now.
Alex
> } else if (n < 64) {
> /* fprs */
> if (gdb_has_xml) {
> return 0;
> }
> - stfq_p(mem_buf, env->fpr[n-32]);
> + stfq_p(mem_buf, end_swapl(env->fpr[n-32]));
> return 8;
> } else {
> switch (n) {
> case 64:
> - return gdb_get_regl(mem_buf, env->nip);
> + return gdb_get_regl(mem_buf, end_swapl(env->nip));
> case 65:
> - return gdb_get_regl(mem_buf, env->msr);
> + return gdb_get_regl(mem_buf, end_swapl(env->msr));
> case 66:
> {
> uint32_t cr = 0;
> @@ -56,20 +69,20 @@ 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);
> + return gdb_get_reg32(mem_buf, end_swap32(cr));
> }
> case 67:
> - return gdb_get_regl(mem_buf, env->lr);
> + return gdb_get_regl(mem_buf, end_swapl(env->lr));
> case 68:
> - return gdb_get_regl(mem_buf, env->ctr);
> + return gdb_get_regl(mem_buf, end_swapl(env->ctr));
> case 69:
> - return gdb_get_regl(mem_buf, env->xer);
> + return gdb_get_regl(mem_buf, end_swapl(env->xer));
> case 70:
> {
> if (gdb_has_xml) {
> return 0;
> }
> - return gdb_get_reg32(mem_buf, env->fpscr);
> + return gdb_get_reg32(mem_buf, end_swap32(env->fpscr));
> }
> }
> }
> @@ -83,47 +96,48 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>
> if (n < 32) {
> /* gprs */
> - env->gpr[n] = ldtul_p(mem_buf);
> + env->gpr[n] = end_swapl(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);
> + env->fpr[n-32] = end_swapl(ldfq_p(mem_buf));
> return 8;
> } else {
> switch (n) {
> case 64:
> - env->nip = ldtul_p(mem_buf);
> + env->nip = end_swapl(ldtul_p(mem_buf));
> return sizeof(target_ulong);
> case 65:
> - ppc_store_msr(env, ldtul_p(mem_buf));
> + ppc_store_msr(env, end_swapl(ldtul_p(mem_buf)));
> return sizeof(target_ulong);
> case 66:
> {
> uint32_t cr = ldl_p(mem_buf);
> int i;
> for (i = 0; i < 8; i++) {
> - env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF;
> + env->crf[i] = end_swap32((cr >> (32 -
> + ((i + 1) * 4))) & 0xF);
> }
> return 4;
> }
> case 67:
> - env->lr = ldtul_p(mem_buf);
> + env->lr = end_swapl(ldtul_p(mem_buf));
> return sizeof(target_ulong);
> case 68:
> - env->ctr = ldtul_p(mem_buf);
> + env->ctr = end_swapl(ldtul_p(mem_buf));
> return sizeof(target_ulong);
> case 69:
> - env->xer = ldtul_p(mem_buf);
> + env->xer = end_swapl(ldtul_p(mem_buf));
> return sizeof(target_ulong);
> case 70:
> /* fpscr */
> if (gdb_has_xml) {
> return 0;
> }
> - store_fpscr(env, ldtul_p(mem_buf), 0xffffffff);
> + store_fpscr(env, end_swapl(ldtul_p(mem_buf)), 0xffffffff);
> return sizeof(target_ulong);
> }
> }
> -- 1.8.3.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
2014-01-14 22:40 ` Alexander Graf
@ 2014-01-14 22:55 ` Peter Maydell
2014-01-14 23:01 ` Alexander Graf
2014-01-15 17:29 ` Ulrich Weigand
1 sibling, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2014-01-14 22:55 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, Ulrich Weigand, Thomas Falcon, QEMU Developers
On 14 January 2014 22:40, Alexander Graf <agraf@suse.de> wrote:
> Uli, I thought ppc64le gdb wasn't finalized yet? What does the gdbstub
> layout look like? Are all fields the same as ppc64(be) but simply byte
> swapped - including FPR ones?
> This is quite invasive (and prone to get wrong). If we really just have
> to swap every single register by its size (which we yet have to confirm
> with Uli) why don't we just wrap this function by another one that takes
> the return value of ppc_cpu_gdb_read_register (the integer size) and
> swaps it in-place in mem_buf? At least we're 100% consistent that way.
Note that we already support "fields in the buffer are in target byte order"
(ie matching TARGET_WORDS_BIGENDIAN) with gdb_get_reg*,
"fields are always LE" (use st*_le_p()) and "fields are always BE"
(use st*_be_p()).
Is the underlying issue here that we might have a CPU which is
in littleendian mode but in a QEMU executable compiled with
TARGET_WORDS_BIGENDIAN ? (If so I can't help feeling that
the gdb stub is only the tip of the iceberg for things that might need
attention...)
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
2014-01-14 22:55 ` Peter Maydell
@ 2014-01-14 23:01 ` Alexander Graf
2014-01-14 23:06 ` Peter Maydell
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2014-01-14 23:01 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-ppc, Ulrich Weigand, Thomas Falcon, QEMU Developers
On 14.01.2014, at 23:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 14 January 2014 22:40, Alexander Graf <agraf@suse.de> wrote:
>> Uli, I thought ppc64le gdb wasn't finalized yet? What does the gdbstub
>> layout look like? Are all fields the same as ppc64(be) but simply byte
>> swapped - including FPR ones?
>
>> This is quite invasive (and prone to get wrong). If we really just have
>> to swap every single register by its size (which we yet have to confirm
>> with Uli) why don't we just wrap this function by another one that takes
>> the return value of ppc_cpu_gdb_read_register (the integer size) and
>> swaps it in-place in mem_buf? At least we're 100% consistent that way.
>
> Note that we already support "fields in the buffer are in target byte order"
> (ie matching TARGET_WORDS_BIGENDIAN) with gdb_get_reg*,
> "fields are always LE" (use st*_le_p()) and "fields are always BE"
> (use st*_be_p()).
>
> Is the underlying issue here that we might have a CPU which is
> in littleendian mode but in a QEMU executable compiled with
> TARGET_WORDS_BIGENDIAN ? (If so I can't help feeling that
> the gdb stub is only the tip of the iceberg for things that might need
> attention...)
Yes, which is going to be the same problem you have for AArch64 :). LE vs BE is really just a register flip. The qemu binary is the same for both when you run system emulation mode.
Alex
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
2014-01-14 23:01 ` Alexander Graf
@ 2014-01-14 23:06 ` Peter Maydell
0 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2014-01-14 23:06 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, Ulrich Weigand, Thomas Falcon, QEMU Developers
On 14 January 2014 23:01, Alexander Graf <agraf@suse.de> wrote:
>> Is the underlying issue here that we might have a CPU which is
>> in littleendian mode but in a QEMU executable compiled with
>> TARGET_WORDS_BIGENDIAN ? (If so I can't help feeling that
>> the gdb stub is only the tip of the iceberg for things that might need
>> attention...)
>
> Yes, which is going to be the same problem you have for AArch64 :).
> LE vs BE is really just a register flip. The qemu binary is the same
> for both when you run system emulation mode.
Right, then I think we really need to look at the issue in a more
holistic fashion. You're essentially trying to make
TARGET_WORDS_BIGENDIAN irrelevant, which is a goal I
thoroughly approve of, but it's pretty thoroughly baked into various
parts of the codebase and I don't think it's going to be easy to
eradicate.
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
2014-01-14 22:40 ` Alexander Graf
2014-01-14 22:55 ` Peter Maydell
@ 2014-01-15 17:29 ` Ulrich Weigand
2014-01-15 17:40 ` Alexander Graf
1 sibling, 1 reply; 9+ messages in thread
From: Ulrich Weigand @ 2014-01-15 17:29 UTC (permalink / raw)
To: Alexander Graf; +Cc: Thomas Falcon, qemu-ppc, QEMU Developers
Alexander Graf <agraf@suse.de> wrote on 14.01.2014 23:40:20:
> On 14.01.2014, at 23:06, Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
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 only if the MSR:LE value is set and if the host machine
> > is big endian.
> >
> > Signed-off-by: Thomas Falcon<tlfalcon@linux.vnet.ibm.com>
>
> Uli, I thought ppc64le gdb wasn't finalized yet? What does the
> gdbstub layout look like? Are all fields the same as ppc64(be) but
> simply byte swapped - including FPR ones?
Hi Alex, yes, the layout of the gdb protocol packets should be the
same as on BE, except that all registers are byte-swapped. This
includes FPRs and VPRs.
Bye,
Ulrich
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers
2014-01-15 17:29 ` Ulrich Weigand
@ 2014-01-15 17:40 ` Alexander Graf
0 siblings, 0 replies; 9+ messages in thread
From: Alexander Graf @ 2014-01-15 17:40 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: Thomas Falcon, qemu-ppc, QEMU Developers
On 15.01.2014, at 18:29, Ulrich Weigand <Ulrich.Weigand@de.ibm.com> wrote:
> Alexander Graf <agraf@suse.de> wrote on 14.01.2014 23:40:20:
>> On 14.01.2014, at 23:06, Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> 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 only if the MSR:LE value is set and if the host machine
>>> is big endian.
>>>
>>> Signed-off-by: Thomas Falcon<tlfalcon@linux.vnet.ibm.com>
>>
>> Uli, I thought ppc64le gdb wasn't finalized yet? What does the
>> gdbstub layout look like? Are all fields the same as ppc64(be) but
>> simply byte swapped - including FPR ones?
>
> Hi Alex, yes, the layout of the gdb protocol packets should be the
> same as on BE, except that all registers are byte-swapped. This
> includes FPRs and VPRs.
Phew - I don't think we have byte swapping primitives above 64bit :).
But good, then let's abstract this whole swappiness one level higher as indicated in my previous email. That way we're guaranteed to not miss any fields by accident.
Alex
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-01-15 17:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-14 21:47 [Qemu-devel] [PATCH] gdbstub: allow byte swapping for reading/writing registers Thomas Falcon
-- strict thread matches above, loose matches on Subject: below --
2014-01-14 22:06 Thomas Falcon
2014-01-14 22:20 ` Peter Maydell
2014-01-14 22:40 ` Alexander Graf
2014-01-14 22:55 ` Peter Maydell
2014-01-14 23:01 ` Alexander Graf
2014-01-14 23:06 ` Peter Maydell
2014-01-15 17:29 ` Ulrich Weigand
2014-01-15 17:40 ` Alexander Graf
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).