[7/7] target-i386: Fix inhibit irq mask handling
diff mbox

Message ID 1456983051-14707-8-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson March 3, 2016, 5:30 a.m. UTC
The patch in 7f0b714 was too simplistic, in that we wound up setting
the flag and then resetting it immediately in gen_eob.

Fixes the reported boot problem with Windows XP.

Reported-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 76 ++++++++++++++++++++++++-------------------------
 1 file changed, 37 insertions(+), 39 deletions(-)

Comments

Paolo Bonzini March 3, 2016, 8:46 a.m. UTC | #1
On 03/03/2016 06:30, Richard Henderson wrote:
> The patch in 7f0b714 was too simplistic, in that we wound up setting
> the flag and then resetting it immediately in gen_eob.
> 
> Fixes the reported boot problem with Windows XP.
> 
> Reported-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-i386/translate.c | 76 ++++++++++++++++++++++++-------------------------
>  1 file changed, 37 insertions(+), 39 deletions(-)

Oops, I forgot to Cc you on my fix for this:

http://permalink.gmane.org/gmane.comp.emulators.qemu/397645

Feel free to pick the one you prefer and send a pull request!

Thanks,

Paolo

> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 7455a18..513b53a 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -2441,12 +2441,19 @@ static void gen_bnd_jmp(DisasContext *s)
>      }
>  }
>  
> -/* generate a generic end of block. Trace exception is also generated
> -   if needed */
> -static void gen_eob(DisasContext *s)
> +/* Generate an end of block. Trace exception is also generated if needed.
> +   If IIM, set HF_INHIBIT_IRQ_MASK if it isn't already set.  */
> +static void gen_eob_inhibit_irq(DisasContext *s, bool inhibit)
>  {
>      gen_update_cc_op(s);
> -    gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
> +
> +    /* If several instructions disable interrupts, only the first does it.  */
> +    if (inhibit && !(s->flags & HF_INHIBIT_IRQ_MASK)) {
> +        gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
> +    } else {
> +        gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
> +    }
> +
>      if (s->tb->flags & HF_RF_MASK) {
>          gen_helper_reset_rf(cpu_env);
>      }
> @@ -2460,6 +2467,12 @@ static void gen_eob(DisasContext *s)
>      s->is_jmp = DISAS_TB_JUMP;
>  }
>  
> +/* End of block, resetting the inhibit irq flag.  */
> +static void gen_eob(DisasContext *s)
> +{
> +    gen_eob_inhibit_irq(s, false);
> +}
> +
>  /* generate a jump to eip. No segment change must happen before as a
>     direct call to the next block may occur */
>  static void gen_jmp_tb(DisasContext *s, target_ulong eip, int tb_num)
> @@ -5193,16 +5206,15 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>          ot = gen_pop_T0(s);
>          gen_movl_seg_T0(s, reg);
>          gen_pop_update(s, ot);
> -        if (reg == R_SS) {
> -            /* if reg == SS, inhibit interrupts/trace. */
> -            /* If several instructions disable interrupts, only the
> -               _first_ does it */
> -            gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
> -            s->tf = 0;
> -        }
> +        /* Note that reg == R_SS in gen_movl_seg_T0 always sets is_jmp.  */
>          if (s->is_jmp) {
>              gen_jmp_im(s->pc - s->cs_base);
> -            gen_eob(s);
> +            if (reg == R_SS) {
> +                s->tf = 0;
> +                gen_eob_inhibit_irq(s, true);
> +            } else {
> +                gen_eob(s);
> +            }
>          }
>          break;
>      case 0x1a1: /* pop fs */
> @@ -5260,16 +5272,15 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>              goto illegal_op;
>          gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
>          gen_movl_seg_T0(s, reg);
> -        if (reg == R_SS) {
> -            /* if reg == SS, inhibit interrupts/trace */
> -            /* If several instructions disable interrupts, only the
> -               _first_ does it */
> -            gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
> -            s->tf = 0;
> -        }
> +        /* Note that reg == R_SS in gen_movl_seg_T0 always sets is_jmp.  */
>          if (s->is_jmp) {
>              gen_jmp_im(s->pc - s->cs_base);
> -            gen_eob(s);
> +            if (reg == R_SS) {
> +                s->tf = 0;
> +                gen_eob_inhibit_irq(s, true);
> +            } else {
> +                gen_eob(s);
> +            }
>          }
>          break;
>      case 0x8c: /* mov Gv, seg */
> @@ -6795,26 +6806,13 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>          }
>          break;
>      case 0xfb: /* sti */
> -        if (!s->vm86) {
> -            if (s->cpl <= s->iopl) {
> -            gen_sti:
> -                gen_helper_sti(cpu_env);
> -                /* interruptions are enabled only the first insn after sti */
> -                /* If several instructions disable interrupts, only the
> -                   _first_ does it */
> -                gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
> -                /* give a chance to handle pending irqs */
> -                gen_jmp_im(s->pc - s->cs_base);
> -                gen_eob(s);
> -            } else {
> -                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
> -            }
> +        if (s->vm86 ? s->iopl == 3 : s->cpl <= s->iopl) {
> +            gen_helper_sti(cpu_env);
> +            /* interruptions are enabled only the first insn after sti */
> +            gen_jmp_im(s->pc - s->cs_base);
> +            gen_eob_inhibit_irq(s, true);
>          } else {
> -            if (s->iopl == 3) {
> -                goto gen_sti;
> -            } else {
> -                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
> -            }
> +            gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
>          }
>          break;
>      case 0x62: /* bound */
>

Patch
diff mbox

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 7455a18..513b53a 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2441,12 +2441,19 @@  static void gen_bnd_jmp(DisasContext *s)
     }
 }
 
-/* generate a generic end of block. Trace exception is also generated
-   if needed */
-static void gen_eob(DisasContext *s)
+/* Generate an end of block. Trace exception is also generated if needed.
+   If IIM, set HF_INHIBIT_IRQ_MASK if it isn't already set.  */
+static void gen_eob_inhibit_irq(DisasContext *s, bool inhibit)
 {
     gen_update_cc_op(s);
-    gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
+
+    /* If several instructions disable interrupts, only the first does it.  */
+    if (inhibit && !(s->flags & HF_INHIBIT_IRQ_MASK)) {
+        gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
+    } else {
+        gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
+    }
+
     if (s->tb->flags & HF_RF_MASK) {
         gen_helper_reset_rf(cpu_env);
     }
@@ -2460,6 +2467,12 @@  static void gen_eob(DisasContext *s)
     s->is_jmp = DISAS_TB_JUMP;
 }
 
+/* End of block, resetting the inhibit irq flag.  */
+static void gen_eob(DisasContext *s)
+{
+    gen_eob_inhibit_irq(s, false);
+}
+
 /* generate a jump to eip. No segment change must happen before as a
    direct call to the next block may occur */
 static void gen_jmp_tb(DisasContext *s, target_ulong eip, int tb_num)
@@ -5193,16 +5206,15 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         ot = gen_pop_T0(s);
         gen_movl_seg_T0(s, reg);
         gen_pop_update(s, ot);
-        if (reg == R_SS) {
-            /* if reg == SS, inhibit interrupts/trace. */
-            /* If several instructions disable interrupts, only the
-               _first_ does it */
-            gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
-            s->tf = 0;
-        }
+        /* Note that reg == R_SS in gen_movl_seg_T0 always sets is_jmp.  */
         if (s->is_jmp) {
             gen_jmp_im(s->pc - s->cs_base);
-            gen_eob(s);
+            if (reg == R_SS) {
+                s->tf = 0;
+                gen_eob_inhibit_irq(s, true);
+            } else {
+                gen_eob(s);
+            }
         }
         break;
     case 0x1a1: /* pop fs */
@@ -5260,16 +5272,15 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             goto illegal_op;
         gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
         gen_movl_seg_T0(s, reg);
-        if (reg == R_SS) {
-            /* if reg == SS, inhibit interrupts/trace */
-            /* If several instructions disable interrupts, only the
-               _first_ does it */
-            gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
-            s->tf = 0;
-        }
+        /* Note that reg == R_SS in gen_movl_seg_T0 always sets is_jmp.  */
         if (s->is_jmp) {
             gen_jmp_im(s->pc - s->cs_base);
-            gen_eob(s);
+            if (reg == R_SS) {
+                s->tf = 0;
+                gen_eob_inhibit_irq(s, true);
+            } else {
+                gen_eob(s);
+            }
         }
         break;
     case 0x8c: /* mov Gv, seg */
@@ -6795,26 +6806,13 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         }
         break;
     case 0xfb: /* sti */
-        if (!s->vm86) {
-            if (s->cpl <= s->iopl) {
-            gen_sti:
-                gen_helper_sti(cpu_env);
-                /* interruptions are enabled only the first insn after sti */
-                /* If several instructions disable interrupts, only the
-                   _first_ does it */
-                gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
-                /* give a chance to handle pending irqs */
-                gen_jmp_im(s->pc - s->cs_base);
-                gen_eob(s);
-            } else {
-                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-            }
+        if (s->vm86 ? s->iopl == 3 : s->cpl <= s->iopl) {
+            gen_helper_sti(cpu_env);
+            /* interruptions are enabled only the first insn after sti */
+            gen_jmp_im(s->pc - s->cs_base);
+            gen_eob_inhibit_irq(s, true);
         } else {
-            if (s->iopl == 3) {
-                goto gen_sti;
-            } else {
-                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-            }
+            gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         }
         break;
     case 0x62: /* bound */