Message ID | 20230829152604.101542-1-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
Series | target/i386: raise FERR interrupt with iothread locked | expand |
On 29/8/23 17:25, 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> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/i386/tcg/sysemu/fpu_helper.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/i386/tcg/sysemu/fpu_helper.c b/target/i386/tcg/sysemu/fpu_helper.c > index 1c3610da3b9..fd8cc72a026 100644 > --- a/target/i386/tcg/sysemu/fpu_helper.c > +++ b/target/i386/tcg/sysemu/fpu_helper.c > @@ -31,7 +31,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; > } > } OK, so: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Maybe for completeness, squash this? : -- >8 -- diff --git a/target/i386/tcg/sysemu/fpu_helper.c b/target/i386/tcg/sysemu/fpu_helper.c index 1c3610da3b..320bd69f43 100644 --- a/target/i386/tcg/sysemu/fpu_helper.c +++ b/target/i386/tcg/sysemu/fpu_helper.c @@ -45,6 +45,8 @@ void cpu_clear_ignne(void) void cpu_set_ignne(void) { CPUX86State *env = &X86_CPU(first_cpu)->env; + + g_assert(qemu_mutex_iothread_locked()); env->hflags2 |= HF2_IGNNE_MASK; /* --- Somehow similar pattern so what about MIPS MTC0?: mtc0_compare() -> helper_mtc0_compare() -> cpu_mips_store_compare() -> qemu_irq_lower() Regards, Phil.
On Tue, Aug 29, 2023 at 5:46 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > On 29/8/23 17:25, 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> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > target/i386/tcg/sysemu/fpu_helper.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/target/i386/tcg/sysemu/fpu_helper.c b/target/i386/tcg/sysemu/fpu_helper.c > > index 1c3610da3b9..fd8cc72a026 100644 > > --- a/target/i386/tcg/sysemu/fpu_helper.c > > +++ b/target/i386/tcg/sysemu/fpu_helper.c > > @@ -31,7 +31,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; > > } > > } > > OK, so: > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Actually Richard has a better (though also slightly incomplete) patch in the bug, so I'll send v2. > void cpu_set_ignne(void) > { > CPUX86State *env = &X86_CPU(first_cpu)->env; > + > + g_assert(qemu_mutex_iothread_locked()); qemu_irq_lower() is fine because it doesn't result in a call to tcg_handle_interrupt(). Paolo > Somehow similar pattern so what about MIPS MTC0?: > > mtc0_compare() -> > helper_mtc0_compare() -> > cpu_mips_store_compare() -> > qemu_irq_lower() Lower is a bit safer
On Tue, 29 Aug 2023 at 17:59, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On Tue, Aug 29, 2023 at 5:46 PM Philippe Mathieu-Daudé > <philmd@linaro.org> wrote: > > > > On 29/8/23 17:25, 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> > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > --- > > > target/i386/tcg/sysemu/fpu_helper.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/target/i386/tcg/sysemu/fpu_helper.c b/target/i386/tcg/sysemu/fpu_helper.c > > > index 1c3610da3b9..fd8cc72a026 100644 > > > --- a/target/i386/tcg/sysemu/fpu_helper.c > > > +++ b/target/i386/tcg/sysemu/fpu_helper.c > > > @@ -31,7 +31,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; > > > } > > > } > > > > OK, so: > > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Actually Richard has a better (though also slightly incomplete) patch > in the bug, so I'll send v2. > > > void cpu_set_ignne(void) > > { > > CPUX86State *env = &X86_CPU(first_cpu)->env; > > + > > + g_assert(qemu_mutex_iothread_locked()); > > qemu_irq_lower() is fine because it doesn't result in a call to > tcg_handle_interrupt(). It does potentially result in calling code in a device model that assumes that it is protected by the iothread lock, though... -- PMM
diff --git a/target/i386/tcg/sysemu/fpu_helper.c b/target/i386/tcg/sysemu/fpu_helper.c index 1c3610da3b9..fd8cc72a026 100644 --- a/target/i386/tcg/sysemu/fpu_helper.c +++ b/target/i386/tcg/sysemu/fpu_helper.c @@ -31,7 +31,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; } }
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> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/i386/tcg/sysemu/fpu_helper.c | 2 ++ 1 file changed, 2 insertions(+)