pci_pcie_cap invalid on AER/EEH enabled PPC?

[Richard A Lary]

On 7/1/2011 12:02 PM, Jon Mason wrote:
> On Fri, Jul 1, 2011 at 1:30 PM, Richard A Lary<rlary at linux.vnet.ibm.com>  wrote:
>> On 7/1/2011 8:24 AM, Jon Mason wrote:
>>>
>>> I recently sent out a number of patches to migrate drivers calling
>>> `pci_find_capability(pdef, PCI_CAP_ID_EXP)` to pci_pcie_cap.  This
>>> function takes uses a PCI-E capability offset that was determined by
>>> calling pci_find_capability during the PCI bus walking.  In response
>>> to one of the patches, James Smart posted:
>>>
>>> "The reason is due to an issue on PPC platforms whereby use of
>>> "pdev->is_pcie" and pci_is_pcie() will erroneously fail under some
>>> conditions, but explicit search for the capability struct via
>>> pci_find_capability() is always successful.   I expect this to be due
>>> a shadowing of pci config space in the hal/platform that isn't
>>> sufficiently built up.  We detected this issue while testing AER/EEH,
>>> and are functional only if the pci_find_capability() option is used."
>>>
>>> See http://marc.info/?l=linux-scsi&m=130946649427828&w=2 for the whole
>>> post.
>>>
>>> Based on his description above pci_pcie_cap
>>> andpci_find_capability(pdef, PCI_CAP_ID_EXP) should be functionally
>>> equivalent.  If this is not safe, then the PCI bus walking code is
>>> most likely busted on EEH enabled PPC systems (and that is a BIG
>>> problem).  Can anyone confirm this is still an issue?
>>
>> Jon,
>>
>> I applied the following debug patch to lpfc driver in a 2.6.32 distro
>> kernel ( I had this one handy, I can try with mainline later today )
>>
>> ---
>>   drivers/scsi/lpfc/lpfc_init.c |   10   10 +    0 -     0 !
>>   1 file changed, 10 insertions(+)
>>
>> Index: b/drivers/scsi/lpfc/lpfc_init.c
>> ===================================================================
>> --- a/drivers/scsi/lpfc/lpfc_init.c
>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>> @@ -3958,6 +3958,16 @@ lpfc_enable_pci_dev(struct lpfc_hba *phb
>>         pci_try_set_mwi(pdev);
>>         pci_save_state(pdev);
>>
>> +       printk(KERN_WARNING "pcicap: is_pcie=%x pci_cap=%x pcie_type=%x\n",
>> +               pdev->is_pcie,
>> +               pdev->pcie_cap,
>> +               pdev->pcie_type);
>> +
>> +       if (pci_is_pcie(pdev))
>> +               printk(KERN_WARNING "pcicap: true\n");
>> +       else
>> +               printk(KERN_WARNING "pcicap: false\n");
>> +
>>         /* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>>         if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>                 pdev->needs_freset = 1;
>>
>> This is output upon driver load on an IBM Power 7 model 8233-E8B server.
>>
>> dmesg | grep pcicap
>> Linux version 2.6.32.42-pcicap-ppc64 (geeko at buildhost) (gcc version 4.3.4
>> [gcc-4_3-branch revision 152973] (SUSE Linux) ) #1 SMP Fri Jul 1 09:31:27
>> PDT 2011
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>> pcicap: is_pcie=0 pci_cap=0 pcie_type=0
>> pcicap: false
>>
>> It would appear that the pcie information is not set in pci_dev structure
>> for
>> this device at the time the driver is being initialized during boot.
>
> Thanks for trying this.  Can you confirm that the other devices in the
> system have this issue as well (or show that it is isolated to the lpr
> device)?  You can add printks in set_pcie_port_type() to verify what
> is being set on bus walking and to see when it is being called with
> respect to when it is being populated by firmware.

Jon,

I will give this suggestion a try and post results

-rich