| Message ID | 20140917080518.4844.99109.stgit@PASHA-ISP |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 17, 2014 at 4:05 PM, Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> wrote: > This patch introduces cpu_set_fpuc() function, which changes fpuc field > of the CPU state and calls update_fp_status() function. > These calls update status of softfloat library and prevent bugs caused > by non-coherent rounding settings of the FPU and softfloat. > > v2 changes: > * Added missed calls and intoduced setter function (as suggested by TeLeMan) > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > --- > target-i386/cpu.c | 2 +- > target-i386/cpu.h | 2 ++ > target-i386/fpu_helper.c | 21 +++++++++++++-------- > target-i386/gdbstub.c | 2 +- > target-i386/machine.c | 2 +- > 5 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 217500c..9f48b1c 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2572,7 +2572,7 @@ static void x86_cpu_reset(CPUState *s) > for (i = 0; i < 8; i++) { > env->fptags[i] = 1; > } > - env->fpuc = 0x37f; > + cpu_set_fpuc(env, 0x37f); > > env->mxcsr = 0x1f80; > env->xstate_bv = XSTATE_FP | XSTATE_SSE; > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index e634d83..6ab8eca 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -1249,6 +1249,7 @@ void QEMU_NORETURN raise_interrupt(CPUX86State *nenv, int intno, int is_int, > /* cc_helper.c */ > extern const uint8_t parity_table[256]; > uint32_t cpu_cc_compute_all(CPUX86State *env1, int op); > +void update_fp_status(CPUX86State *env); > > static inline uint32_t cpu_compute_eflags(CPUX86State *env) > { > @@ -1284,6 +1285,7 @@ static inline void cpu_load_efer(CPUX86State *env, uint64_t val) > > /* fpu_helper.c */ > void cpu_set_mxcsr(CPUX86State *env, uint32_t val); > +void cpu_set_fpuc(CPUX86State *env, uint16_t val); > > /* svm_helper.c */ > void cpu_svm_check_intercept_param(CPUX86State *env1, uint32_t type, > diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c > index 1b2900d..1d4eee3 100644 > --- a/target-i386/fpu_helper.c > +++ b/target-i386/fpu_helper.c > @@ -537,7 +537,7 @@ uint32_t helper_fnstcw(CPUX86State *env) > return env->fpuc; > } > > -static void update_fp_status(CPUX86State *env) > +void update_fp_status(CPUX86State *env) > { > int rnd_type; > > @@ -575,8 +575,7 @@ static void update_fp_status(CPUX86State *env) > > void helper_fldcw(CPUX86State *env, uint32_t val) > { > - env->fpuc = val; > - update_fp_status(env); > + cpu_set_fpuc(env, val); > } > > void helper_fclex(CPUX86State *env) > @@ -595,7 +594,7 @@ void helper_fninit(CPUX86State *env) > { > env->fpus = 0; > env->fpstt = 0; > - env->fpuc = 0x37f; > + cpu_set_fpuc(env, 0x37f); > env->fptags[0] = 1; > env->fptags[1] = 1; > env->fptags[2] = 1; > @@ -1013,11 +1012,11 @@ void helper_fldenv(CPUX86State *env, target_ulong ptr, int data32) > int i, fpus, fptag; > > if (data32) { > - env->fpuc = cpu_lduw_data(env, ptr); > + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); > fpus = cpu_lduw_data(env, ptr + 4); > fptag = cpu_lduw_data(env, ptr + 8); > } else { > - env->fpuc = cpu_lduw_data(env, ptr); > + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); > fpus = cpu_lduw_data(env, ptr + 2); > fptag = cpu_lduw_data(env, ptr + 4); > } > @@ -1046,7 +1045,7 @@ void helper_fsave(CPUX86State *env, target_ulong ptr, int data32) > /* fninit */ > env->fpus = 0; > env->fpstt = 0; > - env->fpuc = 0x37f; > + cpu_set_fpuc(env, 0x37f); > env->fptags[0] = 1; > env->fptags[1] = 1; > env->fptags[2] = 1; > @@ -1157,7 +1156,7 @@ void helper_fxrstor(CPUX86State *env, target_ulong ptr, int data64) > raise_exception(env, EXCP0D_GPF); > } > > - env->fpuc = cpu_lduw_data(env, ptr); > + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); > fpus = cpu_lduw_data(env, ptr + 2); > fptag = cpu_lduw_data(env, ptr + 4); > env->fpstt = (fpus >> 11) & 7; > @@ -1257,6 +1256,12 @@ void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr) > set_flush_to_zero((mxcsr & SSE_FZ) ? 1 : 0, &env->fp_status); > } > > +void cpu_set_fpuc(CPUX86State *env, uint16_t val) > +{ > + env->fpuc = val; > + update_fp_status(env); > +} > + > void helper_ldmxcsr(CPUX86State *env, uint32_t val) > { > cpu_set_mxcsr(env, val); > diff --git a/target-i386/gdbstub.c b/target-i386/gdbstub.c > index 19fe9ad..ff99cfb 100644 > --- a/target-i386/gdbstub.c > +++ b/target-i386/gdbstub.c > @@ -203,7 +203,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) > return x86_cpu_gdb_load_seg(cpu, R_GS, mem_buf); > > case IDX_FP_REGS + 8: > - env->fpuc = ldl_p(mem_buf); > + cpu_set_fpuc(env, ldl_p(mem_buf)); > return 4; > case IDX_FP_REGS + 9: > tmp = ldl_p(mem_buf); > diff --git a/target-i386/machine.c b/target-i386/machine.c > index 16d2f6a..2922c97 100644 > --- a/target-i386/machine.c > +++ b/target-i386/machine.c > @@ -315,13 +315,13 @@ static int cpu_post_load(void *opaque, int version_id) > env->hflags &= ~HF_CPL_MASK; > env->hflags |= (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; > > - /* XXX: restore FPU round state */ > env->fpstt = (env->fpus_vmstate >> 11) & 7; > env->fpus = env->fpus_vmstate & ~0x3800; > env->fptag_vmstate ^= 0xff; > for(i = 0; i < 8; i++) { > env->fptags[i] = (env->fptag_vmstate >> i) & 1; > } > + update_fp_status(env); > > cpu_breakpoint_remove_all(cs, BP_CPU); > cpu_watchpoint_remove_all(cs, BP_CPU); > Reviewed-by: TeLeMan <geleman@gmail.com>
Il 17/09/2014 10:05, Pavel Dovgalyuk ha scritto: > This patch introduces cpu_set_fpuc() function, which changes fpuc field > of the CPU state and calls update_fp_status() function. > These calls update status of softfloat library and prevent bugs caused > by non-coherent rounding settings of the FPU and softfloat. > > v2 changes: > * Added missed calls and intoduced setter function (as suggested by TeLeMan) > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > --- > target-i386/cpu.c | 2 +- > target-i386/cpu.h | 2 ++ > target-i386/fpu_helper.c | 21 +++++++++++++-------- > target-i386/gdbstub.c | 2 +- > target-i386/machine.c | 2 +- > 5 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 217500c..9f48b1c 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2572,7 +2572,7 @@ static void x86_cpu_reset(CPUState *s) > for (i = 0; i < 8; i++) { > env->fptags[i] = 1; > } > - env->fpuc = 0x37f; > + cpu_set_fpuc(env, 0x37f); > > env->mxcsr = 0x1f80; > env->xstate_bv = XSTATE_FP | XSTATE_SSE; > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index e634d83..6ab8eca 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -1249,6 +1249,7 @@ void QEMU_NORETURN raise_interrupt(CPUX86State *nenv, int intno, int is_int, > /* cc_helper.c */ > extern const uint8_t parity_table[256]; > uint32_t cpu_cc_compute_all(CPUX86State *env1, int op); > +void update_fp_status(CPUX86State *env); > > static inline uint32_t cpu_compute_eflags(CPUX86State *env) > { > @@ -1284,6 +1285,7 @@ static inline void cpu_load_efer(CPUX86State *env, uint64_t val) > > /* fpu_helper.c */ > void cpu_set_mxcsr(CPUX86State *env, uint32_t val); > +void cpu_set_fpuc(CPUX86State *env, uint16_t val); > > /* svm_helper.c */ > void cpu_svm_check_intercept_param(CPUX86State *env1, uint32_t type, > diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c > index 1b2900d..1d4eee3 100644 > --- a/target-i386/fpu_helper.c > +++ b/target-i386/fpu_helper.c > @@ -537,7 +537,7 @@ uint32_t helper_fnstcw(CPUX86State *env) > return env->fpuc; > } > > -static void update_fp_status(CPUX86State *env) > +void update_fp_status(CPUX86State *env) > { > int rnd_type; > > @@ -575,8 +575,7 @@ static void update_fp_status(CPUX86State *env) > > void helper_fldcw(CPUX86State *env, uint32_t val) > { > - env->fpuc = val; > - update_fp_status(env); > + cpu_set_fpuc(env, val); > } > > void helper_fclex(CPUX86State *env) > @@ -595,7 +594,7 @@ void helper_fninit(CPUX86State *env) > { > env->fpus = 0; > env->fpstt = 0; > - env->fpuc = 0x37f; > + cpu_set_fpuc(env, 0x37f); > env->fptags[0] = 1; > env->fptags[1] = 1; > env->fptags[2] = 1; > @@ -1013,11 +1012,11 @@ void helper_fldenv(CPUX86State *env, target_ulong ptr, int data32) > int i, fpus, fptag; > > if (data32) { > - env->fpuc = cpu_lduw_data(env, ptr); > + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); > fpus = cpu_lduw_data(env, ptr + 4); > fptag = cpu_lduw_data(env, ptr + 8); > } else { > - env->fpuc = cpu_lduw_data(env, ptr); > + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); > fpus = cpu_lduw_data(env, ptr + 2); > fptag = cpu_lduw_data(env, ptr + 4); > } > @@ -1046,7 +1045,7 @@ void helper_fsave(CPUX86State *env, target_ulong ptr, int data32) > /* fninit */ > env->fpus = 0; > env->fpstt = 0; > - env->fpuc = 0x37f; > + cpu_set_fpuc(env, 0x37f); > env->fptags[0] = 1; > env->fptags[1] = 1; > env->fptags[2] = 1; > @@ -1157,7 +1156,7 @@ void helper_fxrstor(CPUX86State *env, target_ulong ptr, int data64) > raise_exception(env, EXCP0D_GPF); > } > > - env->fpuc = cpu_lduw_data(env, ptr); > + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); > fpus = cpu_lduw_data(env, ptr + 2); > fptag = cpu_lduw_data(env, ptr + 4); > env->fpstt = (fpus >> 11) & 7; > @@ -1257,6 +1256,12 @@ void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr) > set_flush_to_zero((mxcsr & SSE_FZ) ? 1 : 0, &env->fp_status); > } > > +void cpu_set_fpuc(CPUX86State *env, uint16_t val) > +{ > + env->fpuc = val; > + update_fp_status(env); > +} > + > void helper_ldmxcsr(CPUX86State *env, uint32_t val) > { > cpu_set_mxcsr(env, val); > diff --git a/target-i386/gdbstub.c b/target-i386/gdbstub.c > index 19fe9ad..ff99cfb 100644 > --- a/target-i386/gdbstub.c > +++ b/target-i386/gdbstub.c > @@ -203,7 +203,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) > return x86_cpu_gdb_load_seg(cpu, R_GS, mem_buf); > > case IDX_FP_REGS + 8: > - env->fpuc = ldl_p(mem_buf); > + cpu_set_fpuc(env, ldl_p(mem_buf)); > return 4; > case IDX_FP_REGS + 9: > tmp = ldl_p(mem_buf); > diff --git a/target-i386/machine.c b/target-i386/machine.c > index 16d2f6a..2922c97 100644 > --- a/target-i386/machine.c > +++ b/target-i386/machine.c > @@ -315,13 +315,13 @@ static int cpu_post_load(void *opaque, int version_id) > env->hflags &= ~HF_CPL_MASK; > env->hflags |= (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; > > - /* XXX: restore FPU round state */ > env->fpstt = (env->fpus_vmstate >> 11) & 7; > env->fpus = env->fpus_vmstate & ~0x3800; > env->fptag_vmstate ^= 0xff; > for(i = 0; i < 8; i++) { > env->fptags[i] = (env->fptag_vmstate >> i) & 1; > } > + update_fp_status(env); > > cpu_breakpoint_remove_all(cs, BP_CPU); > cpu_watchpoint_remove_all(cs, BP_CPU); > Looks good. Richard, are you going to pick it up or shall I? Paolo
On 09/17/2014 02:16 AM, Paolo Bonzini wrote: > > Looks good. Richard, are you going to pick it up or shall I? Go ahead. r~
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 217500c..9f48b1c 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2572,7 +2572,7 @@ static void x86_cpu_reset(CPUState *s) for (i = 0; i < 8; i++) { env->fptags[i] = 1; } - env->fpuc = 0x37f; + cpu_set_fpuc(env, 0x37f); env->mxcsr = 0x1f80; env->xstate_bv = XSTATE_FP | XSTATE_SSE; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index e634d83..6ab8eca 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1249,6 +1249,7 @@ void QEMU_NORETURN raise_interrupt(CPUX86State *nenv, int intno, int is_int, /* cc_helper.c */ extern const uint8_t parity_table[256]; uint32_t cpu_cc_compute_all(CPUX86State *env1, int op); +void update_fp_status(CPUX86State *env); static inline uint32_t cpu_compute_eflags(CPUX86State *env) { @@ -1284,6 +1285,7 @@ static inline void cpu_load_efer(CPUX86State *env, uint64_t val) /* fpu_helper.c */ void cpu_set_mxcsr(CPUX86State *env, uint32_t val); +void cpu_set_fpuc(CPUX86State *env, uint16_t val); /* svm_helper.c */ void cpu_svm_check_intercept_param(CPUX86State *env1, uint32_t type, diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c index 1b2900d..1d4eee3 100644 --- a/target-i386/fpu_helper.c +++ b/target-i386/fpu_helper.c @@ -537,7 +537,7 @@ uint32_t helper_fnstcw(CPUX86State *env) return env->fpuc; } -static void update_fp_status(CPUX86State *env) +void update_fp_status(CPUX86State *env) { int rnd_type; @@ -575,8 +575,7 @@ static void update_fp_status(CPUX86State *env) void helper_fldcw(CPUX86State *env, uint32_t val) { - env->fpuc = val; - update_fp_status(env); + cpu_set_fpuc(env, val); } void helper_fclex(CPUX86State *env) @@ -595,7 +594,7 @@ void helper_fninit(CPUX86State *env) { env->fpus = 0; env->fpstt = 0; - env->fpuc = 0x37f; + cpu_set_fpuc(env, 0x37f); env->fptags[0] = 1; env->fptags[1] = 1; env->fptags[2] = 1; @@ -1013,11 +1012,11 @@ void helper_fldenv(CPUX86State *env, target_ulong ptr, int data32) int i, fpus, fptag; if (data32) { - env->fpuc = cpu_lduw_data(env, ptr); + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); fpus = cpu_lduw_data(env, ptr + 4); fptag = cpu_lduw_data(env, ptr + 8); } else { - env->fpuc = cpu_lduw_data(env, ptr); + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); fpus = cpu_lduw_data(env, ptr + 2); fptag = cpu_lduw_data(env, ptr + 4); } @@ -1046,7 +1045,7 @@ void helper_fsave(CPUX86State *env, target_ulong ptr, int data32) /* fninit */ env->fpus = 0; env->fpstt = 0; - env->fpuc = 0x37f; + cpu_set_fpuc(env, 0x37f); env->fptags[0] = 1; env->fptags[1] = 1; env->fptags[2] = 1; @@ -1157,7 +1156,7 @@ void helper_fxrstor(CPUX86State *env, target_ulong ptr, int data64) raise_exception(env, EXCP0D_GPF); } - env->fpuc = cpu_lduw_data(env, ptr); + cpu_set_fpuc(env, cpu_lduw_data(env, ptr)); fpus = cpu_lduw_data(env, ptr + 2); fptag = cpu_lduw_data(env, ptr + 4); env->fpstt = (fpus >> 11) & 7; @@ -1257,6 +1256,12 @@ void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr) set_flush_to_zero((mxcsr & SSE_FZ) ? 1 : 0, &env->fp_status); } +void cpu_set_fpuc(CPUX86State *env, uint16_t val) +{ + env->fpuc = val; + update_fp_status(env); +} + void helper_ldmxcsr(CPUX86State *env, uint32_t val) { cpu_set_mxcsr(env, val); diff --git a/target-i386/gdbstub.c b/target-i386/gdbstub.c index 19fe9ad..ff99cfb 100644 --- a/target-i386/gdbstub.c +++ b/target-i386/gdbstub.c @@ -203,7 +203,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) return x86_cpu_gdb_load_seg(cpu, R_GS, mem_buf); case IDX_FP_REGS + 8: - env->fpuc = ldl_p(mem_buf); + cpu_set_fpuc(env, ldl_p(mem_buf)); return 4; case IDX_FP_REGS + 9: tmp = ldl_p(mem_buf); diff --git a/target-i386/machine.c b/target-i386/machine.c index 16d2f6a..2922c97 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -315,13 +315,13 @@ static int cpu_post_load(void *opaque, int version_id) env->hflags &= ~HF_CPL_MASK; env->hflags |= (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; - /* XXX: restore FPU round state */ env->fpstt = (env->fpus_vmstate >> 11) & 7; env->fpus = env->fpus_vmstate & ~0x3800; env->fptag_vmstate ^= 0xff; for(i = 0; i < 8; i++) { env->fptags[i] = (env->fptag_vmstate >> i) & 1; } + update_fp_status(env); cpu_breakpoint_remove_all(cs, BP_CPU); cpu_watchpoint_remove_all(cs, BP_CPU);