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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Jan 11, 2011 at 05:20:15PM +0000, Ian Campbell wrote:
> This is neater than open-coded calls to irq_alloc_desc_at and
> irq_free_desc.
> 
> No intended behavioural change.
> 
> Note that we previously were not checking the return value of
> irq_alloc_desc_at which would be failing for GSI<NR_IRQS_LEGACY
> because the core architecture code has already allocated those for
> us. Hence the additional check against NR_IRQS_LEGACY in
> xen_allocate_irq_gsi.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> ---
>  drivers/xen/events.c |   53 +++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index ae8d45d..74fb216 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -384,7 +384,7 @@ static int get_nr_hw_irqs(void)
>  	return ret;
>  }
>  
> -static int find_unbound_irq(void)
> +static int xen_allocate_irq_dynamic(void)
>  {
>  	struct irq_data *data;
>  	int irq, res;
> @@ -442,6 +442,30 @@ static bool identity_mapped_irq(unsigned irq)
>  	return irq < get_nr_hw_irqs();
>  }
>  
> +static int xen_allocate_irq_gsi(unsigned gsi)
> +{
> +	int irq;
> +
> +	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?

> +		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?

> +
> +	/* Legacy IRQ descriptors are already allocated by the arch. */
> +	if (gsi < NR_IRQS_LEGACY)
> +		return gsi;
> +
> +	irq = irq_alloc_desc_at(gsi, -1);
> +	if (irq < 0)
> +		panic("Unable to allocate to IRQ%d (%d)\n", gsi, irq);
> +
> +	return irq;
> +}
> +
> +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?