[Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Igor Mammedov]

(PS: resend due to wrong qemu-devel mail list address in original email)


Regression bisects to:

commit 8d04fb55dec381bc5105cb47f29d918e579e8cbd
Author: Jan Kiszka <jan.kiszka@siemens.com>
Date:   Thu Feb 23 18:29:11 2017 +0000
    tcg: drop global lock during TCG code execution

with following stack trace:
#2  0x00007fffee48f0c5 in g_assertion_message (domain=domain@entry=0x0, file=file@entry=0x555555c8aa80 "qemu/translate-common.c", line=line@entry=0x22, func=func@entry=0x555555c8aae0 <__FUNCTION__.22084> "tcg_handle_interrupt", message=message@entry=0x7fff781371d0 "assertion failed: (qemu_mutex_iothread_locked())")
    at gtestutils.c:2432
#3  0x00007fffee48f15a in g_assertion_message_expr (domain=0x0, file=0x555555c8aa80 "qemu/translate-common.c", line=0x22, func=0x555555c8aae0 <__FUNCTION__.22084> "tcg_handle_interrupt", expr=<optimized out>)
    at gtestutils.c:2455
#4  0x000055555577808b in tcg_handle_interrupt (cpu=0x55555683a400, mask=0x2)
    at qemu/translate-common.c:34
#5  0x00005555557fe900 in cpu_interrupt (cpu=0x55555683a400, mask=0x2)
    at qemu/include/qom/cpu.h:795
#6  0x00005555557ffb6a in apic_update_irq (s=0x5555568587a0) at qemu/hw/intc/apic.c:372
#7  0x00005555557ff9d4 in apic_set_tpr (s=0x5555568587a0, val=0x0) at qemu/hw/intc/apic.c:304
#8  0x000055555580168d in cpu_set_apic_tpr (dev=0x5555568587a0, val=0x0)
    at qemu/hw/intc/apic_common.c:77
#9  0x00005555558c5edb in helper_write_crN (env=0x555556842690, reg=0x8, t0=0x0)
    at qemu/target/i386/misc_helper.c:159
#10 0x00007fffd66e1020 in code_gen_buffer ()
#11 0x0000555555776f9f in cpu_tb_exec (cpu=0x55555683a400, itb=0x7fffd0fcbf50)
    at qemu/cpu-exec.c:165
#12 0x0000555555777ca4 in cpu_loop_exec_tb (cpu=0x55555683a400, tb=0x7fffd0fcbf50, last_tb=0x7fffd002a968, tb_exit=0x7fffd002a964, sc=0x7fffd002a980) at qemu/cpu-exec.c:566
#13 0x0000555555777fbe in cpu_exec (cpu=0x55555683a400) at qemu/cpu-exec.c:674
#14 0x00005555557a9af0 in tcg_cpu_exec (cpu=0x55555683a400) at qemu/cpus.c:1247
#15 0x00005555557a9d62 in qemu_tcg_cpu_thread_fn (arg=0x55555683a400) at qemu/cpus.c:1340
#16 0x00007fffeffb5dc5 in start_thread (arg=0x7fffd002b700) at pthread_create.c:308
#17 0x00007fffed10a34d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

to reproduce build qemu with following options:

./configure --enable-debug --target-list=x86_64-softmmu

and run qemu in tcg mode with install image from msdn as following:

  qemu-system-x86_64 -m 1G  \
    -cdrom en_windows_10_enterprise_n_version_1607_updated_jan_2017_x64_dvd_9718069.iso

It seems to be affecting Windows x64 variants only.
Assert also happens with WinEnt 8.1/WinServer 2003 Ent x64

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Alex Bennée]

Igor Mammedov <imammedo@redhat.com> writes:

> (PS: resend due to wrong qemu-devel mail list address in original
> email)

Le *sigh* another way of x86 generating IRQs ;-)

Could you test this please?

target/i386/misc_helper: wrap BQL around another IRQ generator

Anything that calls into HW emulation must be protected by the BQL.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

1 file changed, 2 insertions(+)
target/i386/misc_helper.c | 2 ++

modified   target/i386/misc_helper.c
@@ -156,7 +156,9 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
         break;
     case 8:
         if (!(env->hflags2 & HF2_VINTR_MASK)) {
+            qemu_mutex_lock_iothread();
             cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
+            qemu_mutex_unlock_iothread();
         }
         env->v_tpr = t0 & 0x0f;
         break;
--
Alex Bennée

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Igor Mammedov <imammedo@redhat.com> writes:
>
>> (PS: resend due to wrong qemu-devel mail list address in original
>> email)
>
> Le *sigh* another way of x86 generating IRQs ;-)
>
> Could you test this please?

Sorry I missed you'll also need a:

#include "qemu/main-loop.h"

At the top of misc_helper.c

>
> target/i386/misc_helper: wrap BQL around another IRQ generator
>
> Anything that calls into HW emulation must be protected by the BQL.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> 1 file changed, 2 insertions(+)
> target/i386/misc_helper.c | 2 ++
>
> modified   target/i386/misc_helper.c
> @@ -156,7 +156,9 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
>          break;
>      case 8:
>          if (!(env->hflags2 & HF2_VINTR_MASK)) {
> +            qemu_mutex_lock_iothread();
>              cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
> +            qemu_mutex_unlock_iothread();
>          }
>          env->v_tpr = t0 & 0x0f;
>          break;


--
Alex Bennée

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Igor Mammedov]

On Thu, 30 Mar 2017 16:05:58 +0100
Alex Bennée <alex.bennee@linaro.org> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > (PS: resend due to wrong qemu-devel mail list address in original
> > email)  
> 
> Le *sigh* another way of x86 generating IRQs ;-)
> 
> Could you test this please?
> 
> target/i386/misc_helper: wrap BQL around another IRQ generator
> 
> Anything that calls into HW emulation must be protected by the BQL.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
it doesn't help,
it asserts in the same place anyway

> 
> 1 file changed, 2 insertions(+)
> target/i386/misc_helper.c | 2 ++
> 
> modified   target/i386/misc_helper.c
> @@ -156,7 +156,9 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
>          break;
>      case 8:
>          if (!(env->hflags2 & HF2_VINTR_MASK)) {
> +            qemu_mutex_lock_iothread();
>              cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
> +            qemu_mutex_unlock_iothread();
>          }
>          env->v_tpr = t0 & 0x0f;
>          break;
> --
> Alex Bennée
> 

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Alex Bennée]

Igor Mammedov <imammedo@redhat.com> writes:

> On Thu, 30 Mar 2017 16:05:58 +0100
> Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> Igor Mammedov <imammedo@redhat.com> writes:
>>
>> > (PS: resend due to wrong qemu-devel mail list address in original
>> > email)
>>
>> Le *sigh* another way of x86 generating IRQs ;-)
>>
>> Could you test this please?
>>
>> target/i386/misc_helper: wrap BQL around another IRQ generator
>>
>> Anything that calls into HW emulation must be protected by the BQL.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> it doesn't help,
> it asserts in the same place anyway

That's weird. If the lock was already held I could see it failing higher
up but this seems to imply we are dropping a lock as we descend into
hardware emulation.

How often does this leg of the helper get called?

>
>>
>> 1 file changed, 2 insertions(+)
>> target/i386/misc_helper.c | 2 ++
>>
>> modified   target/i386/misc_helper.c
>> @@ -156,7 +156,9 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
>>          break;
>>      case 8:
>>          if (!(env->hflags2 & HF2_VINTR_MASK)) {
>> +            qemu_mutex_lock_iothread();
>>              cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
>> +            qemu_mutex_unlock_iothread();
>>          }
>>          env->v_tpr = t0 & 0x0f;
>>          break;
>> --
>> Alex Bennée
>>


--
Alex Bennée

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Paolo Bonzini]

On 31/03/2017 10:58, Alex Bennée wrote:
> 
> Igor Mammedov <imammedo@redhat.com> writes:
> 
>> On Thu, 30 Mar 2017 16:05:58 +0100
>> Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>> Igor Mammedov <imammedo@redhat.com> writes:
>>>
>>>> (PS: resend due to wrong qemu-devel mail list address in original
>>>> email)
>>>
>>> Le *sigh* another way of x86 generating IRQs ;-)
>>>
>>> Could you test this please?
>>>
>>> target/i386/misc_helper: wrap BQL around another IRQ generator
>>>
>>> Anything that calls into HW emulation must be protected by the BQL.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> it doesn't help,
>> it asserts in the same place anyway
> 
> That's weird. If the lock was already held I could see it failing higher
> up but this seems to imply we are dropping a lock as we descend into
> hardware emulation.

Seems weird to me too.

> How often does this leg of the helper get called?

Continuously under Windows, never on Linux.  But you can also try
kvm-unit-tests probably to trigger it:

$ qemu-system-x86_64 \
   -kernel x86/vmexit.flat -serial stdio \
   --append 'cpuid mov_to_cr8' -device isa-debug-exit,iobase=0xf4


Thanks,

paolo

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Igor Mammedov]

On Fri, 31 Mar 2017 09:58:49 +0100
Alex Bennée <alex.bennee@linaro.org> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Thu, 30 Mar 2017 16:05:58 +0100
> > Alex Bennée <alex.bennee@linaro.org> wrote:
> >  
> >> Igor Mammedov <imammedo@redhat.com> writes:
> >>  
> >> > (PS: resend due to wrong qemu-devel mail list address in original
> >> > email)  
> >>
> >> Le *sigh* another way of x86 generating IRQs ;-)
> >>
> >> Could you test this please?
> >>
> >> target/i386/misc_helper: wrap BQL around another IRQ generator
> >>
> >> Anything that calls into HW emulation must be protected by the BQL.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>  
> > it doesn't help,
> > it asserts in the same place anyway  
> 
> That's weird. If the lock was already held I could see it failing higher
> up but this seems to imply we are dropping a lock as we descend into
> hardware emulation.
I'm sorry,
when testing it I've misplaced locking into helper_read_crN(),
left write path untouched and went hunting for weird behavior.

Anyway,
I've tested you patch again and windows guest boots fine with it.

> How often does this leg of the helper get called?
> 
> >  
> >>
> >> 1 file changed, 2 insertions(+)
> >> target/i386/misc_helper.c | 2 ++
> >>
> >> modified   target/i386/misc_helper.c
> >> @@ -156,7 +156,9 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
> >>          break;
> >>      case 8:
> >>          if (!(env->hflags2 & HF2_VINTR_MASK)) {
> >> +            qemu_mutex_lock_iothread();
> >>              cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
> >> +            qemu_mutex_unlock_iothread();
> >>          }
> >>          env->v_tpr = t0 & 0x0f;
> >>          break;
> >> --
> >> Alex Bennée
> >>  
> 
> 
> --
> Alex Bennée

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Alex Bennée]

Igor Mammedov <imammedo@redhat.com> writes:

> On Fri, 31 Mar 2017 09:58:49 +0100
> Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> Igor Mammedov <imammedo@redhat.com> writes:
>>
>> > On Thu, 30 Mar 2017 16:05:58 +0100
>> > Alex Bennée <alex.bennee@linaro.org> wrote:
>> >
>> >> Igor Mammedov <imammedo@redhat.com> writes:
>> >>
>> >> > (PS: resend due to wrong qemu-devel mail list address in original
>> >> > email)
>> >>
>> >> Le *sigh* another way of x86 generating IRQs ;-)
>> >>
>> >> Could you test this please?
>> >>
>> >> target/i386/misc_helper: wrap BQL around another IRQ generator
>> >>
>> >> Anything that calls into HW emulation must be protected by the BQL.
>> >>
>> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> > it doesn't help,
>> > it asserts in the same place anyway
>>
>> That's weird. If the lock was already held I could see it failing higher
>> up but this seems to imply we are dropping a lock as we descend into
>> hardware emulation.
> I'm sorry,
> when testing it I've misplaced locking into helper_read_crN(),
> left write path untouched and went hunting for weird behavior.
>
> Anyway,
> I've tested you patch again and windows guest boots fine with it.

No problem - I've done the same before ;-)

Can I have a Tested-by?

--
Alex Bennée

Re: [Qemu-devel] qemu-2.9 crashes in tcg_handle_interrupt() during winx64 boot

[Igor Mammedov]

On Wed, 05 Apr 2017 14:26:35 +0100
Alex Bennée <alex.bennee@linaro.org> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Fri, 31 Mar 2017 09:58:49 +0100
> > Alex Bennée <alex.bennee@linaro.org> wrote:
> >  
> >> Igor Mammedov <imammedo@redhat.com> writes:
> >>  
> >> > On Thu, 30 Mar 2017 16:05:58 +0100
> >> > Alex Bennée <alex.bennee@linaro.org> wrote:
> >> >  
> >> >> Igor Mammedov <imammedo@redhat.com> writes:
> >> >>  
> >> >> > (PS: resend due to wrong qemu-devel mail list address in original
> >> >> > email)  
> >> >>
> >> >> Le *sigh* another way of x86 generating IRQs ;-)
> >> >>
> >> >> Could you test this please?
> >> >>
> >> >> target/i386/misc_helper: wrap BQL around another IRQ generator
> >> >>
> >> >> Anything that calls into HW emulation must be protected by the BQL.
> >> >>
> >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>  
> >> > it doesn't help,
> >> > it asserts in the same place anyway  
> >>
> >> That's weird. If the lock was already held I could see it failing higher
> >> up but this seems to imply we are dropping a lock as we descend into
> >> hardware emulation.  
> > I'm sorry,
> > when testing it I've misplaced locking into helper_read_crN(),
> > left write path untouched and went hunting for weird behavior.
> >
> > Anyway,
> > I've tested you patch again and windows guest boots fine with it.  
> 
> No problem - I've done the same before ;-)
It wasn't totally wasteful though,
I've learned how to do nested breakpoints in gdb

> 
> Can I have a Tested-by?
sure,

Tested-by: Igor Mammedov <imammedo@redhat.com>

> 
> --
> Alex Bennée