mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Francisco Beltrán Millalén" <fbeltranmillalen@gmail.com>
To: Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 0/3] PCI/PM: Do not save the config space of an inaccessible device
Date: Wed, 30 Sep 2026 11:19:11 -0300	[thread overview]
Message-ID: <20260930141914.6678-1-fbeltranmillalen@gmail.com> (raw)

When a PCI device becomes inaccessible while the system is suspending,
pci_save_state() stores all ones over its saved config space, and
pci_restore_state() writes that back on resume to a device that answers
again.  On a MacBookPro14,3 this happens to the upstream bridge of a
Thunderbolt 3 controller that drops off the bus while the system is
suspending: after resume the bridge has bus numbers ff/ff/ff and
Secondary Bus Reset asserted, and the xHCI controllers behind it are
removed.  Resume also waits about 65 seconds for a device behind a dead
bridge, because an all-ones Link Status reads as an active link.

Patch 2 makes pci_save_state() refuse to save an inaccessible device,
using pci_dev_config_accessible() from commit e18d1abc3bff ("PCI: Avoid
saving config space state if inaccessible"), so that system suspend is
covered and not only resets.  Patch 1 prepares the USB PCI HCD for it;
without patch 1, patch 2 makes pci_pm_suspend_noirq() warn.  Patch 3
stops the link wait code from taking an all-ones Link Status for an
active link.

Patch 1 touches drivers/usb.  Bjorn, if you take the series, it would
need an ack from Greg or Alan.

Changes since v1:
- v1 2/4 and 3/4 took an all-ones Vendor and Device ID to mean that the
  device was inaccessible, but that is always the case for SR-IOV VFs,
  so they broke saving and restoring VFs (as I said in reply to v1).
  2/3 now uses pci_dev_config_accessible(), which reads the Command and
  Status registers.
- Dropped v1 3/4 ("PCI/PM: Do not restore a config space snapshot that
  is all ones"): it would never restore a VF, and it did not protect
  what it claimed to, as pci_restore_state() restores the PCIe
  capability state before the standard header.
- 1/3: rewrote the commit message and moved the wakeup handling for a
  dead root hub ahead of the early return.  Alan's Acked-by is dropped.
- 2/3: the second accessibility check now runs after the capabilities
  are saved, and state_saved is only set if both checks pass.
- 3/3: also cover pcie_wait_for_link_status().
- The v1 cover letter spoke of an earlier version; that version was
  never posted.
- Based on pci/next.

v1: https://lore.kernel.org/all/20260924124221.12374-1-fbeltranmillalen@gmail.com/

Testing:
On a MacBookPro14,3 (two Alpine Ridge controllers), v6.18.49 with
e18d1abc3bff backported and this series, S3 entered by closing the lid
(158 s asleep), a USB disk on one controller and nothing on the other:

- In pci_pm_suspend_noirq() the bridges of both controllers, including
  the upstream bridge 04:00.0, were inaccessible and their state was not
  saved ("Device config space inaccessible; unable to save state").
- On resume the controller with nothing attached came back: 04:00.0
  kept bus numbers 04/05/79 and Bridge Control 0x0002, the link came up
  at 8 GT/s and its xHCI controller resumed.  Before the series the same
  bridge came back with ff/ff/ff and Bridge Control 0x005f (Secondary
  Bus Reset asserted), and both xHCI controllers were removed.
- The controller with the disk did not come back (its link does not
  train, which is a separate problem); resume waited 1 s for its xHCI
  controller instead of 65 s.
- No "State of device not saved" warning.

With the separate Alpine Ridge quirk applied, the xHCI controllers of an
empty controller are inaccessible in hcd_pci_suspend_noirq(); four S3
cycles went through patch 1 without warnings and everything resumed.

When the machine wakes up again after a few seconds (with the lid open
it does, after about 3.5 s), the empty controller does not come back
either, with v1 as with v2, so there is nothing for the series to
preserve.  The "1 of 2 controllers instead of 0 of 2" in the v1 cover
letter holds only for the longer sleeps.

I have no SR-IOV hardware, so the VF case is untested, and the machine
never reaches the pcie_wait_for_link_status() change.

Francisco Beltrán Millalén (3):
  usb: hcd-pci: Honour pci_save_state() failure
  PCI/PM: Do not save the config space of an inaccessible device
  PCI: Do not mistake an absent device for an active link

 drivers/pci/pci.c          | 69 ++++++++++++++++++++++++++------------
 drivers/usb/core/hcd-pci.c | 15 +++++++--
 2 files changed, 61 insertions(+), 23 deletions(-)

             reply	other threads:[~2026-09-30 14:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30 14:19 Francisco Beltrán Millalén [this message]
2026-09-30 14:19 ` [PATCH v2 1/3] usb: hcd-pci: Honour pci_save_state() failure Francisco Beltrán Millalén
2026-09-30 14:19 ` [PATCH v2 2/3] PCI/PM: Do not save the config space of an inaccessible device Francisco Beltrán Millalén
2026-09-30 14:19 ` [PATCH v2 3/3] PCI: Do not mistake an absent device for an active link Francisco Beltrán Millalén

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=20260930141914.6678-1-fbeltranmillalen@gmail.com \
    --to=fbeltranmillalen@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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

all inboxes | Powered by JetHome®