From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754253AbbGFMH5 (ORCPT ); Mon, 6 Jul 2015 08:07:57 -0400 Received: from mx0b-0016f401.pphosted.com ([67.231.156.173]:3296 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753445AbbGFMHy (ORCPT ); Mon, 6 Jul 2015 08:07:54 -0400 Date: Mon, 6 Jul 2015 20:07:36 +0800 From: Jisheng Zhang To: Thomas Gleixner , Sebastian Hesselbarth , Mark Rutland , Jason Cooper CC: LKML Subject: Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage Message-ID: <20150706200736.3f8ef2b0@xhacker> In-Reply-To: <20150706101543.373582262@linutronix.de> References: <20150706101543.373582262@linutronix.de> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2015-07-06_05:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 kscore.is_bulkscore=0 kscore.compositescore=1 compositescore=0.9 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 kscore.is_spamscore=0 rbsscore=0.9 spamscore=0 urlsuspectscore=0.9 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1506180000 definitions=main-1507060192 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Thomas, On Mon, 6 Jul 2015 10:18:28 +0000 Thomas Gleixner wrote: > The num_ct argument of irq_alloc_domain_generic_chips() tells the core > code how many chip types (for different control flows, > e.g. edge/level) should be allocated. It does not control how many > generic chip instances are created because that's determined from the > irq domain size and the number of interrupts per chip. > > The dw-apb init abuses the num_ct argument for allocating one or two > chip types depending on the number of interrupts. That's completely > wrong because the alternate type is never used. > > This code was obviously never tested on a system which has more than > 32 interrupts as that would have never worked due to the unitialized > second generic chip instance. > > Hand in the proper num_ct=1 and fixup the chip initialization along > with the interrupt handler. > > Signed-off-by: Thomas Gleixner > Cc: Sebastian Hesselbarth > Cc: Jisheng Zhang > Cc: Mark Rutland > Cc: Jason Cooper > --- > drivers/irqchip/irq-dw-apb-ictl.c | 53 +++++++++++++++----------------------- > 1 file changed, 22 insertions(+), 31 deletions(-) > > Index: tip/drivers/irqchip/irq-dw-apb-ictl.c > =================================================================== > --- tip.orig/drivers/irqchip/irq-dw-apb-ictl.c > +++ tip/drivers/irqchip/irq-dw-apb-ictl.c > @@ -25,24 +25,25 @@ > #define APB_INT_MASK_H 0x0c > #define APB_INT_FINALSTATUS_L 0x30 > #define APB_INT_FINALSTATUS_H 0x34 > +#define APB_INT_BASE_OFFSET 0x04 > > static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc) > { > - struct irq_chip *chip = irq_get_chip(irq); > - struct irq_chip_generic *gc = irq_get_handler_data(irq); > - struct irq_domain *d = gc->private; > - u32 stat; > + struct irq_domain *d = irq_desc_get_handler_data(desc); > + struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0); > + struct irq_chip *chip = irq_desc_get_chip(desc); > int n; > > chained_irq_enter(chip, desc); > > - for (n = 0; n < gc->num_ct; n++) { > - stat = readl_relaxed(gc->reg_base + > - APB_INT_FINALSTATUS_L + 4 * n); > + for (n = 0; n < d->gc->num_chips; n++, gc++) { > + u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L); I simply tested this patch, got panic like the following, on berlin, gc->num_chips is always 2, it seems that the the gc->reg_base is null when n=1. [ 0.511313] Unable to handle kernel NULL pointer dereference at virtual address 00000030 [ 0.519669] pgd = c0004000 [ 0.522467] [00000030] *pgd=00000000 [ 0.526183] Internal error: Oops: 5 [#1] PREEMPT SMP ARM [ 0.531671] Modules linked in: [ 0.534849] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.0-next-20150703+ #14 [ 0.542306] Hardware name: Marvell Berlin [ 0.546451] task: ee454000 ti: ee442000 task.ti: ee442000 [ 0.552037] PC is at dw_apb_ictl_handler+0xbc/0x120 [ 0.557084] LR is at console_unlock.part.19+0x31c/0x3c8 [ 0.562485] pc : [] lr : [] psr: 20000193 [ 0.562485] sp : ee443b00 ip : ee443a50 fp : ee443b34 [ 0.574337] r10: 00000000 r9 : 00000008 r8 : 00000001 [ 0.579736] r7 : ee402a64 r6 : ee405cc4 r5 : ee402800 r4 : c04e7f00 [ 0.586478] r3 : 00000000 r2 : 00010001 r1 : 60000193 r0 : 0000000a [ 0.593223] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel [ 0.600861] Control: 10c5387d Table: 0100406a DAC: 00000015 [ 0.606797] Process swapper/0 (pid: 1, stack limit = 0xee442210) [ 0.613002] Stack: (0xee443b00 to 0xee444000) [ 0.617510] 3b00: 00000000 00000001 ee443b24 00000011 00000000 00000000 00000031 ee408000 [ 0.625960] 3b20: ee41d4bc 60000113 ee443b4c ee443b38 c006171c c022a8a4 0000001b c04d6e4c [ 0.634410] 3b40: ee443b74 ee443b50 c0061848 c0061700 ee443ba0 ee443ba0 c04dcfb4 00000021 [ 0.642860] 3b60: ef80200c ef802000 ee443b9c ee443b78 c00093b4 c00617f8 c0063910 c02c3da8 [ 0.651310] 3b80: 60000113 ffffffff ee443bd4 0000001a ee443bf4 ee443ba0 c0013580 c0009398 [ 0.659760] 3ba0: ee41d4e4 60000113 00000001 00000005 ee41d480 ee5d2b40 ee41d4e4 c0247370 [ 0.668210] 3bc0: 0000001a ee41d4bc 60000113 ee443bf4 ee443bf8 ee443be8 c0063910 c02c3da8 [ 0.676659] 3be0: 60000113 ffffffff ee443c34 ee443bf8 c0063910 c02c3d8c c04ed084 c0515e80 [ 0.685109] 3c00: 00000080 00000080 ee443c24 ee5d2b40 00000080 ee41d480 c0247370 0000001a [ 0.693559] 3c20: 00000000 ee5d2b00 ee443c64 ee443c38 c0063bd8 c006361c 00000080 c0515e80 [ 0.702009] 3c40: ee5d2b00 00000080 00000000 ee5d2b0c ee5e8800 c05164d8 ee443c94 ee443c68 [ 0.710459] 3c60: c024a280 c0063b10 c0357f4c ee5d2b00 c0515e80 00000002 00000002 20000113 [ 0.718908] 3c80: 00000000 00000000 ee443cac ee443c98 c024a35c c024a140 c024a2bc c0515e80 [ 0.727358] 3ca0: ee443cd4 ee443cb0 c0249348 c024a2c8 c0515e80 ee4a9400 00000000 ee5e8800 [ 0.735808] 3cc0: 00000000 00000000 ee443ce4 ee443cd8 c02498a0 c0249230 ee443d0c ee443ce8 [ 0.744258] 3ce0: c02442e8 c0249888 00000002 ee4a9400 ee5e8800 ee4a949c ee4b3d80 ee5e895c [ 0.752708] 3d00: ee443d34 ee443d10 c0244f44 c0244298 ee4b3d80 ee4408c8 c02e6610 00500001 [ 0.761158] 3d20: 00000002 ee5e8800 ee443d74 ee443d38 c0239820 c0244e74 c0254ee4 ee533000 [ 0.769608] 3d40: 00000001 00000000 000000d0 ee4408c8 ee4b3d80 c02e675c c0515ce8 c0515ce8 [ 0.778058] 3d60: ee4b3d80 ee001d48 ee443da4 ee443d78 c00ecfdc c02397f4 00000000 00000000 [ 0.786507] 3d80: ee443da4 ee4b3d80 ee4408c8 ee4b3d88 c00ecf2c 00000000 ee443dcc ee443da8 [ 0.794957] 3da0: c00e74cc c00ecf38 ee4b3d80 ee443f34 00000002 ee443e5c 00000000 ee001d48 [ 0.803407] 3dc0: ee443de4 ee443dd0 c00e83e8 c00e73e8 ee454000 ee443e88 ee443e4c ee443de8 [ 0.811857] 3de0: c00f57a0 c00e8390 ee443e88 ee413015 ee443e0c ee443e00 ee443e18 00000000 [ 0.820306] 3e00: 00000000 00000026 00000004 ee4408c8 00000000 00000000 ee40c250 ee001e58 [ 0.828756] 3e20: c00f3dc0 ee443e88 ee4b3d80 ee443f34 00000041 00000000 00000000 00000000 [ 0.837205] 3e40: ee443e84 ee443e50 c00f5dd0 c00f5684 00000001 00000007 0000000e 00000000 [ 0.845655] 3e60: 0000001a ee443f34 ee443e88 00000001 00000000 00000000 ee443f24 ee443e88 [ 0.854105] 3e80: c00f6c1c c00f5d68 ee40c250 ee001e58 0f4756ea 00000007 ee413015 00000000 [ 0.862554] 3ea0: 00000000 ee001088 ee4408c8 00000101 00000004 00000016 00000000 00000000 [ 0.871004] 3ec0: 00000000 ee443ec8 ee41a018 00000000 ee443f14 ee443ee0 c0103478 c02c3d34 [ 0.879453] 3ee0: c00f6514 00000002 00000002 00000002 ee413000 ffffff9c ee413000 00000000 [ 0.887903] 3f00: 00000002 ffffff9c 00000000 ee413000 ffffff9c 00000000 ee443f6c ee443f28 [ 0.896353] 3f20: c00e8738 c00f6bc0 00000000 00000000 c03ade9c 00000002 00000000 00000026 [ 0.904802] 3f40: 00000100 00000001 c03ade9c c03ade9c 00000000 00000000 00000000 00000000 [ 0.913252] 3f60: ee443f7c ee443f70 c00e880c c00e8618 ee443f94 ee443f80 c038eef8 c00e87f4 [ 0.921702] 3f80: 00000000 c02b6138 ee443fac ee443f98 c02b6148 c038ee50 ee442000 00000000 [ 0.930151] 3fa0: 00000000 ee443fb0 c000fa08 c02b6144 00000000 00000000 00000000 00000000 [ 0.938600] 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 0.947049] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 964ec888 865a0c8b [ 0.955491] Backtrace: [ 0.958044] [] (dw_apb_ictl_handler) from [] (generic_handle_irq+0x28/ 0x38) [ 0.967026] r10:60000113 r9:ee41d4bc r8:ee408000 r7:00000031 r6:00000000 r5:00000000 [ 0.975191] r4:00000011 [ 0.977832] [] (generic_handle_irq) from [] (__handle_domain_irq+0x5c/ 0xb4) [ 0.986814] r4:c04d6e4c r3:0000001b [ 0.990548] [] (__handle_domain_irq) from [] (gic_handle_irq+0x28/0x68 ) [ 0.999169] r8:ef802000 r7:ef80200c r6:00000021 r5:c04dcfb4 r4:ee443ba0 r3:ee443ba0 [ 1.007252] [] (gic_handle_irq) from [] (__irq_svc+0x40/0x74) [ 1.014980] Exception stack(0xee443ba0 to 0xee443be8) [ 1.020205] 3ba0: ee41d4e4 60000113 00000001 00000005 ee41d480 ee5d2b40 ee41d4e4 c0247370 [ 1.028655] 3bc0: 0000001a ee41d4bc 60000113 ee443bf4 ee443bf8 ee443be8 c0063910 c02c3da8 [ 1.037100] 3be0: 60000113 ffffffff [ 1.040704] r8:0000001a r7:ee443bd4 r6:ffffffff r5:60000113 r4:c02c3da8 r3:c0063910 [ 1.048783] [] (_raw_spin_unlock_irqrestore) from [] (__setup_irq+0x30 0/0x4f4) [ 1.058040] [] (__setup_irq) from [] (request_threaded_irq+0xd4/0x150) [ 1.066573] r10:ee5d2b00 r9:00000000 r8:0000001a r7:c0247370 r6:ee41d480 r5:00000080 [ 1.074738] r4:ee5d2b40 [ 1.077381] [] (request_threaded_irq) from [] (serial_link_irq_chain+0 x14c/0x188) [ 1.086900] r10:c05164d8 r9:ee5e8800 r8:ee5d2b0c r7:00000000 r6:00000080 r5:ee5d2b00 [ 1.095066] r4:c0515e80 r3:00000080 [ 1.098802] [] (serial_link_irq_chain) from [] (univ8250_setup_irq+0xa 0/0xac) [ 1.107962] r10:00000000 r8:00000000 r7:20000113 r6:00000002 r5:00000002 r4:c0515e80 [ 1.116135] [] (univ8250_setup_irq) from [] (serial8250_do_startup+0x1 24/0x658) [ 1.125475] r4:c0515e80 r3:c024a2bc [ 1.129208] [] (serial8250_do_startup) from [] (serial8250_startup+0x2 4/0x28) [ 1.138369] r10:00000000 r8:00000000 r7:ee5e8800 r6:00000000 r5:ee4a9400 r4:c0515e80 [ 1.146544] [] (serial8250_startup) from [] (uart_port_startup+0x5c/0x 140) [ 1.155442] [] (uart_port_startup) from [] (uart_open+0xdc/0x144) [ 1.163527] r8:ee5e895c r7:ee4b3d80 r6:ee4a949c r5:ee5e8800 r4:ee4a9400 r3:00000002 [ 1.171610] [] (uart_open) from [] (tty_open+0x38/0x344) [ 1.178888] r10:ee5e8800 r8:00000002 r7:00500001 r6:c02e6610 r5:ee4408c8 r4:ee4b3d80 [ 1.187064] [] (tty_open) from [] (chrdev_open+0xb0/0x168) [ 1.194521] r10:ee001d48 r9:ee4b3d80 r8:c0515ce8 r7:c0515ce8 r6:c02e675c r5:ee4b3d80 [ 1.202685] r4:ee4408c8 [ 1.205330] [] (chrdev_open) from [] (do_dentry_open.isra.13+0xf0/0x2f 8) [ 1.214042] r8:00000000 r7:c00ecf2c r6:ee4b3d88 r5:ee4408c8 r4:ee4b3d80 [ 1.221049] [] (do_dentry_open.isra.13) from [] (vfs_open+0x64/0x68) [ 1.229403] r10:ee001d48 r8:00000000 r7:ee443e5c r6:00000002 r5:ee443f34 r4:ee4b3d80 [ 1.237577] [] (vfs_open) from [] (do_last+0x128/0x6e4) [ 1.244766] r4:ee443e88 r3:ee454000 [ 1.248497] [] (do_last) from [] (path_openat+0x74/0x140) [ 1.255866] r10:00000000 r9:00000000 r8:00000000 r7:00000041 r6:ee443f34 r5:ee4b3d80 [ 1.264028] r4:ee443e88 [ 1.266667] [] (path_openat) from [] (do_filp_open+0x68/0xbc) [ 1.274393] r8:00000000 r7:00000000 r6:00000001 r5:ee443e88 r4:ee443f34 [ 1.281398] [] (do_filp_open) from [] (do_sys_open+0x12c/0x1dc) [ 1.289303] r7:00000000 r6:ffffff9c r5:ee413000 r4:00000000 [ 1.295219] [] (do_sys_open) from [] (SyS_open+0x24/0x28) [ 1.302588] r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c03ade9c r4:c03ade9c [ 1.310763] [] (SyS_open) from [] (kernel_init_freeable+0xb4/0x140) [ 1.319037] [] (kernel_init_freeable) from [] (kernel_init+0x10/0xec) [ 1.327480] r5:c02b6138 r4:00000000 [ 1.331216] [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) [ 1.339031] r4:00000000 r3:ee442000 [ 1.342762] Code: e5972004 e59f0064 eb023931 e5973004 (e593a030) > + > while (stat) { > u32 hwirq = ffs(stat) - 1; > - generic_handle_irq(irq_find_mapping(d, > - gc->irq_base + hwirq + 32 * n)); > + u32 virq = irq_find_mapping(d, gc->irq_base + hwirq); > + > + generic_handle_irq(virq); > stat &= ~(1 << hwirq); > } > } > @@ -73,7 +74,7 @@ static int __init dw_apb_ictl_init(struc > struct irq_domain *domain; > struct irq_chip_generic *gc; > void __iomem *iobase; > - int ret, nrirqs, irq; > + int ret, nrirqs, irq, i; > u32 reg; > > /* Map the parent interrupt for the chained handler */ > @@ -128,35 +129,25 @@ static int __init dw_apb_ictl_init(struc > goto err_unmap; > } > > - ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1, > - np->name, handle_level_irq, clr, 0, > - IRQ_GC_MASK_CACHE_PER_TYPE | > + ret = irq_alloc_domain_generic_chips(domain, 32, 1, np->name, > + handle_level_irq, clr, 0, > IRQ_GC_INIT_MASK_CACHE); > if (ret) { > pr_err("%s: unable to alloc irq domain gc\n", np->full_name); > goto err_unmap; > } > > - gc = irq_get_domain_generic_chip(domain, 0); > - gc->private = domain; > - gc->reg_base = iobase; > - > - gc->chip_types[0].regs.mask = APB_INT_MASK_L; > - gc->chip_types[0].regs.enable = APB_INT_ENABLE_L; > - gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit; > - gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit; > - gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume; > - > - if (nrirqs > 32) { > - gc->chip_types[1].regs.mask = APB_INT_MASK_H; > - gc->chip_types[1].regs.enable = APB_INT_ENABLE_H; > - gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit; > - gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit; > - gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume; > + for (i = 0; i < nrirqs / 32; i++) { > + gc = irq_get_domain_generic_chip(domain, i * 32); > + gc->reg_base = iobase + i * APB_INT_BASE_OFFSET; > + gc->chip_types[0].regs.mask = APB_INT_MASK_L; > + gc->chip_types[0].regs.enable = APB_INT_ENABLE_L; > + gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit; > + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit; > + gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume; > } > > - irq_set_handler_data(irq, gc); > - irq_set_chained_handler(irq, dw_apb_ictl_handler); > + irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain); > > return 0; >