Message ID | 1417451109-30276-1-git-send-email-marc.zyngier@arm.com |
---|---|
State | New |
Headers | show |
Hi Marc, On Mon, Dec 1, 2014 at 2:25 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > mach-imx directly references to the irq field in > struct irq_data, and uses this to directly poke hardware register. > > But irq is the *virtual* irq number, something that has nothing > to do with the actual HW irq (stored in the hwirq field). And once > we put the stacked domain code in action, the whole thing explodes, > as these two values are *very* different. > > Just replacing all instances of irq with hwirq fixes the issue. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> I tested your patch and I still have the following problem on a mx6q: [ 0.000000] Unable to handle kernel NULL pointer dereference at virtual address 00000008 [ 0.000000] pgd = 80004000 [ 0.000000] [00000008] *pgd=00000000 [ 0.000000] Internal error: Oops: 5 [#1] SMP ARM [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.18.0-rc6-next-20141201-dirty #341 [ 0.000000] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [ 0.000000] task: 8097c2b0 ti: 80970000 task.ti: 80970000 [ 0.000000] PC is at imx_gpc_irq_unmask+0xc/0x4c [ 0.000000] LR is at imx6q_set_lpm+0x80/0x10c [ 0.000000] pc : [<80026598>] lr : [<8002757c>] psr: 800001d3 [ 0.000000] sp : 80971ea8 ip : 80971eb8 fp : 80971eb4 [ 0.000000] r10: c0818078 r9 : 809d8524 r8 : c0818074 [ 0.000000] r7 : 00000000 r6 : 809d8fec r5 : 00000000 r4 : 00000078 [ 0.000000] r3 : c0818000 r2 : 00000079 r1 : 00000020 r0 : 00000000 [ 0.000000] Flags: Nzcv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel [ 0.000000] Control: 10c5387d Table: 1000404a DAC: 00000015 [ 0.000000] Process swapper/0 (pid: 0, stack limit = 0x80970238) [ 0.000000] Stack: (0x80971ea8 to 0x80972000) [ 0.000000] 1ea0: 80971ed4 80971eb8 8002757c 80026598 809d8524 809d8524 [ 0.000000] 1ec0: 80968a18 809d8524 80971f64 80971ed8 809293f4 80027508 c0818060 00000018 [ 0.000000] 1ee0: 00000000 8097f348 00000000 8097f348 0000001a be7cfe84 c081803c c0818020 [ 0.000000] 1f00: c0818024 c0818028 c0818034 c0818048 c0818060 c0818038 c081802c c0818068 [ 0.000000] 1f20: c0818030 c0818014 c081801c c081806c c0818018 c0818080 016e3600 00000001 [ 0.000000] 1f40: be7cfe84 be0027c0 809c7e2c 00000000 809c7d18 809c7e34 80971fa4 80971f68 [ 0.000000] 1f60: 80950984 80925cac 00000008 00000001 00000000 00000000 00000000 00000000 [ 0.000000] 1f80: 00000001 ffffffff 809d7a00 8095e9d8 412fc09a befffa40 80971fb4 80971fa8 [ 0.000000] 1fa0: 809175fc 809508bc 80971ff4 80971fb8 80913b9c 809175dc ffffffff ffffffff [ 0.000000] 1fc0: 809136d4 00000000 00000000 8095e9d8 809d7c94 80978968 8095e9d4 8097d9fc [ 0.000000] 1fe0: 1000406a 00000000 00000000 80971ff8 10008074 8091395c 00000000 00000000 [ 0.000000] Backtrace: [ 0.000000] [<8002658c>] (imx_gpc_irq_unmask) from [<8002757c>] (imx6q_set_lpm+0x80/0x10c) [ 0.000000] [<800274fc>] (imx6q_set_lpm) from [<809293f4>] (imx6q_clocks_init+0x3754/0x375c) [ 0.000000] r7:809d8524 r6:80968a18 r5:809d8524 r4:809d8524 [ 0.000000] [<80925ca0>] (imx6q_clocks_init) from [<80950984>] (of_clk_init+0xd4/0x1a0) [ 0.000000] r10:809c7e34 r9:809c7d18 r8:00000000 r7:809c7e2c r6:be0027c0 r5:be7cfe84 [ 0.000000] r4:00000001 [ 0.000000] [<809508b0>] (of_clk_init) from [<809175fc>] (time_init+0x2c/0x38) [ 0.000000] r10:befffa40 r9:412fc09a r8:8095e9d8 r7:809d7a00 r6:ffffffff r5:00000001 [ 0.000000] r4:00000000 [ 0.000000] [<809175d0>] (time_init) from [<80913b9c>] (start_kernel+0x24c/0x3d8) [ 0.000000] [<80913950>] (start_kernel) from [<10008074>] (0x10008074) [ 0.000000] r10:00000000 r8:1000406a r7:8097d9fc r6:8095e9d4 r5:80978968 r4:809d7c94 [ 0.000000] Code: 809d84f0 e1a0c00d e92dd800 e24cb004 (e5903008) [ 0.000000] ---[ end trace cb88537fdc8fa200 ]--- [ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task! [ 0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! This issue does not happen on linux-next 20141126, but it stats at 201411267. I haven't bisect it yet, but if you have any ideas, please let me know. Thanks
On 01/12/14 17:00, Fabio Estevam wrote: > Hi Marc, > > On Mon, Dec 1, 2014 at 2:25 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> mach-imx directly references to the irq field in >> struct irq_data, and uses this to directly poke hardware register. >> >> But irq is the *virtual* irq number, something that has nothing >> to do with the actual HW irq (stored in the hwirq field). And once >> we put the stacked domain code in action, the whole thing explodes, >> as these two values are *very* different. >> >> Just replacing all instances of irq with hwirq fixes the issue. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > > I tested your patch and I still have the following problem on a mx6q: [...] > This issue does not happen on linux-next 20141126, but it stats at 201411267. > > I haven't bisect it yet, but if you have any ideas, please let me know. Thanks I do have an idea indeed, as well as a patch for this. I'll put you on Cc, watch that space. Thanks, M.
Hi Marc, Am Montag, den 01.12.2014, 16:25 +0000 schrieb Marc Zyngier: > mach-imx directly references to the irq field in > struct irq_data, and uses this to directly poke hardware register. > > But irq is the *virtual* irq number, something that has nothing > to do with the actual HW irq (stored in the hwirq field). And once > we put the stacked domain code in action, the whole thing explodes, > as these two values are *very* different. > > Just replacing all instances of irq with hwirq fixes the issue. I have tried this on next-20141128, but due to struct irq_data *iomuxc_irq_data = irq_get_irq_data(32); /* now returns NULL */ in arch/arm/mach-imx/pm-imx6q.c it still explodes: Unable to handle kernel NULL pointer dereference at virtual address 00000008 pgd = 80004000 [00000008] *pgd=00000000 Internal error: Oops: 5 [#1] SMP ARM Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.18.0-rc6-next-20141128+ #8437 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) task: 808520e8 ti: 80846000 task.ti: 80846000 PC is at imx_gpc_irq_unmask+0x14/0x54 LR is at imx6q_set_lpm+0x6c/0xe0 pc : [<8001ec90>] lr : [<8001fb6c>] psr: 800001d3 sp : 80847ea0 ip : 8001ec90 fp : 80847eac r10: 00000000 r9 : 80891c4c r8 : c0818080 r7 : 00000000 r6 : 00000000 r5 : 00000078 r4 : 80892068 r3 : c0818000 r2 : 00000079 r1 : 00000020 r0 : 00000000 Flags: Nzcv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel Control: 10c5387d Table: 1000404a DAC: 00000015 Process swapper/0 (pid: 0, stack limit = 0x80846240) Stack: (0x80847ea0 to 0x80848000) 7ea0: 80847ecc 80847eb0 8001fb6c 8001ec88 80891c94 af01e080 8083f9e8 80891c94 7ec0: 80847f54 80847ed0 807dddd0 8001fb0c c0818060 00000018 00000000 80891c4c 7ee0: 00000000 80891c4c c0818024 af6644c0 c081803c c0818020 c0818060 c0818038 7f00: c0818028 c0818034 c0818048 c0818024 c081802c c0818014 8001e3dc c081801c 7f20: 80891c4c c0818000 016e3600 00000001 af6644c0 af002740 808801e4 00200200 7f40: 00100100 00000000 80847f94 80847f58 80807710 807da4bc 80880180 00000001 7f60: 00000000 00000000 00000000 00000000 808917c0 00000001 ffffffff 80820808 7f80: 8084e480 808917c0 80847fa4 80847f98 807d57b8 80807608 80847ff4 80847fa8 7fa0: 807d1bd4 807d5794 ffffffff ffffffff 807d16d0 00000000 00000000 affff9c0 7fc0: 00000000 80820808 00000000 80891a54 8084e4fc 80820804 808531c4 1000406a 7fe0: 412fc09a 00000000 00000000 80847ff8 10008074 807d1960 00000000 00000000 Backtrace: [<8001ec7c>] (imx_gpc_irq_unmask) from [<8001fb6c>] (imx6q_set_lpm+0x6c/0xe0) [<8001fb00>] (imx6q_set_lpm) from [<807dddd0>] (imx6q_clocks_init+0x3920/0x392c) r7:80891c94 r6:8083f9e8 r5:af01e080 r4:80891c94 [<807da4b0>] (imx6q_clocks_init) from [<80807710>] (of_clk_init+0x114/0x1a4) r10:00000000 r9:00100100 r8:00200200 r7:808801e4 r6:af002740 r5:af6644c0 r4:00000001 [<808075fc>] (of_clk_init) from [<807d57b8>] (time_init+0x30/0x38) r10:808917c0 r9:8084e480 r8:80820808 r7:ffffffff r6:00000001 r5:808917c0 r4:00000000 [<807d5788>] (time_init) from [<807d1bd4>] (start_kernel+0x280/0x3dc) [<807d1954>] (start_kernel) from [<10008074>] (0x10008074) r10:00000000 r9:412fc09a r8:1000406a r7:808531c4 r6:80820804 r5:8084e4fc r4:80891a54 Code: e92dd800 e24cb004 e52de004 ebffdbbb (e5903008) ---[ end trace cb88537fdc8fa200 ]--- > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/mach-imx/gpc.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c > index 82ea74e..1455829 100644 > --- a/arch/arm/mach-imx/gpc.c > +++ b/arch/arm/mach-imx/gpc.c > @@ -56,14 +56,14 @@ void imx_gpc_post_resume(void) > > static int imx_gpc_irq_set_wake(struct irq_data *d, unsigned int on) > { > - unsigned int idx = d->irq / 32 - 1; > + unsigned int idx = d->hwirq / 32 - 1; > u32 mask; > > /* Sanity check for SPI irq */ > - if (d->irq < 32) > + if (d->hwirq < 32) > return -EINVAL; > > - mask = 1 << d->irq % 32; > + mask = 1 << d->hwirq % 32; > gpc_wake_irqs[idx] = on ? gpc_wake_irqs[idx] | mask : > gpc_wake_irqs[idx] & ~mask; > > @@ -97,12 +97,12 @@ void imx_gpc_irq_unmask(struct irq_data *d) > u32 val; > > /* Sanity check for SPI irq */ > - if (d->irq < 32) > + if (d->hwirq < 32) > return; > > - reg = gpc_base + GPC_IMR1 + (d->irq / 32 - 1) * 4; > + reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4; > val = readl_relaxed(reg); > - val &= ~(1 << d->irq % 32); > + val &= ~(1 << d->hwirq % 32); > writel_relaxed(val, reg); > } > > @@ -112,12 +112,12 @@ void imx_gpc_irq_mask(struct irq_data *d) > u32 val; > > /* Sanity check for SPI irq */ > - if (d->irq < 32) > + if (d->hwirq < 32) > return; > > - reg = gpc_base + GPC_IMR1 + (d->irq / 32 - 1) * 4; > + reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4; > val = readl_relaxed(reg); > - val |= 1 << (d->irq % 32); > + val |= 1 << (d->hwirq % 32); > writel_relaxed(val, reg); > } >
On Mon, Dec 1, 2014 at 2:25 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > mach-imx directly references to the irq field in > struct irq_data, and uses this to directly poke hardware register. > > But irq is the *virtual* irq number, something that has nothing > to do with the actual HW irq (stored in the hwirq field). And once > we put the stacked domain code in action, the whole thing explodes, > as these two values are *very* different. > > Just replacing all instances of irq with hwirq fixes the issue. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c index 82ea74e..1455829 100644 --- a/arch/arm/mach-imx/gpc.c +++ b/arch/arm/mach-imx/gpc.c @@ -56,14 +56,14 @@ void imx_gpc_post_resume(void) static int imx_gpc_irq_set_wake(struct irq_data *d, unsigned int on) { - unsigned int idx = d->irq / 32 - 1; + unsigned int idx = d->hwirq / 32 - 1; u32 mask; /* Sanity check for SPI irq */ - if (d->irq < 32) + if (d->hwirq < 32) return -EINVAL; - mask = 1 << d->irq % 32; + mask = 1 << d->hwirq % 32; gpc_wake_irqs[idx] = on ? gpc_wake_irqs[idx] | mask : gpc_wake_irqs[idx] & ~mask; @@ -97,12 +97,12 @@ void imx_gpc_irq_unmask(struct irq_data *d) u32 val; /* Sanity check for SPI irq */ - if (d->irq < 32) + if (d->hwirq < 32) return; - reg = gpc_base + GPC_IMR1 + (d->irq / 32 - 1) * 4; + reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4; val = readl_relaxed(reg); - val &= ~(1 << d->irq % 32); + val &= ~(1 << d->hwirq % 32); writel_relaxed(val, reg); } @@ -112,12 +112,12 @@ void imx_gpc_irq_mask(struct irq_data *d) u32 val; /* Sanity check for SPI irq */ - if (d->irq < 32) + if (d->hwirq < 32) return; - reg = gpc_base + GPC_IMR1 + (d->irq / 32 - 1) * 4; + reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4; val = readl_relaxed(reg); - val |= 1 << (d->irq % 32); + val |= 1 << (d->hwirq % 32); writel_relaxed(val, reg); }
mach-imx directly references to the irq field in struct irq_data, and uses this to directly poke hardware register. But irq is the *virtual* irq number, something that has nothing to do with the actual HW irq (stored in the hwirq field). And once we put the stacked domain code in action, the whole thing explodes, as these two values are *very* different. Just replacing all instances of irq with hwirq fixes the issue. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/mach-imx/gpc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)