> Looking at the hypervisor code I couldn't see anything obviously wrong.
I think the culprit is "physdev_unmap_pirq":
if ( is_hvm_domain(d) )
{
spin_lock(&d->event_lock);
gdprintk(XENLOG_WARNING,"d%d, pirq: %d is %x %s, irq: %d\n",
d->domain_id, pirq, domain_pirq_to_emuirq(d, pirq),
domain_pirq_to_emuirq(d, pirq) == IRQ_UNBOUND ? "unbound" : "",
domain_pirq_to_irq(d, pirq));
if ( domain_pirq_to_emuirq(d, pirq) != IRQ_UNBOUND )
ret = unmap_domain_pirq_emuirq(d, pirq);
spin_unlock(&d->event_lock);
if ( domid == DOMID_SELF || ret )
goto free_domain;
It always tells me unbound:
(XEN) physdev.c:237:d14 14, pirq: 54 is ffffffff
(XEN) irq.c:1873:d14 14, nr_pirqs: 56
(XEN) physdev.c:237:d14 14, pirq: 53 is ffffffff
(XEN) irq.c:1873:d14 14, nr_pirqs: 56
(XEN) physdev.c:237:d14 14, pirq: 52 is ffffffff
(XEN) irq.c:1873:d14 14, nr_pirqs: 56
(XEN) physdev.c:237:d14 14, pirq: 51 is ffffffff
(XEN) irq.c:1873:d14 14, nr_pirqs: 56
(XEN) physdev.c:237:d14 14, pirq: 50 is ffffffff
(XEN) irq.c:1873:d14 14, nr_pirqs: 56
(a bit older debug code, so the 'unbound' does not show up here).
Which means that the call to unmap_domain_pirq_emuirq does not happen.
The checks in unmap_domain_pirq_emuirq also look to be depend
on the code being IRQ_UNBOUND.
In other words, all of that code looks to only clear things when
they are !IRQ_UNBOUND.
But the other logic (IRQ_UNBOUND) looks to be missing a removal
in the radix tree:
if ( emuirq != IRQ_PT )
radix_tree_delete(&d->arch.hvm_domain.emuirq_pirq, emuirq);
And I think that is what is causing the leak - the radix tree
needs to be pruned? Or perhaps the allocate_pirq should check
the radix tree for IRQ_UNBOUND ones and re-use them?
> I do note that Xen doesn't free the pirq until it has been unbound by
> the guest. Xen will warn if the guest unmaps a pirq that is still bound
> ("domD: forcing unbind of pirq P"). Is this what is happening? If so,
No. It does not b/c of this check in physdev_unmap_pirq:
if ( domid == DOMID_SELF || ret )
goto free_domain
which jumps over the call to unmap_domain_pirq.
> that would suggest a bug in the guest rather than the hypervisor.
No. But then I am not even using it. See attached module.
>
> David
#include <linux/module.h>
#include <linux/kthread.h>
#include <linux/pagemap.h>
#include <linux/init.h>
#include <xen/xen.h>
#include <xen/page.h>
#include <asm/xen/hypervisor.h>
#include <xen/features.h>
#include <xen/events.h>
MODULE_AUTHOR("Konrad Rzeszutek Wilk <konrad.w...@oracle.com>");
MODULE_DESCRIPTION("alloc_and_unmap");
MODULE_LICENSE("GPL");
MODULE_VERSION("0.1");
static int do_it(void)
{
int rc;
struct physdev_get_free_pirq op_get_free_pirq;
struct physdev_unmap_pirq unmap_irq;
int pirq;
op_get_free_pirq.type = MAP_PIRQ_TYPE_MSI;
rc = HYPERVISOR_physdev_op(PHYSDEVOP_get_free_pirq, &op_get_free_pirq);
if (rc) {
printk(KERN_WARNING "%s:%d rc:%d\n", __func__, __LINE__, rc);
return rc;
}
pirq = op_get_free_pirq.pirq;
unmap_irq.pirq = pirq;
unmap_irq.domid = DOMID_SELF;
rc = HYPERVISOR_physdev_op(PHYSDEVOP_unmap_pirq, &unmap_irq);
if (rc) {
printk(KERN_WARNING "unmap irq failed %d\n", rc);
return rc;
}
printk("PIRQ: %d\n", pirq);
return 0;
}
static int __init alloc_and_unmap_init(void)
{
int i;
for (i = 0; i < 10; i++)
if (do_it())
break;
return 0;
}
static void __exit alloc_and_unmap_exit(void)
{
}
module_init(alloc_and_unmap_init);
module_exit(alloc_and_unmap_exit);
