Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
From: Jisheng Zhang
Date: Mon Jul 06 2015 - 08:08:00 EST
Dear Thomas,
On Mon, 6 Jul 2015 10:18:28 +0000
Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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 <tglx@xxxxxxxxxxxxx>
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx>
> Cc: Jisheng Zhang <jszhang@xxxxxxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Jason Cooper <jason@xxxxxxxxxxxxxx>
> ---
> 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 : [<c022a954>] lr : [<c0060200>] 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] [<c022a898>] (dw_apb_ictl_handler) from [<c006171c>] (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] [<c00616f4>] (generic_handle_irq) from [<c0061848>] (__handle_domain_irq+0x5c/
0xb4)
[ 0.986814] r4:c04d6e4c r3:0000001b
[ 0.990548] [<c00617ec>] (__handle_domain_irq) from [<c00093b4>] (gic_handle_irq+0x28/0x68
)
[ 0.999169] r8:ef802000 r7:ef80200c r6:00000021 r5:c04dcfb4 r4:ee443ba0 r3:ee443ba0
[ 1.007252] [<c000938c>] (gic_handle_irq) from [<c0013580>] (__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] [<c02c3d80>] (_raw_spin_unlock_irqrestore) from [<c0063910>] (__setup_irq+0x30
0/0x4f4)
[ 1.058040] [<c0063610>] (__setup_irq) from [<c0063bd8>] (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] [<c0063b04>] (request_threaded_irq) from [<c024a280>] (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] [<c024a134>] (serial_link_irq_chain) from [<c024a35c>] (univ8250_setup_irq+0xa
0/0xac)
[ 1.107962] r10:00000000 r8:00000000 r7:20000113 r6:00000002 r5:00000002 r4:c0515e80
[ 1.116135] [<c024a2bc>] (univ8250_setup_irq) from [<c0249348>] (serial8250_do_startup+0x1
24/0x658)
[ 1.125475] r4:c0515e80 r3:c024a2bc
[ 1.129208] [<c0249224>] (serial8250_do_startup) from [<c02498a0>] (serial8250_startup+0x2
4/0x28)
[ 1.138369] r10:00000000 r8:00000000 r7:ee5e8800 r6:00000000 r5:ee4a9400 r4:c0515e80
[ 1.146544] [<c024987c>] (serial8250_startup) from [<c02442e8>] (uart_port_startup+0x5c/0x
140)
[ 1.155442] [<c024428c>] (uart_port_startup) from [<c0244f44>] (uart_open+0xdc/0x144)
[ 1.163527] r8:ee5e895c r7:ee4b3d80 r6:ee4a949c r5:ee5e8800 r4:ee4a9400 r3:00000002
[ 1.171610] [<c0244e68>] (uart_open) from [<c0239820>] (tty_open+0x38/0x344)
[ 1.178888] r10:ee5e8800 r8:00000002 r7:00500001 r6:c02e6610 r5:ee4408c8 r4:ee4b3d80
[ 1.187064] [<c02397e8>] (tty_open) from [<c00ecfdc>] (chrdev_open+0xb0/0x168)
[ 1.194521] r10:ee001d48 r9:ee4b3d80 r8:c0515ce8 r7:c0515ce8 r6:c02e675c r5:ee4b3d80
[ 1.202685] r4:ee4408c8
[ 1.205330] [<c00ecf2c>] (chrdev_open) from [<c00e74cc>] (do_dentry_open.isra.13+0xf0/0x2f
8)
[ 1.214042] r8:00000000 r7:c00ecf2c r6:ee4b3d88 r5:ee4408c8 r4:ee4b3d80
[ 1.221049] [<c00e73dc>] (do_dentry_open.isra.13) from [<c00e83e8>] (vfs_open+0x64/0x68)
[ 1.229403] r10:ee001d48 r8:00000000 r7:ee443e5c r6:00000002 r5:ee443f34 r4:ee4b3d80
[ 1.237577] [<c00e8384>] (vfs_open) from [<c00f57a0>] (do_last+0x128/0x6e4)
[ 1.244766] r4:ee443e88 r3:ee454000
[ 1.248497] [<c00f5678>] (do_last) from [<c00f5dd0>] (path_openat+0x74/0x140)
[ 1.255866] r10:00000000 r9:00000000 r8:00000000 r7:00000041 r6:ee443f34 r5:ee4b3d80
[ 1.264028] r4:ee443e88
[ 1.266667] [<c00f5d5c>] (path_openat) from [<c00f6c1c>] (do_filp_open+0x68/0xbc)
[ 1.274393] r8:00000000 r7:00000000 r6:00000001 r5:ee443e88 r4:ee443f34
[ 1.281398] [<c00f6bb4>] (do_filp_open) from [<c00e8738>] (do_sys_open+0x12c/0x1dc)
[ 1.289303] r7:00000000 r6:ffffff9c r5:ee413000 r4:00000000
[ 1.295219] [<c00e860c>] (do_sys_open) from [<c00e880c>] (SyS_open+0x24/0x28)
[ 1.302588] r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c03ade9c r4:c03ade9c
[ 1.310763] [<c00e87e8>] (SyS_open) from [<c038eef8>] (kernel_init_freeable+0xb4/0x140)
[ 1.319037] [<c038ee44>] (kernel_init_freeable) from [<c02b6148>] (kernel_init+0x10/0xec)
[ 1.327480] r5:c02b6138 r4:00000000
[ 1.331216] [<c02b6138>] (kernel_init) from [<c000fa08>] (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;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/