| Message ID | 20210406093557.1073423-3-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleaning up some of the irqdomain features | expand |
On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote: > irq_create_strict_mappings() is a poor way to allow the use of > a linear IRQ domain as a legacy one. Let's be upfront about > it and use a legacy domain when appropriate. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- When running the "mainstone" qemu emulation, this patch results in many (32, actually) runtime warnings such as the following. [ 0.528272] ------------[ cut here ]------------ [ 0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550 irq_domain_associate+0x194/0x1f0 [ 0.528315] error: virq335 is not allocated [ 0.528325] Modules linked in: [ 0.528351] CPU: 0 PID: 1 Comm: swapper Tainted: G W 5.12.0-rc8-next-20210423 #1 [ 0.528372] Hardware name: Intel HCDDBBVA0 Development Platform (aka Mainstone) [ 0.528387] Backtrace: [ 0.528406] [<c06bd188>] (dump_backtrace) from [<c06bd468>] (show_stack+0x20/0x24) [ 0.528441] r7:00000226 r6:c00796e8 r5:00000009 r4:c088d2a0 [ 0.528454] [<c06bd448>] (show_stack) from [<c06c11dc>] (dump_stack+0x28/0x30) [ 0.528479] [<c06c11b4>] (dump_stack) from [<c002a2b8>] (__warn+0xe8/0x110) [ 0.528507] r5:00000009 r4:c0872698 [ 0.528520] [<c002a1d0>] (__warn) from [<c06bdbc0>] (warn_slowpath_fmt+0xa0/0xe0) [ 0.528551] r7:c00796e8 r6:00000226 r5:c0872698 r4:c0872700 [ 0.528564] [<c06bdb24>] (warn_slowpath_fmt) from [<c00796e8>] (irq_domain_associate+0x194/0x1f0) [ 0.528597] r8:00000130 r7:0000014f r6:0000001f r5:00000000 r4:c11bd780 [ 0.528610] [<c0079554>] (irq_domain_associate) from [<c00797a4>] (irq_domain_associate_many+0x60/0xa4) [ 0.528642] r8:00000130 r7:c11bd780 r6:fffffed0 r5:00000150 r4:00000150 [ 0.528655] [<c0079744>] (irq_domain_associate_many) from [<c0079e5c>] (irq_domain_create_legacy+0x5c/0x68) [ 0.528687] r8:00000130 r7:00000130 r6:00000020 r5:00000000 r4:c11bd780 [ 0.528699] [<c0079e00>] (irq_domain_create_legacy) from [<c0079e9c>] (irq_domain_add_legacy+0x34/0x3c) [ 0.528730] r7:c09b1370 r6:c09b1360 r5:c11bd3a0 r4:00000000 [ 0.528743] [<c0079e68>] (irq_domain_add_legacy) from [<c0024f28>] (cplds_probe+0x170/0x1ac) [ 0.528768] [<c0024db8>] (cplds_probe) from [<c0432cec>] (platform_probe+0x50/0xb0) [ 0.528800] r8:c09d2c94 r7:c0aa4f88 r6:c09d2c94 r5:c09b1370 r4:00000000 [ 0.528814] [<c0432c9c>] (platform_probe) from [<c042fc70>] (really_probe+0x100/0x4d4) [ 0.528844] r7:c0aa4f88 r6:00000000 r5:00000000 r4:c09b1370 [ 0.528858] [<c042fb70>] (really_probe) from [<c04300cc>] (driver_probe_device+0x88/0x20c) [ 0.528892] r10:c0974830 r9:c0a70000 r8:c093b224 r7:c0a31de8 r6:c09d2c94 r5:c09d2c94 [ 0.528907] r4:c09b1370 [ 0.528919] [<c0430044>] (driver_probe_device) from [<c04306cc>] (device_driver_attach+0x68/0x70) [ 0.528953] r9:c0a70000 r8:c093b224 r7:c0a31de8 r6:c09d2c94 r5:00000000 r4:c09b1370 [ 0.528969] [<c0430664>] (device_driver_attach) from [<c0430794>] (__driver_attach+0xc0/0x164) [ 0.528997] r7:c0a31de8 r6:c09b1370 r5:c09d2c94 r4:00000000 [ 0.529009] [<c04306d4>] (__driver_attach) from [<c042d8d0>] (bus_for_each_dev+0x84/0xcc) [ 0.529039] r7:c0a31de8 r6:c04306d4 r5:c09d2c94 r4:00000000 [ 0.529052] [<c042d84c>] (bus_for_each_dev) from [<c042f494>] (driver_attach+0x28/0x30) [ 0.529082] r6:00000000 r5:c11bd200 r4:c09d2c94 [ 0.529095] [<c042f46c>] (driver_attach) from [<c042edac>] (bus_add_driver+0x168/0x210) [ 0.529122] [<c042ec44>] (bus_add_driver) from [<c0431304>] (driver_register+0x88/0x120) [ 0.529152] r7:c0a5c7e0 r6:00000000 r5:ffffe000 r4:c09d2c94 [ 0.529165] [<c043127c>] (driver_register) from [<c04329a0>] (__platform_driver_register+0x2c/0x34) [ 0.529191] r5:ffffe000 r4:c094ba64 [ 0.529204] [<c0432974>] (__platform_driver_register) from [<c094ba84>] (cplds_driver_init+0x20/0x28) [ 0.529230] [<c094ba64>] (cplds_driver_init) from [<c000a2a8>] (do_one_initcall+0x60/0x27c) [ 0.529255] [<c000a248>] (do_one_initcall) from [<c093f244>] (kernel_init_freeable+0x158/0x1e4) [ 0.529284] r7:c0974850 r6:00000007 r5:c0c0f720 r4:c09a02fc [ 0.529297] [<c093f0ec>] (kernel_init_freeable) from [<c06c5274>] (kernel_init+0x18/0x110) [ 0.529328] r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c06c525c [ 0.529343] r4:00000000 [ 0.529354] [<c06c525c>] (kernel_init) from [<c0008348>] (ret_from_fork+0x14/0x2c) [ 0.529387] Exception stack(0xc0bdffb0 to 0xc0bdfff8) [ 0.529467] ffa0: 00000000 00000000 00000000 00000000 [ 0.529587] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 0.529684] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 0.529726] r5:c06c525c r4:00000000 [ 0.529752] ---[ end trace d199929d2b87e077 ]--- Bisect log attached. Guenter --- # bad: [e3d35712f85ac84fb06234848f6c043ab418cf8b] Add linux-next specific files for 20210423 # good: [bf05bf16c76bb44ab5156223e1e58e26dfe30a88] Linux 5.12-rc8 git bisect start 'HEAD' 'v5.12-rc8' # good: [d4b5d9d94679a18bfa4ccdafd19876d58777911e] Merge remote-tracking branch 'crypto/master' git bisect good d4b5d9d94679a18bfa4ccdafd19876d58777911e # good: [27628e42fe59a698e66b671bf1e1f01f6a3fe765] Merge remote-tracking branch 'tip/auto-latest' git bisect good 27628e42fe59a698e66b671bf1e1f01f6a3fe765 # bad: [bc6c3ae4f662fc719d0bf144f150f72cab8912d4] Merge remote-tracking branch 'vfio/next' git bisect bad bc6c3ae4f662fc719d0bf144f150f72cab8912d4 # bad: [5ff5b00609c64a043ccd5bc92273c132b33f7f9a] Merge remote-tracking branch 'driver-core/driver-core-next' git bisect bad 5ff5b00609c64a043ccd5bc92273c132b33f7f9a # bad: [c878be9c883153797d5749620e58f180cc429e88] Merge remote-tracking branch 'kvm/next' git bisect bad c878be9c883153797d5749620e58f180cc429e88 # good: [52acd22faa1af8a0514ccd075a6978ac97986425] KVM: Boost vCPU candidate in user mode which is delivering interrupt git bisect good 52acd22faa1af8a0514ccd075a6978ac97986425 # good: [988aab640a6c46ab9552e65c2c3a8d577a4e30f3] rcu: Make rcu_gp_cleanup() be noinline for tracing git bisect good 988aab640a6c46ab9552e65c2c3a8d577a4e30f3 # bad: [6603c2d8bd6cc7fa591fd3b4232bf25b65a0ea8f] Merge remote-tracking branch 'ftrace/for-next' git bisect bad 6603c2d8bd6cc7fa591fd3b4232bf25b65a0ea8f # good: [c658797f1a70561205a224be0c8be64977ed64e8] tracing: Add method for recording "func_repeats" events git bisect good c658797f1a70561205a224be0c8be64977ed64e8 # good: [46135d6f878ab00261d4a2082d620bfb41019aab] irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection git bisect good 46135d6f878ab00261d4a2082d620bfb41019aab # bad: [05d7bf817019890e4d049e0b851940c596adbd9b] dt-bindings: interrupt-controller: Add IDT 79RC3243x Interrupt Controller git bisect bad 05d7bf817019890e4d049e0b851940c596adbd9b # bad: [1a0b05e435544cd53cd3936bdab425d88784b71a] irqdomain: Get rid of irq_create_strict_mappings() git bisect bad 1a0b05e435544cd53cd3936bdab425d88784b71a # bad: [5f8b938bd790cff6542c7fe3c1495c71f89fef1b] irqchip/jcore-aic: Kill use of irq_create_strict_mappings() git bisect bad 5f8b938bd790cff6542c7fe3c1495c71f89fef1b # bad: [b68761da01114a64b9c521975c3bca6d10eeb950] ARM: PXA: Kill use of irq_create_strict_mappings() git bisect bad b68761da01114a64b9c521975c3bca6d10eeb950 # first bad commit: [b68761da01114a64b9c521975c3bca6d10eeb950] ARM: PXA: Kill use of irq_create_strict_mappings()
Hi Guenter, Thanks for the heads up. On Mon, 26 Apr 2021 23:39:42 +0100, Guenter Roeck <linux@roeck-us.net> wrote: > > On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote: > > irq_create_strict_mappings() is a poor way to allow the use of > > a linear IRQ domain as a legacy one. Let's be upfront about > > it and use a legacy domain when appropriate. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > When running the "mainstone" qemu emulation, this patch results > in many (32, actually) runtime warnings such as the following. > > [ 0.528272] ------------[ cut here ]------------ > [ 0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550 irq_domain_associate+0x194/0x1f0 > [ 0.528315] error: virq335 is not allocated [...] This looks like a case of CONFIG_SPARSE_IRQ, combined with a lack of brain engagement. I've come up with the following patch, which lets the kernel boot in QEMU without screaming (other than the lack of a rootfs...). Please let me know if this helps. Thanks, M. From 4d7f6ddbbfdff1c9f029bafca79020d3294dc32c Mon Sep 17 00:00:00 2001 From: Marc Zyngier <maz@kernel.org> Date: Tue, 27 Apr 2021 09:00:28 +0100 Subject: [PATCH] ARM: PXA: Fix cplds irqdesc allocation when using legacy mode The Mainstone PXA platform uses CONFIG_SPARSE_IRQ, and thus we cannot rely on the irq descriptors to be readilly allocated before creating the irqdomain in legacy mode. The kernel then complains loudly about not being able to associate the interrupt in the domain -- can't blame it. Fix it by allocating the irqdescs upfront in the legacy case. Fixes: b68761da0111 ("ARM: PXA: Kill use of irq_create_strict_mappings()") Reported-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm/mach-pxa/pxa_cplds_irqs.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c b/arch/arm/mach-pxa/pxa_cplds_irqs.c index ec0d9b094744..bddfc7cd5d40 100644 --- a/arch/arm/mach-pxa/pxa_cplds_irqs.c +++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c @@ -121,8 +121,13 @@ static int cplds_probe(struct platform_device *pdev) return fpga->irq; base_irq = platform_get_irq(pdev, 1); - if (base_irq < 0) + if (base_irq < 0) { base_irq = 0; + } else { + ret = devm_irq_alloc_descs(&pdev->dev, base_irq, base_irq, CPLDS_NB_IRQ, 0); + if (ret < 0) + return ret; + } res = platform_get_resource(pdev, IORESOURCE_MEM, 0); fpga->base = devm_ioremap_resource(&pdev->dev, res);
On 4/27/21 1:30 AM, Marc Zyngier wrote: > Hi Guenter, > > Thanks for the heads up. > > On Mon, 26 Apr 2021 23:39:42 +0100, > Guenter Roeck <linux@roeck-us.net> wrote: >> >> On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote: >>> irq_create_strict_mappings() is a poor way to allow the use of >>> a linear IRQ domain as a legacy one. Let's be upfront about >>> it and use a legacy domain when appropriate. >>> >>> Signed-off-by: Marc Zyngier <maz@kernel.org> >>> --- >> >> When running the "mainstone" qemu emulation, this patch results >> in many (32, actually) runtime warnings such as the following. >> >> [ 0.528272] ------------[ cut here ]------------ >> [ 0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550 irq_domain_associate+0x194/0x1f0 >> [ 0.528315] error: virq335 is not allocated > > [...] > > This looks like a case of CONFIG_SPARSE_IRQ, combined with a lack of > brain engagement. I've come up with the following patch, which lets > the kernel boot in QEMU without screaming (other than the lack of a > rootfs...). > > Please let me know if this helps. > It does. Tested-by: Guenter Roeck <linux@roeck-us.net> Thanks, Guenter > Thanks, > > M. > > From 4d7f6ddbbfdff1c9f029bafca79020d3294dc32c Mon Sep 17 00:00:00 2001 > From: Marc Zyngier <maz@kernel.org> > Date: Tue, 27 Apr 2021 09:00:28 +0100 > Subject: [PATCH] ARM: PXA: Fix cplds irqdesc allocation when using legacy mode > > The Mainstone PXA platform uses CONFIG_SPARSE_IRQ, and thus we > cannot rely on the irq descriptors to be readilly allocated > before creating the irqdomain in legacy mode. The kernel then > complains loudly about not being able to associate the interrupt > in the domain -- can't blame it. > > Fix it by allocating the irqdescs upfront in the legacy case. > > Fixes: b68761da0111 ("ARM: PXA: Kill use of irq_create_strict_mappings()") > Reported-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm/mach-pxa/pxa_cplds_irqs.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c b/arch/arm/mach-pxa/pxa_cplds_irqs.c > index ec0d9b094744..bddfc7cd5d40 100644 > --- a/arch/arm/mach-pxa/pxa_cplds_irqs.c > +++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c > @@ -121,8 +121,13 @@ static int cplds_probe(struct platform_device *pdev) > return fpga->irq; > > base_irq = platform_get_irq(pdev, 1); > - if (base_irq < 0) > + if (base_irq < 0) { > base_irq = 0; > + } else { > + ret = devm_irq_alloc_descs(&pdev->dev, base_irq, base_irq, CPLDS_NB_IRQ, 0); > + if (ret < 0) > + return ret; > + } > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > fpga->base = devm_ioremap_resource(&pdev->dev, res); >
diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c b/arch/arm/mach-pxa/pxa_cplds_irqs.c index 45c19ca96f7a..ec0d9b094744 100644 --- a/arch/arm/mach-pxa/pxa_cplds_irqs.c +++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c @@ -147,22 +147,20 @@ static int cplds_probe(struct platform_device *pdev) } irq_set_irq_wake(fpga->irq, 1); - fpga->irqdomain = irq_domain_add_linear(pdev->dev.of_node, - CPLDS_NB_IRQ, - &cplds_irq_domain_ops, fpga); + if (base_irq) + fpga->irqdomain = irq_domain_add_legacy(pdev->dev.of_node, + CPLDS_NB_IRQ, + base_irq, 0, + &cplds_irq_domain_ops, + fpga); + else + fpga->irqdomain = irq_domain_add_linear(pdev->dev.of_node, + CPLDS_NB_IRQ, + &cplds_irq_domain_ops, + fpga); if (!fpga->irqdomain) return -ENODEV; - if (base_irq) { - ret = irq_create_strict_mappings(fpga->irqdomain, base_irq, 0, - CPLDS_NB_IRQ); - if (ret) { - dev_err(&pdev->dev, "couldn't create the irq mapping %d..%d\n", - base_irq, base_irq + CPLDS_NB_IRQ); - return ret; - } - } - return 0; }
irq_create_strict_mappings() is a poor way to allow the use of a linear IRQ domain as a legacy one. Let's be upfront about it and use a legacy domain when appropriate. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm/mach-pxa/pxa_cplds_irqs.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)