Arnd Bergmann wrote:
> On Maandag 31 Januar 2005 22:35, Brian King wrote:
> 
>>Matthew Wilcox wrote:
>>
>>>Basically, ppc64's config ops are broken and need to check the offset
>>>being read.  Here's i386:
>>>
>>>static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v
>>>alue)
>>>{
>>>        unsigned long flags;
>>>
>>>        if ((bus > 255) || (devfn > 255) || (reg > 255)) 
>>>                return -EINVAL;
>>
>>Here is a pure ppc64 implementation that does this.
> 
> 
> Actually, it doesn't:
> 
> 
>>+static int config_access_valid(struct device_node *dn, int where)
>>+{
>>+       struct device_node *hose_dn = dn->phb->arch_data;
>>+
>>+       if (where < 256 || hose_dn->pci_ext_config_space)
>>+               return 1;
> 
> 
> This needs a check for (where < 4096) in case of PCIe or PCI-X.

Done.

>>@@ -62,6 +72,8 @@ static int rtas_read_config(struct devic
>>                return PCIBIOS_DEVICE_NOT_FOUND;
>>        if (where & (size - 1))
>>                return PCIBIOS_BAD_REGISTER_NUMBER;
>>+       if (!config_access_valid(dn, where))
>>+               return PCIBIOS_BAD_REGISTER_NUMBER;
>> 
>>        addr = (dn->busno << 16) | (dn->devfn << 8) | where;
> 
> 
> addr is still wrong, see my previous mail.

Fixed.


>>@@ -285,6 +309,7 @@ static int __devinit setup_phb(struct de
>>        phb->arch_data = dev;
>>        phb->ops = &rtas_pci_ops;
>>        phb->buid = get_phb_buid(dev);
>>+       get_phb_config_space_type(dev);
>> 
>>        return 0;
>> }
> 
> 
> Isn't the config space size a property of the PCI device instead of the
> host bridge? For a PCI device behind a PCIe host bridge, this could
> still lead to an incorrect config space accesses.

It is a property of both. Accessing config space beyond the first 256 
bytes will only work if both the PCI device and the host bridge support 
it. The problem I ran into was generic pci code issuing a config read to 
offset 256 after checking that the device supports it when the host 
bridge did not support it.

> PS: I got a permanent fatal error from <linux-pci@vger.kernel.org>, does
> that list actually exist?

Sorry about that... Should be fixed on this thread now. I checked the 
archives and saw a thread related to adding another L: line to the 
MAINTAINERS file for the linux-pci list. Greg - was some flavor of that 
patch going in?

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center