Re: [RFC/PATCH 4/4] [POWERPC] pci: Disable IO/Mem on a device whenresources can't be allocated

[Benjamin Herrenschmidt]

On Mon, 2007-12-24 at 00:23 -0700, Grant Grundler wrote:
> On Tue, Dec 18, 2007 at 10:01:15AM +1100, Benjamin Herrenschmidt wrote:
> > This patch changes the PowerPC 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 <benh@xxxxxxxxxxxxxxxxxxx>
> ....
> > @@ -1062,8 +1065,12 @@ static void __init pcibios_allocate_reso
> >  				disabled = !(command & PCI_COMMAND_IO);
> >  			else
> >  				disabled = !(command & PCI_COMMAND_MEMORY);
> > -			if (pass == disabled)
> > -				alloc_resource(dev, idx);
> > +			if (pass == disabled && alloc_resource(dev, idx)) {
> > +				command &= ~(r->flags & (IORESOURCE_IO |
> > +							 IORESOURCE_MEM));
> 
> While this may be ok for PPC, in general, wouldn't we want to only disable
> which ever type of resource that couldn't be allocated?

This is exactly what's supposed to be happening, but the code is buggy
and nobody noticed :-) (I'm mixing up IORESOURCE_* flags and
PCI_COMMAND_* flags). Thanks for reviewing !

> ie make two calls: alloc_resource_io() and alloc_resource_mem() and disable
> the respective flag if the alloc call fails?

No need for 2 calls, just disable whatever type the resource is, but
using the right bits instead of what my code incorrectly does.

> Thus a device which was enable and programmed by BIOS could remain functional
> despite one resource not being allocated.

Yes, that's what intended by the above code, if I didn't have mixed up
the flags.

Ben.


--
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/