From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH 3/4] xen: events: add
 xen_allocate_irq_{dynamic, gsi} and xen_free_irq
Date: Tue, 11 Jan 2011 13:46:25 -0500
Message-ID: <20110111184625.GB29378@dumpdata.com>
References: <1294766389.3831.5902.camel@zakaz.uk.xensource.com>
	<1294766416-11407-3-git-send-email-ian.campbell@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <xen-devel-bounces@lists.xensource.com>
Content-Disposition: inline
In-Reply-To: <1294766416-11407-3-git-send-email-ian.campbell@citrix.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>, xen-devel@lists.xensource.com, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

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?