Re: [PATCH] PCI: Drop unnecessary retries when restoring BARs

On Mon, May 11, 2026 at 05:24:49PM +0200, Rafael J. Wysocki wrote:
> On Fri, May 8, 2026 at 11:43 PM Bjorn Helgaas <[email protected]> wrote:
> > On Fri, May 08, 2026 at 02:51:37PM +0200, Rafael J. Wysocki wrote:
> > > On Fri, May 8, 2026 at 2:17 AM Bjorn Helgaas <[email protected]> wrote:
> > > > On Tue, May 05, 2026 at 12:43:17PM +0200, Rafael J. Wysocki wrote:
> > > > > On Mon, May 4, 2026 at 11:17 PM Bjorn Helgaas <[email protected]> wrote:
> ...

> > > >     platform_pci_set_power_state(dev, PCI_D0);
> > > >
> > > >     pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
> > > >     state = pmcsr & PCI_PM_CTRL_STATE_MASK;
> > > >     if (state == PCI_D0)
> > > >       goto end;
> > > >
> > > >     pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, 0);
> > > >     if (state == PCI_D3hot)
> > > >       pci_dev_d3_sleep(dev);
> > > >     ...
> > > >
> > > >   end:
> ...

> > OK, so after platform_pci_set_power_state(PCI_D0), the device may be
> > in D3hot in some cases.  Then there's a D3hot -> D0 transition, and we
> > do pci_dev_d3_sleep(), but not pci_dev_wait().  Sec 2.3.1 says RRS is
> > permitted after a D3hot to D0uninitialized, and pci_dev_wait() is what
> > waits and retries for the RRS case.
> >
> > It looks to me like we need pci_dev_wait() after that transition when
> > No_Soft_Reset == 0, i.e., something like the below.  Do you think this
> > is unnecessary?
> 
> Well, let me turn this around: Is there any evidence that it is necessary?

PCIe r7.0, sec 2.3.1, includes this:

  ◦ For Configuration Requests only, if Device Readiness Status is not
    supported, following reset it is permitted for a Function to
    terminate the request and indicate that it is temporarily unable
    to process the Request, but will be able to process the Request in
    the future - in this case, the Request Retry Status (RRS)
    Completion Status must be used (see § Section 6.6 ). Valid reset
    conditions after which a device/Function is permitted to return
    RRS in response to a Configuration Request are:

    ▪ Cold, Warm, and Hot Resets
    ▪ FLRs
    ▪ A reset initiated in response to a D3Hot to D0uninitialized
      device state transition

When No_Soft_Reset == 0, the D3hot->D0 transition is to
D0uninitialized, so I think the device is allowed to respond with RRS.

For the D3hot->D0 case, pci_power_up() already calls
pci_dev_d3_sleep(), which waits the 10ms required by sec 5.9, but it
does not currently account for an RRS response after that.

My argument is that if the device *does* respond with RRS, subsequent
config reads in callers of pci_power_up() will see invalid ~0 data
even though the device may become ready in the future.

For example, pci_set_full_power_state() reads PCI_PM_CTRL and may
incorrectly complain "Refused to change power state from D3hot to D0".

> I guess if it is not necessary, there will be no harm, but this is a
> performance-sensitive code path.

If the device is already Configuration-Ready, pci_dev_wait() will do
one additional config read, see a valid Vendor/Device ID, and won't
wait at all.

Bjorn