Thanks Richard for your feedback. I am going to correct the patch and
resubmit it.
Best regards,
Jaume
On Sun, Jun 22, 2014 at 8:55 PM, Richard Henderson <r...@twiddle.net> wrote:
> On 06/22/2014 07:55 AM, Jaume Martí wrote:
>> - cpu_x86_fsave(env, fpstate_addr, 1);
>> - fpstate->status = fpstate->sw;
>> - magic = 0xffff;
>> + cpu_x86_fsave(env, fpstate_addr);
>> + fpstate->status = fpstate->sw;
>> + magic = 0xffff;
>
> This patch needs to be split into format fixes and the actual change to be
> reviewed.
>
>> - /* KVM-only so far */
>> - uint16_t fpop;
>> + union {
>> + uint32_t tcg;
>> + uint16_t kvm;
>> + } fpop;
>
> This is highly questionable.
>
>> .fields = (VMStateField[]) {
>> - VMSTATE_UINT16(env.fpop, X86CPU),
>> + VMSTATE_UINT16(env.fpop.kvm, X86CPU),
>
> You're breaking save/restore in tcg. KVM is not required for migration.
>
>> + if (non_control_x87_instr(modrm, b)) {
>> + tcg_gen_movi_i32(cpu_fpop, ((b & 0x7) << 8) | (modrm & 0xff));
>> + tcg_gen_movi_tl(cpu_fpip, pc_start - s->cs_base);
>> + tcg_gen_movi_i32(cpu_fpcs, env->segs[R_CS].selector);
>> + }
>
> I strongly suspect you can implement this feature without having to add 3
> (largely redundant) register writes to every x87 instruction executed.
>
> See how restore_state_to_opc works to compute the value of CC_OP during
> translation. You can do the same thing to recover these three values.
>
> You do have to sync these values before normal exits from the TB, but you only
> have to do that once, not once for every insn executed. See gen_update_cc_op.
>
>
> r~
