[PATCH v2] target/i386: raise FERR interrupt with iothread locked

Paolo Bonzini posted 1 patch 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230830103141.138775-1-pbonzini@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
target/i386/tcg/sysemu/fpu_helper.c | 6 ++++++
target/i386/tcg/translate.c         | 8 +++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
[PATCH v2] target/i386: raise FERR interrupt with iothread locked
Posted by Paolo Bonzini 8 months ago
Otherwise tcg_handle_interrupt() triggers an assertion failure:

  #5  0x0000555555c97369 in tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:83
  #6  tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:81
  #7  0x0000555555b4d58b in pic_irq_request (opaque=<optimized out>, irq=<optimized out>, level=1) at ../hw/i386/x86.c:555
  #8  0x0000555555b4f218 in gsi_handler (opaque=0x5555579423d0, n=13, level=1) at ../hw/i386/x86.c:611
  #9  0x00007fffa42bde14 in code_gen_buffer ()
  #10 0x0000555555c724bb in cpu_tb_exec (cpu=cpu@entry=0x555557434cb0, itb=<optimized out>, tb_exit=tb_exit@entry=0x7fffe9bfd658) at ../accel/tcg/cpu-exec.c:457

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1808
Reported-by: NyanCatTW1 <https://gitlab.com/a0939712328>
Co-developed-by: Richard Henderson <richard.henderson@linaro.org>'
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/sysemu/fpu_helper.c | 6 ++++++
 target/i386/tcg/translate.c         | 8 +++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/target/i386/tcg/sysemu/fpu_helper.c b/target/i386/tcg/sysemu/fpu_helper.c
index 1c3610da3b9..93506cdd94e 100644
--- a/target/i386/tcg/sysemu/fpu_helper.c
+++ b/target/i386/tcg/sysemu/fpu_helper.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 #include "cpu.h"
 #include "hw/irq.h"
 
@@ -31,7 +32,9 @@ void x86_register_ferr_irq(qemu_irq irq)
 void fpu_check_raise_ferr_irq(CPUX86State *env)
 {
     if (ferr_irq && !(env->hflags2 & HF2_IGNNE_MASK)) {
+        qemu_mutex_lock_iothread();
         qemu_irq_raise(ferr_irq);
+        qemu_mutex_unlock_iothread();
         return;
     }
 }
@@ -45,6 +48,9 @@ void cpu_clear_ignne(void)
 void cpu_set_ignne(void)
 {
     CPUX86State *env = &X86_CPU(first_cpu)->env;
+
+    assert(qemu_mutex_iothread_locked());
+
     env->hflags2 |= HF2_IGNNE_MASK;
     /*
      * We get here in response to a write to port F0h.  The chipset should
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 90c7b32f362..e0a622941cb 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -4619,7 +4619,11 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
                 case 0x0a: /* grp d9/2 */
                     switch (rm) {
                     case 0: /* fnop */
-                        /* check exceptions (FreeBSD FPU probe) */
+                        /*
+                         * check exceptions (FreeBSD FPU probe)
+                         * needs to be treated as I/O because of ferr_irq
+                         */
+                        translator_io_start(&s->base);
                         gen_helper_fwait(cpu_env);
                         update_fip = false;
                         break;
@@ -5548,6 +5552,8 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
             (HF_MP_MASK | HF_TS_MASK)) {
             gen_exception(s, EXCP07_PREX);
         } else {
+            /* needs to be treated as I/O because of ferr_irq */
+            translator_io_start(&s->base);
             gen_helper_fwait(cpu_env);
         }
         break;
-- 
2.41.0
Re: [PATCH v2] target/i386: raise FERR interrupt with iothread locked
Posted by Richard Henderson 8 months ago
On 8/30/23 03:31, Paolo Bonzini wrote:
> Otherwise tcg_handle_interrupt() triggers an assertion failure:
> 
>    #5  0x0000555555c97369 in tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:83
>    #6  tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:81
>    #7  0x0000555555b4d58b in pic_irq_request (opaque=<optimized out>, irq=<optimized out>, level=1) at ../hw/i386/x86.c:555
>    #8  0x0000555555b4f218 in gsi_handler (opaque=0x5555579423d0, n=13, level=1) at ../hw/i386/x86.c:611
>    #9  0x00007fffa42bde14 in code_gen_buffer ()
>    #10 0x0000555555c724bb in cpu_tb_exec (cpu=cpu@entry=0x555557434cb0, itb=<optimized out>, tb_exit=tb_exit@entry=0x7fffe9bfd658) at ../accel/tcg/cpu-exec.c:457
> 
> Cc:qemu-stable@nongnu.org
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1808
> Reported-by: NyanCatTW1<https://gitlab.com/a0939712328>
> Co-developed-by: Richard Henderson<richard.henderson@linaro.org>'
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   target/i386/tcg/sysemu/fpu_helper.c | 6 ++++++
>   target/i386/tcg/translate.c         | 8 +++++++-
>   2 files changed, 13 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Re: [PATCH v2] target/i386: raise FERR interrupt with iothread locked
Posted by Philippe Mathieu-Daudé 8 months ago
On 30/8/23 12:31, Paolo Bonzini wrote:
> Otherwise tcg_handle_interrupt() triggers an assertion failure:
> 
>    #5  0x0000555555c97369 in tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:83
>    #6  tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:81
>    #7  0x0000555555b4d58b in pic_irq_request (opaque=<optimized out>, irq=<optimized out>, level=1) at ../hw/i386/x86.c:555
>    #8  0x0000555555b4f218 in gsi_handler (opaque=0x5555579423d0, n=13, level=1) at ../hw/i386/x86.c:611
>    #9  0x00007fffa42bde14 in code_gen_buffer ()
>    #10 0x0000555555c724bb in cpu_tb_exec (cpu=cpu@entry=0x555557434cb0, itb=<optimized out>, tb_exit=tb_exit@entry=0x7fffe9bfd658) at ../accel/tcg/cpu-exec.c:457
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1808
> Reported-by: NyanCatTW1 <https://gitlab.com/a0939712328>
> Co-developed-by: Richard Henderson <richard.henderson@linaro.org>'
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/sysemu/fpu_helper.c | 6 ++++++
>   target/i386/tcg/translate.c         | 8 +++++++-
>   2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/tcg/sysemu/fpu_helper.c b/target/i386/tcg/sysemu/fpu_helper.c
> index 1c3610da3b9..93506cdd94e 100644
> --- a/target/i386/tcg/sysemu/fpu_helper.c
> +++ b/target/i386/tcg/sysemu/fpu_helper.c
> @@ -18,6 +18,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
>   #include "cpu.h"
>   #include "hw/irq.h"
>   
> @@ -31,7 +32,9 @@ void x86_register_ferr_irq(qemu_irq irq)
>   void fpu_check_raise_ferr_irq(CPUX86State *env)
>   {
>       if (ferr_irq && !(env->hflags2 & HF2_IGNNE_MASK)) {
> +        qemu_mutex_lock_iothread();
>           qemu_irq_raise(ferr_irq);
> +        qemu_mutex_unlock_iothread();
>           return;
>       }
>   }
> @@ -45,6 +48,9 @@ void cpu_clear_ignne(void)
>   void cpu_set_ignne(void)
>   {
>       CPUX86State *env = &X86_CPU(first_cpu)->env;
> +
> +    assert(qemu_mutex_iothread_locked());
> +
>       env->hflags2 |= HF2_IGNNE_MASK;
>       /*
>        * We get here in response to a write to port F0h.  The chipset should
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 90c7b32f362..e0a622941cb 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -4619,7 +4619,11 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>                   case 0x0a: /* grp d9/2 */
>                       switch (rm) {
>                       case 0: /* fnop */
> -                        /* check exceptions (FreeBSD FPU probe) */
> +                        /*
> +                         * check exceptions (FreeBSD FPU probe)
> +                         * needs to be treated as I/O because of ferr_irq
> +                         */
> +                        translator_io_start(&s->base);

To the best of my knowledge:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>                           gen_helper_fwait(cpu_env);
>                           update_fip = false;
>                           break;
> @@ -5548,6 +5552,8 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>               (HF_MP_MASK | HF_TS_MASK)) {
>               gen_exception(s, EXCP07_PREX);
>           } else {
> +            /* needs to be treated as I/O because of ferr_irq */
> +            translator_io_start(&s->base);
>               gen_helper_fwait(cpu_env);
>           }
>           break;