Re: Deadlocks by p2m_lock and event_lock

["Andres Lagar-Cavilla" <andres@lagarcavilla.org>]

> Hi, Tim and Andres
> The patch fix part of this issue. In handle_mmio, function hvmemul_do_io()
> is called and p2m lock was held again by calling get_gfn_unshare(), still
> trigger a deadlocks.
>
> (XEN) Xen call trace:
> (XEN)    [<ffff82c4801261a3>] _spin_lock+0x1b/0xa8
> (XEN)    [<ffff82c4801070d3>] notify_via_xen_event_channel+0x21/0x106
> (XEN)    [<ffff82c4801b6883>] hvm_buffered_io_send+0x1f1/0x21b
> (XEN)    [<ffff82c4801bbd3a>] stdvga_intercept_mmio+0x491/0x4c7
> (XEN)    [<ffff82c4801b5d58>] hvm_io_intercept+0x218/0x244
> (XEN)    [<ffff82c4801aa931>] hvmemul_do_io+0x55a/0x716
> (XEN)    [<ffff82c4801aab1a>] hvmemul_do_mmio+0x2d/0x2f
> (XEN)    [<ffff82c4801ab239>] hvmemul_write+0x181/0x1a2
> (XEN)    [<ffff82c4801963f0>] x86_emulate+0xcad3/0xfbdf
> (XEN)    [<ffff82c4801a9d2e>] hvm_emulate_one+0x120/0x1af
> (XEN)    [<ffff82c4801b63cb>] handle_mmio+0x4e/0x1d1
> (XEN)    [<ffff82c4801afd72>] hvm_hap_nested_page_fault+0x210/0x37f
> (XEN)    [<ffff82c4801d2419>] vmx_vmexit_handler+0x1523/0x17d0
>
> Thanks,

Thanks for the report, very useful. I'll look into it
Andres

> -Xudong
>
>> -----Original Message-----
>> From: Tim Deegan [mailto:tim@xen.org]
>> Sent: Saturday, March 10, 2012 12:56 AM
>> To: Andres Lagar-Cavilla
>> Cc: Hao, Xudong; Keir Fraser; xen-devel@lists.xensource.com; Zhang,
>> Xiantao;
>> JBeulich@suse.com
>> Subject: Re: [Xen-devel] Deadlocks by p2m_lock and event_lock
>>
>> At 08:29 -0800 on 09 Mar (1331281767), Andres Lagar-Cavilla wrote:
>> > >> I don't know about the event lock, but it seems unwise to call in
>> > >> to handle_mmio with a gfn lock held.  How about fixing the other
>> path?
>> > >>
>> > >> diff -r 04673ecb9d78 xen/arch/x86/hvm/hvm.c
>> > >> --- a/xen/arch/x86/hvm/hvm.c	Thu Mar 08 16:40:05 2012 +0000
>> > >> +++ b/xen/arch/x86/hvm/hvm.c	Fri Mar 09 11:15:25 2012 +0000
>> > >> @@ -1324,10 +1324,11 @@ int hvm_hap_nested_page_fault(unsigned l
>> > >>      if ( (p2mt == p2m_mmio_dm) ||
>> > >>           (access_w && (p2mt == p2m_ram_ro)) )
>> > >>      {
>> > >> +        put_gfn(p2m->domain, gfn);
>> > >>          if ( !handle_mmio() )
>> > >>              hvm_inject_exception(TRAP_gp_fault, 0, 0);
>> > >>          rc = 1;
>> > >> -        goto out_put_gfn;
>> > >> +        goto out;
>> > >>      }
>> > >>
>> > >>  #ifdef __x86_64__
>> > >> @@ -1379,6 +1380,7 @@ int hvm_hap_nested_page_fault(unsigned l
>> > >>
>> > >>  out_put_gfn:
>> > >>      put_gfn(p2m->domain, gfn);
>> > >> +out:
>> > >>      if ( paged )
>> > >>          p2m_mem_paging_populate(v->domain, gfn);
>> > >>      if ( req_ptr )
>> > >
>> > > Yes, that's fine to release the p2m lock earlier than handle_mmio.
>> >
>> > Ack
>>
>> OK, applied.
>>
>> Tim.
>