Re: [RFC PATCH] PCI: Support Immediate Readiness on devices without PM capabilities

[Vipin Sharma <vipinsh@google.com>]

On 2025-06-24 10:16:37, Sean Christopherson wrote:
> Query support for Immediate Readiness irrespective of whether or not the
> device supports PM capabilities, as nothing in the PCIe spec suggests that
> Immediate Readiness is in any way dependent on PM functionality.

After reading spec I also arrived at the same conclusion, this can be
done irrespective of PM functionality. For example, wait after FLR
behavior which are done using PCI Express Capability is also covered by
this Immediate Readiness bit.

> 
> Opportunistically add a comment to explain why "errors" during PM setup
> are effectively ignored.
> 
> Fixes: d6112f8def51 ("PCI: Add support for Immediate Readiness")
> Cc: David Matlack <dmatlack@google.com>
> Cc: Vipin Sharma <vipinsh@google.com>
> Cc: Aaron Lewis <aaronlewis@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> 
> RFC as I'm not entirely sure this is useful/correct.
> 
> Found by inspection when debugging a VFIO VF passthrough issue that turned out
> to be 907a7a2e5bf4 ("PCI/PM: Set up runtime PM even for devices without PCI PM").
> 
> The folks on the Cc list are looking at parallelizing VF assignment to avoid
> serializing the 100ms wait on FLR.  I'm hoping we'll get lucky and the VFs in
> question do (or can) support PCI_STATUS_IMM_READY.
> 
>  drivers/pci/pci.c | 40 +++++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 15 deletions(-)
> 
> +void pci_pm_init(struct pci_dev *dev)
> +{
> +	u16 status;
> +
> +	device_enable_async_suspend(&dev->dev);
> +	dev->wakeup_prepared = false;
> +
> +	dev->pm_cap = 0;
> +	dev->pme_support = 0;
> +
> +	/*
> +	 * Note, support for the PCI PM spec is optional for legacy PCI devices
> +	 * and for VFs.  Continue on even if no PM capabilities are supported.
> +	 */
> +	__pci_pm_init(dev);
>  
>  	pci_read_config_word(dev, PCI_STATUS, &status);
>  	if (status & PCI_STATUS_IMM_READY)
>  		dev->imm_ready = 1;

I don't see a reason to keep it in pci_pm_init then. This should be
moved out of this function, maybe in the caller or its own function.
> -poweron:
> +
>  	pci_pm_power_up_and_verify_state(dev);
>  	pm_runtime_forbid(&dev->dev);
>  	pm_runtime_set_active(&dev->dev);
> 
> base-commit: 86731a2a651e58953fc949573895f2fa6d456841
> -- 
> 2.50.0.714.g196bf9f422-goog
>