netdev - Re: Warning: myri10ge/sky2/vxge/r8169/niu/bnx2/bnx2x/igb/e100e/cxgb3/mlx4/tg3/vxge : removal of PCI_CAP_ID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Tue, 5 Jul 2011 22:14:57 -0500
From:	Jon Mason <jdmason@...zu.us>
To:	James Smart <james.smart@...lex.com>
Cc:	netdev@...r.kernel.org, Richard Lary <rlary@...ibm.com>,
	"davem@...emloft.net" <davem@...emloft.net>, gallatin@...i.com,
	shemminger@...ux-foundation.org, nic_swsd@...ltek.com,
	romieu@...zoreil.com, eilong@...adcom.com, mchan@...adcom.com,
	jeffrey.t.kirsher@...el.com, e1000-devel@...ts.sourceforge.net,
	divy@...lsio.com, yevgenyp@...lanox.co.il, mcarlson@...adcom.com
Subject: Re: Warning: myri10ge/sky2/vxge/r8169/niu/bnx2/bnx2x/igb/e100e/cxgb3/mlx4/tg3/vxge
 : removal of PCI_CAP_ID_EXP

On Fri, Jul 1, 2011 at 10:28 AM, Jon Mason <jdmason@...zu.us> wrote:
> On Fri, Jul 1, 2011 at 9:49 AM, James Smart <james.smart@...lex.com> wrote:
>> All,
>>
>> I wanted to communicate a potential warning to those drivers that had a
>> patch submitted to replace config space searches of PCI_CAP_ID_EXP with
>> shorthand options such as is_pcie and pci_is_pcie().
>>
>> Testing with the lpfc driver and AER/EEH identified cases where the
>> short-hand search options would fail on PPC platforms.  The only successful
>> option in all cases was the explicit search via PCI_CAP_ID_EXP.   Therefore,
>> I recommend that this change not be accepted until the platform level issue
>> can be identified and corrected.
>
> pci_is_pcie checks for a PCI-E capability offset that was determined
> by calling pci_find_capability during the PCI bus walking.  Based on
> your description above this 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).
>
> I have e-mailed the PPC and PCI mailing lists to verify the issue.

Per Richard Lary's testing, this is not an issue with the latest kernel.
http://www.spinics.net/lists/linux-pci/msg11350.html

Thanks,
Jon


>
> Thanks,
> Jon
>
>
>>
>> -- james s
>>
>>
>>
>> On 6/30/2011 4:41 PM, James Smart wrote:
>>>
>>> Jon,
>>>
>>> I must NACK this patch to the lpfc driver and recommend that all other
>>> patches
>>> which replace pci_find_capability(pdef, PCI_CAP_ID_EXP) with
>>> "pci_is_pcie(pdev)" are NACK'd as well.
>>>
>>> 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.
>>>
>>> -- james s
>>>
>>>
>>>
>>> On 6/27/2011 1:39 PM, Jon Mason wrote:
>>>>
>>>> The PCIE capability offset is saved during PCI bus walking.  It will
>>>> remove an unnecessary search in the PCI configuration space if this
>>>> value is referenced instead of reacquiring it.  Also, pci_is_pcie is a
>>>> better way of determining if the device is PCIE or not (as it uses the
>>>> same saved PCIE capability offset).
>>>>
>>>> Signed-off-by: Jon Mason<jdmason@...zu.us>
>>>> ---
>>>>   drivers/scsi/lpfc/lpfc_init.c |    2 +-
>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/lpfc/lpfc_init.c
>>>> b/drivers/scsi/lpfc/lpfc_init.c
>>>> index 148b98d..9000ad0 100644
>>>> --- a/drivers/scsi/lpfc/lpfc_init.c
>>>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>>>> @@ -3970,7 +3970,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba)
>>>>        pci_save_state(pdev);
>>>>
>>>>        /* PCIe EEH recovery on powerpc platforms needs fundamental reset
>>>> */
>>>> -       if (pci_find_capability(pdev, PCI_CAP_ID_EXP))
>>>> +       if (pci_is_pcie(pdev))
>>>>                pdev->needs_freset = 1;
>>>>
>>>>        return 0;
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>>> the body of a message to majordomo@...r.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html