Stop ARM boards crashing when CUPS is loaded - 2.6.35-rc5
in [Kernel]
|
Prev: kmemleak: Add DocBook style comments to kmemleak.c
Next: [PATCH] [RFC] CFQ: Make prio_trees per cfq group basis to improve IO performance
From: Lee Jones on 16 Jul 2010 05:10 On 15/07/10 21:06, Russell King - ARM Linux wrote: > On Thu, Jul 15, 2010 at 01:02:14PM -0700, Andrew Morton wrote: >>> BugLink: http://bugs.launchpad.net/bugs/601226 >>> >>> When CUPS loads, it tries to load several drivers that it may need. >>> When one of these drivers, specifically parport_pc is loaded on ARM >>> based systems, it causes a segmentation fault as the ISA addresses >>> which are attempted are not writable on non-PC based architectures. >>> This code prevents ISA addresses from being attempted except on x86. >>> >> >> That sounds like a pretty serious problem. But presumably it isn't - >> otherwise it would have been fixed earlier! Well it is a problem on OMAP based boards. I've personally tested it on OMAP3 and OMAP4 based machines. Perhaps the memory is not reserved correctly. >> So what actions are required to trigger this bug and why aren't others >> seeing it? Probably because most other chips either manage to reserve the memory successfully, or do not attempt to load the parport_pc driver, either as a module or built-in. All I have to do to recreate this bug is load the module or build in the parport_pc driver. > Note that we have machines which have ISA parallel ports, so it's not > this simple. Do they have ISA parallel ports, or do they just pretend to? I found this scattered about: /* * We don't actually have real ISA nor PCI buses, but there is so many * drivers out there that might just work if we fake them... */ Clearly parport_pc isn't one of them. :) > Why not just avoid selecting and building parport_pc on these machines? > I mean, the Beagleboard doesn't have PCI nor ISA, so why is parport_pc > being built for production use? I am happy to make a Kconfig change to disallow OMAP builds from building the parport_pc driver. Do you think this would be more sensible? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Lee Jones on 16 Jul 2010 05:40 > The best solution is probably for the parport code to go through a > modernisation cycle like the serial code did, essentially using > platform devices to pass the base addresses. This would make the > driver more portable, and eliminates this problem entirely (because > platforms which don't have parports won't register the platform device(s) > necessary for parport to even probe illegal addresses.) This sounds brilliant - when are you going to start? </kidding> In all seriousness, do you think anyone is likely to undertake this work anytime soon? I am seeing this problem in a distribution which is due for release in October. I have no problem implementing a config change in the meantime, but as you say, a more _correct_ and portable solution should be sought. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Lee Jones on 16 Jul 2010 07:20 On 16/07/10 10:32, Lee Jones wrote: >> The best solution is probably for the parport code to go through a >> modernisation cycle like the serial code did, essentially using >> platform devices to pass the base addresses. This would make the >> driver more portable, and eliminates this problem entirely (because >> platforms which don't have parports won't register the platform device(s) >> necessary for parport to even probe illegal addresses.) I have lobbied for one of our partners to conduct this work, but I don't think this would be something that is in their interest to fix. Nevertheless, I am currently waiting on a reply from them. FAO GregKH, Do you think this would be something that may interest you and your Linux drivers project? Or perhaps a student who wants to get their feet wet and play with some platform driver code. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Lee Jones on 16 Jul 2010 12:50 On 16/07/10 17:23, Milton Miller wrote: > On Fri Jul 16 2010 about 05:33:06 EST, Lee Jones wrote: >>> The best solution is probably for the parport code to go through a >>> modernisation cycle like the serial code did, essentially using >>> platform devices to pass the base addresses. This would make the >>> driver more portable, and eliminates this problem entirely (because >>> platforms which don't have parports won't register the platform device(s) >>> necessary for parport to even probe illegal addresses.) >> >> This sounds brilliant - when are you going to start? </kidding> > > It has a long time ago ... > > drivers/parport/parport_pc.c calls parport_pc_find_nonpci_ports, > which is in asm/parport.h I'm not entirely sure what you're trying to say here? How does that help with the platformisation of the driver? >> In all seriousness, do you think anyone is likely to undertake this >> work anytime soon? I am seeing this problem in a distribution which >> is due for release in October. I have no problem implementing a config >> change in the meantime, but as you say, a more _correct_ and portable >> solution should be sought. > > Why not replace the arm asm/parport.h with asm-generic/parport.h which > already has a check for CONFIG_ISA, which appears to only be selected > on a few ARM platforms? static int __devinit parport_pc_find_isa_ports(int autoirq, int autodma); static int __devinit parport_pc_find_nonpci_ports(int autoirq, int autodma) { #ifdef CONFIG_ISA return parport_pc_find_isa_ports(autoirq, autodma); #else return 0; #endif } That's perfect! This would work a treat. Surely this #ifdef should be in all the parport.h files which call parport_pc_find_isa_ports? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Milton Miller on 16 Jul 2010 15:40
On Fri, 16 Jul 2010 about 10:42:23 -0600 Lee Jones wrote: >On 16/07/10 17:23, Milton Miller wrote: > > On Fri Jul 16 2010 about 05:33:06 EST, Lee Jones wrote: > > > > The best solution is probably for the parport code to go through a > > > > modernisation cycle like the serial code did, essentially using > > > > platform devices to pass the base addresses. This would make the > > > > driver more portable, and eliminates this problem entirely (because > > > > platforms which don't have parports won't register the platform device(s) > > > > necessary for parport to even probe illegal addresses.) > > > > > > This sounds brilliant - when are you going to start? </kidding> > > > > It has a long time ago ... > > > > drivers/parport/parport_pc.c calls parport_pc_find_nonpci_ports, > > which is in asm/parport.h > >I'm not entirely sure what you're trying to say here? > >How does that help with the platformisation of the driver? I was reading quickly, but my point was the code already defers to the architecture the method of finding the ports to scan, which is the important part. I just looked at the 8250 code and it abuses the platform device model. Instead of a platform device for each port, it has multiple port descriptions set via platform_data in one or a a few device instances. It then only uses this information to fill in its internal array of all ports, which drives the actual registration with the serial core. It also registers a platform device to to get suspend and resume hooks, but now has to scan its list of ports for all the instances driven from this "device". Yech. Please don't use this as an example of a modern driver. > > > > In all seriousness, do you think anyone is likely to undertake this > > > work anytime soon? I am seeing this problem in a distribution which > > > is due for release in October. I have no problem implementing a config > > > change in the meantime, but as you say, a more _correct_ and portable > > > solution should be sought. > > > > Why not replace the arm asm/parport.h with asm-generic/parport.h which > > already has a check for CONFIG_ISA, which appears to only be selected > > on a few ARM platforms? > >static int __devinit parport_pc_find_isa_ports(int autoirq, int autodma); >static int __devinit parport_pc_find_nonpci_ports(int autoirq, int autodma) >{ >#ifdef CONFIG_ISA > return parport_pc_find_isa_ports(autoirq, autodma); >#else > return 0; >#endif >} > >That's perfect! > >This would work a treat. > >Surely this #ifdef should be in all the parport.h files which call >parport_pc_find_isa_ports? No, as CONFIG_ISA is supposed to be ISA slots, and other architectures may frequently have 8250 ports at the pc legacy port numbers without ISA slots. milton -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |