Re: [Qemu-devel] PATCH for bugs 661696 and 1248376: target-i386: x87 exception pointers using TCG.

[Richard Henderson]

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~