qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] target/i386: two interrupt shadow fixes
@ 2025-05-02 15:41 Paolo Bonzini
  2025-05-02 15:41 ` [PATCH 1/2] target/i386: do not trigger IRQ shadow for LSS Paolo Bonzini
  2025-05-02 15:41 ` [PATCH 2/2] target/i386: do not block singlestep for STI Paolo Bonzini
  0 siblings, 2 replies; 3+ messages in thread
From: Paolo Bonzini @ 2025-05-02 15:41 UTC (permalink / raw)
  To: qemu-devel

LSS should not trigger the interrupt shadow, unlike MOV and POP to SS.

STI should trigger the interrupt shadow but only MOV and POP to SS
should block singlestep.

Paolo

Paolo Bonzini (2):
  target/i386: do not trigger IRQ shadow for LSS
  target/i386: do not block singlestep for STI

 target/i386/tcg/translate.c | 33 +++++++++++++++++++++------------
 target/i386/tcg/emit.c.inc  |  4 ++--
 2 files changed, 23 insertions(+), 14 deletions(-)

-- 
2.49.0



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

* [PATCH 1/2] target/i386: do not trigger IRQ shadow for LSS
  2025-05-02 15:41 [PATCH 0/2] target/i386: two interrupt shadow fixes Paolo Bonzini
@ 2025-05-02 15:41 ` Paolo Bonzini
  2025-05-02 15:41 ` [PATCH 2/2] target/i386: do not block singlestep for STI Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2025-05-02 15:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-stable

Because LSS need not trigger an IRQ shadow, gen_movl_seg can't just use
the destination register to decide whether to inhibit IRQs.  Add an
argument.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/translate.c | 27 ++++++++++++++++-----------
 target/i386/tcg/emit.c.inc  |  4 ++--
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 8a641951cd1..a4e935b043b 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2026,27 +2026,32 @@ static void gen_op_movl_seg_real(DisasContext *s, X86Seg seg_reg, TCGv seg)
 
 /* move SRC to seg_reg and compute if the CPU state may change. Never
    call this function with seg_reg == R_CS */
-static void gen_movl_seg(DisasContext *s, X86Seg seg_reg, TCGv src)
+static void gen_movl_seg(DisasContext *s, X86Seg seg_reg, TCGv src, bool inhibit_irq)
 {
     if (PE(s) && !VM86(s)) {
         TCGv_i32 sel = tcg_temp_new_i32();
 
         tcg_gen_trunc_tl_i32(sel, src);
         gen_helper_load_seg(tcg_env, tcg_constant_i32(seg_reg), sel);
-        /* abort translation because the addseg value may change or
-           because ss32 may change. For R_SS, translation must always
-           stop as a special handling must be done to disable hardware
-           interrupts for the next instruction */
-        if (seg_reg == R_SS) {
-            s->base.is_jmp = DISAS_EOB_INHIBIT_IRQ;
-        } else if (CODE32(s) && seg_reg < R_FS) {
+
+        /* For move to DS/ES/SS, the addseg or ss32 flags may change.  */
+        if (CODE32(s) && seg_reg < R_FS) {
             s->base.is_jmp = DISAS_EOB_NEXT;
         }
     } else {
         gen_op_movl_seg_real(s, seg_reg, src);
-        if (seg_reg == R_SS) {
-            s->base.is_jmp = DISAS_EOB_INHIBIT_IRQ;
-        }
+    }
+
+    /*
+     * For MOV or POP to SS (but not LSS) translation must always
+     * stop as a special handling must be done to disable hardware
+     * interrupts for the next instruction.
+     *
+     * DISAS_EOB_INHIBIT_IRQ is a superset of DISAS_EOB_NEXT which
+     * might have been set above.
+     */
+    if (inhibit_irq) {
+        s->base.is_jmp = DISAS_EOB_INHIBIT_IRQ;
     }
 }
 
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index e3166e70a5b..1a7fab9333a 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -342,7 +342,7 @@ static void gen_writeback(DisasContext *s, X86DecodedInsn *decode, int opn, TCGv
         break;
     case X86_OP_SEG:
         /* Note that gen_movl_seg takes care of interrupt shadow and TF.  */
-        gen_movl_seg(s, op->n, s->T0);
+        gen_movl_seg(s, op->n, v, op->n == R_SS);
         break;
     case X86_OP_INT:
         if (op->has_ea) {
@@ -2382,7 +2382,7 @@ static void gen_lxx_seg(DisasContext *s, X86DecodedInsn *decode, int seg)
     gen_op_ld_v(s, MO_16, s->T1, s->A0);
 
     /* load the segment here to handle exceptions properly */
-    gen_movl_seg(s, seg, s->T1);
+    gen_movl_seg(s, seg, s->T1, false);
 }
 
 static void gen_LDS(DisasContext *s, X86DecodedInsn *decode)
-- 
2.49.0



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

* [PATCH 2/2] target/i386: do not block singlestep for STI
  2025-05-02 15:41 [PATCH 0/2] target/i386: two interrupt shadow fixes Paolo Bonzini
  2025-05-02 15:41 ` [PATCH 1/2] target/i386: do not trigger IRQ shadow for LSS Paolo Bonzini
@ 2025-05-02 15:41 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2025-05-02 15:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-stable

STI will trigger a singlestep exception even if it has inhibit-IRQ
behavior.  Do not suppress single-step for all IRQ-inhibiting
instructions, instead special case MOV SS and POP SS.

Cc: qemu-stable@nongnu.org
Fixes: f0f0136abba ("target/i386: no single-step exception after MOV or POP SS", 2024-05-25)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/translate.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index a4e935b043b..ed43c95c1d9 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2047,11 +2047,15 @@ static void gen_movl_seg(DisasContext *s, X86Seg seg_reg, TCGv src, bool inhibit
      * stop as a special handling must be done to disable hardware
      * interrupts for the next instruction.
      *
+     * This is the last instruction, so it's okay to overwrite
+     * HF_TF_MASK; the next TB will start with the flag set.
+     *
      * DISAS_EOB_INHIBIT_IRQ is a superset of DISAS_EOB_NEXT which
      * might have been set above.
      */
     if (inhibit_irq) {
         s->base.is_jmp = DISAS_EOB_INHIBIT_IRQ;
+        s->flags &= ~HF_TF_MASK;
     }
 }
 
@@ -2302,7 +2306,7 @@ gen_eob(DisasContext *s, int mode)
     if (mode == DISAS_EOB_RECHECK_TF) {
         gen_helper_rechecking_single_step(tcg_env);
         tcg_gen_exit_tb(NULL, 0);
-    } else if ((s->flags & HF_TF_MASK) && mode != DISAS_EOB_INHIBIT_IRQ) {
+    } else if (s->flags & HF_TF_MASK) {
         gen_helper_single_step(tcg_env);
     } else if (mode == DISAS_JUMP &&
                /* give irqs a chance to happen */
-- 
2.49.0



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

end of thread, other threads:[~2025-05-02 15:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-02 15:41 [PATCH 0/2] target/i386: two interrupt shadow fixes Paolo Bonzini
2025-05-02 15:41 ` [PATCH 1/2] target/i386: do not trigger IRQ shadow for LSS Paolo Bonzini
2025-05-02 15:41 ` [PATCH 2/2] target/i386: do not block singlestep for STI Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).