linux-kernel - Re: [PATCH] arm: irq: Allow for specification of no preallocated irqs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 19 Jan 2012 17:04:44 -0600
From:	Rob Herring <robherring2@...il.com>
To:	Michael Bohan <mbohan@...eaurora.org>
CC:	grant.likely@...retlab.ca, tglx@...utronix.de,
	Russell King <linux@....linux.org.uk>,
	linux-arm-msm@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm: irq: Allow for specification of no preallocated
 irqs

On 01/19/2012 04:43 PM, Michael Bohan wrote:
> For cases with SPARSE_IRQ enabled, irqs preallocated with
> arch_probe_nr_irqs() are already marked as allocated in the
> allocated_irqs bitmap. As a consequence, irq chip drivers that
> allocate irqs will feel one of two behaviors:
> 
> 1. An allocation will succeed with the starting irq_base one
> more than the preallocated irqs. This will thus waste the
> preceeding interrupt resources that were preallocated, unless a
> legacy chip driver happens to assume ownership of these by some
> platform definition. The GIC driver is a typical primary chip
> driver, and abides to the allocation APIs. So this can be a
> problem in many trivial usecases.
> 
> 2. An allocation will fail with < 0. This can also happen in the
> GIC driver, which interprets this value as meaning the irq_descs
> are already preallocated. But in Device Tree configurations, the
> fallback irq_base is -1. This results in an invalid irq_base
> value.
> 
> Looking forward, we are moving towards a world where preallocation
> of irqs is no longer necessary. irq_domain is scoped to handle all
> irq_desc allocations in the future. Thus, we should support
> configurations where the platform wants to preallocate no irqs.
> 
> One easy way to achieve this is to allow for
> machine_desc->nr_irqs < 0, which indicates not to preallocate any
> interrupts.
> 
> Signed-off-by: Michael Bohan <mbohan@...eaurora.org>

I don't know if you saw my recent series on NR_IRQS clean-up (Make
mach/irqs.h optional).

No doubt that arch_probe_nr_irqs is doing the wrong thing on ARM, but no
pre-allocation is not what we want either. We ultimately want
arch_probe_nr_irqs to return NR_IRQS_LEGACY (16) to reserve IRQ0 (aka
NO_IRQ) and legacy ISA IRQs. With my series, NR_IRQS is set to
NR_IRQS_LEGACY for SPARSE_IRQ. You can accomplish the same thing without
that series by setting .nr_irqs to NR_IRQS for non-DT and to
NR_IRQS_LEGACY for DT. For platforms to work in single kernel builds,
they will need to select SPARSE_IRQ.

Rob

> ---
>  arch/arm/include/asm/mach/arch.h |    2 +-
>  arch/arm/kernel/irq.c            |   14 ++++++++++++--
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
> index d7692ca..cc6506a 100644
> --- a/arch/arm/include/asm/mach/arch.h
> +++ b/arch/arm/include/asm/mach/arch.h
> @@ -22,7 +22,7 @@ struct machine_desc {
>  	const char *const 	*dt_compat;	/* array of device tree
>  						 * 'compatible' strings	*/
>  
> -	unsigned int		nr_irqs;	/* number of IRQs */
> +	int			nr_irqs;	/* number of IRQs */
>  
>  #ifdef CONFIG_ZONE_DMA
>  	unsigned long		dma_zone_size;	/* size of DMA-able area */
> diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
> index 3efd82c..f74b173 100644
> --- a/arch/arm/kernel/irq.c
> +++ b/arch/arm/kernel/irq.c
> @@ -129,8 +129,18 @@ void __init init_IRQ(void)
>  #ifdef CONFIG_SPARSE_IRQ
>  int __init arch_probe_nr_irqs(void)
>  {
> -	nr_irqs = machine_desc->nr_irqs ? machine_desc->nr_irqs : NR_IRQS;
> -	return nr_irqs;
> +	/*
> +	 * machine_desc->nr_irqs < 0 is a special case that
> +	 * specifies not to preallocate any irq_descs.
> +	 */
> +	if (machine_desc->nr_irqs < 0) {
> +		nr_irqs = 0;
> +		return nr_irqs;
> +	} else {
> +		nr_irqs = machine_desc->nr_irqs ?
> +			  machine_desc->nr_irqs : NR_IRQS;
> +		return nr_irqs;
> +	}
>  }
>  #endif
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/