* [PATCH 0/4] Fix incorrect accesses to XER
@ 2021-10-14 22:32 matheus.ferst
2021-10-14 22:32 ` [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs matheus.ferst
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: matheus.ferst @ 2021-10-14 22:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: laurent, Matheus Ferst, richard.henderson, groug, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
This patch series fixes some uses of env->xer that were left behind when
SO, OV, and CA bits were separated into other fields of CPUPPCState. At
the time, all of them weren't in target-specific files/directories,
which probably made it harder to track what needed to be changed.
Currently, this still is the case for linux-user/elfload.c. Is there
any particular reason for that? I can try to provide a patch to split
it.
Matheus Ferst (4):
linux-user/ppc: Fix XER access in save/restore_user_regs
target/ppc: Fix XER access in gdbstub
linux-user: Fix XER access in ppc version of elf_core_copy_regs
target/ppc: Fix XER access in monitor
linux-user/elfload.c | 2 +-
linux-user/ppc/signal.c | 9 +++--
target/ppc/cpu.c | 2 +-
target/ppc/cpu.h | 2 +-
target/ppc/gdbstub.c | 8 ++--
target/ppc/monitor.c | 9 ++++-
tests/tcg/ppc64/Makefile.target | 2 +
tests/tcg/ppc64le/Makefile.target | 2 +
tests/tcg/ppc64le/signal_save_restore_xer.c | 42 +++++++++++++++++++++
9 files changed, 67 insertions(+), 11 deletions(-)
create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs
2021-10-14 22:32 [PATCH 0/4] Fix incorrect accesses to XER matheus.ferst
@ 2021-10-14 22:32 ` matheus.ferst
2021-10-14 23:43 ` Richard Henderson
2021-10-15 0:30 ` David Gibson
2021-10-14 22:32 ` [PATCH 2/4] target/ppc: Fix XER access in gdbstub matheus.ferst
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: matheus.ferst @ 2021-10-14 22:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: laurent, Matheus Ferst, richard.henderson, groug, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
We should use cpu_read_xer/cpu_write_xer to save/restore the complete
register since some of its bits are in other fields of CPUPPCState. A
test is added to prevent future regressions.
Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
linux-user/ppc/signal.c | 9 +++--
tests/tcg/ppc64/Makefile.target | 2 +
tests/tcg/ppc64le/Makefile.target | 2 +
tests/tcg/ppc64le/signal_save_restore_xer.c | 42 +++++++++++++++++++++
4 files changed, 52 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c
diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index edfad28a37..4413a4a28f 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -244,7 +244,7 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
__put_user(env->nip, &frame->mc_gregs[TARGET_PT_NIP]);
__put_user(env->ctr, &frame->mc_gregs[TARGET_PT_CTR]);
__put_user(env->lr, &frame->mc_gregs[TARGET_PT_LNK]);
- __put_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]);
+ __put_user(cpu_read_xer(env), &frame->mc_gregs[TARGET_PT_XER]);
for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
ccr |= env->crf[i] << (32 - ((i + 1) * 4));
@@ -319,6 +319,7 @@ static void restore_user_regs(CPUPPCState *env,
{
target_ulong save_r2 = 0;
target_ulong msr;
+ target_ulong xer;
target_ulong ccr;
int i;
@@ -334,9 +335,11 @@ static void restore_user_regs(CPUPPCState *env,
__get_user(env->nip, &frame->mc_gregs[TARGET_PT_NIP]);
__get_user(env->ctr, &frame->mc_gregs[TARGET_PT_CTR]);
__get_user(env->lr, &frame->mc_gregs[TARGET_PT_LNK]);
- __get_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]);
+
+ __get_user(xer, &frame->mc_gregs[TARGET_PT_XER]);
+ cpu_write_xer(env, xer);
+
__get_user(ccr, &frame->mc_gregs[TARGET_PT_CCR]);
-
for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
env->crf[i] = (ccr >> (32 - ((i + 1) * 4))) & 0xf;
}
diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
index a6a4ddaeca..6ab7934fdf 100644
--- a/tests/tcg/ppc64/Makefile.target
+++ b/tests/tcg/ppc64/Makefile.target
@@ -23,4 +23,6 @@ run-plugin-byte_reverse-with-%:
$(call skip-test, "RUN of byte_reverse ($*)", "not built")
endif
+PPC64_TESTS += signal_save_restore_xer
+
TESTS += $(PPC64_TESTS)
diff --git a/tests/tcg/ppc64le/Makefile.target b/tests/tcg/ppc64le/Makefile.target
index c0c14ffbad..5e65b1590d 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -22,4 +22,6 @@ run-plugin-byte_reverse-with-%:
$(call skip-test, "RUN of byte_reverse ($*)", "not built")
endif
+PPC64LE_TESTS += signal_save_restore_xer
+
TESTS += $(PPC64LE_TESTS)
diff --git a/tests/tcg/ppc64le/signal_save_restore_xer.c b/tests/tcg/ppc64le/signal_save_restore_xer.c
new file mode 100644
index 0000000000..e4f8a07dd7
--- /dev/null
+++ b/tests/tcg/ppc64le/signal_save_restore_xer.c
@@ -0,0 +1,42 @@
+#include <assert.h>
+#include <stdint.h>
+#include <signal.h>
+#include <sys/user.h>
+
+#define XER_SO (1 << 31)
+#define XER_OV (1 << 30)
+#define XER_CA (1 << 29)
+#define XER_OV32 (1 << 19)
+#define XER_CA32 (1 << 18)
+
+uint64_t saved;
+
+void sigill_handler(int sig, siginfo_t *si, void *ucontext)
+{
+ ucontext_t *uc = ucontext;
+ uc->uc_mcontext.regs->nip += 4;
+ saved = uc->uc_mcontext.regs->xer;
+ uc->uc_mcontext.regs->xer |= XER_OV | XER_OV32;
+}
+
+int main(void)
+{
+ uint64_t initial = XER_CA | XER_CA32, restored;
+ struct sigaction sa = {
+ .sa_sigaction = sigill_handler,
+ .sa_flags = SA_SIGINFO
+ };
+
+ sigaction(SIGILL, &sa, NULL);
+
+ asm("mtspr 1, %1\n\t"
+ ".long 0x0\n\t"
+ "mfspr %0, 1\n\t"
+ : "=r" (restored)
+ : "r" (initial));
+
+ assert(saved == initial);
+ assert(restored == (XER_OV | XER_OV32 | XER_CA | XER_CA32));
+
+ return 0;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] target/ppc: Fix XER access in gdbstub
2021-10-14 22:32 [PATCH 0/4] Fix incorrect accesses to XER matheus.ferst
2021-10-14 22:32 ` [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs matheus.ferst
@ 2021-10-14 22:32 ` matheus.ferst
2021-10-14 23:43 ` Richard Henderson
2021-10-15 0:31 ` David Gibson
2021-10-14 22:32 ` [PATCH 3/4] linux-user: Fix XER access in ppc version of elf_core_copy_regs matheus.ferst
2021-10-14 22:32 ` [PATCH 4/4] target/ppc: Fix XER access in monitor matheus.ferst
3 siblings, 2 replies; 13+ messages in thread
From: matheus.ferst @ 2021-10-14 22:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: laurent, Matheus Ferst, richard.henderson, groug, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
The value of XER is split in multiple fields of CPUPPCState, like
env->xer and env->so. To get/set the whole register from gdb, we should
use cpu_read_xer/cpu_write_xer.
Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/gdbstub.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 1808a150e4..105c2f7dd1 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -159,7 +159,7 @@ int ppc_cpu_gdb_read_register(CPUState *cs, GByteArray *buf, int n)
gdb_get_regl(buf, env->ctr);
break;
case 69:
- gdb_get_reg32(buf, env->xer);
+ gdb_get_reg32(buf, cpu_read_xer(env));
break;
case 70:
gdb_get_reg32(buf, env->fpscr);
@@ -217,7 +217,7 @@ int ppc_cpu_gdb_read_register_apple(CPUState *cs, GByteArray *buf, int n)
gdb_get_reg64(buf, env->ctr);
break;
case 69 + 32:
- gdb_get_reg32(buf, env->xer);
+ gdb_get_reg32(buf, cpu_read_xer(env));
break;
case 70 + 32:
gdb_get_reg64(buf, env->fpscr);
@@ -269,7 +269,7 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
env->ctr = ldtul_p(mem_buf);
break;
case 69:
- env->xer = ldl_p(mem_buf);
+ cpu_write_xer(env, ldl_p(mem_buf));
break;
case 70:
/* fpscr */
@@ -319,7 +319,7 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, uint8_t *mem_buf, int n)
env->ctr = ldq_p(mem_buf);
break;
case 69 + 32:
- env->xer = ldl_p(mem_buf);
+ cpu_write_xer(env, ldl_p(mem_buf));
break;
case 70 + 32:
/* fpscr */
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] linux-user: Fix XER access in ppc version of elf_core_copy_regs
2021-10-14 22:32 [PATCH 0/4] Fix incorrect accesses to XER matheus.ferst
2021-10-14 22:32 ` [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs matheus.ferst
2021-10-14 22:32 ` [PATCH 2/4] target/ppc: Fix XER access in gdbstub matheus.ferst
@ 2021-10-14 22:32 ` matheus.ferst
2021-10-14 23:44 ` Richard Henderson
2021-10-14 22:32 ` [PATCH 4/4] target/ppc: Fix XER access in monitor matheus.ferst
3 siblings, 1 reply; 13+ messages in thread
From: matheus.ferst @ 2021-10-14 22:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: richard.henderson, laurent, groug, Lucas Mateus Castro,
Matheus Ferst, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
env->xer doesn't hold some bits of XER, like OV and CA. To write the
complete register in the core dump we should read XER value with
cpu_read_xer.
Reported-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
linux-user/elfload.c | 2 +-
target/ppc/cpu.c | 2 +-
target/ppc/cpu.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 01e9a833fb..e760786a98 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -896,7 +896,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUPPCState *en
(*regs)[33] = tswapreg(env->msr);
(*regs)[35] = tswapreg(env->ctr);
(*regs)[36] = tswapreg(env->lr);
- (*regs)[37] = tswapreg(env->xer);
+ (*regs)[37] = tswapreg(cpu_read_xer(env));
for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
ccr |= env->crf[i] << (32 - ((i + 1) * 4));
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 7ad9bd6044..f933d9f2bd 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -27,7 +27,7 @@
#include "helper_regs.h"
#include "sysemu/tcg.h"
-target_ulong cpu_read_xer(CPUPPCState *env)
+target_ulong cpu_read_xer(const CPUPPCState *env)
{
if (is_isa300(env)) {
return env->xer | (env->so << XER_SO) |
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 500205229c..67a143941f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2418,7 +2418,7 @@ enum {
/*****************************************************************************/
#define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
-target_ulong cpu_read_xer(CPUPPCState *env);
+target_ulong cpu_read_xer(const CPUPPCState *env);
void cpu_write_xer(CPUPPCState *env, target_ulong xer);
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] target/ppc: Fix XER access in monitor
2021-10-14 22:32 [PATCH 0/4] Fix incorrect accesses to XER matheus.ferst
` (2 preceding siblings ...)
2021-10-14 22:32 ` [PATCH 3/4] linux-user: Fix XER access in ppc version of elf_core_copy_regs matheus.ferst
@ 2021-10-14 22:32 ` matheus.ferst
2021-10-14 23:45 ` Richard Henderson
3 siblings, 1 reply; 13+ messages in thread
From: matheus.ferst @ 2021-10-14 22:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: laurent, Matheus Ferst, richard.henderson, groug, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
We can't read env->xer directly, as it does not contain some bits of
XER. Instead, we should have a callback that uses cpu_read_xer to read
the complete register.
Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/monitor.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
index a475108b2d..0b805ef6e9 100644
--- a/target/ppc/monitor.c
+++ b/target/ppc/monitor.c
@@ -44,6 +44,13 @@ static target_long monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
return u;
}
+static target_long monitor_get_xer(Monitor *mon, const struct MonitorDef *md,
+ int val)
+{
+ CPUArchState *env = mon_get_cpu_env(mon);
+ return cpu_read_xer(env);
+}
+
static target_long monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
int val)
{
@@ -85,7 +92,7 @@ const MonitorDef monitor_defs[] = {
{ "decr", 0, &monitor_get_decr, },
{ "ccr|cr", 0, &monitor_get_ccr, },
/* Machine state register */
- { "xer", offsetof(CPUPPCState, xer) },
+ { "xer", 0, &monitor_get_xer },
{ "msr", offsetof(CPUPPCState, msr) },
{ "tbu", 0, &monitor_get_tbu, },
{ "tbl", 0, &monitor_get_tbl, },
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs
2021-10-14 22:32 ` [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs matheus.ferst
@ 2021-10-14 23:43 ` Richard Henderson
2021-10-15 14:54 ` Matheus K. Ferst
2021-10-15 0:30 ` David Gibson
1 sibling, 1 reply; 13+ messages in thread
From: Richard Henderson @ 2021-10-14 23:43 UTC (permalink / raw)
To: matheus.ferst, qemu-devel, qemu-ppc; +Cc: laurent, groug, david
On 10/14/21 3:32 PM, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> We should use cpu_read_xer/cpu_write_xer to save/restore the complete
> register since some of its bits are in other fields of CPUPPCState. A
> test is added to prevent future regressions.
>
> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> linux-user/ppc/signal.c | 9 +++--
> tests/tcg/ppc64/Makefile.target | 2 +
> tests/tcg/ppc64le/Makefile.target | 2 +
> tests/tcg/ppc64le/signal_save_restore_xer.c | 42 +++++++++++++++++++++
> 4 files changed, 52 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c
The code is good so,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> + sigaction(SIGILL, &sa, NULL);
> +
> + asm("mtspr 1, %1\n\t"
> + ".long 0x0\n\t"
While Appendix B does guarantee that "0" is and always will be an invalid instruction, I
wonder if the test itself would be clearer (i.e. self-documenting the intent) using
SIGTRAP and "trap".
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] target/ppc: Fix XER access in gdbstub
2021-10-14 22:32 ` [PATCH 2/4] target/ppc: Fix XER access in gdbstub matheus.ferst
@ 2021-10-14 23:43 ` Richard Henderson
2021-10-15 0:31 ` David Gibson
1 sibling, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2021-10-14 23:43 UTC (permalink / raw)
To: matheus.ferst, qemu-devel, qemu-ppc; +Cc: laurent, groug, david
On 10/14/21 3:32 PM, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst<matheus.ferst@eldorado.org.br>
>
> The value of XER is split in multiple fields of CPUPPCState, like
> env->xer and env->so. To get/set the whole register from gdb, we should
> use cpu_read_xer/cpu_write_xer.
>
> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
> Signed-off-by: Matheus Ferst<matheus.ferst@eldorado.org.br>
> ---
> target/ppc/gdbstub.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] linux-user: Fix XER access in ppc version of elf_core_copy_regs
2021-10-14 22:32 ` [PATCH 3/4] linux-user: Fix XER access in ppc version of elf_core_copy_regs matheus.ferst
@ 2021-10-14 23:44 ` Richard Henderson
0 siblings, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2021-10-14 23:44 UTC (permalink / raw)
To: matheus.ferst, qemu-devel, qemu-ppc
Cc: laurent, Lucas Mateus Castro, groug, david
On 10/14/21 3:32 PM, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst<matheus.ferst@eldorado.org.br>
>
> env->xer doesn't hold some bits of XER, like OV and CA. To write the
> complete register in the core dump we should read XER value with
> cpu_read_xer.
>
> Reported-by: Lucas Mateus Castro (alqotel)<lucas.araujo@eldorado.org.br>
> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
> Signed-off-by: Matheus Ferst<matheus.ferst@eldorado.org.br>
> ---
> linux-user/elfload.c | 2 +-
> target/ppc/cpu.c | 2 +-
> target/ppc/cpu.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] target/ppc: Fix XER access in monitor
2021-10-14 22:32 ` [PATCH 4/4] target/ppc: Fix XER access in monitor matheus.ferst
@ 2021-10-14 23:45 ` Richard Henderson
0 siblings, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2021-10-14 23:45 UTC (permalink / raw)
To: matheus.ferst, qemu-devel, qemu-ppc; +Cc: laurent, groug, david
On 10/14/21 3:32 PM, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst<matheus.ferst@eldorado.org.br>
>
> We can't read env->xer directly, as it does not contain some bits of
> XER. Instead, we should have a callback that uses cpu_read_xer to read
> the complete register.
>
> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
> Signed-off-by: Matheus Ferst<matheus.ferst@eldorado.org.br>
> ---
> target/ppc/monitor.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs
2021-10-14 22:32 ` [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs matheus.ferst
2021-10-14 23:43 ` Richard Henderson
@ 2021-10-15 0:30 ` David Gibson
1 sibling, 0 replies; 13+ messages in thread
From: David Gibson @ 2021-10-15 0:30 UTC (permalink / raw)
To: matheus.ferst; +Cc: laurent, richard.henderson, qemu-ppc, qemu-devel, groug
[-- Attachment #1: Type: text/plain, Size: 4780 bytes --]
On Thu, Oct 14, 2021 at 07:32:31PM -0300, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> We should use cpu_read_xer/cpu_write_xer to save/restore the complete
> register since some of its bits are in other fields of CPUPPCState. A
> test is added to prevent future regressions.
>
> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Applied to ppc-for-6.2, thanks.
> ---
> linux-user/ppc/signal.c | 9 +++--
> tests/tcg/ppc64/Makefile.target | 2 +
> tests/tcg/ppc64le/Makefile.target | 2 +
> tests/tcg/ppc64le/signal_save_restore_xer.c | 42 +++++++++++++++++++++
> 4 files changed, 52 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c
>
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index edfad28a37..4413a4a28f 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -244,7 +244,7 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
> __put_user(env->nip, &frame->mc_gregs[TARGET_PT_NIP]);
> __put_user(env->ctr, &frame->mc_gregs[TARGET_PT_CTR]);
> __put_user(env->lr, &frame->mc_gregs[TARGET_PT_LNK]);
> - __put_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]);
> + __put_user(cpu_read_xer(env), &frame->mc_gregs[TARGET_PT_XER]);
>
> for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
> ccr |= env->crf[i] << (32 - ((i + 1) * 4));
> @@ -319,6 +319,7 @@ static void restore_user_regs(CPUPPCState *env,
> {
> target_ulong save_r2 = 0;
> target_ulong msr;
> + target_ulong xer;
> target_ulong ccr;
>
> int i;
> @@ -334,9 +335,11 @@ static void restore_user_regs(CPUPPCState *env,
> __get_user(env->nip, &frame->mc_gregs[TARGET_PT_NIP]);
> __get_user(env->ctr, &frame->mc_gregs[TARGET_PT_CTR]);
> __get_user(env->lr, &frame->mc_gregs[TARGET_PT_LNK]);
> - __get_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]);
> +
> + __get_user(xer, &frame->mc_gregs[TARGET_PT_XER]);
> + cpu_write_xer(env, xer);
> +
> __get_user(ccr, &frame->mc_gregs[TARGET_PT_CCR]);
> -
> for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
> env->crf[i] = (ccr >> (32 - ((i + 1) * 4))) & 0xf;
> }
> diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
> index a6a4ddaeca..6ab7934fdf 100644
> --- a/tests/tcg/ppc64/Makefile.target
> +++ b/tests/tcg/ppc64/Makefile.target
> @@ -23,4 +23,6 @@ run-plugin-byte_reverse-with-%:
> $(call skip-test, "RUN of byte_reverse ($*)", "not built")
> endif
>
> +PPC64_TESTS += signal_save_restore_xer
> +
> TESTS += $(PPC64_TESTS)
> diff --git a/tests/tcg/ppc64le/Makefile.target b/tests/tcg/ppc64le/Makefile.target
> index c0c14ffbad..5e65b1590d 100644
> --- a/tests/tcg/ppc64le/Makefile.target
> +++ b/tests/tcg/ppc64le/Makefile.target
> @@ -22,4 +22,6 @@ run-plugin-byte_reverse-with-%:
> $(call skip-test, "RUN of byte_reverse ($*)", "not built")
> endif
>
> +PPC64LE_TESTS += signal_save_restore_xer
> +
> TESTS += $(PPC64LE_TESTS)
> diff --git a/tests/tcg/ppc64le/signal_save_restore_xer.c b/tests/tcg/ppc64le/signal_save_restore_xer.c
> new file mode 100644
> index 0000000000..e4f8a07dd7
> --- /dev/null
> +++ b/tests/tcg/ppc64le/signal_save_restore_xer.c
> @@ -0,0 +1,42 @@
> +#include <assert.h>
> +#include <stdint.h>
> +#include <signal.h>
> +#include <sys/user.h>
> +
> +#define XER_SO (1 << 31)
> +#define XER_OV (1 << 30)
> +#define XER_CA (1 << 29)
> +#define XER_OV32 (1 << 19)
> +#define XER_CA32 (1 << 18)
> +
> +uint64_t saved;
> +
> +void sigill_handler(int sig, siginfo_t *si, void *ucontext)
> +{
> + ucontext_t *uc = ucontext;
> + uc->uc_mcontext.regs->nip += 4;
> + saved = uc->uc_mcontext.regs->xer;
> + uc->uc_mcontext.regs->xer |= XER_OV | XER_OV32;
> +}
> +
> +int main(void)
> +{
> + uint64_t initial = XER_CA | XER_CA32, restored;
> + struct sigaction sa = {
> + .sa_sigaction = sigill_handler,
> + .sa_flags = SA_SIGINFO
> + };
> +
> + sigaction(SIGILL, &sa, NULL);
> +
> + asm("mtspr 1, %1\n\t"
> + ".long 0x0\n\t"
> + "mfspr %0, 1\n\t"
> + : "=r" (restored)
> + : "r" (initial));
> +
> + assert(saved == initial);
> + assert(restored == (XER_OV | XER_OV32 | XER_CA | XER_CA32));
> +
> + return 0;
> +}
--
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] 13+ messages in thread
* Re: [PATCH 2/4] target/ppc: Fix XER access in gdbstub
2021-10-14 22:32 ` [PATCH 2/4] target/ppc: Fix XER access in gdbstub matheus.ferst
2021-10-14 23:43 ` Richard Henderson
@ 2021-10-15 0:31 ` David Gibson
1 sibling, 0 replies; 13+ messages in thread
From: David Gibson @ 2021-10-15 0:31 UTC (permalink / raw)
To: matheus.ferst; +Cc: laurent, richard.henderson, qemu-ppc, qemu-devel, groug
[-- Attachment #1: Type: text/plain, Size: 2422 bytes --]
On Thu, Oct 14, 2021 at 07:32:32PM -0300, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> The value of XER is split in multiple fields of CPUPPCState, like
> env->xer and env->so. To get/set the whole register from gdb, we should
> use cpu_read_xer/cpu_write_xer.
>
> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Applied to ppc-for-6.2.
> ---
> target/ppc/gdbstub.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
> index 1808a150e4..105c2f7dd1 100644
> --- a/target/ppc/gdbstub.c
> +++ b/target/ppc/gdbstub.c
> @@ -159,7 +159,7 @@ int ppc_cpu_gdb_read_register(CPUState *cs, GByteArray *buf, int n)
> gdb_get_regl(buf, env->ctr);
> break;
> case 69:
> - gdb_get_reg32(buf, env->xer);
> + gdb_get_reg32(buf, cpu_read_xer(env));
> break;
> case 70:
> gdb_get_reg32(buf, env->fpscr);
> @@ -217,7 +217,7 @@ int ppc_cpu_gdb_read_register_apple(CPUState *cs, GByteArray *buf, int n)
> gdb_get_reg64(buf, env->ctr);
> break;
> case 69 + 32:
> - gdb_get_reg32(buf, env->xer);
> + gdb_get_reg32(buf, cpu_read_xer(env));
> break;
> case 70 + 32:
> gdb_get_reg64(buf, env->fpscr);
> @@ -269,7 +269,7 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> env->ctr = ldtul_p(mem_buf);
> break;
> case 69:
> - env->xer = ldl_p(mem_buf);
> + cpu_write_xer(env, ldl_p(mem_buf));
> break;
> case 70:
> /* fpscr */
> @@ -319,7 +319,7 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, uint8_t *mem_buf, int n)
> env->ctr = ldq_p(mem_buf);
> break;
> case 69 + 32:
> - env->xer = ldl_p(mem_buf);
> + cpu_write_xer(env, ldl_p(mem_buf));
> break;
> case 70 + 32:
> /* fpscr */
--
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] 13+ messages in thread
* Re: [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs
2021-10-14 23:43 ` Richard Henderson
@ 2021-10-15 14:54 ` Matheus K. Ferst
2021-10-15 18:24 ` Richard Henderson
0 siblings, 1 reply; 13+ messages in thread
From: Matheus K. Ferst @ 2021-10-15 14:54 UTC (permalink / raw)
To: Richard Henderson, qemu-devel, qemu-ppc; +Cc: laurent, groug, david
On 14/10/2021 20:43, Richard Henderson wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você
> possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de
> e-mail suspeito entre imediatamente em contato com o DTI.
>
> On 10/14/21 3:32 PM, matheus.ferst@eldorado.org.br wrote:
>> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>
>> We should use cpu_read_xer/cpu_write_xer to save/restore the complete
>> register since some of its bits are in other fields of CPUPPCState. A
>> test is added to prevent future regressions.
>>
>> Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>> linux-user/ppc/signal.c | 9 +++--
>> tests/tcg/ppc64/Makefile.target | 2 +
>> tests/tcg/ppc64le/Makefile.target | 2 +
>> tests/tcg/ppc64le/signal_save_restore_xer.c | 42 +++++++++++++++++++++
>> 4 files changed, 52 insertions(+), 3 deletions(-)
>> create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c
>
> The code is good so,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>> + sigaction(SIGILL, &sa, NULL);
>> +
>> + asm("mtspr 1, %1\n\t"
>> + ".long 0x0\n\t"
>
> While Appendix B does guarantee that "0" is and always will be an
> invalid instruction, I
> wonder if the test itself would be clearer (i.e. self-documenting the
> intent) using
> SIGTRAP and "trap".
>
>
> r~
It would be better, but cpu_loop is currently calling cpu_abort for
POWERPC_EXCP_TRAP, so the test would fail. I'll see if I can fix that in
another patch, and then we can change the test to use trap.
--
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs
2021-10-15 14:54 ` Matheus K. Ferst
@ 2021-10-15 18:24 ` Richard Henderson
0 siblings, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2021-10-15 18:24 UTC (permalink / raw)
To: Matheus K. Ferst, qemu-devel, qemu-ppc; +Cc: laurent, groug, david
On 10/15/21 7:54 AM, Matheus K. Ferst wrote:
>> While Appendix B does guarantee that "0" is and always will be an invalid instruction, I
>> wonder if the test itself would be clearer (i.e. self-documenting the intent) using
>> SIGTRAP and "trap".
>>
>>
>> r~
>
> It would be better, but cpu_loop is currently calling cpu_abort for POWERPC_EXCP_TRAP, so
> the test would fail. I'll see if I can fix that in another patch, and then we can change
> the test to use trap.
Fair enough.
r~
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-10-15 18:28 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14 22:32 [PATCH 0/4] Fix incorrect accesses to XER matheus.ferst
2021-10-14 22:32 ` [PATCH 1/4] linux-user/ppc: Fix XER access in save/restore_user_regs matheus.ferst
2021-10-14 23:43 ` Richard Henderson
2021-10-15 14:54 ` Matheus K. Ferst
2021-10-15 18:24 ` Richard Henderson
2021-10-15 0:30 ` David Gibson
2021-10-14 22:32 ` [PATCH 2/4] target/ppc: Fix XER access in gdbstub matheus.ferst
2021-10-14 23:43 ` Richard Henderson
2021-10-15 0:31 ` David Gibson
2021-10-14 22:32 ` [PATCH 3/4] linux-user: Fix XER access in ppc version of elf_core_copy_regs matheus.ferst
2021-10-14 23:44 ` Richard Henderson
2021-10-14 22:32 ` [PATCH 4/4] target/ppc: Fix XER access in monitor matheus.ferst
2021-10-14 23:45 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.