[V8,4/5] PCI/ASPM: save power on values during bridge init

Commit Message

Patel, Mayurkumar April 21, 2017, 7:46 a.m. UTC

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.
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?

 

>I can easily see people complaining the other way around. There
>could be some boot FW that doesn't know what ASPM is and this particular
>system could rely on the compile time option for enabling power options.
>Maybe, a command line option will be required for them to keep the existing
>behavior.
>
>>   - Deprecate kernel boot parameters (possibly keep pcie_aspm=off for
>> debugging use).
>>   - Use /sys/module/pcie_aspm/parameters/policy for run-time
>> system-wide  control, including for future hot-added devices.
>>   - Remove CONFIG_PCIEASPM_DEBUG and enable that code always, so we
>> have fine-grained run-time control.
>>
>
>Runtime control sounds like a better plan. We need hooks into the system
>power management policy.
>
>>> Maybe, we are missing a HPP option from the PCI spec.
>> That's an interesting idea.  _HPX does have provision for manipulating
>> Link Control bits (see ACPI r5.0, sec 6.2.8.3), but I don't think very
>> many systems implement it.  And there's currently no connection
>> between program_hpp_type2() and aspm.c, so I'm a little worried that
>> we might have issues if a system did implement an _HPX that sets any
>> of the ASPM bits.
>
>I looked at the spec some more. These are there to restore the register
>settings following hotplug insertion. I agree it won't play nice with ASPM
>as the control bits need to be enabled in coordination with the upstream
>device.
>
>I think the right approach is to let the userspace feed the required
>policy to the kernel like you suggested. Then, it needs to be per port
>via link_state to have the most flexibility.
>
>
>--
>Sinan Kaya
>Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
>Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: [phone], www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Comments

Sinan Kaya April 21, 2017, 1:50 p.m. UTC | #1

On 4/21/2017 3:46 AM, Patel, Mayurkumar wrote:
> If we want to follow above approach then shall we consider having something similar as following?

Do you see this problem if you boot with pcie_aspm.policy=powersave option?

Patel, Mayurkumar April 21, 2017, 2:13 p.m. UTC | #2

>

>On 4/21/2017 3:46 AM, Patel, Mayurkumar wrote:

>> If we want to follow above approach then shall we consider having something similar as following?

>

>Do you see this problem if you boot with pcie_aspm.policy=powersave option?

>


No problems. with pcie_aspm.policy=powersave(L1SS are not enabled in this case
but L1 stays ok all the time after many Power(hotplug) cycles but I think that is expected with this policy)
and pcie_aspm.policy=powersupersave (L1/L1SS both stays ok all the time).

>

>--

>Sinan Kaya

>Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.

>Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: [phone], www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Bjorn Helgaas April 25, 2017, 6:45 p.m. UTC | #3

On Fri, Apr 21, 2017 at 2:46 AM, Patel, Mayurkumar
<[e-mail]> 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

Patel, Mayurkumar May 2, 2017, 12:02 p.m. UTC | #4

Hi Bjorn

>

>On Fri, Apr 21, 2017 at 2:46 AM, Patel, Mayurkumar

><[e-mail]> 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."

>


Yes, you are right and per spec also it makes sense that ASPM needs to be disabled.
But, if POLICY_DEFAULT is set then, shouldn't BIOS take care of disabling ASPM?



>> 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?

>


Yes, it is hot-added device. Also, I understand, for POLICY_DEFAULT, OS would/should not touch ASPM(enable/disable),
but BIOS could still (enable/disable), right?

Currently, what happens in my system is as following, (each 2nd power cycle/hotplug of Endpoint disables ASPM):


First Power cycle (When ASPM L1 is already enabled):
device gets powered off -> there are no Link status events, so no pcie hotplug interrupt and pcie_aspm_exit_link_state() triggered.
When the device gets powered on again -> Link down/Link up events are coming back to back. 
First Link down is served. (BIOS checks for the Link status and enables ASPM already, as the device is
actually powered back). OS calls pcie_aspm_exit_link_state() and ASPM gets disabled by OS.

Second Power cycle (When ASPM L1 is disabled after above):
device gets powered off -> there are link status events, pcie hotplug interrupt is triggered and pcie_aspm_exit_link_state() triggered.
OS disables ASPM. BIOS checks Link status and disables ASPM too.
When the device gets powered on -> BIOS enables ASPM and as this is pcie hotplug insertion, OS
does not interfere and we have ASPM enabled.

The above sequence happens each 2nd power cycle of the hotplug device.

So One could still argue if POLICY_DEFAULT is set, then why OS disables ASPM if it is not meant to touch configuration.
This is why I proposed following kind of change, so that OS would not touch ASPM, if POLICY_DEFAULT is set.
Also, With the below change, everything relies on BIOS for ASPM when POLICY_DEFAULT is set and I see above problem
gets resolved. Also, the existing ASPM behavior does not have impact, unless specific BIOS does not disable ASPM on
Root Port when device gets removed.



>Bjorn

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: [phone], www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Bjorn Helgaas May 3, 2017, 9:10 p.m. UTC | #5

On Tue, May 02, 2017 at 12:02:53PM +0000, Patel, Mayurkumar wrote:
> >On Fri, Apr 21, 2017 at 2:46 AM, Patel, Mayurkumar
> ><[e-mail]> wrote:
> >>>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."
> 
> Yes, you are right and per spec also it makes sense that ASPM needs
> to be disabled.  But, if POLICY_DEFAULT is set then, shouldn't BIOS
> take care of disabling ASPM?

No, I don't think so.  POLICY_DEFAULT is a Linux thing and BIOS
doesn't know anything about it.

ASPM can be configured by BIOS before handoff to Linux, but after
handoff it should be managed either entirely by BIOS or entirely by
Linux.  If BIOS wants to retain ASPM control, it would have to tell
the OS *not* to use ASPM, and it would have to use ACPI hotplug.  In
this case, POLICY_DEFAULT is irrelevant because Linux shouldn't do
anything with ASPM.

But normally BIOS allows Linux to control ASPM, and we would use
native PCIe hotplug (pciehp) instead of ACPI hotplug, and BIOS has no
opportunity to enable or disable ASPM on hotplug events.

> >> 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?
> 
> Yes, it is hot-added device. Also, I understand, for POLICY_DEFAULT,
> OS would/should not touch ASPM(enable/disable), but BIOS could still
> (enable/disable), right?

Maybe I'm misunderstanding your question.  There are two questions
here:

  1) Does the BIOS allow Linux to manage ASPM?

  2) If Linux does manage ASPM, what policy does it use?

If BIOS doesn't allow Linux to manage ASPM, POLICY_DEFAULT is
irrelevant.  If BIOS does allow Linux to manage ASPM, POLICY_DEFAULT
means Linux should use the settings made by BIOS.  The user could
select a different policy, and then Linux would change the ASPM
configuration accordingly.

> Currently, what happens in my system is as following, (each 2nd
> power cycle/hotplug of Endpoint disables ASPM):
> 
> 
> First Power cycle (When ASPM L1 is already enabled): device gets
> powered off -> there are no Link status events, so no pcie hotplug
> interrupt and pcie_aspm_exit_link_state() triggered.  

If the Downstream Port leading to your Endpoint is hotplug capable,
doesn't the spec require that it can report link state changes (PCIe
r3.1, sec 7.8.6, 7.8.10, 7.8.11)?

> When the device gets powered on again -> Link down/Link up events
> are coming back to back.  First Link down is served. (BIOS checks
> for the Link status and enables ASPM already, as the device is
> actually powered back). OS calls pcie_aspm_exit_link_state() and
> ASPM gets disabled by OS.
> 
> Second Power cycle (When ASPM L1 is disabled after above): device
> gets powered off -> there are link status events, pcie hotplug
> interrupt is triggered and pcie_aspm_exit_link_state() triggered.
> OS disables ASPM. BIOS checks Link status and disables ASPM too.
> When the device gets powered on -> BIOS enables ASPM and as this is
> pcie hotplug insertion, OS does not interfere and we have ASPM
> enabled.

I don't understand this sequence.  If we're using native PCIe hotplug,
BIOS should not be involved to enable ASPM when the device is powered
on.

> The above sequence happens each 2nd power cycle of the hotplug
> device.
> 
> So One could still argue if POLICY_DEFAULT is set, then why OS
> disables ASPM if it is not meant to touch configuration.  This is
> why I proposed following kind of change, so that OS would not touch
> ASPM, if POLICY_DEFAULT is set.  Also, With the below change,
> everything relies on BIOS for ASPM when POLICY_DEFAULT is set and I
> see above problem gets resolved. Also, the existing ASPM behavior
> does not have impact, unless specific BIOS does not disable ASPM on
> Root Port when device gets removed.

Patel, Mayurkumar May 15, 2017, 9:10 a.m. UTC | #6

Hi Bjorn

Thanks a lot for your reply and explanations. Sorry for my late reply due to some other emergencies.
>
>On Tue, May 02, 2017 at 12:02:53PM +0000, Patel, Mayurkumar wrote:
>> >On Fri, Apr 21, 2017 at 2:46 AM, Patel, Mayurkumar
>> ><[e-mail]> wrote:
>> >>>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."
>>
>> Yes, you are right and per spec also it makes sense that ASPM needs
>> to be disabled.  But, if POLICY_DEFAULT is set then, shouldn't BIOS
>> take care of disabling ASPM?
>
>No, I don't think so.  POLICY_DEFAULT is a Linux thing and BIOS
>doesn't know anything about it.
>
>ASPM can be configured by BIOS before handoff to Linux, but after
>handoff it should be managed either entirely by BIOS or entirely by
>Linux.  If BIOS wants to retain ASPM control, it would have to tell
>the OS *not* to use ASPM, and it would have to use ACPI hotplug.  In
>this case, POLICY_DEFAULT is irrelevant because Linux shouldn't do
>anything with ASPM.
>
>But normally BIOS allows Linux to control ASPM, and we would use
>native PCIe hotplug (pciehp) instead of ACPI hotplug, and BIOS has no
>opportunity to enable or disable ASPM on hotplug events.
>

BIOS that I am having, has an SMI handler Which gets triggered upon
Hotplug (Data Link Layer State Changed) Interrupt Which configures ASPM L1/L1SS in BIOS
and We are still using Native Hotplug driver. Sounds like BIOS we have in our System,
does not inform OS that it wants control ASPM.

>> >> 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?
>>
>> Yes, it is hot-added device. Also, I understand, for POLICY_DEFAULT,
>> OS would/should not touch ASPM(enable/disable), but BIOS could still
>> (enable/disable), right?
>
>Maybe I'm misunderstanding your question.  There are two questions
>here:
>
>  1) Does the BIOS allow Linux to manage ASPM?
>
>  2) If Linux does manage ASPM, what policy does it use?
>
>If BIOS doesn't allow Linux to manage ASPM, POLICY_DEFAULT is
>irrelevant.  If BIOS does allow Linux to manage ASPM, POLICY_DEFAULT
>means Linux should use the settings made by BIOS.  The user could
>select a different policy, and then Linux would change the ASPM
>configuration accordingly.
>

Ok understood.

>> Currently, what happens in my system is as following, (each 2nd
>> power cycle/hotplug of Endpoint disables ASPM):
>>
>>
>> First Power cycle (When ASPM L1 is already enabled): device gets
>> powered off -> there are no Link status events, so no pcie hotplug
>> interrupt and pcie_aspm_exit_link_state() triggered.
>
>If the Downstream Port leading to your Endpoint is hotplug capable,
>doesn't the spec require that it can report link state changes (PCIe
>r3.1, sec 7.8.6, 7.8.10, 7.8.11)?
>
>> When the device gets powered on again -> Link down/Link up events
>> are coming back to back.  First Link down is served. (BIOS checks
>> for the Link status and enables ASPM already, as the device is
>> actually powered back). OS calls pcie_aspm_exit_link_state() and
>> ASPM gets disabled by OS.
>>
>> Second Power cycle (When ASPM L1 is disabled after above): device
>> gets powered off -> there are link status events, pcie hotplug
>> interrupt is triggered and pcie_aspm_exit_link_state() triggered.
>> OS disables ASPM. BIOS checks Link status and disables ASPM too.
>> When the device gets powered on -> BIOS enables ASPM and as this is
>> pcie hotplug insertion, OS does not interfere and we have ASPM
>> enabled.
>
>I don't understand this sequence.  If we're using native PCIe hotplug,
>BIOS should not be involved to enable ASPM when the device is powered
>on.
>
>> The above sequence happens each 2nd power cycle of the hotplug
>> device.
>>
>> So One could still argue if POLICY_DEFAULT is set, then why OS
>> disables ASPM if it is not meant to touch configuration.  This is
>> why I proposed following kind of change, so that OS would not touch
>> ASPM, if POLICY_DEFAULT is set.  Also, With the below change,
>> everything relies on BIOS for ASPM when POLICY_DEFAULT is set and I
>> see above problem gets resolved. Also, the existing ASPM behavior
>> does not have impact, unless specific BIOS does not disable ASPM on
>> Root Port when device gets removed.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: [phone], www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Patch

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 1dfa10c..bf5be6d 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -940,7 +940,8 @@  void pcie_aspm_exit_link_state(struct pci_dev *pdev)
        parent_link = link->parent;

        /* All functions are removed, so just disable ASPM for the link */
-       pcie_config_aspm_link(link, 0);
+       if (aspm_policy != POLICY_DEFAULT)
+               pcie_config_aspm_link(link, 0);
        list_del(&link->sibling);
        list_del(&link->link);
        /* Clock PM is for endpoint device */