archive mirror
 help / color / mirror / Atom feed
From: Kai-Heng Feng <>
To: Bjorn Helgaas <>
Cc: Bjorn Helgaas <>,
	"Derrick, Jonathan" <>,
	Lorenzo Pieralisi <>,
	Rob Herring <>,,,
	Ian Kumlien <>
Subject: Re: [PATCH 2/2] PCI: vmd: Enable ASPM for mobile platforms
Date: Thu, 8 Oct 2020 12:19:35 +0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20201007133002.GA3236436@bjorn-Precision-5520>

> On Oct 7, 2020, at 21:30, Bjorn Helgaas <> wrote:
> On Wed, Oct 07, 2020 at 12:26:19PM +0800, Kai-Heng Feng wrote:
>>> On Oct 6, 2020, at 03:19, Bjorn Helgaas <> wrote:
>>> On Tue, Oct 06, 2020 at 02:40:32AM +0800, Kai-Heng Feng wrote:
>>>>> On Oct 3, 2020, at 06:18, Bjorn Helgaas <> wrote:
>>>>> On Wed, Sep 30, 2020 at 04:24:54PM +0800, Kai-Heng Feng wrote:
> ...
>>>> I wonder whether other devices that add PCIe domain have the same
>>>> behavior?  Maybe it's not a special case at all...
>>> What other devices are these?
>> Controllers which add PCIe domain.
> I was looking for specific examples, not just a restatement of what
> you said before.  I'm just curious because there are a lot of
> controllers I'm not familiar with, and I can't think of an example.
>>>> I understand the end goal is to keep consistency for the entire ASPM
>>>> logic. However I can't think of any possible solution right now.
>>>>> - If we built with CONFIG_PCIEASPM_POWERSAVE=y, would that solve the
>>>>>  SoC power state problem?
>>>> Yes.
>>>>> - What issues would CONFIG_PCIEASPM_POWERSAVE=y introduce?
>>>> This will break many systems, at least for the 1st Gen Ryzen
>>>> desktops and laptops.
>>>> All PCIe ASPM are not enabled by BIOS, and those systems immediately
>>>> freeze once ASPM is enabled.
>>> That indicates a defect in the Linux ASPM code.  We should fix that.
>>> It should be safe to use CONFIG_PCIEASPM_POWERSAVE=y on every system.
>> On those systems ASPM are also not enabled on Windows. So I think
>> ASPM are disabled for a reason.
> If the platform knows ASPM needs to be disabled, it should be using
> ACPI_FADT_NO_ASPM or _OSC to prevent the OS from using it.  And if
> CONFIG_PCIEASPM_POWERSAVE=y means Linux enables ASPM when it
> shouldn't, that's a Linux bug that we need to fix.

Yes that's a bug which fixed by Ian's new patch.

>>> Are there bug reports for these? The info we would need to start with
>>> includes "lspci -vv" and dmesg log (with CONFIG_PCIEASPM_DEFAULT=y).
>>> If a console log with CONFIG_PCIEASPM_POWERSAVE=y is available, that
>>> might be interesting, too.  We'll likely need to add some
>>> instrumentation and do some experimentation, but in principle, this
>>> should be fixable.
>> Doing this is asking users to use hardware settings that ODM/OEM
>> never tested, and I think the risk is really high.
> What?  That's not what I said at all.  I'm asking for information
> about these hangs so we can fix them.  I'm not suggesting that you
> should switch to CONFIG_PCIEASPM_POWERSAVE=y for the distro.

Ah, I thought your suggestion is switching to CONFIG_PCIEASPM_POWERSAVE=y, because I sense you want to use that to cover the VMD ASPM this patch tries to solve.

Do we have a conclusion how to enable VMD ASPM with CONFIG_PCIEASPM_DEFAULT=y?

> Let's back up.  You said:
>  CONFIG_PCIEASPM_POWERSAVE=y ... will break many systems, at least
>  for the 1st Gen Ryzen desktops and laptops.
>  All PCIe ASPM are not enabled by BIOS, and those systems immediately
>  freeze once ASPM is enabled.
> These system hangs might be caused by (1) some hardware issue that
> causes a hang when ASPM is enabled even if it is configured correctly
> or (2) Linux configuring ASPM incorrectly.

It's (2) here.

> For case (1), the platform should be using ACPI_FADT_NO_ASPM or _OSC
> to prevent the OS from enabling ASPM.  Linux should pay attention to
> If the platform *should* use these mechanisms but doesn't, the
> solution is a quirk, not the folklore that "we can't use
> CONFIG_PCIEASPM_POWERSAVE=y because it breaks some systems."

The platform in question doesn't prevent OS from enabling ASPM.

> For case (2), we should fix Linux so it configures ASPM correctly.
> We cannot use the build-time CONFIG_PCIEASPM settings to avoid these
> hangs.  We need to fix the Linux run-time code so the system operates
> correctly no matter what CONFIG_PCIEASPM setting is used.
> We have sysfs knobs to control ASPM (see 72ea91afbfb0 ("PCI/ASPM: Add
> sysfs attributes for controlling ASPM link states")).  They can do the
> same thing at run-time as CONFIG_PCIEASPM_POWERSAVE=y does at
> build-time.  If those knobs cause hangs on 1st Gen Ryzen systems, we
> need to fix that.

Ian's patch solves the issue, at least for the systems I have.


> Bjorn

  parent reply	other threads:[~2020-10-08  4:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30  8:24 [PATCH 1/2] PCI/ASPM: Add helper to enable ASPM link Kai-Heng Feng
2020-09-30  8:24 ` [PATCH 2/2] PCI: vmd: Enable ASPM for mobile platforms Kai-Heng Feng
2020-10-02 22:18   ` Bjorn Helgaas
2020-10-05 18:40     ` Kai-Heng Feng
2020-10-05 19:19       ` Bjorn Helgaas
2020-10-07  4:26         ` Kai-Heng Feng
2020-10-07  9:28           ` Ian Kumlien
2020-10-07 13:30           ` Bjorn Helgaas
2020-10-07 13:44             ` Ian Kumlien
2020-10-08  4:19             ` Kai-Heng Feng [this message]
2020-10-09 14:34               ` Ian Kumlien

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).