Skip to contentSkip to conversation

ARM: OMAP4: Fix array size for irq_target_cpu

PATCH New Tony Lindgren 3 participants 5 messages Sep 5, 2012

Subsystems (3)
lore
Share & tools
lore

Tony Lindgren

author maintainer
#0 lore raw

PATCH 1/1 ARM: OMAP4: Fix array size for irq_target_cpu

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.

arch/arm/mach-omap2/omap-wakeupgen.c +1 -1
@@ -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;
#1 lore raw

PATCH 1/1 ARM: OMAP4: Fix array size for irq_target_cpu

Quoted 6 lines · 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++)

Quoted 8 lines
        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.

Looks good.

#2 lore raw

PATCH 1/1 ARM: OMAP4: Fix array size for irq_target_cpu

In reply to Tony Lindgren

Hi Tony,

Quoted 10 lines · 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.

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 <[email protected]>
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).

Cc Santosh Shilimkar <[email protected]>
arch/arm/mach-omap2/omap-wakeupgen.c +2 -2
index d3d8971..bec55e1 100644
@@ -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

#3 lore raw

PATCH 1/1 ARM: OMAP4: Fix array size for irq_target_cpu

Quoted 11 lines · Benoit Cousson

Hi Tony,

Quoted 7 lines · 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;
Quoted 8 lines
Quoted 3 lines

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

author maintainer
#4 lore raw

PATCH 1/1 ARM: OMAP4: Fix array size for irq_target_cpu

Quoted 16 lines · Shilimkar, Santosh
Quoted 15 lines · Benoit Cousson

Hi Tony,

Quoted 10 lines · 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.

Quoted 8 lines
Quoted 4 lines

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