target-i386: fix interrupt shadow

Commit Message

Paolo Bonzini March 1, 2016, 2:09 p.m. UTC

The handling of the interrupt shadow is subtle.  QEMU's check to stop the
interrupt shadow needs to check the state after the _penultimate_
instruction.  Because the interrupt shadow is only enabled at the end of
a translation block, and it makes the next, the state at the
penultimate instruction is stored in the current translation block's
flags.  Fix gen_eob to check it correctly.

This fixes Windows XP.

Reported-by: Hervé Poussineau <[e-mail]>
Fixes: 7f0b7141b4c7deab51efd8ee1e83eab2d9b7a9ea
Signed-off-by: Paolo Bonzini <[e-mail]>
---
 target-i386/translate.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Hervé Poussineau March 1, 2016, 7:41 p.m. UTC | #1

Le 01/03/2016 15:09, Paolo Bonzini a écrit :
> The handling of the interrupt shadow is subtle.  QEMU's check to stop the
> interrupt shadow needs to check the state after the _penultimate_
> instruction.  Because the interrupt shadow is only enabled at the end of
> a translation block, and it makes the next, the state at the
> penultimate instruction is stored in the current translation block's
> flags.  Fix gen_eob to check it correctly.
>
> This fixes Windows XP.
>
> Reported-by: Hervé Poussineau <[e-mail]>
> Fixes: 7f0b7141b4c7deab51efd8ee1e83eab2d9b7a9ea
> Signed-off-by: Paolo Bonzini <[e-mail]>

Tested-by: Hervé Poussineau <[e-mail]>

Patch

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 9171929..0ed2ee9 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2424,7 +2424,14 @@  static void gen_bnd_jmp(DisasContext *s)
 static void gen_eob(DisasContext *s)
 {
     gen_update_cc_op(s);
-    gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
+    /* If the last instruction of the previous block inhibited IRQ,
+     * re-enable interrupts here.  The interrupt shadow never lasts
+     * more than one instruction, hence interrupts must always be
+     * re-enabled by the one-instruction tb with HF_INHIBIT_IRQ_MASK.
+     */
+    if ((s->tb->flags & HF_INHIBIT_IRQ_MASK) != 0) {
+        gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
+    }
     if (s->tb->flags & HF_RF_MASK) {
         gen_helper_reset_rf(cpu_env);
     }
@@ -5173,8 +5180,6 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         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;
         }
@@ -5240,8 +5245,6 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         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;
         }
@@ -6778,8 +6781,6 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             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);