Re: [Xen-devel] [PATCH v3 14/25] x86emul: make all FPU emulation use the stub

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/12/17 14:09, Jan Beulich wrote:
> While this means quite some reduction of (source) code, the main
> purpose is to no longer have exceptions raised from other than stubs.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> for the reduction
alone, but with a recommendation.

> @@ -4266,37 +4262,13 @@ x86_emulate(
>              emulate_fpu_insn_stub(0xd8, modrm);
>              break;
>          default:
> +        fpu_memsrc32:
>              ASSERT(ea.type == OP_MEM);
>              if ( (rc = ops->read(ea.mem.seg, ea.mem.off, &src.val,
>                                   4, ctxt)) != X86EMUL_OKAY )
>                  goto done;
> -            switch ( modrm_reg & 7 )
> -            {
> -            case 0: /* fadd */
> -                emulate_fpu_insn_memsrc("fadds", src.val);
> -                break;
> -            case 1: /* fmul */
> -                emulate_fpu_insn_memsrc("fmuls", src.val);
> -                break;
> -            case 2: /* fcom */
> -                emulate_fpu_insn_memsrc("fcoms", src.val);
> -                break;
> -            case 3: /* fcomp */
> -                emulate_fpu_insn_memsrc("fcomps", src.val);
> -                break;
> -            case 4: /* fsub */
> -                emulate_fpu_insn_memsrc("fsubs", src.val);
> -                break;
> -            case 5: /* fsubr */
> -                emulate_fpu_insn_memsrc("fsubrs", src.val);
> -                break;
> -            case 6: /* fdiv */
> -                emulate_fpu_insn_memsrc("fdivs", src.val);
> -                break;
> -            case 7: /* fdivr */
> -                emulate_fpu_insn_memsrc("fdivrs", src.val);
> -                break;
> -            }
> +            emulate_fpu_insn_memsrc(b, modrm_reg, src.val);

The modrm_reg & 7 should be visible here to make it obvious that the rex
prefix if any is dropped.  It is probably best to duplicate up the &7
because the one inside the macro is for encoding safety, and the
compiler can trivially combine the two.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel