From nobody Sun Feb 8 22:54:04 2026 Received: from mail-dy1-f178.google.com (mail-dy1-f178.google.com [74.125.82.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 675C83093B5 for ; Thu, 8 Jan 2026 23:42:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767915743; cv=none; b=V0rzNObsOC/Yg8UW9m0pzI8T5LuW8wbxPvhrsxUh3biHa2jJFL9XPMm7zvXOIlkEiMmrnZ0L6ZfsnomX2V7VrT+NiaIW10v4XwMgL+2LgOUYNxJIdWn5B9e+MBUCRsqxjuRXqp5aKBW4BIritjndpn8KgoynN8Lw9sA4/rVLMSk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767915743; c=relaxed/simple; bh=q1WJ/tknsGfRiD8moFkTOed5nAXvTgq3ret8KECqKMw=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type; b=oB0vMPHA/aoyLh4JOlg3Zi7NcpLHC87/SJmEnQ4xjyNvDgw0n573FXbFqOoksCFttyFTpdV1ipNiRyxxzhBOjOYJKRaZeAHCt+Sn3ITfXuwhlOG9YVdxkoLxYnmtu3fJny6+bQGEGpUlqUFoSjTn+YVgqCoKrt4BIPvNNB2OzOY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=YVE73Gy7; arc=none smtp.client-ip=74.125.82.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="YVE73Gy7" Received: by mail-dy1-f178.google.com with SMTP id 5a478bee46e88-2ae2eb49b4bso1644760eec.0 for ; Thu, 08 Jan 2026 15:42:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1767915739; x=1768520539; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=W9inY4Dn7b85AeLEK8fRSnnQcqPoH5Aqe2MencsS1B8=; b=YVE73Gy7t/2eDAz2riBA4AndgK3O6wVgLcGalxxylWXNCAkaNuo2Vw7MqkPlWoGqG2 vLP0HTHSRYPbV//BOzUCHPtxq/OONL1RbDWy2w9fJl8nZNmfF4PLMoDNZ6VhKUK1lYgz Qzj1QPF81Ta6O07MX4W5txTKM2hKHTIhB+FFM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767915739; x=1768520539; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=W9inY4Dn7b85AeLEK8fRSnnQcqPoH5Aqe2MencsS1B8=; b=paP+xDoiRXv6+eWK2qB1FLcCLJowODde9mIipUYI5iMRwicF1GGwKE081xECGrTg4Z ZyMGaDdmeHyB18vZsJBz/BaaD99I4f2VLvXY6UZgFETKb0cFsgl7OVMQUJYblD3gK42U 1xYe5cjHu0deankiyqb6o+ehQgmz/04jVk9LYwn9Qp2APemCboy2m2A0m26LCyxVDjkD 3ivE2WO+3zO1e29Qx4lE0LW44gGS2RofMit4sH+Kc5DoM2NEcwWZPnZueS46TlZB8mpu CdV+S6Skjus6n5ZXoFh2s8WN/VA4eEfp48av8MpVVnI6q+uYlpSDEVBmO/PG5/7q+Pvf TXOg== X-Forwarded-Encrypted: i=1; AJvYcCVvtNeypk5WjhCukf23pgr8PYpTETWlUg/nCAexMysRysSlsfhlf6o00VTpiazkJz1KbFaFu3mpY/ScQmo=@vger.kernel.org X-Gm-Message-State: AOJu0YzBG0j35Kqu5BHkpzwK/tR8MW7hnm4SMEYrZohA+ynrJR3L3XB8 2ayZ27ZZGfR8xAzTQ9cRNdCxly8rtXRuZYSspwroXRzgF+Iv6113wSUJEXWOT5vPnjv8M49Ze2K ax4U= X-Gm-Gg: AY/fxX7HecEM2bCGntLSgH+xvQxPJsldcOkXT05jWyoV3lai3S5nNLQjzof3lh+Wu65 vGSR5f9++QnAnvYPFdFbaBMgqu0OGZv847C3pIEXIgN5KPQhF3rKVvScVeidudyBmGGAKEtygB3 6t5SlHomiUUTNxKACTzFESrZe5AcOPPuHoEfhxaEwvvfBzMeubcskc4Z4BAhbxWLcYZ62KM3bF5 tZIpR5LxieTt19wEazZYTMRHw4GfH7gX1cSmwZ9xR/lJQZM7IyVY0AO63S4L+V5C3eyKHtJMfZT PDRDgUp/EikQ63JLcgSMOj59gD246JJBg+KbLmUJTpezTNQrtwIboV9k2vHOjeVGUi2C7jQkIjR IWgitxD+sKXHJ57tyjkbrq/R1W4XzJ+u5UV33mTHyeAN/x8UG6Kc9F9CARSxTfWvQGIX0KIWKXY pWqyQZWmDmgwjM3eHb9tSWgqewFLMK4/xbkQzAREoZDrwB3hJKHw== X-Google-Smtp-Source: AGHT+IEd5xjYdSwbAVqr9+dYrpsdym27z2YDGMurEb5Fj2LJWyZXFOnDjVN4fI56rz6x0MybpuCRFA== X-Received: by 2002:a05:7301:6790:b0:2b0:59da:f798 with SMTP id 5a478bee46e88-2b17d294b83mr8744620eec.21.1767915739474; Thu, 08 Jan 2026 15:42:19 -0800 (PST) Received: from localhost ([2a00:79e0:2e7c:8:d9f4:70dd:b942:60f7]) by smtp.gmail.com with UTF8SMTPSA id 5a478bee46e88-2b1706a5d3dsm9358795eec.13.2026.01.08.15.42.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Jan 2026 15:42:18 -0800 (PST) From: Brian Norris To: Bjorn Helgaas Cc: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= , Heiner Kallweit , linux-pci@vger.kernel.org, Manivannan Sadhasivam , Rajat Jain , Ajay Agarwal , linux-kernel@vger.kernel.org, Brian Norris Subject: [PATCH] PCI/ASPM: Allow PCI PM L1 substates without ASPM Date: Thu, 8 Jan 2026 15:42:01 -0800 Message-ID: <20260108154200.1.I7beb66162d35904e7e05830a666de03ed75e6b76@changeid> X-Mailer: git-send-email 2.52.0.457.g6b5491de43-goog Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Configuration of ASPM and PCI PM should be mostly orthogonal, where PCI PM L1 substates can be enabled/disabled independently of ASPM L1 substates. The main restriction is that one cannot enter L1 substates without entering L1.0 first. In practice, this means we cannot enable ASPM L1 substates if ASPM L1.0 is disabled. Notably, PCI PM L1.0 cannot be disabled [*]. However, we're enforcing unexpected artificial dependencies between PCI PM and ASPM. Consider the following sequence on a given device: 1. Initial state: L1.0, L1.1, L1.2 ASPM enabled L1.1, L1.2 PCI PM enabled 2. We disable ASPM L1.0 via sysfs echo 0 > /sys/bus/pci/devices/.../link/l1_aspm Expected: L1.0, L1.1, L1.2 ASPM disabled L1.1, L1.2 PCI PM enabled Actual: L1.0, L1.1, L1.2 ASPM disabled L1.1, L1.2 PCI PM disabled I suspect this is an accidental misapplication of ASPM requirements to the PCI PM configuration. There are similar artificial dependencies: 1. enabling L1.x PCI PM unnecessarily implies enabling L1.0 ASPM 2. pci_{enable,disable}_link_state() have the same bug Relax these dependencies by: (a) narrowing "PCIE_LINK_STATE_L1SS" to only mean ASPM L1 substates, and updating some corresponding naming/constants (b) applying the restriction (that L1 substates require L1.0 support) only to the ASPM variant. --- [*] PCI PM L1.0 does not have a configuration bit to disable it; it is entered whenever the downstream component leaves D0. (PCIe r6.1 5.3.2) =3D=3D Behavioral impact =3D=3D Besides correcting sysfs behavior, this may have some impact on other drivers. For instance, drivers that call pci_disable_link_state(pdev, PCIE_LINK_STATE_L1); will now only disable ASPM L1.{0,1,2}; L1.{1,2} PCI PM may still be possible. I don't expect this to be a problem, since ASPM is typically the problematic one -- PCI PM typically only takes effect when a device is runtime suspended (D3). =3D=3D Historical note =3D=3D It seems this mistake originates in commit aeda9adebab8 ("PCI/ASPM: Configure L1 substate settings"), where PCIE_LINK_STATE_L1SS was previously named ASPM_STATE_L1SS, although it represented both ASPM and PCI PM substates. This error was propagated further in the other commits marked with Fixes. Fixes: aeda9adebab8 ("PCI/ASPM: Configure L1 substate settings") ++ Fixes: aff5d0552da4 ("PCI/ASPM: Add L1 PM substate support to pci_disable_l= ink_state()") ++ Fixes: 72ea91afbfb0 ("PCI/ASPM: Add sysfs attributes for controlling ASPM l= ink states") ++ Fixes: 80950a546089 ("PCI/ASPM: Set ASPM_STATE_L1 when driver enables L1.1 = or L1.2") ++ Signed-off-by: Brian Norris --- drivers/pci/pcie/aspm.c | 32 ++++++++++++++++---------------- include/linux/pci.h | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index cedea47a3547..4d1e79886518 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -221,9 +221,8 @@ static_assert(PCIE_LINK_STATE_L0S =3D=3D (PCIE_LINK_STA= TE_L0S_UP | PCIE_LINK_STATE_L PCIE_LINK_STATE_L1_2_PCIPM) #define PCIE_LINK_STATE_L1_2_MASK (PCIE_LINK_STATE_L1_2 |\ PCIE_LINK_STATE_L1_2_PCIPM) -#define PCIE_LINK_STATE_L1SS (PCIE_LINK_STATE_L1_1 |\ - PCIE_LINK_STATE_L1_1_PCIPM |\ - PCIE_LINK_STATE_L1_2_MASK) +#define PCIE_LINK_STATE_L1_SS_ASPM (PCIE_LINK_STATE_L1_1 |\ + PCIE_LINK_STATE_L1_2) =20 struct pcie_link_state { struct pci_dev *pdev; /* Upstream component of the Link */ @@ -902,8 +901,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state *= link, int blacklist) } } =20 -/* Configure the ASPM L1 substates. Caller must disable L1 first. */ -static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state) +/* Configure the L1 substates. Caller must disable L1 first. */ +static void pcie_config_l1ss(struct pcie_link_state *link, u32 state) { u32 val =3D 0; struct pci_dev *child =3D link->downstream, *parent =3D link->pdev; @@ -953,9 +952,9 @@ static void pcie_config_aspm_link(struct pcie_link_stat= e *link, u32 state) /* Enable only the states that were not explicitly disabled */ state &=3D (link->aspm_capable & ~link->aspm_disable); =20 - /* Can't enable any substates if L1 is not enabled */ + /* Can't enable any ASPM substates if L1 is not enabled */ if (!(state & PCIE_LINK_STATE_L1)) - state &=3D ~PCIE_LINK_STATE_L1SS; + state &=3D ~PCIE_LINK_STATE_L1_SS_ASPM; =20 /* Spec says both ports must be in D0 before enabling PCI PM substates*/ if (parent->current_state !=3D PCI_D0 || child->current_state !=3D PCI_D0= ) { @@ -994,8 +993,9 @@ static void pcie_config_aspm_link(struct pcie_link_stat= e *link, u32 state) pcie_config_aspm_dev(child, 0); pcie_config_aspm_dev(parent, 0); =20 - if (link->aspm_capable & PCIE_LINK_STATE_L1SS) - pcie_config_aspm_l1ss(link, state); + if (link->aspm_capable & (PCIE_LINK_STATE_L1_SS_PCIPM | + PCIE_LINK_STATE_L1_SS_ASPM)) + pcie_config_l1ss(link, state); =20 pcie_config_aspm_dev(parent, upstream); list_for_each_entry(child, &linkbus->devices, bus_list) @@ -1376,9 +1376,9 @@ static u8 pci_calc_aspm_disable_mask(int state) { state &=3D ~PCIE_LINK_STATE_CLKPM; =20 - /* L1 PM substates require L1 */ + /* L1 ASPM substates require L1 ASPM */ if (state & PCIE_LINK_STATE_L1) - state |=3D PCIE_LINK_STATE_L1SS; + state |=3D PCIE_LINK_STATE_L1_SS_ASPM; =20 return state; } @@ -1387,8 +1387,8 @@ static u8 pci_calc_aspm_enable_mask(int state) { state &=3D ~PCIE_LINK_STATE_CLKPM; =20 - /* L1 PM substates require L1 */ - if (state & PCIE_LINK_STATE_L1SS) + /* L1 ASPM substates require L1 ASPM */ + if (state & PCIE_LINK_STATE_L1_SS_ASPM) state |=3D PCIE_LINK_STATE_L1; =20 return state; @@ -1626,13 +1626,13 @@ static ssize_t aspm_attr_store_common(struct device= *dev, =20 if (state_enable) { link->aspm_disable &=3D ~state; - /* need to enable L1 for substates */ - if (state & PCIE_LINK_STATE_L1SS) + /* need to enable L1 for ASPM substates */ + if (state & PCIE_LINK_STATE_L1_SS_ASPM) link->aspm_disable &=3D ~PCIE_LINK_STATE_L1; } else { link->aspm_disable |=3D state; if (state & PCIE_LINK_STATE_L1) - link->aspm_disable |=3D PCIE_LINK_STATE_L1SS; + link->aspm_disable |=3D PCIE_LINK_STATE_L1_SS_ASPM; } =20 pcie_config_aspm_link(link, policy_to_aspm_state(link)); diff --git a/include/linux/pci.h b/include/linux/pci.h index 864775651c6f..5781aff12748 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1865,7 +1865,7 @@ static inline int pcie_set_target_speed(struct pci_de= v *port, #endif =20 #define PCIE_LINK_STATE_L0S (BIT(0) | BIT(1)) /* Upstr/dwnstr L0s */ -#define PCIE_LINK_STATE_L1 BIT(2) /* L1 state */ +#define PCIE_LINK_STATE_L1 BIT(2) /* ASPM L1 state */ #define PCIE_LINK_STATE_L1_1 BIT(3) /* ASPM L1.1 state */ #define PCIE_LINK_STATE_L1_2 BIT(4) /* ASPM L1.2 state */ #define PCIE_LINK_STATE_L1_1_PCIPM BIT(5) /* PCI-PM L1.1 state */ --=20 2.52.0.457.g6b5491de43-goog