pci_pcie_cap invalid on AER/EEH enabled PPC?

[Jon Mason]

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.

Thanks,
Jon

>
> -rich
>
>
>