Re: [Xen-devel] [PATCH] xen: reuse the same pirq allocated whendriver load first time

From: Konrad Rzeszutek Wilk
Date: Tue May 21 2013 - 09:41:31 EST

> 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) )
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);
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.wilk@xxxxxxxxxx>");

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())
return 0;
static void __exit alloc_and_unmap_exit(void)