From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Andres Lagar-Cavilla" <andres@lagarcavilla.org>
Subject: Re: Deadlocks by p2m_lock and event_lock
Date: Tue, 13 Mar 2012 11:26:06 -0700
Message-ID: <109443395be5f3e323fa7d0d1977f8b6.squirrel@webmail.lagarcavilla.org>
References: <403610A45A2B5242BD291EDAE8B37D300FCE3236@SHSMSX102.ccr.corp.intel.com>
	<20120309112010.GB83422@ocelot.phlegethon.org>
	<403610A45A2B5242BD291EDAE8B37D300FCE32B2@SHSMSX102.ccr.corp.intel.com>
	<763b511f59616a274ff142d62f55f7bf.squirrel@webmail.lagarcavilla.org>
	<20120309165544.GE83422@ocelot.phlegethon.org>
	<403610A45A2B5242BD291EDAE8B37D300FCE7398@SHSMSX102.ccr.corp.intel.com>
Reply-To: andres@lagarcavilla.org
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <403610A45A2B5242BD291EDAE8B37D300FCE7398@SHSMSX102.ccr.corp.intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: "Hao, Xudong" <xudong.hao@intel.com>
Cc: Keir Fraser <keir@xen.org>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Tim Deegan <tim@xen.org>, "JBeulich@suse.com" <jbeulich@suse.com>, "Zhang,
	Xiantao" <xiantao.zhang@intel.com>
List-Id: xen-devel@lists.xenproject.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.

I have a question before I dive into lock untangling

msix_capability_init ->
p2m_change_entry_type_global(dev->domain, p2m_mmio_direct, p2m_mmio_direct);

Huh? This achieves ... nothing. Almost. It flushes a bunch of TLBs, but
that can be done with significantly less effort. Am I missing something?

Andres
>
> (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,
> -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.
>