Re: [PATCH 3/4] xen: events: add xen_allocate_irq_{dynamic, gsi} and xen_free_irq

[Ian Campbell <Ian.Campbell@eu.citrix.com>]

On Tue, 2011-01-11 at 18:46 +0000, Konrad Rzeszutek Wilk wrote:
> > +     if (!identity_mapped_irq(gsi) &&
> > +         (xen_initial_domain() || !xen_pv_domain()))
> 
> Perhaps 'xen_hvm_domain()' would sound better? That way there
> are less _not_ expressions to think through?

This is deliberately just the inverse of the test I removed from the
callsite in xen_map_pirq_gsi, modulo an application or two of De
Morgan's:

-       /* If we are a PV guest, we don't have GSIs (no ACPI passed). Therefore
-        * we are using the !xen_initial_domain() to drop in the function.*/
-       if (identity_mapped_irq(gsi) || (!xen_initial_domain() &&
-                               xen_pv_domain())) {
-               irq = gsi;
-               irq_alloc_desc_at(irq, -1);
-       } else
-               irq = find_unbound_irq();
+       irq = xen_allocate_irq_gsi(gsi);

This patch is just the refactoring step before the meat of the change in
the following patch where this complex expression goes away. 

> > +             return xen_allocate_irq_dynamic();
> 
> Ok, so this ends up allocating an IRQ for all non-physical
> IRQs, such as the spinlock, call IPI, and so on, correct? 

The overall effect should be identical to before this patch. 

> > +static void xen_free_irq(unsigned irq)
> > +{
> > +     irq_free_desc(irq);
> This is still OK even if the IRQ is < NR_IRQS_LEGACY? You mention
> "Legacy IRQ descriptors are already allocated by the arch" so I would
> think that the arch would take care of de-allocating those?

Hmm. Interesting question. I suspect you are right but I can't think
howto convince the system to deallocate such an interrupt anyway. I'll
dig around the code a little and convince myself as best I can that
adding a check+return there is correct.

Ian.