Re: [PATCH v4 1/3] PCI: Add d3cold as general reset method

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Missing IOMMU preparation and completion calls in the `pci_d3cold_reset` flow.
- [High] Missing wait for device readiness (CRS polling) after transitioning from D3cold back to D0.
- [High] Bypassing device quirks and user constraints (`dev->no_d3cold` and `dev->d3cold_allowed`) for D3cold transitions.
--

commit 6ccb7429bcb1b8c8a16f329c1b8caf009c316653
Author: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
Subject: PCI: Add d3cold as general reset method

This commit adds a new PCI reset method using D3cold power cycling for
single-function devices. It utilizes ACPI _PR3 power resources to physically
cut power to the device.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8f7cfcc000901..839903b596982 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c

[ ... ]

> +static int pci_d3cold_reset(struct pci_dev *dev, bool probe)
> +{
> +	int ret;
> +
> +	if (dev->multifunction)
> +		return -ENOTTY;
> +
> +	if (!pci_pr3_present(dev))
> +		return -ENOTTY;
> +
> +	if (probe)
> +		return 0;

Could this transition devices into D3cold even if they have the state
explicitly disabled via quirks or user constraints?

Should this check dev->no_d3cold and dev->d3cold_allowed before proceeding
with the transition?

> +
> +	if (dev->current_state != PCI_D0)
> +		return -EINVAL;
> +
> +	ret = pci_set_power_state(dev, PCI_D3cold);

Does this reset method need to pause IOMMU fault handling and wait for
pending DMA transactions before dropping physical power?

Other reset methods like pci_pm_reset() call pci_dev_reset_iommu_prepare()
before performing the hardware reset. Is similar synchronization required
here to prevent IOMMU faults?

> +	if (ret)
> +		return ret;
> +
> +	return pci_set_power_state(dev, PCI_D0);
> +}

Does the device require time to reinitialize its link and internal state
after returning to PCI_D0?

The generic reset handler will immediately invoke pci_dev_restore() after
this returns. Should this function call pci_dev_wait() to check for
Configuration Request Retry Status (CRS) clearance, similar to how
pci_pm_reset() waits?

Also, if pci_dev_reset_iommu_prepare() is needed earlier in this function,
should pci_dev_reset_iommu_done() be called here before returning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518124836.460805-1-jtornosm@redhat.com?part=1