Re: Deadlocks by p2m_lock and event_lock

[Tim Deegan <tim@xen.org>]

Hi, 

At 10:58 +0000 on 09 Mar (1331290728), Hao, Xudong wrote:
> ====CPU0===
> map_domain_pirq()    Grab event_lock
>   /
> Pci_enable_msi()
>   /
> msix_capability_init()
>   /
> p2m_change_entry_type_global()   Trying to acquire p2m_lock
> 
> ====CPU9===
> hvm_hap_nested_page_fault() -> get_gfn_type_access()   Grab p2m_lock
>   /
> handle_mmio()
>   /
> ...
>   /
> notify_via_xen_event_channel()    Trying to acquire event_lock
> 
> 
> The event_lock is used anywhere in Xen, I only have a patch of workaround this issue for proposal, but not for the final fix. Any good suggestion?
> 
> diff -r f61120046915 xen/arch/x86/irq.c
> --- a/xen/arch/x86/irq.c	Wed Mar 07 11:50:31 2012 +0100
> +++ b/xen/arch/x86/irq.c	Sat Mar 10 02:06:18 2012 +0800
> @@ -1875,10 +1875,12 @@ int map_domain_pirq(
>          if ( !cpu_has_apic )
>              goto done;
>  
> +        spin_unlock(&d->event_lock);
>          pdev = pci_get_pdev(msi->seg, msi->bus, msi->devfn);
>          ret = pci_enable_msi(msi, &msi_desc);
>          if ( ret )
>              goto done;
> +        spin_lock(&d->event_lock);
>  
>          spin_lock_irqsave(&desc->lock, flags);
> 
> Best Regards,
> Xudong Hao

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 )