On Wednesday, June 02, 2010 09:53:23 am Mike Habeck wrote: > Bjorn Helgaas wrote: > > I'm still having trouble reconciling the stated purpose, i.e., the > > changelog, with the behavior. The changelog implies that the patch > > is required to make >16 devices with I/O BARs work at all, but per > > Mike Habeck, the patch just gets rid of some warnings and maybe helps > > with hot-add of devices using I/O space. > > greaterthan 16 devices will still work if the BIOS doesn't assign > the I/O BARs and the kernel does (including the devices that don't > get assigned due to the kernel running out of I/O Port resources). > And when the kernel runs out of I/O Port space it will warn for > those devices it couldn't assign: (example): > > pci 0002:03:00.0: BAR 5: can't allocate I/O resource [0x0-0x7f] [For anybody else grepping for this message, it is now "can't assign", not "can't allocate".] > But if the kernel assigns all the I/O Port space to these devices > we know don't need it (thus the reason the BIOS didn't assign it) > then I believe hot-add of devices using I/O space will fail. Yes, I understand and agree with this motivation, though I hope "pci=nobar" is recognized as only a short-term workaround. I think it's completely unacceptable in terms of the user experience. Using DMI quirks to turn it on automatically removes some user pain, but it's not a real solution and still leaves a maintenance problem. How about if we write a changelog that mentions hot-add of devices that need I/O space? :-) > There is also the issue that quirk_system_pci_resources() thinks > those unassigned I/O resources are using I/O port space 0x0-0xFF. > Since the BIOS never assigned the BAR the kernel reads it as > having a base of 0x0 and a limit of whatever the BAR size is when > it writes all 1's to obtain the size. So that results in > quirk_system_pci_resources() disabling pnp devices: (example): > > pnp 00:11: io resource (0x92-0x92) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling [...] > > One could argue this is a quirk_system_pci_resources() issue and > should be handled there rather than "zeroing out the resource if > the bios didn't assign it" as the patch does, but what the patch > is attempting to do (as stated above) is to allow the BIOS a way > to not assign resources it knows are not needed and thus make sure > the kernel doesn't override that... and in doing that the quirk > issue goes away too. This is definitely a problem, but I think it should be handled separately. The fact that this patch makes the "overlap" messages go away is a nice coincidence, but it doesn't fix the underlying issue. I think setting the resource start/end to zero to "disable" it as in this patch (and elsewhere) is just a hack -- we no longer know the size of the BAR, the struct resource no longer corresponds to the BAR contents, and I don't think we do anything with the PCI_COMMAND_IO/PCI_COMMAND_MEM bits to actually disable the BAR, so the device probably still responds at whatever we left in the BAR. In quirk_system_pci_resources(), we set IORESOURCE_DISABLED rather than setting the PNP resource start/end to zero, but that's a bit of a hack, too. All that does is prevent the PNP motherboard driver (pnp/system.c) from requesting that region. We don't actually do anything to stop the device from responding to that address. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
