linux-kernel - Re: [PATCH] PCI: pciehp: Include the Data Link Layer State Changed bit when clearing the Slot Status register's event bits

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53A0C763.7080003@roeck-us.net>
Date:	Tue, 17 Jun 2014 15:55:31 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Bjorn Helgaas <bhelgaas@...gle.com>,
	Myron Stowe <myron.stowe@...hat.com>
CC:	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Rajat Jain <rajatjain.linux@...il.com>
Subject: Re: [PATCH] PCI: pciehp: Include the Data Link Layer State Changed
 bit when clearing the Slot Status register's event bits

On 06/17/2014 02:07 PM, Bjorn Helgaas wrote:
> On Tue, Jun 17, 2014 at 1:27 PM, Myron Stowe <myron.stowe@...hat.com> wrote:
>> During PCIe hot-plug initialization - pciehp_probe - data structures
>> related to slot capabilities are set up.  As part of this set up, ISRs are
>> put in place to handle slot events and all event bits are cleared out.
>>
>> This patch adds the Data Link Layer State Changed (PCI_EXP_SLTSTA_DLLSC)
>> Slot Status bit to the event bits that are cleared out during
>> initialization.
>
> I think this is related to e48f1b67f668 ("PCI: pciehp: Use link change
> notifications for hot-plug and removal").  Prior to that, pcie_isr()
> didn't look at the PCI_EXP_SLTSTA_DLLSC bit.
>
> Apparently there's a non-public report of spurious messages like this
> at boot-time, with no actual hotplug events:
>
>    pciehp 0000:82:04.0:pcie24: slot(4): Link Up event
>    pciehp 0000:82:04.0:pcie24: Device 0000:83:00.0 already exists at
> 0000:83:00, cannot hot-add
>    pciehp 0000:82:04.0:pcie24: Cannot add device at 0000:83:00
>

I think I have seen that message once in a while in our systems.
Rajat, didn't we talk about this a while ago ?

Guenter

> Device 0000:83:00.0 was enumerated previously, during the normal PCI
> device enumeration.  I suspect DLLSC was set by the hardware when the
> link came up after power-on, and it remained set until Linux booted.
> Then when we take the first pciehp interrupt, we notice DLLSC is set
> and think it's a new link state change.
>
> This might be system-specific, because some BIOSes might clear DLLSC
> before handing off to the OS.  Or my theory might be all wet.
>
> But I think we should clear DLLSC in any case.  We clear all the other
> RW1C bits in Slot Status, and I think we should clear DLLSC as well.
>
> I'd like to include a bugzilla or mailing list reference for the
> spurious message, and I'd also like confirmation that this changes
> actually fixes it.
>
> Bjorn
>
>> Reference:
>>    PCI-SIG.  PCI Express Base Specification Revision 4.0 Version 0.3
>>    (PCI-SIG, 2014): 7.8.11. Slot Status Register (Offset 1Ah).
>>
>> Signed-off-by: Myron Stowe <myron.stowe@...hat.com>
>> ---
>>
>>   drivers/pci/hotplug/pciehp_hpc.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
>> index 42914e0..0568416 100644
>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -794,7 +794,7 @@ struct controller *pcie_init(struct pcie_device *dev)
>>          pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
>>                  PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
>>                  PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
>> -               PCI_EXP_SLTSTA_CC);
>> +               PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC);
>>
>>          /* Disable software notification */
>>          pcie_disable_notification(ctrl);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/