On Tue, Jul 5, 2011 at 10:41 AM, Richard A Lary <rlary@xxxxxxxxxxxxxxxxxx> wrote: > 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@xxxxxxxxxxxxxxxxxx> >>> 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@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. Yes, it is. of_create_pci_dev() calls set_pcie_port_type() http://lxr.linux.no/linux+v2.6.39/arch/powerpc/kernel/pci_of_scan.c#L144 That function sets pdev->pcie_cap http://lxr.linux.no/linux+v2.6.39/drivers/pci/probe.c#L896 So, it should be set. It looks like there is a bug in of_create_pci_dev, as set_pcie_port_type is being called BEFORE the BARs are setup. If you move set_pcie_port_type prior to pci_device_add (perhaps even after), then I bet the issue is resolved. Thanks, Jon > 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
