[PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Tony Lindgren]

If NR_IRQS is less than MAX_IRQS, we end up writing past the
irq_target_cpu array in omap_wakeupgen_init():

/* Associate all the IRQs to boot CPU like GIC init does. */
for (i = 0; i < max_irqs; i++)
	irq_target_cpu[i] = boot_cpu;

This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
set to 16. Without this patch we're overwriting other data during
the boot.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -47,7 +47,7 @@
 static void __iomem *wakeupgen_base;
 static void __iomem *sar_base;
 static DEFINE_SPINLOCK(wakeupgen_lock);
-static unsigned int irq_target_cpu[NR_IRQS];
+static unsigned int irq_target_cpu[MAX_IRQS];
 static unsigned int irq_banks = MAX_NR_REG_BANKS;
 static unsigned int max_irqs = MAX_IRQS;
 static unsigned int omap_secure_apis;

[PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Tony Lindgren]

If NR_IRQS is less than MAX_IRQS, we end up writing past the
irq_target_cpu array in omap_wakeupgen_init():

/* Associate all the IRQs to boot CPU like GIC init does. */
for (i = 0; i < max_irqs; i++)
	irq_target_cpu[i] = boot_cpu;

This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
set to 16. Without this patch we're overwriting other data during
the boot.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -47,7 +47,7 @@
 static void __iomem *wakeupgen_base;
 static void __iomem *sar_base;
 static DEFINE_SPINLOCK(wakeupgen_lock);
-static unsigned int irq_target_cpu[NR_IRQS];
+static unsigned int irq_target_cpu[MAX_IRQS];
 static unsigned int irq_banks = MAX_NR_REG_BANKS;
 static unsigned int max_irqs = MAX_IRQS;
 static unsigned int omap_secure_apis;

Re: [PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Shilimkar, Santosh]

On Wed, Sep 5, 2012 at 5:33 AM, Tony Lindgren <tony@atomide.com> wrote:
>
> If NR_IRQS is less than MAX_IRQS, we end up writing past the
> irq_target_cpu array in omap_wakeupgen_init():
>
> /* Associate all the IRQs to boot CPU like GIC init does. */
> for (i = 0; i < max_irqs; i++)
>         irq_target_cpu[i] = boot_cpu;
>
> This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> set to 16. Without this patch we're overwriting other data during
> the boot.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
Looks good.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Shilimkar, Santosh]

On Wed, Sep 5, 2012 at 5:33 AM, Tony Lindgren <tony@atomide.com> wrote:
>
> If NR_IRQS is less than MAX_IRQS, we end up writing past the
> irq_target_cpu array in omap_wakeupgen_init():
>
> /* Associate all the IRQs to boot CPU like GIC init does. */
> for (i = 0; i < max_irqs; i++)
>         irq_target_cpu[i] = boot_cpu;
>
> This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> set to 16. Without this patch we're overwriting other data during
> the boot.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
Looks good.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Re: [PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Benoit Cousson]

Hi Tony,

On 09/05/2012 02:03 AM, Tony Lindgren wrote:
> If NR_IRQS is less than MAX_IRQS, we end up writing past the
> irq_target_cpu array in omap_wakeupgen_init():
> 
> /* Associate all the IRQs to boot CPU like GIC init does. */
> for (i = 0; i < max_irqs; i++)
> 	irq_target_cpu[i] = boot_cpu;
> 
> This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> set to 16. Without this patch we're overwriting other data during
> the boot.

In fact I already sent a patch to fix that when I started my SPARSE_IRQ cleanup, but it looks like it was never merged :-(
I guess I forgot a little bit that series.

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/086076.html


Author: Benoit Cousson <b-cousson@ti.com>
Date:   Thu Feb 23 18:44:27 2012 +0100

    ARM: OMAP2+: wakeupgen: Fix wrong array size for irq_target_cpu
    
    The wakeupgen was wrongly allocating an array based on the
    NR_IRQS value (410 on OMAP4) whereas it is just capable of handling 128
    entries.
    Moreover with SPARSE_IRQ, the NR_IRQS number might be 16, and thus
    cannot handle the proper number of entries. It will generate an oops as
    soon a driver will request an IRQ > 16.
    
    Allocate the array using the fixed MAX_IRQS value (128).
    
    Signed-off-by: Benoit Cousson <b-cousson@ti.com>
    Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>

diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wak
index d3d8971..bec55e1 100644
--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -45,7 +45,7 @@ static void __iomem *wakeupgen_base;
 static void __iomem *sar_base;
 static DEFINE_PER_CPU(u32 [NR_REG_BANKS], irqmasks);
 static DEFINE_SPINLOCK(wakeupgen_lock);
-static unsigned int irq_target_cpu[NR_IRQS];
+static unsigned int irq_target_cpu[MAX_IRQS];
 
 /*
  * Static helper functions.
@@ -379,7 +379,7 @@ int __init omap_wakeupgen_init(void)
         */
 
        /* Associate all the IRQs to boot CPU like GIC init does. */
-       for (i = 0; i < NR_IRQS; i++)
+       for (i = 0; i < MAX_IRQS; i++)
                irq_target_cpu[i] = boot_cpu;
 
        irq_hotplug_init();


My original series was doing a little bit more:

0cc3fdc ARM: OMAP: irqs: Set NR_IRQS to NR_IRQS_LEGACY for CONFIG_SPARSE_IRQ
116263d ARM: OMAP2+: gpmc: Use irq_alloc_descs instead of static IRQ range
47b6c8c ARM: OMAP2+: wakeupgen: Fix wrong array size for irq_target_cpu
9017329 ARM: OMAP: irqs: Delete irqs-44xx.h file
b9bb0df ARM: OMAP2+: l3_noc: Remove references to static IRQ defines

But I guess that all of that is now gone with your latest series.

Regards,
Benoit

[PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Benoit Cousson]

Hi Tony,

On 09/05/2012 02:03 AM, Tony Lindgren wrote:
> If NR_IRQS is less than MAX_IRQS, we end up writing past the
> irq_target_cpu array in omap_wakeupgen_init():
> 
> /* Associate all the IRQs to boot CPU like GIC init does. */
> for (i = 0; i < max_irqs; i++)
> 	irq_target_cpu[i] = boot_cpu;
> 
> This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> set to 16. Without this patch we're overwriting other data during
> the boot.

In fact I already sent a patch to fix that when I started my SPARSE_IRQ cleanup, but it looks like it was never merged :-(
I guess I forgot a little bit that series.

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/086076.html


Author: Benoit Cousson <b-cousson@ti.com>
Date:   Thu Feb 23 18:44:27 2012 +0100

    ARM: OMAP2+: wakeupgen: Fix wrong array size for irq_target_cpu
    
    The wakeupgen was wrongly allocating an array based on the
    NR_IRQS value (410 on OMAP4) whereas it is just capable of handling 128
    entries.
    Moreover with SPARSE_IRQ, the NR_IRQS number might be 16, and thus
    cannot handle the proper number of entries. It will generate an oops as
    soon a driver will request an IRQ > 16.
    
    Allocate the array using the fixed MAX_IRQS value (128).
    
    Signed-off-by: Benoit Cousson <b-cousson@ti.com>
    Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>

diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wak
index d3d8971..bec55e1 100644
--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -45,7 +45,7 @@ static void __iomem *wakeupgen_base;
 static void __iomem *sar_base;
 static DEFINE_PER_CPU(u32 [NR_REG_BANKS], irqmasks);
 static DEFINE_SPINLOCK(wakeupgen_lock);
-static unsigned int irq_target_cpu[NR_IRQS];
+static unsigned int irq_target_cpu[MAX_IRQS];
 
 /*
  * Static helper functions.
@@ -379,7 +379,7 @@ int __init omap_wakeupgen_init(void)
         */
 
        /* Associate all the IRQs to boot CPU like GIC init does. */
-       for (i = 0; i < NR_IRQS; i++)
+       for (i = 0; i < MAX_IRQS; i++)
                irq_target_cpu[i] = boot_cpu;
 
        irq_hotplug_init();


My original series was doing a little bit more:

0cc3fdc ARM: OMAP: irqs: Set NR_IRQS to NR_IRQS_LEGACY for CONFIG_SPARSE_IRQ
116263d ARM: OMAP2+: gpmc: Use irq_alloc_descs instead of static IRQ range
47b6c8c ARM: OMAP2+: wakeupgen: Fix wrong array size for irq_target_cpu
9017329 ARM: OMAP: irqs: Delete irqs-44xx.h file
b9bb0df ARM: OMAP2+: l3_noc: Remove references to static IRQ defines

But I guess that all of that is now gone with your latest series.

Regards,
Benoit

Re: [PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Shilimkar, Santosh]

On Wed, Sep 5, 2012 at 5:11 PM, Benoit Cousson <b-cousson@ti.com> wrote:
>
> Hi Tony,
>
> On 09/05/2012 02:03 AM, Tony Lindgren wrote:
> > If NR_IRQS is less than MAX_IRQS, we end up writing past the
> > irq_target_cpu array in omap_wakeupgen_init():
> >
> > /* Associate all the IRQs to boot CPU like GIC init does. */
> > for (i = 0; i < max_irqs; i++)
> >       irq_target_cpu[i] = boot_cpu;
> >
> > This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> > set to 16. Without this patch we're overwriting other data during
> > the boot.
>
> In fact I already sent a patch to fix that when I started my SPARSE_IRQ
> cleanup, but it looks like it was never merged :-(
> I guess I forgot a little bit that series.
>
Indeed. I remember acking few patches in that series.
Infact this patch set was the basis on which I was talking to Arnd
that SPARSE_IRQ
should work on OMAP. Didn't think that it has not made it yet in mainline.

Regards
Santosh

[PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Shilimkar, Santosh]

On Wed, Sep 5, 2012 at 5:11 PM, Benoit Cousson <b-cousson@ti.com> wrote:
>
> Hi Tony,
>
> On 09/05/2012 02:03 AM, Tony Lindgren wrote:
> > If NR_IRQS is less than MAX_IRQS, we end up writing past the
> > irq_target_cpu array in omap_wakeupgen_init():
> >
> > /* Associate all the IRQs to boot CPU like GIC init does. */
> > for (i = 0; i < max_irqs; i++)
> >       irq_target_cpu[i] = boot_cpu;
> >
> > This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> > set to 16. Without this patch we're overwriting other data during
> > the boot.
>
> In fact I already sent a patch to fix that when I started my SPARSE_IRQ
> cleanup, but it looks like it was never merged :-(
> I guess I forgot a little bit that series.
>
Indeed. I remember acking few patches in that series.
Infact this patch set was the basis on which I was talking to Arnd
that SPARSE_IRQ
should work on OMAP. Didn't think that it has not made it yet in mainline.

Regards
Santosh

Re: [PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Tony Lindgren]

* Shilimkar, Santosh <santosh.shilimkar@ti.com> [120905 06:17]:
> On Wed, Sep 5, 2012 at 5:11 PM, Benoit Cousson <b-cousson@ti.com> wrote:
> >
> > Hi Tony,
> >
> > On 09/05/2012 02:03 AM, Tony Lindgren wrote:
> > > If NR_IRQS is less than MAX_IRQS, we end up writing past the
> > > irq_target_cpu array in omap_wakeupgen_init():
> > >
> > > /* Associate all the IRQs to boot CPU like GIC init does. */
> > > for (i = 0; i < max_irqs; i++)
> > >       irq_target_cpu[i] = boot_cpu;
> > >
> > > This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> > > set to 16. Without this patch we're overwriting other data during
> > > the boot.
> >
> > In fact I already sent a patch to fix that when I started my SPARSE_IRQ
> > cleanup, but it looks like it was never merged :-(
> > I guess I forgot a little bit that series.
> >
> Indeed. I remember acking few patches in that series.
> Infact this patch set was the basis on which I was talking to Arnd
> that SPARSE_IRQ
> should work on OMAP. Didn't think that it has not made it yet in mainline.

Bummer. That means duplicate wasted effort was done :(

Looks like the original patch no longer applies because of commit
247c445c0 (ARM: OMAP5: Add the WakeupGen IP updates). So I'll just
update the comments to mention Benoit's original patch.

I guess the lesson here is that everybody must follow up on their own
patches to make sure they get merged. Especially on fixes.

Benoit, can you please check if some parts of your earlier series are
still needed in the testing-cleanup branch?

BTW, there was also Felipe's TWL removal patch that was done in a slightly
different way "[PATCH] arm: omap: drop unused IRQ defines:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/103274.html# 

Regards,

Tony

[PATCH] ARM: OMAP4: Fix array size for irq_target_cpu

[Tony Lindgren]

* Shilimkar, Santosh <santosh.shilimkar@ti.com> [120905 06:17]:
> On Wed, Sep 5, 2012 at 5:11 PM, Benoit Cousson <b-cousson@ti.com> wrote:
> >
> > Hi Tony,
> >
> > On 09/05/2012 02:03 AM, Tony Lindgren wrote:
> > > If NR_IRQS is less than MAX_IRQS, we end up writing past the
> > > irq_target_cpu array in omap_wakeupgen_init():
> > >
> > > /* Associate all the IRQs to boot CPU like GIC init does. */
> > > for (i = 0; i < max_irqs; i++)
> > >       irq_target_cpu[i] = boot_cpu;
> > >
> > > This can happen if SPARSE_IRQ is enabled as by default NR_IRQS is
> > > set to 16. Without this patch we're overwriting other data during
> > > the boot.
> >
> > In fact I already sent a patch to fix that when I started my SPARSE_IRQ
> > cleanup, but it looks like it was never merged :-(
> > I guess I forgot a little bit that series.
> >
> Indeed. I remember acking few patches in that series.
> Infact this patch set was the basis on which I was talking to Arnd
> that SPARSE_IRQ
> should work on OMAP. Didn't think that it has not made it yet in mainline.

Bummer. That means duplicate wasted effort was done :(

Looks like the original patch no longer applies because of commit
247c445c0 (ARM: OMAP5: Add the WakeupGen IP updates). So I'll just
update the comments to mention Benoit's original patch.

I guess the lesson here is that everybody must follow up on their own
patches to make sure they get merged. Especially on fixes.

Benoit, can you please check if some parts of your earlier series are
still needed in the testing-cleanup branch?

BTW, there was also Felipe's TWL removal patch that was done in a slightly
different way "[PATCH] arm: omap: drop unused IRQ defines:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/103274.html# 

Regards,

Tony