All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: sashiko-bot@kernel.org
Cc: linux-pci@vger.kernel.org, sashiko@lists.linux.dev,
	Bjorn Helgaas <helgaas@kernel.org>,
	Marco Nenciarini <mnencia@kcore.it>,
	Michal Winiarski <michal.winiarski@intel.com>,
	Ilpo Jarvinen <ilpo.jarvinen@linux.intel.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Eric Chanudet <echanude@redhat.com>,
	Jean Guyader <jean.guyader@gmail.com>,
	Alex Williamson <alex@shazbot.org>, Sinan Kaya <okaya@kernel.org>
Subject: Re: [PATCH] PCI: Drop unnecessary retries when restoring BARs
Date: Mon, 4 May 2026 09:49:36 +0200	[thread overview]
Message-ID: <afhPkLynt7vS793r@wunner.de> (raw)
In-Reply-To: <20260503135109.46193C2BCB4@smtp.kernel.org>

On Sun, May 03, 2026 at 01:51:08PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] Removing the read-back and retry loop for BAR restoration in
> `pci_restore_state()` introduces a risk of silent regressions for
> hardware resuming from non-FLR resets (such as D3hot to D0 transitions
> or custom driver resets). The commit incorrectly assumes the 60s delay
> from `pci_dev_wait()` covers all usages, but standard PM resume paths
> only delay for 10ms (`PCI_PM_D3_WAIT`) before calling `pci_restore_state()`.
> Historically, hardware that needed slightly longer to accept
> configuration writes relied on the 10x 1ms retry loop to successfully
> restore BARs. By removing both the retry and the read-back verification,
> BAR writes to slow devices will be silently dropped, leaving hardware
> unconfigured and causing MMIO accesses to result in IOMMU faults or
> kernel crashes.

Hallucination alert:

PCI_PM_D3_WAIT does not exist, it was renamed to PCI_PM_D3HOT_WAIT
six years ago by commit 3789af9a13e5, which went into v5.10.

The macro is used in:

pci_pm_resume_noirq()
  pci_pm_default_resume_early()
    pci_pm_power_up_and_verify_state()
      pci_power_up()
        pci_dev_d3_sleep()

However before pci_power_up() calls pci_dev_d3_sleep(), it reads
the PMCSR register and errors out if config space is inaccessible.

Hence when pci_restore_state() is invoked a bit later, config space
can be assumed to be accessible.

sashiko reviews are a mixed bag, I think this one doesn't even reach
junior developer level. :(

Thanks,

Lukas

  reply	other threads:[~2026-05-04  7:49 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-03 13:34 [PATCH] PCI: Drop unnecessary retries when restoring BARs Lukas Wunner
2026-05-03 13:51 ` sashiko-bot
2026-05-04  7:49   ` Lukas Wunner [this message]
2026-05-04 17:09     ` Bjorn Helgaas
2026-05-04 19:31       ` Rafael J. Wysocki
2026-05-04 21:17         ` Bjorn Helgaas
2026-05-05 10:43           ` Rafael J. Wysocki
2026-05-08  0:17             ` Bjorn Helgaas
2026-05-08 12:51               ` Rafael J. Wysocki
2026-05-08 21:43                 ` Bjorn Helgaas
2026-05-11 15:24                   ` Rafael J. Wysocki
2026-05-12  0:01                     ` Bjorn Helgaas
2026-05-12 10:49                       ` Rafael J. Wysocki
2026-05-22 22:31 ` Bjorn Helgaas

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=afhPkLynt7vS793r@wunner.de \
    --to=lukas@wunner.de \
    --cc=alex@shazbot.org \
    --cc=echanude@redhat.com \
    --cc=helgaas@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jean.guyader@gmail.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=michal.winiarski@intel.com \
    --cc=mnencia@kcore.it \
    --cc=okaya@kernel.org \
    --cc=rafael@kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=sashiko@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.