Re: [Qemu-devel] [PATCH] target-i386: Only provide CMOV and friends if feature bit set

[Richard Henderson <rth@twiddle.net>]

On 08/20/2013 05:59 AM, Peter Maydell wrote:
> Ping^2! This has been reviewed and I've checked that the
> patch still applies to master.

Reviewed-by: Richard Henderson <rth@twiddle.net>

> 
> thanks
> -- PMM
> 
> On 25 July 2013 17:54, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Ping!
>>
>> (patchwork url: http://patchwork.ozlabs.org/patch/259148/)
>>
>> thanks
>> -- PMM
>>
>> On 15 July 2013 18:21, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> The instructions CMOVcc, FCMOVcc and F[U]COMI[P] should only be
>>> present if the CMOV feature bit is set. Add missing feature bit
>>> checks so we correctly fault if emulating a 486 or 586.
>>> This fixes bug LP:1201446.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>  target-i386/translate.c |   19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/target-i386/translate.c b/target-i386/translate.c
>>> index 6550c27..f75e3b1 100644
>>> --- a/target-i386/translate.c
>>> +++ b/target-i386/translate.c
>>> @@ -6434,12 +6434,18 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>>>                  }
>>>                  break;
>>>              case 0x1d: /* fucomi */
>>> +                if (!(s->cpuid_features & CPUID_CMOV)) {
>>> +                    goto illegal_op;
>>> +                }
>>>                  gen_update_cc_op(s);
>>>                  gen_helper_fmov_FT0_STN(cpu_env, tcg_const_i32(opreg));
>>>                  gen_helper_fucomi_ST0_FT0(cpu_env);
>>>                  set_cc_op(s, CC_OP_EFLAGS);
>>>                  break;
>>>              case 0x1e: /* fcomi */
>>> +                if (!(s->cpuid_features & CPUID_CMOV)) {
>>> +                    goto illegal_op;
>>> +                }
>>>                  gen_update_cc_op(s);
>>>                  gen_helper_fmov_FT0_STN(cpu_env, tcg_const_i32(opreg));
>>>                  gen_helper_fcomi_ST0_FT0(cpu_env);
>>> @@ -6495,6 +6501,9 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>>>                  }
>>>                  break;
>>>              case 0x3d: /* fucomip */
>>> +                if (!(s->cpuid_features & CPUID_CMOV)) {
>>> +                    goto illegal_op;
>>> +                }
>>>                  gen_update_cc_op(s);
>>>                  gen_helper_fmov_FT0_STN(cpu_env, tcg_const_i32(opreg));
>>>                  gen_helper_fucomi_ST0_FT0(cpu_env);
>>> @@ -6502,6 +6511,9 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>>>                  set_cc_op(s, CC_OP_EFLAGS);
>>>                  break;
>>>              case 0x3e: /* fcomip */
>>> +                if (!(s->cpuid_features & CPUID_CMOV)) {
>>> +                    goto illegal_op;
>>> +                }
>>>                  gen_update_cc_op(s);
>>>                  gen_helper_fmov_FT0_STN(cpu_env, tcg_const_i32(opreg));
>>>                  gen_helper_fcomi_ST0_FT0(cpu_env);
>>> @@ -6518,6 +6530,10 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>>>                          (JCC_BE << 1),
>>>                          (JCC_P << 1),
>>>                      };
>>> +
>>> +                    if (!(s->cpuid_features & CPUID_CMOV)) {
>>> +                        goto illegal_op;
>>> +                    }
>>>                      op1 = fcmov_cc[op & 3] | (((op >> 3) & 1) ^ 1);
>>>                      l1 = gen_new_label();
>>>                      gen_jcc1_noeob(s, op1, l1);
>>> @@ -6889,6 +6905,9 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>>>          gen_ldst_modrm(env, s, modrm, OT_BYTE, OR_TMP0, 1);
>>>          break;
>>>      case 0x140 ... 0x14f: /* cmov Gv, Ev */
>>> +        if (!(s->cpuid_features & CPUID_CMOV)) {
>>> +            goto illegal_op;
>>> +        }
>>>          ot = dflag + OT_WORD;
>>>          modrm = cpu_ldub_code(env, s->pc++);
>>>          reg = ((modrm >> 3) & 7) | rex_r;
>>> --
>>> 1.7.9.5
>>>
>>>