linux-kernel - Re: [PATCH V8 4/5] PCI/ASPM: save power on values during bridge init

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: <CAErSpo7TO1rfG6MDYuohyXL4WbaGCJdV8y3Ke-uL6VjGRb1hvw@mail.gmail.com>
Date:   Tue, 25 Apr 2017 13:45:13 -0500
From:   Bjorn Helgaas <bhelgaas@...gle.com>
To:     "Patel, Mayurkumar" <mayurkumar.patel@...el.com>
Cc:     Sinan Kaya <okaya@...eaurora.org>,
        Bjorn Helgaas <helgaas@...nel.org>,
        Rajat Jain <rajatja@...gle.com>,
        Rajat Jain <rajatxjain@...il.com>,
        David Daney <david.daney@...ium.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        Timur Tabi <timur@...eaurora.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Julia Lawall <Julia.Lawall@...6.fr>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Yinghai Lu <yinghai@...nel.org>,
        Shawn Lin <shawn.lin@...k-chips.com>,
        linux-arm <linux-arm-kernel@...ts.infradead.org>,
        Myron Stowe <myron.stowe@...hat.com>
Subject: Re: [PATCH V8 4/5] PCI/ASPM: save power on values during bridge init

On Fri, Apr 21, 2017 at 2:46 AM, Patel, Mayurkumar
<mayurkumar.patel@...el.com> wrote:
> Hi Bjorn/Kaya,
>
>
>>
>>On 4/17/2017 12:38 PM, Bjorn Helgaas wrote:
>>>> Like you said, what do we do by default is the question. Should we opt
>>>> for safe like we are doing, or try to save some power.
>>> I think safety is paramount.  Every user should be able to boot safely
>>> without any kernel parameters.  We don't want users to have a problem
>>> booting and then have to search for a workaround like booting with
>>> "pcie_aspm=off".  Most users will never do that.
>>>
>>
>>OK, no problem with leaving the behavior as it is.
>>
>>My initial approach was #2. We knew this way that user had full control
>>over the ASPM policy by changing the BIOS option. Then, Mayurkumar
>>complained that ASPM is not enabled following a hotplug insertion to an
>>empty slot. That's when I switched to #3 as it sounded like a good thing
>>to have for us.
>>
>>> Here's a long-term strawman proposal, see what you think:
>>>
>>>   - Deprecate CONFIG_PCIEASPM_DEFAULT, CONFIG_PCIEASPM_POWERSAVE, etc.
>>>   - Default aspm_policy is POLICY_DEFAULT always.
>>>   - POLICY_DEFAULT means Linux doesn't touch anything: if BIOS enabled
>>> ASPM, we leave it that way; we leave ASPM disabled on hot-added
>>> devices.
>>
> I am also ok with leaving the same behavior as now.
> But still following is something open I feel besides, Which may be there in your comments redundantly.
> The current problem is, pcie_aspm_exit_link_state() disables the ASPM configuration even
> if POLICY_DEFAULT was set.

We call pcie_aspm_exit_link_state() when removing an endpoint.  When
we remove an endpoint, I think disabling ASPM is the right thing to
do.  The spec (PCIe r3.1, sec 5.4.1.3) says "Software must not enable
L0s in either direction on a given Link unless components on both
sides of the Link each support L0s; otherwise, the result is
undefined."

> I am seeing already following problem(or may be influence) with it. The Endpoint I have does not have
> does not have "Presence detect change" mechanism. Hot plug is working with Link status events.
> When link is in L1 or L1SS and if EP is powered off, no Link status change event are triggered (It might be
> the expected behavior in L1 or L1SS).  When next time EP is powered on there are link down and
> link up events coming one after other. BIOS enables ASPM on Root port and Endpoint, but while
> processing link status down, pcie_aspm_exit_link_state() clears the ASPM already which were enabled by BIOS.
> If we want to follow above approach then shall we consider having something similar as following?

The proposal was to leave ASPM disabled on hot-added devices.  If the
endpoint was powered off and powered back on again, I think that
device looks like a hot-added device, doesn't it?

Bjorn