Re: [RFC/PATCH]] x86: pci: Disable IO/Mem on a device when resources can't be allocated
On Wed, 2007-12-19 at 16:43 +0300, Ivan Kokshaysky wrote: > On Wed, Dec 19, 2007 at 04:10:19PM +1100, Benjamin Herrenschmidt wrote: > > This patch changes the x86 PCI code to disable IO and/or Memory > > decoding on a PCI device when a resource of that type failed to > > be allocated. > > Oh, this opens up a can of worms ;-) > > In ideal world, the patch would be perfectly valid. But on x86 we have > at least two firmware layers (E820 and ACPI), each with its own (often > totally crazy) view on system resources. OTOH, we cannot completely ignore > the firmware - otherwise the resource allocator could step onto some > hidden/undocumented system decode ranges... > One of the typical reasons for "dangling BAR" on x86 is that a resource > allocation failed because BIOS has reserved address region for the > very same BAR ;-) > > Perhaps you've seen most recent illustration of x86 resource allocation > problems: > http://lkml.org/lkml/2007/12/17/429 Yeah, gives me headaches. > Plus there are lots of x86 hardware like host bridges with a BAR > representing the system RAM, IOAPIC BARs with some high order bits > hardwired to "1" and so on. Which also doesn't make life any easier. Ok. We tent do have quirks for those on powerpc. > That's why I still think that res->parent check is not enough for > x86 and we need some resource flag that tells generic PCI code > "We failed to allocate this resource, but please don't touch it!". > Some code is already using IORESOURCE_PCI_FIXED for that purpose, so it > would suffice. Yup, possibly. On the other hand, it's also used for other things. It normally means a fixed decode resource, such as IDE in legacy mode. If that conflicts for real, then the only option -is- to disable the device. The problem on x86 seems to be to differenciate what conflicts for real and what is just this resource management crackpot. > On the other hand, with that flag we can move all those horrible > exceptions like PCI_CLASS_BRIDGE_HOST (which nearly breaks alpha, BTW) > and PCI_CLASS_SYSTEM_PIC from generic code to arch/x86 where it belongs. Heh, possibly yeah :-) > > I'll wait for more comments today and post the whole 5 again tomorrow > > as official candidates for inclusion :-) (BTW. What is people general > > feeling about inline vs. non inline for the functions in pci.c ?) > > I think inlines are prettier, but not allowing direct use of the _flag > function is a valid argument too. So I'm fine with both. Ok. I'll keep the x86 patch out for now though. I'll let others sort out what happens on this platform. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC/PATCH]] x86: pci: Disable IO/Mem on a device when resources can't be allocated
On Wed, Dec 19, 2007 at 04:10:19PM +1100, Benjamin Herrenschmidt wrote: > This patch changes the x86 PCI code to disable IO and/or Memory > decoding on a PCI device when a resource of that type failed to > be allocated. Oh, this opens up a can of worms ;-) In ideal world, the patch would be perfectly valid. But on x86 we have at least two firmware layers (E820 and ACPI), each with its own (often totally crazy) view on system resources. OTOH, we cannot completely ignore the firmware - otherwise the resource allocator could step onto some hidden/undocumented system decode ranges... One of the typical reasons for "dangling BAR" on x86 is that a resource allocation failed because BIOS has reserved address region for the very same BAR ;-) Perhaps you've seen most recent illustration of x86 resource allocation problems: http://lkml.org/lkml/2007/12/17/429 Plus there are lots of x86 hardware like host bridges with a BAR representing the system RAM, IOAPIC BARs with some high order bits hardwired to "1" and so on. Which also doesn't make life any easier. That's why I still think that res->parent check is not enough for x86 and we need some resource flag that tells generic PCI code "We failed to allocate this resource, but please don't touch it!". Some code is already using IORESOURCE_PCI_FIXED for that purpose, so it would suffice. On the other hand, with that flag we can move all those horrible exceptions like PCI_CLASS_BRIDGE_HOST (which nearly breaks alpha, BTW) and PCI_CLASS_SYSTEM_PIC from generic code to arch/x86 where it belongs. > I'll wait for more comments today and post the whole 5 again tomorrow > as official candidates for inclusion :-) (BTW. What is people general > feeling about inline vs. non inline for the functions in pci.c ?) I think inlines are prettier, but not allowing direct use of the _flag function is a valid argument too. So I'm fine with both. Ivan. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC/PATCH]] x86: pci: Disable IO/Mem on a device when resources can't be allocated
This patch changes the x86 PCI code to disable IO and/or Memory decoding on a PCI device when a resource of that type failed to be allocated. This is done to avoid having unallocated dangling BARs enabled that might try to decode on top of other devices. If a proper resource is assigned later on, then pci_enable_device{,_io,_mem} will take care of re-enabling decoding. Signed-off-by: Benjamin Herrenschmidt <[EMAIL PROTECTED]> --- On top of my previous 4 patches to replace pci_enable_device_bars() I don't have any x86 at hand to test it... this will probably want to simmer a little bit in -mm, though it shouldn't have any problem of user setups that don't already have dodgy resource setup. In fact, it might turn weird latent bugs into right away visible ones. I'll wait for more comments today and post the whole 5 again tomorrow as official candidates for inclusion :-) (BTW. What is people general feeling about inline vs. non inline for the functions in pci.c ?) Index: linux-merge/arch/x86/pci/i386.c === --- linux-merge.orig/arch/x86/pci/i386.c2007-12-19 16:00:31.0 +1100 +++ linux-merge/arch/x86/pci/i386.c 2007-12-19 16:02:20.0 +1100 @@ -143,12 +143,32 @@ static void __init pcibios_allocate_bus_ } } +static inline int __init pcibios_alloc_resource(struct pci_dev *dev, int idx) +{ + struct resource *pr, *r = &dev->resource[idx]; + + DBG("PCI: Resource %08lx-%08lx (f=%lx, d=%d, p=%d)\n", + r->start, r->end, r->flags, disabled, pass); + + pr = pci_find_parent_resource(dev, r); + if (!pr || request_resource(pr, r) < 0) { + printk(KERN_ERR "PCI: Cannot allocate " + "resource region %d of device %s\n", + idx, pci_name(dev)); + /* We'll assign a new address later */ + r->end -= r->start; + r->start = 0; + return -EBUSY; + } + return 0; +} + static void __init pcibios_allocate_resources(int pass) { struct pci_dev *dev = NULL; int idx, disabled; u16 command; - struct resource *r, *pr; + struct resource *r; for_each_pci_dev(dev) { pci_read_config_word(dev, PCI_COMMAND, &command); @@ -162,20 +182,11 @@ static void __init pcibios_allocate_reso disabled = !(command & PCI_COMMAND_IO); else disabled = !(command & PCI_COMMAND_MEMORY); - if (pass == disabled) { - DBG("PCI: Resource %08lx-%08lx " - "(f=%lx, d=%d, p=%d)\n", - r->start, r->end, r->flags, disabled, pass); - pr = pci_find_parent_resource(dev, r); - if (!pr || request_resource(pr, r) < 0) { - printk(KERN_ERR "PCI: Cannot allocate " - "resource region %d " - "of device %s\n", - idx, pci_name(dev)); - /* We'll assign a new address later */ - r->end -= r->start; - r->start = 0; - } + if (pass == disabled && pcibios_alloc_resource(dev, idx)) { + command &= ~(r->flags & (IORESOURCE_IO | +IORESOURCE_MEM)); + pci_write_config_word(dev, + PCI_COMMAND, command); } } if (!pass) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/