[PULL 0/3] Linux user for 5.2 patches

[PULL 0/3] Linux user for 5.2 patches

[Laurent Vivier]

The following changes since commit 43afbbd9fea1b255cc81f5f4bfd0b6a88826c735:

  Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2020-11-09-tag' =
into staging (2020-11-09 20:29:04 +0000)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request

for you to fetch changes up to c3ab5df2f5c466d998917f2c707e206322063dcd:

  linux-user/sparc: Don't zero high half of PC, NPC, PSR in sigreturn (2020-1=
1-10 07:54:22 +0100)

----------------------------------------------------------------
Some linux-user/sparc fixes

----------------------------------------------------------------

Peter Maydell (3):
  linux-user/sparc: Fix errors in target_ucontext structures
  linux-user/sparc: Correct set/get_context handling of fp and i7
  linux-user/sparc: Don't zero high half of PC, NPC, PSR in sigreturn

 linux-user/sparc/signal.c | 62 ++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 30 deletions(-)

--=20
2.28.0

[PULL 1/3] linux-user/sparc: Fix errors in target_ucontext structures

[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

[PULL 2/3] linux-user/sparc: Correct set/get_context handling of fp and i7

[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

[PULL 3/3] linux-user/sparc: Don't zero high half of PC, NPC, PSR in sigreturn

[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

Re: [PULL 0/3] Linux user for 5.2 patches

[Peter Maydell]

On Tue, 10 Nov 2020 at 08:33, Laurent Vivier <laurent@vivier.eu> wrote:
>
> The following changes since commit 43afbbd9fea1b255cc81f5f4bfd0b6a88826c735:
>
>   Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2020-11-09-tag' =
> into staging (2020-11-09 20:29:04 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request
>
> for you to fetch changes up to c3ab5df2f5c466d998917f2c707e206322063dcd:
>
>   linux-user/sparc: Don't zero high half of PC, NPC, PSR in sigreturn (2020-1=
> 1-10 07:54:22 +0100)
>
> ----------------------------------------------------------------
> Some linux-user/sparc fixes
>
> ----------------------------------------------------------------
>
> Peter Maydell (3):
>   linux-user/sparc: Fix errors in target_ucontext structures
>   linux-user/sparc: Correct set/get_context handling of fp and i7
>   linux-user/sparc: Don't zero high half of PC, NPC, PSR in sigreturn


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM