Re: [Qemu-devel] [PATCH] target/i386: Fixed CR0.TS check in gen_sse

[Paolo Bonzini <pbonzini@redhat.com>]

On 29/05/2018 12:19, Alexandro Sanchez Bach wrote:
> The function `gen_sse` assumes all of its instructions require CR0.TS=0.
> However, integer extensions at `0F 38 F[0-F]` and `0F 3A F[0-F]` such as
> CRC32, MOVBE, ADX, BMI1, BMI2 that are handled by `gen_sse` are not supposed
> to throw an exception in this scenario. This causes issues while booting
> some FreeBSD-based guests.
> 
> Reported-by: Alexandro Sanchez Bach <alexandro@phi.nz>
> Signed-off-by: Alexandro Sanchez Bach <alexandro@phi.nz>
> Cc: qemu-stable@nongnu.org
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 7c21814676..079ab7afef 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -3049,8 +3049,16 @@ static void gen_sse(CPUX86State *env, DisasContext
> *s, int b,
>              is_xmm = 1;
>          }
>      }
> +
> +    modrm = x86_ldub_code(env, s);
> +    reg = ((modrm >> 3) & 7);
> +    if (is_xmm)
> +        reg |= rex_r;
> +    mod = (modrm >> 6) & 3;

The problem here is that EMMS and FEMMS instructions do not have a
mod/rm byte.  However, this change should apply easily on top of another
patch to move EMMS and FEMMS out of gen_sse.

I'll queue the patch, and wait applying it until I have time to do that
other change (or someone else does it ;)).

Thanks,

Paolo

>      /* simple MMX/SSE operation */
> -    if (s->flags & HF_TS_MASK) {
> +    if (s->flags & HF_TS_MASK
> +        && ((b != 0x38 && b != 0x3A) || !(modrm & 0xF0))) {
>          gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
>          return;
>      }
> @@ -3084,11 +3092,6 @@ static void gen_sse(CPUX86State *env, DisasContext
> *s, int b,
>          gen_helper_enter_mmx(cpu_env);
>      }
>  
> -    modrm = x86_ldub_code(env, s);
> -    reg = ((modrm >> 3) & 7);
> -    if (is_xmm)
> -        reg |= rex_r;
> -    mod = (modrm >> 6) & 3;
>      if (sse_fn_epp == SSE_SPECIAL) {
>          b |= (b1 << 8);
>          switch(b) {
> 
> 
>