pci_pcie_cap invalid on AER/EEH enabled PPC?

[Richard A Lary]

On 7/1/2011 1:00 PM, Richard A Lary wrote:
> 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

On Power PC platforms, set_pcie_port_type() is not called.  On Power PC,
pci_dev structure is initialized by of_create_pci_dev().  However, the
structure member pcie_cap is NOT computed nor set in this function.

The information used to populate pci_dev comes from the Power PC
device_tree passed to the OS by Open Firmware.

Based upon standing Power PC design, we cannot support patches
which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with
pci_is_pcie(pdev) on Power PC platforms.

-rich