On Fri, Apr 21, 2017 at 2:46 AM, Patel, Mayurkumar <mayurkumar.patel@xxxxxxxxx> 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