mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

linux-user/sparc: Correct sparc64_get/set_context() FPU handling

Browse Source 
The handling of the FPU state in sparc64_get_context() and
sparc64_set_context() is not the same as what the kernel actually
does: we unconditionally read and write the FP registers and the
FSR, GSR and FPRS, but the kernel logic is more complicated:
 * in get_context the kernel has code for saving FPU registers,
   but it is hidden inside an "if (fenab) condition and the
   fenab flag is always set to 0 (inside an "#if 1" which has
   been in the kernel for over 15 years). So the effect is that
   the FPU state part is always written as zeroes.
 * in set_context the kernel looks at the fenab field in the
   structure from the guest, and only restores the state if
   it is set; it also looks at the structure's FPRS to see
   whether either the upper or lower or both halves of the
   register file have valid data.

Bring our implementations into line with the kernel:
 * in get_context:
    - clear the entire target_ucontext at the top of the
      function (as the kernel does)
    - then don't write the FPU state, so those fields remain zero
    - this fixes Coverity issue CID 1432305 by deleting the code
      it was complaining about
 * in set_context:
    - check the fenab and the fpsr to decide which parts of
      the FPU data to restore, if any
    - instead of setting the FPU registers by doing two
      32-bit loads and filling in the .upper and .lower parts
      of the CPU_Double union separately, just do a 64-bit
      load of the whole register at once. This fixes Coverity
      issue CID 1432303 because we now access the dregs[] part
      of the mcfpu_fregs union rather than the sregs[] part
      (which is not large enough to actually cover the whole of
      the data, so we were accessing off the end of sregs[])

We change both functions in a single commit to avoid potentially
breaking bisection.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201106152738.26026-2-peter.maydell@linaro.org>
[lv: fix FPRS_DU loop s/31/32/]
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
This commit is contained in:
Peter Maydell 2020-11-06 15:27:35 +00:00 committed by Laurent Vivier
parent 6addf06a3c
commit 246ff44295
2 changed files with 46 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
linux-user/sparc/signal.c
View File

@ -402,8 +402,10 @@ void sparc64_set_context(CPUSPARCState *env)
abi_ulong ucp_addr;
struct target_ucontext *ucp;
target_mc_gregset_t *grp;
target_mc_fpu_t *fpup;
abi_ulong pc, npc, tstate;
unsigned int i;
unsigned char fenab;
ucp_addr = env->regwptr[WREG_O0];
if (!lock_user_struct(VERIFY_READ, ucp, ucp_addr, 1)) {
@ -467,26 +469,42 @@ void sparc64_set_context(CPUSPARCState *env)
__get_user(env->regwptr[WREG_FP], &(ucp->tuc_mcontext.mc_fp));
__get_user(env->regwptr[WREG_I7], &(ucp->tuc_mcontext.mc_i7));
/* 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:
* __get_user(fenab, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_enab));
*/
__get_user(env->fprs, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_fprs));
{
uint32_t *src = ucp->tuc_mcontext.mc_fpregs.mcfpu_fregs.sregs;
for (i = 0; i < 64; i++, src++) {
if (i & 1) {
__get_user(env->fpr[i/2].l.lower, src);
} else {
__get_user(env->fpr[i/2].l.upper, src);
fpup = &ucp->tuc_mcontext.mc_fpregs;
__get_user(fenab, &(fpup->mcfpu_enab));
if (fenab) {
abi_ulong fprs;
/*
* We use the FPRS from the guest only in deciding whether
* to restore the upper, lower, or both banks of the FPU regs.
* The kernel here writes the FPU register data into the
* process's current_thread_info state and unconditionally
* clears FPRS and TSTATE_PEF: this disables the FPU so that the
* next FPU-disabled trap will copy the data out of
* current_thread_info and into the real FPU registers.
* QEMU doesn't need to handle lazy-FPU-state-restoring like that,
* so we always load the data directly into the FPU registers
* and leave FPRS and TSTATE_PEF alone (so the FPU stays enabled).
* Note that because we (and the kernel) always write zeroes for
* the fenab and fprs in sparc64_get_context() none of this code
* will execute unless the guest manually constructed or changed
* the context structure.
*/
__get_user(fprs, &(fpup->mcfpu_fprs));
if (fprs & FPRS_DL) {
for (i = 0; i < 16; i++) {
__get_user(env->fpr[i].ll, &(fpup->mcfpu_fregs.dregs[i]));
}
}
if (fprs & FPRS_DU) {
for (i = 16; i < 32; i++) {
__get_user(env->fpr[i].ll, &(fpup->mcfpu_fregs.dregs[i]));
}
}
__get_user(env->fsr, &(fpup->mcfpu_fsr));
__get_user(env->gsr, &(fpup->mcfpu_gsr));
}
__get_user(env->fsr,
&(ucp->tuc_mcontext.mc_fpregs.mcfpu_fsr));
__get_user(env->gsr,
&(ucp->tuc_mcontext.mc_fpregs.mcfpu_gsr));
unlock_user_struct(ucp, ucp_addr, 0);
return;
do_sigsegv:
@ -509,7 +527,9 @@ void sparc64_get_context(CPUSPARCState *env)
if (!lock_user_struct(VERIFY_WRITE, ucp, ucp_addr, 0)) {
goto do_sigsegv;
}
memset(ucp, 0, sizeof(*ucp));
mcp = &ucp->tuc_mcontext;
grp = &mcp->mc_gregs;
@ -572,19 +592,11 @@ void sparc64_get_context(CPUSPARCState *env)
__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;
for (i = 0; i < 64; i++, dst++) {
if (i & 1) {
__put_user(env->fpr[i/2].l.lower, dst);
} else {
__put_user(env->fpr[i/2].l.upper, dst);
}
}
}
__put_user(env->fsr, &(mcp->mc_fpregs.mcfpu_fsr));
__put_user(env->gsr, &(mcp->mc_fpregs.mcfpu_gsr));
__put_user(env->fprs, &(mcp->mc_fpregs.mcfpu_fprs));
/*
* We don't write out the FPU state. This matches the kernel's
* implementation (which has the code for doing this but
* hidden behind an "if (fenab)" where fenab is always 0).
*/
if (err)
goto do_sigsegv;

4
target/sparc/cpu.h
View File

@ -156,7 +156,9 @@ enum {
#define PS_IE (1<<1)
#define PS_AG (1<<0) /* v9, zero on UA2007 */
#define FPRS_FEF (1<<2)
#define FPRS_DL (1 << 0)
#define FPRS_DU (1 << 1)
#define FPRS_FEF (1 << 2)
#define HS_PRIV (1<<2)
#endif