All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] target/i386: Fixed CR0.TS check in gen_sse
@ 2018-05-29 10:19 Alexandro Sanchez Bach
  2018-06-27  9:30 ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandro Sanchez Bach @ 2018-05-29 10:19 UTC (permalink / raw)
  To: qemu-devel

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;
+
     /* 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) {

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] target/i386: Fixed CR0.TS check in gen_sse
  2018-05-29 10:19 [Qemu-devel] [PATCH] target/i386: Fixed CR0.TS check in gen_sse Alexandro Sanchez Bach
@ 2018-06-27  9:30 ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2018-06-27  9:30 UTC (permalink / raw)
  To: Alexandro Sanchez Bach, qemu-devel

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) {
> 
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] target/i386: Fixed CR0.TS check in gen_sse
  2018-06-26 17:39 Alexandro Sanchez Bach
@ 2018-06-26 17:51 ` Peter Maydell
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2018-06-26 17:51 UTC (permalink / raw)
  To: Alexandro Sanchez Bach
  Cc: QEMU Developers, Paolo Bonzini, Richard Henderson, Eduardo Habkost

Ccing the target/i386 maintainers...

thanks
-- PMM

On 26 June 2018 at 18:39, Alexandro Sanchez Bach <alexandro@phi.nz> wrote:
> Ping.
>
> -----Original Message-----
> From: Alexandro Sanchez Bach <alexandro@phi.nz>
> Sent: Tuesday, May 29, 2018 12:20
> To: 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> Subject: [PATCH] target/i386: Fixed CR0.TS check in gen_sse
>
> 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;
> +
>      /* 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) {
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] target/i386: Fixed CR0.TS check in gen_sse
@ 2018-06-26 17:39 Alexandro Sanchez Bach
  2018-06-26 17:51 ` Peter Maydell
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandro Sanchez Bach @ 2018-06-26 17:39 UTC (permalink / raw)
  To: qemu-devel, 'Paolo Bonzini', peter.maydell

Ping.

-----Original Message-----
From: Alexandro Sanchez Bach <alexandro@phi.nz> 
Sent: Tuesday, May 29, 2018 12:20
To: 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
Subject: [PATCH] target/i386: Fixed CR0.TS check in gen_sse

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;
+
     /* 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) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-06-27  9:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-29 10:19 [Qemu-devel] [PATCH] target/i386: Fixed CR0.TS check in gen_sse Alexandro Sanchez Bach
2018-06-27  9:30 ` Paolo Bonzini
2018-06-26 17:39 Alexandro Sanchez Bach
2018-06-26 17:51 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.