archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <>
To: Yicong Yang <>
	"Rafael J. Wysocki" <>
Subject: Re: [PATCH] PCI/ACPI: Decouple the negotiation of ASPM and other PCIe services
Date: Thu, 7 Apr 2022 10:42:57 -0500	[thread overview]
Message-ID: <20220407154257.GA235990@bhelgaas> (raw)
In-Reply-To: <>

[+cc Rafael]

On Thu, Apr 07, 2022 at 09:16:02PM +0800, Yicong Yang wrote:
> Currently we regard ASPM as a necessary PCIe service and if it's disabled
> by pcie_aspm=off we cannot enable other services like AER and hotplug.
> However the ASPM is just one of the PCIe services and other services
> mentioned no dependency on this. So this patch decouples the negotiation
> of ASPM and other PCIe services, then we can make use of other services
> in the absence of ASPM.

Why do you want to boot with "pcie_aspm=off"?  If we have to use a
PCI-related parameter to boot, something is already wrong, so if
there's a problem that requires ASPM to be disabled, we should fix
that first.

If there's a known hardware or firmware issue with ASPM, we should
quirk it so users don't have to discover this parameter.

> Aaron Sierra tried to fix this originally:

Yes.  My question from that review is still open:

  But Rafael added ACPI_PCIE_REQ_SUPPORT with 415e12b23792 ("PCI/ACPI:
  Request _OSC control once for each root bridge (v3)") [1], apparently
  related to a bug [2].  I assume there was some reason for requiring
  all those things together, so I'd really like his comments.


Rafael clearly said in [1] that we need to:

  ... check if all of the requisite _OSC support bits are set before
  calling acpi_pci_osc_control_set() for a given root complex.

We would still need to explain why Rafael thought all these _OSC
support bits were required, but now they're not.

_OSC does not negotiate directly for control of ASPM (though of course
it *does* negotiate for control of the PCIe Capability, which contains
the ASPM control bits), but the PCI Firmware spec, r3.3, sec 4.5.3, has
this comment in a sample _OSC implementation:

  // Only allow native hot plug control if the OS supports:
  // * ASPM
  // * Clock PM
  // * MSI/MSI-X

which matches the current ACPI_PCIE_REQ_SUPPORT.

So I think I would approach this by reworking the _OSC negotiation so

Advertising support for ASPM doesn't mean Linux has to actually
*enable* it, so we could make a different mechanism to prevent use of
ASPM if we have a device or platform quirk or we're booting with

> Signed-off-by: Yicong Yang <>
> ---
>  drivers/acpi/pci_root.c | 2 --
>  1 file changed, 2 deletions(-)
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 6f9e75d14808..16fa7c5a11ad 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -37,8 +37,6 @@ static int acpi_pci_root_scan_dependent(struct acpi_device *adev)
>  }
>  static const struct acpi_device_id root_device_ids[] = {
> -- 
> 2.24.0

  reply	other threads:[~2022-04-07 15:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07 13:16 [PATCH] PCI/ACPI: Decouple the negotiation of ASPM and other PCIe services Yicong Yang
2022-04-07 15:42 ` Bjorn Helgaas [this message]
2022-04-07 16:41   ` Rafael J. Wysocki
2022-04-11  9:30     ` Yicong Yang

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 \
    --in-reply-to=20220407154257.GA235990@bhelgaas \ \ \ \ \ \ \ \ \ \ \

* 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).