On Fri, Nov 26, 2010 at 10:53 PM, ChALkeR <chalk...@gmail.com> wrote: >> Please also consider fixing FSAVE and FXSAVE. > > FSAVE also works, i checked twice (code and test). > FSAVE in QEMU calls FSTENV. > > op_helper.c: >> void helper_fsave(target_ulong ptr, int data32) >> { >> CPU86_LDouble tmp; >> int i; >> >> helper_fstenv(ptr, data32);
Sorry, I missed that. > Not sure how FXSAVE operates, that has to be checked in the manuals. > But the patch does not break anything and fixes FSTENV and FSAVE instructions. Yes, I just wish for more fixes. I think FRSTOR/FXRSTOR also need to update env->fpip etc. >> In this place, the data would be saved for every instruction. But I >> think only FPU instructions should update the fields. > > I do not understand. The patch stores eip only for FPU instructions. > This is correct. > FSTENV should put the eip of the last FPU instruction in the stack. > The helper is called at the end of every instruction on floating-point > case of the switch. > There should be some additional data, too, but the correct operation > of fpip is provided by the patch. > > translate.c, line 2461: >> /************************/ >> /* floats */ >> case 0xd8 ... 0xdf: I thought the store happened at the end of disas_insn(). Then this is OK. >> You can simply use tcg_gen_movi_tl() and tcg_gen_st_tl(), no need for >> a helper except for the data pointer. > > Can you fix it, please, if I did something wrong? It's not wrong but just less than optimal, there's some overhead with helper calls. Something like: tcg_gen_movi_tl(cpu_tmp0, pc_start - s->cs_base); tcg_gen_st_tl(cpu_tmp0, cpu_env, offsetof(CPUState, fpip)); Likewise for CS and instruction. Actually the data pointer also seems to be in A0, so we can also store that: tcg_gen_st_tl(cpu_A0, cpu_env, offsetof(CPUState, fpdp)); > Sorry for my bad English. No problem, I suppose most people on this list are not native English speakers anyway.