public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jian-Hong Pan <jhp@endlessos.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Johan Hovold" <johan@kernel.org>,
	"David Box" <david.e.box@linux.intel.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Kuppuswamy Sathyanarayanan"
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	"Nirmal Patel" <nirmal.patel@linux.intel.com>,
	"Jonathan Derrick" <jonathan.derrick@linux.dev>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux@endlessos.org, "Jian-Hong Pan" <jhp@endlessos.org>
Subject: [PATCH v9 3/3] PCI: vmd: Save/restore PCIe bridge states before/after pci_reset_bus()
Date: Tue, 24 Sep 2024 15:29:00 +0800	[thread overview]
Message-ID: <20240924072858.15929-3-jhp@endlessos.org> (raw)
In-Reply-To: <20240924070551.14976-2-jhp@endlessos.org>

PCI devices' parameters on the VMD bus have been programmed properly
originally. But, cleared after pci_reset_bus() and have not been restored
correctly. This leads the link's L1.2 between PCIe Root Port and child
device gets wrong configs.

Here is a failed example on ASUS B1400CEAE with enabled VMD. Both PCIe
bridge and NVMe device should have the same LTR1.2_Threshold value.
However, they are configured as different values in this case:

10000:e0:06.0 PCI bridge [0604]: Intel Corporation 11th Gen Core Processor PCIe Controller [8086:9a09] (rev 01) (prog-if 00 [Normal decode])
  ...
  Capabilities: [200 v1] L1 PM Substates
    L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+
      PortCommonModeRestoreTime=45us PortTPowerOnTime=50us
    L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1-
      T_CommonMode=0us LTR1.2_Threshold=0ns
    L1SubCtl2: T_PwrOn=0us

10000:e1:00.0 Non-Volatile memory controller [0108]: Sandisk Corp WD Blue SN550 NVMe SSD [15b7:5009] (rev 01) (prog-if 02 [NVM Express])
  ...
  Capabilities: [900 v1] L1 PM Substates
    L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- L1_PM_Substates+
      PortCommonModeRestoreTime=32us PortTPowerOnTime=10us
    L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1-
      T_CommonMode=0us LTR1.2_Threshold=101376ns
    L1SubCtl2: T_PwrOn=50us

Here is VMD mapped PCI device tree:

-+-[0000:00]-+-00.0  Intel Corporation Device 9a04
 | ...
 \-[10000:e0]-+-06.0-[e1]----00.0  Sandisk Corp WD Blue SN550 NVMe SSD
              \-17.0  Intel Corporation Tiger Lake-LP SATA Controller

When pci_reset_bus() resets the bus [e1] of the NVMe, it only saves and
restores NVMe's state before and after reset. Because bus [e1] has only one
device: 10000:e1:00.0 NVMe. The PCIe bridge is missed. However, when it
restores the NVMe's state, it also restores the ASPM L1SS between the PCIe
bridge and the NVMe by pci_restore_aspm_l1ss_state(). The NVMe's L1SS is
restored correctly. But, the PCIe bridge's L1SS is restored with the wrong
value 0x0. Becuase, the PCIe bridge's state is not saved before reset.
That is why pci_restore_aspm_l1ss_state() gets the parent device (PCIe
bridge)'s saved state capability data and restores L1SS with value 0.

So, save the PCIe bridge's state before pci_reset_bus(). Then, restore the
state after pci_reset_bus(). The restoring state also consumes the saving
state.

Link: https://www.spinics.net/lists/linux-pci/msg159267.html
Signed-off-by: Jian-Hong Pan <jhp@endlessos.org>
---
v9:
- Drop the v8 fix about drivers/pci/pcie/aspm.c. Use this in VMD instead.

 drivers/pci/controller/vmd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 11870d1fc818..2820005165b4 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -938,9 +938,16 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		if (!list_empty(&child->devices)) {
 			dev = list_first_entry(&child->devices,
 					       struct pci_dev, bus_list);
+			/* To avoid pci_reset_bus restore wrong ASPM L1SS
+			 * configuration due to missed saving parent device's
+			 * states, save & restore the parent device's states
+			 * as well.
+			 */
+			pci_save_state(dev->bus->self);
 			ret = pci_reset_bus(dev);
 			if (ret)
 				pci_warn(dev, "can't reset device: %d\n", ret);
+			pci_restore_state(dev->bus->self);
 
 			break;
 		}
-- 
2.46.1


  parent reply	other threads:[~2024-09-24  7:30 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-24  7:05 [PATCH v9 0/3] PCI: vmd: Enable PCI PM's L1 substates of remapped PCIe Root Port and NVMe Jian-Hong Pan
2024-09-24  7:14 ` [PATCH v9 1/3] PCI: vmd: Set PCI devices to D0 before enable PCI PM's L1 substates Jian-Hong Pan
2024-09-24  7:26 ` [PATCH v9 2/3] PCI/ASPM: Add notes about enabling PCI-PM L1SS to pci_enable_link_state(_locked) Jian-Hong Pan
2024-09-24  7:29 ` Jian-Hong Pan [this message]
2024-09-26  2:51   ` [PATCH v9 3/3] PCI: vmd: Save/restore PCIe bridge states before/after pci_reset_bus() David E. Box
2024-09-26  4:05     ` Jian-Hong Pan
2024-09-26 10:52       ` Ilpo Järvinen
2024-09-26 13:22         ` Ilpo Järvinen
2024-09-27 10:42           ` Jian-Hong Pan
2024-09-26 22:52         ` David E. Box

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20240924072858.15929-3-jhp@endlessos.org \
    --to=jhp@endlessos.org \
    --cc=david.e.box@linux.intel.com \
    --cc=helgaas@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=johan@kernel.org \
    --cc=jonathan.derrick@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@endlessos.org \
    --cc=nirmal.patel@linux.intel.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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