RE: [PATCH]vtd: Fix for irq bind failure after PCI attaching 32 times

["Zhang, Fengzhe" <fengzhe.zhang@intel.com>]

Hi, Stefano,

Here is the calling graph that cause the bug:

unregister_real_device (ioemu)
    |
    +----> pt_msix_disable (ioemu)
            |
            +----> xc_domain_unbind_msi_irq (ioemu)
            |       |
            |       +----> do_domctl (xen) ----> arch_do_domctl (xen) ----> pt_irq_destroy_bind_vtd (xen)
            |              |
            |              +----> unmap_domain_pirq_emuirq (xen)  //freed pirq_to_emuirq
            |
            +----> xc_physdev_unmap_pirq (ioemu)
                   |
                   +----> do_physdev_op (xen) 
                           |
                           +----> physdev_unmap_pirq (xen)
                                   |
                                   +----> unmap_domain_pirq_emuirq (xen)  //found pirq_to_emuirq already freed, abort
                                   |
                                   +----> unmap_domain_pirq (xen)    //not called

The code path you mentioned is not taken for VF dev as its ptdev->machine_irq is 0.


-----Original Message-----
From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] 
Sent: Wednesday, January 26, 2011 7:11 PM
To: Zhang, Fengzhe
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH]vtd: Fix for irq bind failure after PCI attaching 32 times

On Wed, 26 Jan 2011, Zhang, Fengzhe wrote:
> vtd: Fix for irq bind failure after PCI attaching 32 times
> 
> Originally when detaching a PCI device, pirq_to_emuirq and pirq_to_irq are freed via hypercall do_physdev_op. Now in function pt_irq_destroy_bind_vtd, duplicated logic is added to free pirq_to_emuirq, but not pirq_to_irq. This causes do_physdev_op fail to free both emuirq and irq. After attaching a PCI device for 32 times, irq resources run out. This patch removes the redundant logic.
> 
> Signed-off-by: Fengzhe Zhang <fengzhe.zhang@intel.com>
> 

It looks OK in principle, but if the theory is that we should always
call xc_physdev_unmap_pirq after xc_domain_unbind_pt_irq, I can find an
instance of xc_domain_unbind_pt_irq without any corresponding
xc_physdev_unmap_pirq.

Take a look at hw/pass-through.c:pt_reset_interrupt_and_io_mapping in
qemu:

if (ptdev->msi_trans_en == 0 && ptdev->machine_irq)
{
    if (xc_domain_unbind_pt_irq(xc_handle, domid, ptdev->machine_irq,
                    PT_IRQ_TYPE_PCI, 0, e_device, e_intx, 0))
        PT_LOG("Error: Unbinding of interrupt failed!\n");
}

but there is no following xc_physdev_unmap_pirq if MSI and MSIX are
disabled.