| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250720190140.2639200-1-david.e.box@linux.intel.com>
Date: Sun, 20 Jul 2025 12:01:37 -0700
From: "David E. Box" <david.e.box@...ux.intel.com>
To: rafael@...nel.org,
bhelgaas@...gle.com,
vicamo.yang@...onical.com,
kenny@...ix.com,
ilpo.jarvinen@...ux.intel.com,
nirmal.patel@...ux.intel.com,
mani@...nel.org
Cc: "David E. Box" <david.e.box@...ux.intel.com>,
linux-pm@...r.kernel.org,
linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH] PCI/ASPM: Allow controller drivers to override default ASPM and CLKPM link state
Synthetic PCIe hierarchies, such as those created by Intel VMD, are not
visible to firmware and do not receive BIOS-provided default ASPM and CLKPM
configuration. As a result, devices behind such domains operate without
proper power management, regardless of platform intent.
To address this, allow controller drivers to supply an override for the
default link state by setting aspm_dflt_link_state for their associated
pci_host_bridge. During link initialization, if this field is non-zero,
ASPM and CLKPM defaults are derived from its value instead of being taken
from BIOS.
This mechanism enables drivers like VMD to achieve platform-aligned power
savings by statically defining the expected link configuration at
enumeration time, without relying on runtime calls such as
pci_enable_link_state(), which are ineffective when ASPM is disabled
globally.
This approach avoids per-controller hacks in ASPM core logic and provides a
general mechanism for domains that require explicit control over link power
state defaults.
Link: https://lore.kernel.org/linux-pm/0b166ece-eeec-ba5d-2212-50d995611cef@panix.com
Signed-off-by: David E. Box <david.e.box@...ux.intel.com>
---
Changes 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/controller/vmd.c | 12 +++++++++---
drivers/pci/pcie/aspm.c | 13 +++++++++++--
include/linux/pci.h | 4 ++++
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 8df064b62a2f..6f0de95c87fd 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -730,7 +730,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)
{
@@ -770,7 +770,6 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
* 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;
}
@@ -785,6 +784,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
resource_size_t membar2_offset = 0x2000;
struct pci_bus *child;
struct pci_dev *dev;
+ struct pci_host_bridge *vmd_host_bridge;
int ret;
/*
@@ -911,8 +911,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
return -ENODEV;
}
+ vmd_host_bridge = to_pci_host_bridge(vmd->bus->bridge);
+
+#ifdef CONFIG_PCIEASPM
+ vmd_host_bridge->aspm_dflt_link_state = PCIE_LINK_STATE_ALL;
+#endif
+
vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
- to_pci_host_bridge(vmd->bus->bridge));
+ vmd_host_bridge);
vmd_attach_resources(vmd);
if (vmd->irq_domain)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 29fcb0689a91..6f5b34b172f9 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -380,6 +380,7 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
u16 reg16;
struct pci_dev *child;
struct pci_bus *linkbus = link->pdev->subordinate;
+ struct pci_host_bridge *host = pci_find_host_bridge(link->pdev->bus);
/* All functions should have the same cap and state, take the worst */
list_for_each_entry(child, &linkbus->devices, bus_list) {
@@ -394,7 +395,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 (host && host->aspm_dflt_link_state & PCIE_LINK_STATE_CLKPM)
+ link->clkpm_default = 1;
+ else
+ link->clkpm_default = enabled;
link->clkpm_capable = capable;
link->clkpm_disable = blacklist ? 1 : 0;
}
@@ -794,6 +798,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
u32 parent_lnkcap, child_lnkcap;
u16 parent_lnkctl, child_lnkctl;
struct pci_bus *linkbus = parent->subordinate;
+ struct pci_host_bridge *host;
if (blacklist) {
/* Set enabled/disable so that we will disable ASPM later */
@@ -866,7 +871,11 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
}
/* Save default state */
- link->aspm_default = link->aspm_enabled;
+ host = pci_find_host_bridge(parent->bus);
+ if (host && host->aspm_dflt_link_state)
+ link->aspm_default = host->aspm_dflt_link_state;
+ else
+ link->aspm_default = link->aspm_enabled;
/* Setup initial capable state. Will be updated later */
link->aspm_capable = link->aspm_support;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 05e68f35f392..930028bf52b4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -614,6 +614,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_dflt_link_state; /* Controller provided link state */
+#endif
+
/* Resource alignment requirements */
resource_size_t (*align_resource)(struct pci_dev *dev,
const struct resource *res,
base-commit: d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af
--
2.43.0
Powered by blists - more mailing lists