[PATCH] PCI: Report configuration access errors when requesting FLR

[PATCH] PCI: Report configuration access errors when requesting FLR

[Baek Kyu, Kim]

PCIe and AF FLR requests can fail after pci_dev_reset_iommu_prepare()
succeeds. pcie_flr() and pci_af_flr() ignore the config access result.
A failed request can then be reported as a successful reset when an
Immediate Readiness device skips the wait or a later config read works.

Check the request access in both paths and convert PCIBIOS_* errors to
negative errno values. Use the existing done path so IOMMU cleanup still
runs after a successful prepare. Leave successful requests and readiness
handling unchanged.

Fixes: 8dd7f8036c12 ("PCI: add support for function level reset")
Fixes: 1ca887970a39 ("PCI: Extend pci_reset_function() to support PCI Advanced Features")
Signed-off-by: Baek Kyu, Kim <pocj8ur4in@gmail.com>
---
An arm64 QEMU/KVM kernel used a synthetic PCI host with the normal PCI
enumeration and sysfs reset paths. A one-shot Device Control read
failure (PCIBIOS_DEVICE_NOT_FOUND) after IOMMU prepare issued no FLR,
but the original sysfs reset returned success. The change returned
-ENODEV while preserving the IOMMU cleanup path. No physical endpoint
or ATS/IOMMU domain restoration was tested.

 drivers/pci/pci.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d79d2ee4ac69b..41fae1b1050c7 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4429,7 +4429,12 @@ int pcie_flr(struct pci_dev *dev)
 		return ret;
 	}
 
-	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
+	ret = pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
+				       PCI_EXP_DEVCTL_BCR_FLR);
+	if (ret) {
+		ret = pcibios_err_to_errno(ret);
+		goto done;
+	}
 
 	if (dev->imm_ready)
 		goto done;
@@ -4506,7 +4511,11 @@ static int pci_af_flr(struct pci_dev *dev, bool probe)
 		return ret;
 	}
 
-	pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
+	ret = pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
+	if (ret) {
+		ret = pcibios_err_to_errno(ret);
+		goto done;
+	}
 
 	if (dev->imm_ready)
 		goto done;
-- 
2.55.0

Re: [PATCH] PCI: Report configuration access errors when requesting FLR

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI: non-ENOTTY error in pcie_flr() and pci_af_flr() blocks reset fallback
--

commit 4b411204f19ff61d536131d18996cdae7c3f35b9
Author: Baek Kyu, Kim <pocj8ur4in@gmail.com>

PCI: Report configuration access errors when requesting FLR

This patch checks the configuration access return values when initiating FLR
in pcie_flr() and pci_af_flr(), converting PCIBIOS_* error codes to negative
errno values and jumping to the IOMMU cleanup path on failure.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index bca3a5d84cd9f..6c6ccbdd19f98 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4530,7 +4530,12 @@ int pcie_flr(struct pci_dev *dev)
>  		return ret;
>  	}
>  
> -	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
> +	ret = pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> +				       PCI_EXP_DEVCTL_BCR_FLR);
> +	if (ret) {
> +		ret = pcibios_err_to_errno(ret);
> +		goto done;
> +	}
>  
>  	if (dev->imm_ready)
>  		goto done;
> @@ -4607,7 +4612,11 @@ static int pci_af_flr(struct pci_dev *dev, bool probe)
>  		return ret;
>  	}
>  
> -	pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
> +	ret = pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
> +	if (ret) {
> +		ret = pcibios_err_to_errno(ret);
> +		goto done;
> +	}

[Severity: Medium]
Does returning pcibios_err_to_errno(ret) (such as -ENODEV or -EIO) instead of
-ENOTTY here prevent __pci_reset_function_locked() from falling back to other
reset mechanisms?

In drivers/pci/pci.c, __pci_reset_function_locked() iterates over
dev->reset_methods[] and aborts the fallback chain whenever a reset method
returns an error other than -ENOTTY:

	for (i = 0; i < PCI_NUM_RESET_METHODS; i++) {
		m = dev->reset_methods[i];
		if (!m)
			return -ENOTTY;

		method = &pci_reset_fn_methods[m];
		pci_dbg(dev, "trying %s\n", method->name);
		rc = method->reset_fn(dev, PCI_RESET_DO_RESET);
		if (!rc)
			return 0;

		pci_dbg(dev, "%s failed with %d\n", method->name, rc);
		if (rc != -ENOTTY)
			return rc;
	}

Commits 91295d79d658 ("PCI: Handle FLR failure and allow other reset types")
and ad05c16b290b ("PCI: Fail FLR when config space is inaccessible")
established that pcie_flr() and pci_af_flr() return -ENOTTY when endpoint
configuration space is inaccessible or when pci_dev_wait() fails, so that
subsequent reset methods (such as slot reset or secondary bus reset via the
upstream bridge) can still recover the device.

In pci_af_flr(), if the device is disconnected or its configuration space is
inaccessible, pci_read_config_byte(dev, pos + PCI_AF_CAP, &cap) sets cap to
0xff via PCI_SET_ERROR_RESPONSE(), which passes the PCI_AF_CAP_TP and
PCI_AF_CAP_FLR bit checks, and then pci_write_config_byte() returns
PCIBIOS_DEVICE_NOT_FOUND. Similarly, in pcie_flr(), configuration space can
become inaccessible after the initial pci_dev_config_accessible() check while
polling in pci_wait_for_pending_transaction().

Previously, non-imm_ready devices proceeded to pci_dev_wait() and returned
-ENOTTY, allowing fallback to slot or bus reset. With this change, both
functions return -ENODEV or -EIO and abort the reset fallback loop. Should
these failure paths return -ENOTTY instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179343099854.144556.14547372045411279536.c116@gmail.com?part=1