* [PATCH v3 0/2] target/ppc: Fix vector registers access in gdbstub for little-endian
@ 2021-08-26 14:56 matheus.ferst
2021-08-26 14:56 ` [PATCH v3 1/2] include/qemu/int128.h: introduce bswap128s matheus.ferst
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: matheus.ferst @ 2021-08-26 14:56 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: peter.maydell, philmd, richard.henderson, groug, Matheus Ferst, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
PPC gdbstub code has two possible swaps of the 64-bit elements of AVR
registers: in gdb_get_avr_reg/gdb_set_avr_reg (based on msr_le) and in
gdb_get_reg128/ldq_p (based on TARGET_WORDS_BIGENDIAN).
In softmmu, only the first is done, because TARGET_WORDS_BIGENDIAN is
always true. In user mode, both are being done, resulting in swapped
high and low doublewords of AVR registers in little-endian binaries.
We fix this by moving the first swap to ppc_maybe_bswap_register, which
already handles the endianness swap of each element's value in softmmu
and does nothing in user mode.
Based-on: <20210826141446.2488609-1-matheus.ferst@eldorado.org.br>
Matheus Ferst (2):
include/qemu/int128.h: introduce bswap128s
target/ppc: fix vector registers access in gdbstub for little-endian
include/qemu/int128.h | 17 ++++++++++++++++-
target/ppc/gdbstub.c | 32 +++++++-------------------------
2 files changed, 23 insertions(+), 26 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] include/qemu/int128.h: introduce bswap128s
2021-08-26 14:56 [PATCH v3 0/2] target/ppc: Fix vector registers access in gdbstub for little-endian matheus.ferst
@ 2021-08-26 14:56 ` matheus.ferst
2021-08-26 14:56 ` [PATCH v3 2/2] target/ppc: fix vector registers access in gdbstub for little-endian matheus.ferst
2021-08-27 2:43 ` [PATCH v3 0/2] target/ppc: Fix " David Gibson
2 siblings, 0 replies; 5+ messages in thread
From: matheus.ferst @ 2021-08-26 14:56 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: peter.maydell, philmd, richard.henderson, groug, Matheus Ferst, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
Changes the current bswap128 implementation to use __builtin_bswap128
when available, adds a bswap128 implementation for !CONFIG_INT128
builds, and introduces bswap128s based on bswap128.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
include/qemu/int128.h | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index e36c6e6db5..aa7a532727 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -1,9 +1,9 @@
#ifndef INT128_H
#define INT128_H
-#ifdef CONFIG_INT128
#include "qemu/bswap.h"
+#ifdef CONFIG_INT128
typedef __int128_t Int128;
static inline Int128 int128_make64(uint64_t a)
@@ -155,7 +155,11 @@ static inline void int128_subfrom(Int128 *a, Int128 b)
static inline Int128 bswap128(Int128 a)
{
+#if __has_builtin(__builtin_bswap128)
+ return __builtin_bswap128(a);
+#else
return int128_make128(bswap64(int128_gethi(a)), bswap64(int128_getlo(a)));
+#endif
}
#else /* !CONFIG_INT128 */
@@ -342,5 +346,16 @@ static inline void int128_subfrom(Int128 *a, Int128 b)
*a = int128_sub(*a, b);
}
+static inline Int128 bswap128(Int128 a)
+{
+ return int128_make128(bswap64(a.hi), bswap64(a.lo));
+}
+
#endif /* CONFIG_INT128 */
+
+static inline void bswap128s(Int128 *s)
+{
+ *s = bswap128(*s);
+}
+
#endif /* INT128_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] target/ppc: fix vector registers access in gdbstub for little-endian
2021-08-26 14:56 [PATCH v3 0/2] target/ppc: Fix vector registers access in gdbstub for little-endian matheus.ferst
2021-08-26 14:56 ` [PATCH v3 1/2] include/qemu/int128.h: introduce bswap128s matheus.ferst
@ 2021-08-26 14:56 ` matheus.ferst
2021-08-26 15:17 ` Philippe Mathieu-Daudé
2021-08-27 2:43 ` [PATCH v3 0/2] target/ppc: Fix " David Gibson
2 siblings, 1 reply; 5+ messages in thread
From: matheus.ferst @ 2021-08-26 14:56 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: peter.maydell, philmd, richard.henderson, groug, Matheus Ferst, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
As vector registers are stored in host endianness, we shouldn't swap its
64-bit elements in user mode. Add a 16-byte case in
ppc_maybe_bswap_register to handle the reordering of elements in softmmu
and remove avr_need_swap which is now unused.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
The fix of the order of Int128 fields is in the based-on patchset.
---
target/ppc/gdbstub.c | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 09ff1328d4..1808a150e4 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -101,6 +101,8 @@ void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len)
bswap32s((uint32_t *)mem_buf);
} else if (len == 8) {
bswap64s((uint64_t *)mem_buf);
+ } else if (len == 16) {
+ bswap128s((Int128 *)mem_buf);
} else {
g_assert_not_reached();
}
@@ -389,15 +391,6 @@ const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name)
}
#endif
-static bool avr_need_swap(CPUPPCState *env)
-{
-#ifdef HOST_WORDS_BIGENDIAN
- return msr_le;
-#else
- return !msr_le;
-#endif
-}
-
#if !defined(CONFIG_USER_ONLY)
static int gdb_find_spr_idx(CPUPPCState *env, int n)
{
@@ -486,14 +479,9 @@ static int gdb_get_avr_reg(CPUPPCState *env, GByteArray *buf, int n)
if (n < 32) {
ppc_avr_t *avr = cpu_avr_ptr(env, n);
- if (!avr_need_swap(env)) {
- gdb_get_reg128(buf, avr->u64[0] , avr->u64[1]);
- } else {
- gdb_get_reg128(buf, avr->u64[1] , avr->u64[0]);
- }
+ gdb_get_reg128(buf, avr->VsrD(0), avr->VsrD(1));
mem_buf = gdb_get_reg_ptr(buf, 16);
- ppc_maybe_bswap_register(env, mem_buf, 8);
- ppc_maybe_bswap_register(env, mem_buf + 8, 8);
+ ppc_maybe_bswap_register(env, mem_buf, 16);
return 16;
}
if (n == 32) {
@@ -515,15 +503,9 @@ static int gdb_set_avr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
{
if (n < 32) {
ppc_avr_t *avr = cpu_avr_ptr(env, n);
- ppc_maybe_bswap_register(env, mem_buf, 8);
- ppc_maybe_bswap_register(env, mem_buf + 8, 8);
- if (!avr_need_swap(env)) {
- avr->u64[0] = ldq_p(mem_buf);
- avr->u64[1] = ldq_p(mem_buf + 8);
- } else {
- avr->u64[1] = ldq_p(mem_buf);
- avr->u64[0] = ldq_p(mem_buf + 8);
- }
+ ppc_maybe_bswap_register(env, mem_buf, 16);
+ avr->VsrD(0) = ldq_p(mem_buf);
+ avr->VsrD(1) = ldq_p(mem_buf + 8);
return 16;
}
if (n == 32) {
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] target/ppc: fix vector registers access in gdbstub for little-endian
2021-08-26 14:56 ` [PATCH v3 2/2] target/ppc: fix vector registers access in gdbstub for little-endian matheus.ferst
@ 2021-08-26 15:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-26 15:17 UTC (permalink / raw)
To: matheus.ferst, qemu-devel, qemu-ppc
Cc: peter.maydell, richard.henderson, groug, david
On 8/26/21 4:56 PM, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> As vector registers are stored in host endianness, we shouldn't swap its
> 64-bit elements in user mode. Add a 16-byte case in
> ppc_maybe_bswap_register to handle the reordering of elements in softmmu
> and remove avr_need_swap which is now unused.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> The fix of the order of Int128 fields is in the based-on patchset.
> ---
> target/ppc/gdbstub.c | 32 +++++++-------------------------
> 1 file changed, 7 insertions(+), 25 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 0/2] target/ppc: Fix vector registers access in gdbstub for little-endian
2021-08-26 14:56 [PATCH v3 0/2] target/ppc: Fix vector registers access in gdbstub for little-endian matheus.ferst
2021-08-26 14:56 ` [PATCH v3 1/2] include/qemu/int128.h: introduce bswap128s matheus.ferst
2021-08-26 14:56 ` [PATCH v3 2/2] target/ppc: fix vector registers access in gdbstub for little-endian matheus.ferst
@ 2021-08-27 2:43 ` David Gibson
2 siblings, 0 replies; 5+ messages in thread
From: David Gibson @ 2021-08-27 2:43 UTC (permalink / raw)
To: matheus.ferst
Cc: peter.maydell, richard.henderson, qemu-devel, groug, qemu-ppc, philmd
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]
On Thu, Aug 26, 2021 at 11:56:54AM -0300, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> PPC gdbstub code has two possible swaps of the 64-bit elements of AVR
> registers: in gdb_get_avr_reg/gdb_set_avr_reg (based on msr_le) and in
> gdb_get_reg128/ldq_p (based on TARGET_WORDS_BIGENDIAN).
>
> In softmmu, only the first is done, because TARGET_WORDS_BIGENDIAN is
> always true. In user mode, both are being done, resulting in swapped
> high and low doublewords of AVR registers in little-endian binaries.
>
> We fix this by moving the first swap to ppc_maybe_bswap_register, which
> already handles the endianness swap of each element's value in softmmu
> and does nothing in user mode.
Applied to ppc-for-6.2, thanks.
>
> Based-on: <20210826141446.2488609-1-matheus.ferst@eldorado.org.br>
>
> Matheus Ferst (2):
> include/qemu/int128.h: introduce bswap128s
> target/ppc: fix vector registers access in gdbstub for little-endian
>
> include/qemu/int128.h | 17 ++++++++++++++++-
> target/ppc/gdbstub.c | 32 +++++++-------------------------
> 2 files changed, 23 insertions(+), 26 deletions(-)
>
--
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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-27 2:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-26 14:56 [PATCH v3 0/2] target/ppc: Fix vector registers access in gdbstub for little-endian matheus.ferst
2021-08-26 14:56 ` [PATCH v3 1/2] include/qemu/int128.h: introduce bswap128s matheus.ferst
2021-08-26 14:56 ` [PATCH v3 2/2] target/ppc: fix vector registers access in gdbstub for little-endian matheus.ferst
2021-08-26 15:17 ` Philippe Mathieu-Daudé
2021-08-27 2:43 ` [PATCH v3 0/2] target/ppc: Fix " David Gibson
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).