* [PULL 1/3] linux-user/sparc: Fix errors in target_ucontext structures
2020-11-10 8:30 [PULL 0/3] Linux user for 5.2 patches Laurent Vivier
@ 2020-11-10 8:30 ` Laurent Vivier
2020-11-10 8:30 ` [PULL 2/3] linux-user/sparc: Correct set/get_context handling of fp and i7 Laurent Vivier
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2020-11-10 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Richard Henderson, Laurent Vivier
From: Peter Maydell <peter.maydell@linaro.org>
The various structs that make up the SPARC target_ucontext had some
errors:
* target structures must not include fields which are host pointers,
which might be the wrong size. These should be abi_ulong instead
* because we don't have the 'long double' part of the mcfpu_fregs
union in our version of the target_mc_fpu struct, we need to
manually force it to be 16-aligned
In particular, the lack of 16-alignment caused sparc64_get_context()
and sparc64_set_context() to read and write all the registers at the
wrong offset, which triggered a guest glibc stack check in
siglongjmp:
*** longjmp causes uninitialized stack frame ***: terminated
when trying to run bash.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201105212314.9628-2-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/sparc/signal.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index d796f50f665b..57ea1593bfc9 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -349,10 +349,15 @@ typedef abi_ulong target_mc_greg_t;
typedef target_mc_greg_t target_mc_gregset_t[SPARC_MC_NGREG];
struct target_mc_fq {
- abi_ulong *mcfq_addr;
+ abi_ulong mcfq_addr;
uint32_t mcfq_insn;
};
+/*
+ * Note the manual 16-alignment; the kernel gets this because it
+ * includes a "long double qregs[16]" in the mcpu_fregs union,
+ * which we can't do.
+ */
struct target_mc_fpu {
union {
uint32_t sregs[32];
@@ -362,11 +367,11 @@ struct target_mc_fpu {
abi_ulong mcfpu_fsr;
abi_ulong mcfpu_fprs;
abi_ulong mcfpu_gsr;
- struct target_mc_fq *mcfpu_fq;
+ abi_ulong mcfpu_fq;
unsigned char mcfpu_qcnt;
unsigned char mcfpu_qentsz;
unsigned char mcfpu_enab;
-};
+} __attribute__((aligned(16)));
typedef struct target_mc_fpu target_mc_fpu_t;
typedef struct {
@@ -377,7 +382,7 @@ typedef struct {
} target_mcontext_t;
struct target_ucontext {
- struct target_ucontext *tuc_link;
+ abi_ulong tuc_link;
abi_ulong tuc_flags;
target_sigset_t tuc_sigmask;
target_mcontext_t tuc_mcontext;
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 2/3] linux-user/sparc: Correct set/get_context handling of fp and i7
2020-11-10 8:30 [PULL 0/3] Linux user for 5.2 patches Laurent Vivier
2020-11-10 8:30 ` [PULL 1/3] linux-user/sparc: Fix errors in target_ucontext structures Laurent Vivier
@ 2020-11-10 8:30 ` Laurent Vivier
2020-11-10 8:30 ` [PULL 3/3] linux-user/sparc: Don't zero high half of PC, NPC, PSR in sigreturn Laurent Vivier
2020-11-10 12:22 ` [PULL 0/3] Linux user for 5.2 patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2020-11-10 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Richard Henderson, Laurent Vivier
From: Peter Maydell <peter.maydell@linaro.org>
Because QEMU's user-mode emulation just directly accesses guest CPU
state, for SPARC the guest register window state is not the same in
the sparc64_get_context() and sparc64_set_context() functions as it
is for the real kernel's versions of those functions. Specifically,
for the kernel it has saved the user space state such that the O*
registers go into a pt_regs struct as UREG_I*, and the I* registers
have been spilled onto the userspace stack. For QEMU, we haven't
done that, so the guest's O* registers are still in WREG_O* and the
I* registers in WREG_I*.
The code was already accessing the O* registers correctly for QEMU,
but had copied the kernel code for accessing the I* registers off the
userspace stack. Replace this with direct accesses to fp and i7 in
the CPU state, and add a comment explaining why we differ from the
kernel code here.
This fix is sufficient to get bash to a shell prompt.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201105212314.9628-3-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/sparc/signal.c | 47 ++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index 57ea1593bfc9..c315704b3895 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -403,7 +403,6 @@ void sparc64_set_context(CPUSPARCState *env)
struct target_ucontext *ucp;
target_mc_gregset_t *grp;
abi_ulong pc, npc, tstate;
- abi_ulong fp, i7, w_addr;
unsigned int i;
ucp_addr = env->regwptr[WREG_O0];
@@ -447,6 +446,15 @@ void sparc64_set_context(CPUSPARCState *env)
__get_user(env->gregs[5], (&(*grp)[SPARC_MC_G5]));
__get_user(env->gregs[6], (&(*grp)[SPARC_MC_G6]));
__get_user(env->gregs[7], (&(*grp)[SPARC_MC_G7]));
+
+ /*
+ * Note that unlike the kernel, we didn't need to mess with the
+ * guest register window state to save it into a pt_regs to run
+ * the kernel. So for us the guest's O regs are still in WREG_O*
+ * (unlike the kernel which has put them in UREG_I* in a pt_regs)
+ * and the fp and i7 are still in WREG_I6 and WREG_I7 and don't
+ * need to be written back to userspace memory.
+ */
__get_user(env->regwptr[WREG_O0], (&(*grp)[SPARC_MC_O0]));
__get_user(env->regwptr[WREG_O1], (&(*grp)[SPARC_MC_O1]));
__get_user(env->regwptr[WREG_O2], (&(*grp)[SPARC_MC_O2]));
@@ -456,18 +464,9 @@ void sparc64_set_context(CPUSPARCState *env)
__get_user(env->regwptr[WREG_O6], (&(*grp)[SPARC_MC_O6]));
__get_user(env->regwptr[WREG_O7], (&(*grp)[SPARC_MC_O7]));
- __get_user(fp, &(ucp->tuc_mcontext.mc_fp));
- __get_user(i7, &(ucp->tuc_mcontext.mc_i7));
+ __get_user(env->regwptr[WREG_FP], &(ucp->tuc_mcontext.mc_fp));
+ __get_user(env->regwptr[WREG_I7], &(ucp->tuc_mcontext.mc_i7));
- w_addr = TARGET_STACK_BIAS + env->regwptr[WREG_O6];
- if (put_user(fp, w_addr + offsetof(struct target_reg_window, ins[6]),
- abi_ulong) != 0) {
- goto do_sigsegv;
- }
- if (put_user(i7, w_addr + offsetof(struct target_reg_window, ins[7]),
- abi_ulong) != 0) {
- goto do_sigsegv;
- }
/* FIXME this does not match how the kernel handles the FPU in
* its sparc64_set_context implementation. In particular the FPU
* is only restored if fenab is non-zero in:
@@ -501,7 +500,6 @@ void sparc64_get_context(CPUSPARCState *env)
struct target_ucontext *ucp;
target_mc_gregset_t *grp;
target_mcontext_t *mcp;
- abi_ulong fp, i7, w_addr;
int err;
unsigned int i;
target_sigset_t target_set;
@@ -553,6 +551,15 @@ void sparc64_get_context(CPUSPARCState *env)
__put_user(env->gregs[5], &((*grp)[SPARC_MC_G5]));
__put_user(env->gregs[6], &((*grp)[SPARC_MC_G6]));
__put_user(env->gregs[7], &((*grp)[SPARC_MC_G7]));
+
+ /*
+ * Note that unlike the kernel, we didn't need to mess with the
+ * guest register window state to save it into a pt_regs to run
+ * the kernel. So for us the guest's O regs are still in WREG_O*
+ * (unlike the kernel which has put them in UREG_I* in a pt_regs)
+ * and the fp and i7 are still in WREG_I6 and WREG_I7 and don't
+ * need to be fished out of userspace memory.
+ */
__put_user(env->regwptr[WREG_O0], &((*grp)[SPARC_MC_O0]));
__put_user(env->regwptr[WREG_O1], &((*grp)[SPARC_MC_O1]));
__put_user(env->regwptr[WREG_O2], &((*grp)[SPARC_MC_O2]));
@@ -562,18 +569,8 @@ void sparc64_get_context(CPUSPARCState *env)
__put_user(env->regwptr[WREG_O6], &((*grp)[SPARC_MC_O6]));
__put_user(env->regwptr[WREG_O7], &((*grp)[SPARC_MC_O7]));
- w_addr = TARGET_STACK_BIAS + env->regwptr[WREG_O6];
- fp = i7 = 0;
- if (get_user(fp, w_addr + offsetof(struct target_reg_window, ins[6]),
- abi_ulong) != 0) {
- goto do_sigsegv;
- }
- if (get_user(i7, w_addr + offsetof(struct target_reg_window, ins[7]),
- abi_ulong) != 0) {
- goto do_sigsegv;
- }
- __put_user(fp, &(mcp->mc_fp));
- __put_user(i7, &(mcp->mc_i7));
+ __put_user(env->regwptr[WREG_FP], &(mcp->mc_fp));
+ __put_user(env->regwptr[WREG_I7], &(mcp->mc_i7));
{
uint32_t *dst = ucp->tuc_mcontext.mc_fpregs.mcfpu_fregs.sregs;
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 3/3] linux-user/sparc: Don't zero high half of PC, NPC, PSR in sigreturn
2020-11-10 8:30 [PULL 0/3] Linux user for 5.2 patches Laurent Vivier
2020-11-10 8:30 ` [PULL 1/3] linux-user/sparc: Fix errors in target_ucontext structures Laurent Vivier
2020-11-10 8:30 ` [PULL 2/3] linux-user/sparc: Correct set/get_context handling of fp and i7 Laurent Vivier
@ 2020-11-10 8:30 ` Laurent Vivier
2020-11-10 12:22 ` [PULL 0/3] Linux user for 5.2 patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2020-11-10 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Richard Henderson, Laurent Vivier
From: Peter Maydell <peter.maydell@linaro.org>
The function do_sigreturn() tries to store the PC, NPC and PSR in
uint32_t local variables, which implicitly drops the high half of
these fields for 64-bit guests.
The usual effect was that a guest which used signals would crash on
return from a signal unless it was lucky enough to take it while the
PC was in the low 4GB of the address space. In particular, Debian
/bin/dash and /bin/bash would segfault after executing external
commands.
Use abi_ulong, which is the type these fields all have in the
__siginfo_t struct.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201105212314.9628-4-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/sparc/signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index c315704b3895..d12adc8e6ff9 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -247,7 +247,7 @@ long do_sigreturn(CPUSPARCState *env)
{
abi_ulong sf_addr;
struct target_signal_frame *sf;
- uint32_t up_psr, pc, npc;
+ abi_ulong up_psr, pc, npc;
target_sigset_t set;
sigset_t host_set;
int i;
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread