Skip to contentSkip to conversation

PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

PATCH In review David E. Box 4 participants 17 messages Aug 25, 2025 – Oct 6, 2025

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/2 PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

---
Changes in V3:
-- Changed pci_host_get_default_pcie_link_state() argument name from
parent to dev.
-- Applied changelog tags

Changes in V2:

-- Host field name changed to aspm_default_link_state. -- Added get/set functions for aspm_default_link_state. Only the
setter is exported. Added a kernel-doc describing usage and particulars around meaning of 0.

Changes in V1 from RFC:

-- Rename field to aspm_dflt_link_state since it stores
PCIE_LINK_STATE_XXX flags, not a policy enum.
-- Move the field to struct pci_host_bridge since it's being applied to
the entire host bridge per Mani's suggestion.
-- During testing noticed that clkpm remained disabled and this was
also handled by the formerly used pci_enable_link_state(). Add a check in pcie_clkpm_cap_init() as well to enable clkpm during init.

 drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
 include/linux/pci.h     |  9 +++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)
drivers/pci/pcie/aspm.c +40 -2
index 919a05b97647..851ca3d68e55 100644
@@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 	pcie_set_clkpm_nocheck(link, enable);
 }
 
+/**
+ * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
+ * @host: host bridge on which to apply the defaults
+ * @state: PCIE_LINK_STATE_XXX flags
+ *
+ * Allows a PCIe controller driver to specify the default ASPM and/or
+ * Clock Power Management (CLKPM) link state mask that will be used
+ * for links under this host bridge during ASPM/CLKPM capability init.
+ *
+ * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
+ * to override the firmware-discovered defaults.
+ *
+ * Interpretation of aspm_default_link_state:
+ *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
+ *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
+ *              values discovered in hardware/firmware
+ *
+ * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
+ */
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state)
+{
+	host->aspm_default_link_state = state;
+}
+EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
+
+static u32 pci_host_get_default_pcie_link_state(struct pci_dev *dev)
+{
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+	return host ? host->aspm_default_link_state : 0;
+}
+
 static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	int capable = 1, enabled = 1;
@@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 			enabled = 0;
 	}
 	link->clkpm_enabled = enabled;
-	link->clkpm_default = enabled;
+	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
+		link->clkpm_default = 1;
+	else
+		link->clkpm_default = enabled;
 	link->clkpm_capable = capable;
 	link->clkpm_disable = blacklist ? 1 : 0;
 }
@@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	}
 
 	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
+	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
+	if (!link->aspm_default)
+		link->aspm_default = link->aspm_enabled;
 
 	/* Setup initial capable state. Will be updated later */
 	link->aspm_capable = link->aspm_support;
include/linux/pci.h +9 -0
index 59876de13860..8947cbaf9fa6 100644
@@ -620,6 +620,10 @@ struct pci_host_bridge {
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
 	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
 
+#ifdef CONFIG_PCIEASPM
+	unsigned int	aspm_default_link_state;	/* Controller-provided default */
+#endif
+
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
 			const struct resource *res,
@@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
 int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
 int pci_enable_link_state(struct pci_dev *pdev, int state);
 int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state);
 void pcie_no_aspm(void);
 bool pcie_aspm_support_enabled(void);
 bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
 { return 0; }
 static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
 { return 0; }
+static inline void
+pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+				     unsigned int state) { }
 static inline void pcie_no_aspm(void) { }
 static inline bool pcie_aspm_support_enabled(void) { return false; }
 static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9

Signature · 1 line
2.43.0
#1 lore raw

PATCH v3 2/2 PCI: vmd: Use pci_host_set_default_pcie_link_state() to set ASPM defaults

Now that pci_host_set_default_pcie_link_state() exists, set the VMD child domain with PCIE_LINK_STATE_ALL at bridge creation so core ASPM uses those defaults during ASPM and CLKPM capability init.

Also remove the unneeded pci_set_power_state_locked(pdev, PCI_D0) and pci_enable_link_state_locked() calls now that the links are configured during enumeration.

This aligns VMD behavior with platform expectations without per-controller ASPM tweaks at runtime.

---
Changes in V3:
-- Applied tags to changelog

Changes in V2:

-- Separated VMD changes into new patch.
-- Changed comment for VMD_FEAT_BIOS_PM_QUIRK to remove ASPM -- Removed pci_set_power_state() and pci_enable_link_state_locked()
calls in vmd_pm_enable_quirk()
-- Use pci_host_set_default_pcie_link_state()

 drivers/pci/controller/vmd.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)
drivers/pci/controller/vmd.c +8 -14
index b679c7f28f51..b99e01a57ddb 100644
@@ -71,10 +71,9 @@ enum vmd_features {
 	VMD_FEAT_CAN_BYPASS_MSI_REMAP		= (1 << 4),
 
 	/*
-	 * Enable ASPM on the PCIE root ports and set the default LTR of the
-	 * storage devices on platforms where these values are not configured by
-	 * BIOS. This is needed for laptops, which require these settings for
-	 * proper power management of the SoC.
+	 * Program default LTR values for storage devices on platforms where
+	 * firmware did not. Required on many laptops for proper SoC power
+	 * management.
 	 */
 	VMD_FEAT_BIOS_PM_QUIRK		= (1 << 5),
 };
@@ -733,7 +732,7 @@ static void vmd_copy_host_bridge_flags(struct pci_host_bridge *root_bridge,
 }
 
 /*
- * Enable ASPM and LTR settings on devices that aren't configured by BIOS.
+ * Enable LTR settings on devices that aren't configured by BIOS.
  */
 static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 {
@@ -747,7 +746,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
 	if (!pos)
-		goto out_state_change;
+		return 0;
 
 	/*
 	 * Skip if the max snoop LTR is non-zero, indicating BIOS has set it
@@ -755,7 +754,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	 */
 	pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, &ltr_reg);
 	if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK)))
-		goto out_state_change;
+		return 0;
 
 	/*
 	 * Set the default values to the maximum required by the platform to
@@ -767,13 +766,6 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
 	pci_info(pdev, "VMD: Default LTR value set by driver\n");
 
-out_state_change:
-	/*
-	 * Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
-	 * PCIe r6.0, sec 5.5.4.
-	 */
-	pci_set_power_state_locked(pdev, PCI_D0);
-	pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
 	return 0;
 }
 
@@ -921,6 +913,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
 			       "domain"), "Can't create symlink to domain\n");
 
+	pci_host_set_default_pcie_link_state(to_pci_host_bridge(vmd->bus->bridge),
+					     PCIE_LINK_STATE_ALL);
 	vmd_acpi_begin();
 
 	pci_scan_child_bus(vmd->bus);
Signature · 1 line
2.43.0
#2 lore raw
In reply to David E. Box
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

This feels a little scary, so feel free to convince me it can't be done :)

Quoted 137 lines

---
Changes in V3:
-- Changed pci_host_get_default_pcie_link_state() argument name from
parent to dev.
-- Applied changelog tags

Changes in V2:

-- Host field name changed to aspm_default_link_state. -- Added get/set functions for aspm_default_link_state. Only the
setter is exported. Added a kernel-doc describing usage and particulars around meaning of 0.

Changes in V1 from RFC:

-- Rename field to aspm_dflt_link_state since it stores
PCIE_LINK_STATE_XXX flags, not a policy enum.
-- Move the field to struct pci_host_bridge since it's being applied to
the entire host bridge per Mani's suggestion.
-- During testing noticed that clkpm remained disabled and this was
also handled by the formerly used pci_enable_link_state(). Add a check in pcie_clkpm_cap_init() as well to enable clkpm during init.

 drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
 include/linux/pci.h     |  9 +++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)
drivers/pci/pcie/aspm.c +40 -2
index 919a05b97647..851ca3d68e55 100644
@@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 	pcie_set_clkpm_nocheck(link, enable);
 }
 
+/**
+ * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
+ * @host: host bridge on which to apply the defaults
+ * @state: PCIE_LINK_STATE_XXX flags
+ *
+ * Allows a PCIe controller driver to specify the default ASPM and/or
+ * Clock Power Management (CLKPM) link state mask that will be used
+ * for links under this host bridge during ASPM/CLKPM capability init.
+ *
+ * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
+ * to override the firmware-discovered defaults.
+ *
+ * Interpretation of aspm_default_link_state:
+ *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
+ *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
+ *              values discovered in hardware/firmware
+ *
+ * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
+ */
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state)
+{
+	host->aspm_default_link_state = state;
+}
+EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
+
+static u32 pci_host_get_default_pcie_link_state(struct pci_dev *dev)
+{
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+	return host ? host->aspm_default_link_state : 0;
+}
+
 static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	int capable = 1, enabled = 1;
@@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 			enabled = 0;
 	}
 	link->clkpm_enabled = enabled;
-	link->clkpm_default = enabled;
+	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
+		link->clkpm_default = 1;
+	else
+		link->clkpm_default = enabled;
 	link->clkpm_capable = capable;
 	link->clkpm_disable = blacklist ? 1 : 0;
 }
@@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	}
 
 	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
+	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
+	if (!link->aspm_default)
+		link->aspm_default = link->aspm_enabled;
 
 	/* Setup initial capable state. Will be updated later */
 	link->aspm_capable = link->aspm_support;
include/linux/pci.h +9 -0
index 59876de13860..8947cbaf9fa6 100644
@@ -620,6 +620,10 @@ struct pci_host_bridge {
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
 	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
 
+#ifdef CONFIG_PCIEASPM
+	unsigned int	aspm_default_link_state;	/* Controller-provided default */
+#endif
+
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
 			const struct resource *res,
@@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
 int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
 int pci_enable_link_state(struct pci_dev *pdev, int state);
 int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state);
 void pcie_no_aspm(void);
 bool pcie_aspm_support_enabled(void);
 bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
 { return 0; }
 static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
 { return 0; }
+static inline void
+pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+				     unsigned int state) { }
 static inline void pcie_no_aspm(void) { }
 static inline bool pcie_aspm_support_enabled(void) { return false; }
 static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9

Signature · 1 line
2.43.0
author
#3 lore raw
Quoted 12 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 7 lines
Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

I agree with the principle. The intent isn’t for VMD (or any controller) to override valid platform policy. It’s to handle synthetic domains where the platform doesn’t provide any policy path (no effective _OSC/FADT for the child hierarchy). In those cases, the controller is the only agent that knows the topology and can supply sane defaults.

I’m happy to tighten the patch to explicitly cover synthetic domains only. Instead of an API, we could have a boolean flag 'aspm_synthetic_domain'. When set by the controller, we can do:

    if (host_bridge->aspm_synthetic_domain)
            link->aspm_default = PCIE_LINK_STATE_ALL;

This at least addresses your concern about policy decision, leaving it to the core to determine how these domains are handled rather than an ABI that lets domains set policy.

Quoted 14 lines

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

Quoted 8 lines

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

This feels a little scary, so feel free to convince me it can't be done :)

I understand your direction of enabling all advertised states by default (subject to FADT/_OSC and user settings). To explore that, I’ll send an RFC in parallel with this patch that proposes a baby step, e.g. add instrumentation so we can see where BIOS left capabilities unused, and make it opt-in via a boot param so we can evaluate impact safely.

So this series will handle the VMD gap directly, and the RFC can kick off the wider discussion about defaults on ACPI-managed hosts. Does that sound like a reasonable approach and split?

David

Quoted 138 lines
Quoted 138 lines

---
Changes in V3:
-- Changed pci_host_get_default_pcie_link_state() argument name from
parent to dev.
-- Applied changelog tags

Changes in V2:

-- Host field name changed to aspm_default_link_state. -- Added get/set functions for aspm_default_link_state. Only the
setter is exported. Added a kernel-doc describing usage and particulars around meaning of 0.

Changes in V1 from RFC:

-- Rename field to aspm_dflt_link_state since it stores
PCIE_LINK_STATE_XXX flags, not a policy enum.
-- Move the field to struct pci_host_bridge since it's being applied to
the entire host bridge per Mani's suggestion.
-- During testing noticed that clkpm remained disabled and this was
also handled by the formerly used pci_enable_link_state(). Add a check in pcie_clkpm_cap_init() as well to enable clkpm during init.

 drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
 include/linux/pci.h     |  9 +++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)
drivers/pci/pcie/aspm.c +40 -2
index 919a05b97647..851ca3d68e55 100644
@@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 	pcie_set_clkpm_nocheck(link, enable);
 }
 
+/**
+ * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
+ * @host: host bridge on which to apply the defaults
+ * @state: PCIE_LINK_STATE_XXX flags
+ *
+ * Allows a PCIe controller driver to specify the default ASPM and/or
+ * Clock Power Management (CLKPM) link state mask that will be used
+ * for links under this host bridge during ASPM/CLKPM capability init.
+ *
+ * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
+ * to override the firmware-discovered defaults.
+ *
+ * Interpretation of aspm_default_link_state:
+ *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
+ *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
+ *              values discovered in hardware/firmware
+ *
+ * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
+ */
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state)
+{
+	host->aspm_default_link_state = state;
+}
+EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
+
+static u32 pci_host_get_default_pcie_link_state(struct pci_dev *dev)
+{
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+	return host ? host->aspm_default_link_state : 0;
+}
+
 static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	int capable = 1, enabled = 1;
@@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 			enabled = 0;
 	}
 	link->clkpm_enabled = enabled;
-	link->clkpm_default = enabled;
+	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
+		link->clkpm_default = 1;
+	else
+		link->clkpm_default = enabled;
 	link->clkpm_capable = capable;
 	link->clkpm_disable = blacklist ? 1 : 0;
 }
@@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	}
 
 	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
+	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
+	if (!link->aspm_default)
+		link->aspm_default = link->aspm_enabled;
 
 	/* Setup initial capable state. Will be updated later */
 	link->aspm_capable = link->aspm_support;
include/linux/pci.h +9 -0
index 59876de13860..8947cbaf9fa6 100644
@@ -620,6 +620,10 @@ struct pci_host_bridge {
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
 	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
 
+#ifdef CONFIG_PCIEASPM
+	unsigned int	aspm_default_link_state;	/* Controller-provided default */
+#endif
+
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
 			const struct resource *res,
@@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
 int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
 int pci_enable_link_state(struct pci_dev *pdev, int state);
 int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state);
 void pcie_no_aspm(void);
 bool pcie_aspm_support_enabled(void);
 bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
 { return 0; }
 static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
 { return 0; }
+static inline void
+pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+				     unsigned int state) { }
 static inline void pcie_no_aspm(void) { }
 static inline bool pcie_aspm_support_enabled(void) { return false; }
 static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9

Signature · 1 line
2.43.0
#4 lore raw
Quoted 60 lines · David Box
Quoted 12 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 7 lines
Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

I agree with the principle. The intent isn’t for VMD (or any controller) to override valid platform policy. It’s to handle synthetic domains where the platform doesn’t provide any policy path (no effective _OSC/FADT for the child hierarchy). In those cases, the controller is the only agent that knows the topology and can supply sane defaults.

I’m happy to tighten the patch to explicitly cover synthetic domains only. Instead of an API, we could have a boolean flag 'aspm_synthetic_domain'. When set by the controller, we can do:

    if (host_bridge->aspm_synthetic_domain)
            link->aspm_default = PCIE_LINK_STATE_ALL;

This at least addresses your concern about policy decision, leaving it to the core to determine how these domains are handled rather than an ABI that lets domains set policy.

Quoted 13 lines

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

Quoted 8 lines

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

This feels a little scary, so feel free to convince me it can't be done :)

Quoted 6 lines

I understand your direction of enabling all advertised states by default (subject to FADT/_OSC and user settings). To explore that, I’ll send an RFC in parallel with this patch that proposes a baby step, e.g. add instrumentation so we can see where BIOS left capabilities unused, and make it opt-in via a boot param so we can evaluate impact safely.

The instrumentation, absolutely. We need something about what was already enabled and when we change things.

Quoted 3 lines

So this series will handle the VMD gap directly, and the RFC can kick off the wider discussion about defaults on ACPI-managed hosts. Does that sound like a reasonable approach and split?

I don't really want a parallel approach because I don't think it would ever converge again. BUT I think you're still OK for VMD, because I think the default should be PCIE_LINK_STATE_ALL, and when we carve out the exceptions that would not be in vmd.c, and it's easy to say that there's no exception for VMD.

author
#5 lore raw
Quoted 75 lines · Bjorn Helgaas
Quoted 60 lines · David Box
Quoted 12 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 7 lines
Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

I agree with the principle. The intent isn’t for VMD (or any controller) to override valid platform policy. It’s to handle synthetic domains where the platform doesn’t provide any policy path (no effective _OSC/FADT for the child hierarchy). In those cases, the controller is the only agent that knows the topology and can supply sane defaults.

I’m happy to tighten the patch to explicitly cover synthetic domains only. Instead of an API, we could have a boolean flag 'aspm_synthetic_domain'. When set by the controller, we can do:

    if (host_bridge->aspm_synthetic_domain)
            link->aspm_default = PCIE_LINK_STATE_ALL;

This at least addresses your concern about policy decision, leaving it to the core to determine how these domains are handled rather than an ABI that lets domains set policy.

Quoted 13 lines

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

Quoted 8 lines

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

This feels a little scary, so feel free to convince me it can't be done :)

Quoted 6 lines

I understand your direction of enabling all advertised states by default (subject to FADT/_OSC and user settings). To explore that, I’ll send an RFC in parallel with this patch that proposes a baby step, e.g. add instrumentation so we can see where BIOS left capabilities unused, and make it opt-in via a boot param so we can evaluate impact safely.

The instrumentation, absolutely. We need something about what was already enabled and when we change things.

Quoted 3 lines

So this series will handle the VMD gap directly, and the RFC can kick off the wider discussion about defaults on ACPI-managed hosts. Does that sound like a reasonable approach and split?

Quoted 5 lines

I don't really want a parallel approach because I don't think it would ever converge again. BUT I think you're still OK for VMD, because I think the default should be PCIE_LINK_STATE_ALL, and when we carve out the exceptions that would not be in vmd.c, and it's easy to say that there's no exception for VMD.

While I agree this is a better overall direction, it still won’t cover VMD in the “FADT disallows OS ASPM control” case. VMD (and others) are already using pci_enable_link_state() to set PCIE_LINK_STATE_ALL. But that doesn’t apply when aspm_disabled = 1, which is the primary issue this patch addresses — moving away from a runtime control setting to init-time and allowing policy to be set even when aspm_disabled = 1.

I want to be clear that this is needed because VMD is NOT fully ACPI-compliant. There is AML code, but it’s deliberately obfuscated to prevent the OS from enumerating the devices natively [1]. VMD was designed to give the driver complete control over the domain configuration, and that includes power management settings.

So I can send a patch to make the default PCIE_LINK_STATE_ALL, but I would still need a follow-on in that series that does something like:

	if (host_bridge->synthetic_domain)
		link->aspm_default = PCIE_LINK_STATE_ALL;

That carve-out is still required, because without it synthetic domains like VMD would inherit invalid or nonexistent BIOS defaults. The key difference is that only vmd.c would set itself as synthetic_domain; ASPM core still decides what policy applies.

[1] https://lore.kernel.org/linux-pci/4352365.LvFx2qVVIh@kreacher/

David

#6 lore raw
Quoted 98 lines · David Box
Quoted 75 lines · Bjorn Helgaas
Quoted 60 lines · David Box
Quoted 12 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 7 lines
Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

I agree with the principle. The intent isn’t for VMD (or any controller) to override valid platform policy. It’s to handle synthetic domains where the platform doesn’t provide any policy path (no effective _OSC/FADT for the child hierarchy). In those cases, the controller is the only agent that knows the topology and can supply sane defaults.

I’m happy to tighten the patch to explicitly cover synthetic domains only. Instead of an API, we could have a boolean flag 'aspm_synthetic_domain'. When set by the controller, we can do:

    if (host_bridge->aspm_synthetic_domain)
            link->aspm_default = PCIE_LINK_STATE_ALL;

This at least addresses your concern about policy decision, leaving it to the core to determine how these domains are handled rather than an ABI that lets domains set policy.

Quoted 13 lines

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

Quoted 8 lines

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

This feels a little scary, so feel free to convince me it can't be done :)

Quoted 6 lines

I understand your direction of enabling all advertised states by default (subject to FADT/_OSC and user settings). To explore that, I’ll send an RFC in parallel with this patch that proposes a baby step, e.g. add instrumentation so we can see where BIOS left capabilities unused, and make it opt-in via a boot param so we can evaluate impact safely.

The instrumentation, absolutely. We need something about what was already enabled and when we change things.

Quoted 3 lines

So this series will handle the VMD gap directly, and the RFC can kick off the wider discussion about defaults on ACPI-managed hosts. Does that sound like a reasonable approach and split?

Quoted 5 lines

I don't really want a parallel approach because I don't think it would ever converge again. BUT I think you're still OK for VMD, because I think the default should be PCIE_LINK_STATE_ALL, and when we carve out the exceptions that would not be in vmd.c, and it's easy to say that there's no exception for VMD.

While I agree this is a better overall direction, it still won’t cover VMD in the “FADT disallows OS ASPM control” case. VMD (and others) are already using pci_enable_link_state() to set PCIE_LINK_STATE_ALL. But that doesn’t apply when aspm_disabled = 1, which is the primary issue this patch addresses — moving away from a runtime control setting to init-time and allowing policy to be set even when aspm_disabled = 1.

I want to be clear that this is needed because VMD is NOT fully ACPI-compliant. There is AML code, but it’s deliberately obfuscated to prevent the OS from enumerating the devices natively [1]. VMD was designed to give the driver complete control over the domain configuration, and that includes power management settings.

So I can send a patch to make the default PCIE_LINK_STATE_ALL, but I would still need a follow-on in that series that does something like:

Quoted 9 lines
	if (host_bridge->synthetic_domain)
		link->aspm_default = PCIE_LINK_STATE_ALL;

That carve-out is still required, because without it synthetic domains like VMD would inherit invalid or nonexistent BIOS defaults. The key difference is that only vmd.c would set itself as synthetic_domain; ASPM core still decides what policy applies.

Both VMD and the non-ACPI host bridge controllers are mostly same functionality wise, so I don't think the 'synthetic_domain' property makes sense.

IMO, it is better to go with the API introduced in this patch for VMD as it clearly tells us the host bridge driver is overriding for a purpose and I can set PCIE_LINK_STATE_ALL for DT based platforms separately.

- Mani

Signature · 1 line
மணிவண்ணன் சதாசிவம்
#7 lore raw
In reply to Bjorn Helgaas
Quoted 33 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

Quoted 6 lines

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

I'm not sure about the ACPI world, but for devicetree platforms, BIOS or the bootloader won't configure ASPM for the devices (mostly). So the baby step would be to set PCIE_LINK_STATE_ALL for all devicetree platforms :)

We did similar thing for enabling D3Hot for bridges earlier:
a5fb3ff63287 ("PCI: Allow PCI bridges to go to D3Hot on all non-x86")

- Mani

Signature · 1 line
மணிவண்ணன் சதாசிவம்
#8 lore raw
Quoted 40 lines · Manivannan Sadhasivam
Quoted 33 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

Quoted 5 lines

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

Quoted 4 lines

I'm not sure about the ACPI world, but for devicetree platforms, BIOS or the bootloader won't configure ASPM for the devices (mostly). So the baby step would be to set PCIE_LINK_STATE_ALL for all devicetree platforms :)

Yes. How likely would this be to break something?

Before doing that, I think we need to add some logging, at least at pci_dbg(), of what is already enabled and what we change, so we have some kind of hint when things do break.

#9 lore raw
Quoted 41 lines · Bjorn Helgaas
Quoted 40 lines · Manivannan Sadhasivam
Quoted 38 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

Quoted 7 lines
Quoted 4 lines

I'm not sure about the ACPI world, but for devicetree platforms, BIOS or the bootloader won't configure ASPM for the devices (mostly). So the baby step would be to set PCIE_LINK_STATE_ALL for all devicetree platforms :)

Yes. How likely would this be to break something?

I don't know :) If we want to bite the bullet, let's do it. Otherwise, let's keep waiting for the time to come ;) For sure there will be breakages reported, but it won't be like on x86 which people use for day to day work. There are IBM Power and MIPS based servers using devicetree as well, so they might also get impacted.

But it would be the optimal test bed if we ever want to start enabling ASPM by default.

Quoted 3 lines

Before doing that, I think we need to add some logging, at least at pci_dbg(), of what is already enabled and what we change, so we have some kind of hint when things do break.

Sure. Even if we want to make it pci_info(), I think it would be worth doing it as it will make the reporting much easier.

Let me know if I can proceed with this.

- Mani

Signature · 1 line
மணிவண்ணன் சதாசிவம்
#10 lore raw
Quoted 58 lines · Manivannan Sadhasivam
Quoted 41 lines · Bjorn Helgaas
Quoted 40 lines · Manivannan Sadhasivam
Quoted 38 lines · Bjorn Helgaas
Quoted 11 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

Quoted 3 lines

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

I think this kind of sidesteps the real issue. Drivers for host controllers or PCI devices should tell us about *broken* things, but not about things advertised by the hardware and available for use.

The only documented policy controls I'm aware of for ASPM are:

- FADT "PCIe ASPM Controls" bit ("if set, OS must not enable ASPM control on this platform")

- _OSC negotiation for control of the PCIe Capability (OS is only allowed to write PCI_EXP_LNKCTL if platform has granted control to the OS)

I think what we *should* be doing is enabling ASPM when it's advertised, subject to those platform policy controls and user choices like CONFIG_PCIEASPM_PERFORMANCE/POWERSAVE/etc and sysfs attributes.

So basically I think link->aspm_default should be PCIE_LINK_STATE_ALL without drivers doing anything at all. Maybe we have to carve out exceptions, e.g., "VMD hierarchies are exempt from _OSC," or "devices on x86 systems before 2026 can't enable more ASPM than BIOS did," or whatever. Is there any baby step we can make in that direction?

Quoted 6 lines
Quoted 4 lines

I'm not sure about the ACPI world, but for devicetree platforms, BIOS or the bootloader won't configure ASPM for the devices (mostly). So the baby step would be to set PCIE_LINK_STATE_ALL for all devicetree platforms :)

Yes. How likely would this be to break something?

I don't know :) If we want to bite the bullet, let's do it. Otherwise, let's keep waiting for the time to come ;) For sure there will be breakages reported, but it won't be like on x86 which people use for day to day work. There are IBM Power and MIPS based servers using devicetree as well, so they might also get impacted.

But it would be the optimal test bed if we ever want to start enabling ASPM by default.

Quoted 8 lines
Quoted 3 lines

Before doing that, I think we need to add some logging, at least at pci_dbg(), of what is already enabled and what we change, so we have some kind of hint when things do break.

Sure. Even if we want to make it pci_info(), I think it would be worth doing it as it will make the reporting much easier.

Let me know if I can proceed with this.

Yes, I think so. I would likely target v6.19 for upstream to give us more time, but if we can get in -next for a bit now, that would be great.

#11 lore raw
Quoted 1 line · Bjorn Helgaas

but if we can get in -next for a bit now, that would be great.

Yes, please 🙂 Other than some my-hardware-specific gcc flags, this effort in its various forms is the only diff between Linus' master and what I run, and for nearly 3 years now; it'd be great to see it in mainline (and a hearty thank you all for getting us nearly there!)

-Kenny

Signature · 1 line
Kenneth R. Crudup / Sr. SW Engineer, Scott County Consulting, Orange County CA
#12 lore raw
Quoted 10 lines · Kenneth Crudup
Quoted 1 line · Bjorn Helgaas

but if we can get in -next for a bit now, that would be great.

Yes, please 🙂 Other than some my-hardware-specific gcc flags, this effort in its various forms is the only diff between Linus' master and what I run, and for nearly 3 years now; it'd be great to see it in mainline (and a hearty thank you all for getting us nearly there!)

ASPM patches targeting devicetree plaforms are now in pci/aspm and merged to pci/next for testing. David could send a followup to add the VMD driver now reusing the pcie_{aspm/clkpm}_override_default_link_state() helpers.

https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aspm

- Mani

Signature · 1 line
மணிவண்ணன் சதாசிவம்
#13 lore raw
Quoted 3 lines · Manivannan Sadhasivam

ASPM patches targeting devicetree plaforms are now in pci/aspm and merged to pci/next for testing.
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aspm

Yes and I see they're in Linus' master now.

Quoted 2 lines

David could send a followup to add the VMD driver now reusing the pcie_{aspm/clkpm}_override_default_link_state() helpers.

David? Your old commit doesn't apply cleanly now; I'll give it a shot trying to redo it with the new API from above, but as this is your baby maybe you could refactor it as well?

Thanks,

-Kenny

Signature · 1 line
Kenneth R. Crudup / Sr. SW Engineer, Scott County Consulting, Orange County CA
Tested-by on v3 1/2
#14 lore raw
In reply to David E. Box
Quoted 149 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

---
Changes in V3:
-- Changed pci_host_get_default_pcie_link_state() argument name from
parent to dev.
-- Applied changelog tags

Changes in V2:

-- Host field name changed to aspm_default_link_state. -- Added get/set functions for aspm_default_link_state. Only the
setter is exported. Added a kernel-doc describing usage and particulars around meaning of 0.

Changes in V1 from RFC:

-- Rename field to aspm_dflt_link_state since it stores
PCIE_LINK_STATE_XXX flags, not a policy enum.
-- Move the field to struct pci_host_bridge since it's being applied to
the entire host bridge per Mani's suggestion.
-- During testing noticed that clkpm remained disabled and this was
also handled by the formerly used pci_enable_link_state(). Add a check in pcie_clkpm_cap_init() as well to enable clkpm during init.

  drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
  include/linux/pci.h     |  9 +++++++++
  2 files changed, 49 insertions(+), 2 deletions(-)
drivers/pci/pcie/aspm.c +40 -2
index 919a05b97647..851ca3d68e55 100644
@@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
  	pcie_set_clkpm_nocheck(link, enable);
  }
 
+/**
+ * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
+ * @host: host bridge on which to apply the defaults
+ * @state: PCIE_LINK_STATE_XXX flags
+ *
+ * Allows a PCIe controller driver to specify the default ASPM and/or
+ * Clock Power Management (CLKPM) link state mask that will be used
+ * for links under this host bridge during ASPM/CLKPM capability init.
+ *
+ * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
+ * to override the firmware-discovered defaults.
+ *
+ * Interpretation of aspm_default_link_state:
+ *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
+ *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
+ *              values discovered in hardware/firmware
+ *
+ * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
+ */
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state)
+{
+	host->aspm_default_link_state = state;
+}
+EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
+
+static u32 pci_host_get_default_pcie_link_state(struct pci_dev *dev)
+{
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+	return host ? host->aspm_default_link_state : 0;
+}
+
  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
  {
  	int capable = 1, enabled = 1;
@@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
  			enabled = 0;
  	}
  	link->clkpm_enabled = enabled;
-	link->clkpm_default = enabled;
+	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
+		link->clkpm_default = 1;
+	else
+		link->clkpm_default = enabled;
  	link->clkpm_capable = capable;
  	link->clkpm_disable = blacklist ? 1 : 0;
  }
@@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
  	}
 
  	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
+	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
+	if (!link->aspm_default)
+		link->aspm_default = link->aspm_enabled;
 
  	/* Setup initial capable state. Will be updated later */
  	link->aspm_capable = link->aspm_support;
include/linux/pci.h +9 -0
index 59876de13860..8947cbaf9fa6 100644
@@ -620,6 +620,10 @@ struct pci_host_bridge {
  	unsigned int	size_windows:1;		/* Enable root bus sizing */
  	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
 
+#ifdef CONFIG_PCIEASPM
+	unsigned int	aspm_default_link_state;	/* Controller-provided default */
+#endif
+
  	/* Resource alignment requirements */
  	resource_size_t (*align_resource)(struct pci_dev *dev,
  			const struct resource *res,
@@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
  int pci_enable_link_state(struct pci_dev *pdev, int state);
  int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state);
  void pcie_no_aspm(void);
  bool pcie_aspm_support_enabled(void);
  bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
  { return 0; }
  static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
  { return 0; }
+static inline void
+pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+				     unsigned int state) { }
  static inline void pcie_no_aspm(void) { }
  static inline bool pcie_aspm_support_enabled(void) { return false; }
  static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9

Signature · 1 line
Kenneth R. Crudup / Sr. SW Engineer, Scott County Consulting, Orange County CA
#15 lore raw

Any movement on this?

Anything keeping it from being in pm-next (at least I haven't seen it in a precursory look for it)?

Thanks,

-Kenny

Quoted 161 lines · Kenneth Crudup
Quoted 156 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

---
Changes in V3:
   -- Changed pci_host_get_default_pcie_link_state() argument name from parent to dev.
   -- Applied changelog tags

Changes in V2:

   -- Host field name changed to aspm_default_link_state. -- Added get/set functions for aspm_default_link_state. Only the setter is exported. Added a kernel-doc describing usage and particulars around meaning of 0.

Changes in V1 from RFC:

   -- Rename field to aspm_dflt_link_state since it stores PCIE_LINK_STATE_XXX flags, not a policy enum.
   -- Move the field to struct pci_host_bridge since it's being applied to the entire host bridge per Mani's suggestion.
   -- During testing noticed that clkpm remained disabled and this was also handled by the formerly used pci_enable_link_state(). Add a check in pcie_clkpm_cap_init() as well to enable clkpm during init.

  drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++-- include/linux/pci.h     |  9 +++++++++

  2 files changed, 49 insertions(+), 2 deletions(-)
drivers/pci/pcie/aspm.c +2 -0
index 919a05b97647..851ca3d68e55 100644
@@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state 
 *link, int enable)
       pcie_set_clkpm_nocheck(link, enable);
   }
+/**
+ * pci_host_set_default_pcie_link_state - set controller-provided 

default ASPM/CLKPM mask
+ * @host: host bridge on which to apply the defaults
+ * @state: PCIE_LINK_STATE_XXX flags
+ *
+ * Allows a PCIe controller driver to specify the default ASPM and/or
+ * Clock Power Management (CLKPM) link state mask that will be used
+ * for links under this host bridge during ASPM/CLKPM capability init.
+ *
+ * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
+ * to override the firmware-discovered defaults.
+ *
+ * Interpretation of aspm_default_link_state:
+ *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
+ *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
+ *              values discovered in hardware/firmware
+ *
+ * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
+ */

drivers/pci/pcie/aspm.c +14 -0
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+                      unsigned int state)
+{
+    host->aspm_default_link_state = state;
+}
+EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
+
+static u32 pci_host_get_default_pcie_link_state(struct pci_dev *dev)
+{
+    struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+    return host ? host->aspm_default_link_state : 0;
+}
+

  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist) {
      int capable = 1, enabled = 1;

drivers/pci/pcie/aspm.c +4 -1
@@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct 
 pcie_link_state *link, int blacklist)
               enabled = 0;
       }
       link->clkpm_enabled = enabled;
-    link->clkpm_default = enabled;
+    if (pci_host_get_default_pcie_link_state(link->pdev) & 
 PCIE_LINK_STATE_CLKPM)
+        link->clkpm_default = 1;
+    else
+        link->clkpm_default = enabled;
       link->clkpm_capable = capable;

      link->clkpm_disable = blacklist ? 1 : 0;
  }

drivers/pci/pcie/aspm.c +3 -1
@@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct 
 pcie_link_state *link, int blacklist)
       }
       /* Save default state */
-    link->aspm_default = link->aspm_enabled;
+    link->aspm_default = pci_host_get_default_pcie_link_state(parent);
+    if (!link->aspm_default)
+        link->aspm_default = link->aspm_enabled;
       /* Setup initial capable state. Will be updated later */
       link->aspm_capable = link->aspm_support;
include/linux/pci.h +0 -0
index 59876de13860..8947cbaf9fa6 100644
@@ -620,6 +620,10 @@ struct pci_host_bridge {

      unsigned int    size_windows:1;        /* Enable root bus sizing */ unsigned int    msi_domain:1;        /* Bridge wants MSI domain */ +#ifdef CONFIG_PCIEASPM
+    unsigned int    aspm_default_link_state;    /* Controller- provided default */

include/linux/pci.h +2 -0
+#endif
+

      /* Resource alignment requirements */ resource_size_t (*align_resource)(struct pci_dev *dev, const struct resource *res,

include/linux/pci.h +2 -0
@@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, 
 int state);
   int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
   int pci_enable_link_state(struct pci_dev *pdev, int state);
   int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+                      unsigned int state);
   void pcie_no_aspm(void);
   bool pcie_aspm_support_enabled(void);

  bool pcie_aspm_enabled(struct pci_dev *pdev);

include/linux/pci.h +3 -0
@@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct 
 pci_dev *pdev, int state)
   { return 0; }
   static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
   { return 0; }
+static inline void
+pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+                     unsigned int state) { }
   static inline void pcie_no_aspm(void) { }
   static inline bool pcie_aspm_support_enabled(void) { return false; }

  static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9

Signature · 1 line
Kenneth R. Crudup / Sr. SW Engineer, Scott County Consulting, Orange County CA
Tested-by on v3 1/2
#16 lore raw
In reply to David E. Box
Quoted 149 lines · David E. Box

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM defaults. Devices in such domains may therefore run without the intended power management.

Add a host-bridge mechanism that lets controller drivers supply their own defaults. A new aspm_default_link_state field in struct pci_host_bridge is set via pci_host_set_default_pcie_link_state(). During link initialization, if this field is non-zero, ASPM and CLKPM defaults come from it instead of BIOS.

This enables drivers like VMD to align link power management with platform expectations and avoids embedding controller-specific quirks in ASPM core logic.

---
Changes in V3:
-- Changed pci_host_get_default_pcie_link_state() argument name from
parent to dev.
-- Applied changelog tags

Changes in V2:

-- Host field name changed to aspm_default_link_state. -- Added get/set functions for aspm_default_link_state. Only the
setter is exported. Added a kernel-doc describing usage and particulars around meaning of 0.

Changes in V1 from RFC:

-- Rename field to aspm_dflt_link_state since it stores
PCIE_LINK_STATE_XXX flags, not a policy enum.
-- Move the field to struct pci_host_bridge since it's being applied to
the entire host bridge per Mani's suggestion.
-- During testing noticed that clkpm remained disabled and this was
also handled by the formerly used pci_enable_link_state(). Add a check in pcie_clkpm_cap_init() as well to enable clkpm during init.

  drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
  include/linux/pci.h     |  9 +++++++++
  2 files changed, 49 insertions(+), 2 deletions(-)
drivers/pci/pcie/aspm.c +40 -2
index 919a05b97647..851ca3d68e55 100644
@@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
  	pcie_set_clkpm_nocheck(link, enable);
  }
 
+/**
+ * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
+ * @host: host bridge on which to apply the defaults
+ * @state: PCIE_LINK_STATE_XXX flags
+ *
+ * Allows a PCIe controller driver to specify the default ASPM and/or
+ * Clock Power Management (CLKPM) link state mask that will be used
+ * for links under this host bridge during ASPM/CLKPM capability init.
+ *
+ * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
+ * to override the firmware-discovered defaults.
+ *
+ * Interpretation of aspm_default_link_state:
+ *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
+ *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
+ *              values discovered in hardware/firmware
+ *
+ * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
+ */
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state)
+{
+	host->aspm_default_link_state = state;
+}
+EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
+
+static u32 pci_host_get_default_pcie_link_state(struct pci_dev *dev)
+{
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+
+	return host ? host->aspm_default_link_state : 0;
+}
+
  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
  {
  	int capable = 1, enabled = 1;
@@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
  			enabled = 0;
  	}
  	link->clkpm_enabled = enabled;
-	link->clkpm_default = enabled;
+	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
+		link->clkpm_default = 1;
+	else
+		link->clkpm_default = enabled;
  	link->clkpm_capable = capable;
  	link->clkpm_disable = blacklist ? 1 : 0;
  }
@@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
  	}
 
  	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
+	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
+	if (!link->aspm_default)
+		link->aspm_default = link->aspm_enabled;
 
  	/* Setup initial capable state. Will be updated later */
  	link->aspm_capable = link->aspm_support;
include/linux/pci.h +9 -0
index 59876de13860..8947cbaf9fa6 100644
@@ -620,6 +620,10 @@ struct pci_host_bridge {
  	unsigned int	size_windows:1;		/* Enable root bus sizing */
  	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
 
+#ifdef CONFIG_PCIEASPM
+	unsigned int	aspm_default_link_state;	/* Controller-provided default */
+#endif
+
  	/* Resource alignment requirements */
  	resource_size_t (*align_resource)(struct pci_dev *dev,
  			const struct resource *res,
@@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
  int pci_enable_link_state(struct pci_dev *pdev, int state);
  int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state);
  void pcie_no_aspm(void);
  bool pcie_aspm_support_enabled(void);
  bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
  { return 0; }
  static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
  { return 0; }
+static inline void
+pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+				     unsigned int state) { }
  static inline void pcie_no_aspm(void) { }
  static inline bool pcie_aspm_support_enabled(void) { return false; }
  static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9

Signature · 1 line
Kenneth R. Crudup / Sr. SW Engineer, Scott County Consulting, Orange County CA