1. Patchew
  2. linux
  3. View series

[PATCH] PCI: Stop waiting for link status after config read failure

Yury Murashka posted 1 patch 3 weeks, 1 day ago
There is a newer version of this series
drivers/pci/pci.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
[PATCH] PCI: Stop waiting for link status after config read failure
Posted by Yury Murashka 3 weeks, 1 day ago 
With a nested PCIe topology with multiple layers of hotplug, a link can go
down near the bottom of the topology shortly before a link above it goes
down. In that case, pcie_wait_for_link_status() can wait for the full
timeout while every read of the link status register fails because the
device has disappeared.

Return immediately when reading the link status fails so event processing
can continue.

Signed-off-by: Yury Murashka <yurypm@arista.com>
Co-authored-by: James Sewart <jamessewart@arista.com>
---
 drivers/pci/pci.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be..a568d5ac1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4565,8 +4565,9 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
  * @use_lt: Use the LT bit if TRUE, or the DLLLA bit if FALSE.
  * @active: Waiting for active or inactive?
  *
- * Return 0 if successful, or -ETIMEDOUT if status has not changed within
- * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return 0 if successful, -ENODEV if the link status cannot be read, or
+ * -ETIMEDOUT if status has not changed within PCIE_LINK_RETRAIN_TIMEOUT_MS
+ * milliseconds.
  */
 static int pcie_wait_for_link_status(struct pci_dev *pdev,
 				     bool use_lt, bool active)
@@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
 
 	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
 	do {
-		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
+		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
+			return -ENODEV;
 		if ((lnksta & lnksta_mask) == lnksta_match)
 			return 0;
 		msleep(1);
@@ -4603,8 +4605,9 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
  * according to @use_lt.  It is not verified whether the use of the DLLLA
  * bit is valid.
  *
- * Return 0 if successful, or -ETIMEDOUT if training has not completed
- * within PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return 0 if successful, -ENODEV if the link status cannot be read, or
+ * -ETIMEDOUT if training has not completed within
+ * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
  */
 int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 {

base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3
-- 
2.51.0
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Lukas Wunner 3 weeks, 1 day ago 
On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
> With a nested PCIe topology with multiple layers of hotplug, a link can go
> down near the bottom of the topology shortly before a link above it goes
> down. In that case, pcie_wait_for_link_status() can wait for the full
> timeout while every read of the link status register fails because the
> device has disappeared.
> 
> Return immediately when reading the link status fails so event processing
> can continue.
[...]
> +++ b/drivers/pci/pci.c
> @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>  
>  	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
>  	do {
> -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> +			return -ENODEV;
>  		if ((lnksta & lnksta_mask) == lnksta_match)
>  			return 0;
>  		msleep(1);

It might be clearer if you check for pci_dev_is_disconnected() directly
instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
generated as a side effect of the device being gone.

Thanks,

Lukas
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Yury M. 3 weeks, 1 day ago 
what to you think about this check:
if ((pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta) ||  
PCI_POSSIBLE_ERROR(lnksta)) && pci_dev_is_disconnected(pdev))
     return -ENODEV;

On 9/4/26 13:20, Lukas Wunner wrote:
> On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
>> With a nested PCIe topology with multiple layers of hotplug, a link can go
>> down near the bottom of the topology shortly before a link above it goes
>> down. In that case, pcie_wait_for_link_status() can wait for the full
>> timeout while every read of the link status register fails because the
>> device has disappeared.
>>
>> Return immediately when reading the link status fails so event processing
>> can continue.
> [...]
>> +++ b/drivers/pci/pci.c
>> @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>>   
>>   	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
>>   	do {
>> -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
>> +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
>> +			return -ENODEV;
>>   		if ((lnksta & lnksta_mask) == lnksta_match)
>>   			return 0;
>>   		msleep(1);
> It might be clearer if you check for pci_dev_is_disconnected() directly
> instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
> generated as a side effect of the device being gone.
>
> Thanks,
>
> Lukas
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Lukas Wunner 3 weeks ago 
On Fri, Sep 04, 2026 at 04:36:12PM +0100, Yury M. wrote:
> On 9/4/26 13:20, Lukas Wunner wrote:
> > On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
> > > With a nested PCIe topology with multiple layers of hotplug, a link
> > > can go down near the bottom of the topology shortly before a link
> > > above it goes down. In that case, pcie_wait_for_link_status() can
> > > wait for the full timeout while every read of the link status register
> > > fails because the device has disappeared.
> > > 
> > > Return immediately when reading the link status fails so event processing
> > > can continue.
> > [...]
> > > +++ b/drivers/pci/pci.c
> > > @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
> > >   	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
> > >   	do {
> > > -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> > > +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> > > +			return -ENODEV;
> > >   		if ((lnksta & lnksta_mask) == lnksta_match)
> > >   			return 0;
> > >   		msleep(1);
> > 
> > It might be clearer if you check for pci_dev_is_disconnected() directly
> > instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
> > generated as a side effect of the device being gone.
> 
> what to you think about this check:
> 
> if ((pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta) ||
>     PCI_POSSIBLE_ERROR(lnksta)) && pci_dev_is_disconnected(pdev))
> 	return -ENODEV;

It's repetitive because pcie_capability_read_word() already checks
internally for pci_dev_is_disconnected():

  pcie_capability_read_word()
    pci_read_config_word()
      pci_dev_is_disconnected()

I just thought that since you specifically want to bail out in the
hot-removal case, it might be clearer to check pci_dev_is_disconnected()
in pcie_wait_for_link_status() before performing the config space read.

But I don't feel strongly either way and checking the return value of
pcie_capability_read_word() is a viable approach as well.

Thanks,

Lukas
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Yury M. 5 days, 12 hours ago 
On 9/5/26 11:06, Lukas Wunner wrote:
> It's repetitive because pcie_capability_read_word() already checks
> internally for pci_dev_is_disconnected():
>
>    pcie_capability_read_word()
>      pci_read_config_word()
>        pci_dev_is_disconnected()
>
> I just thought that since you specifically want to bail out in the
> hot-removal case, it might be clearer to check pci_dev_is_disconnected()
> in pcie_wait_for_link_status() before performing the config space read.
>
> But I don't feel strongly either way and checking the return value of
> pcie_capability_read_word() is a viable approach as well.

I've sent v3 of the patch, returning to validating the 
|pcie_capability_read_word()| return value.

Reasoning:

 1.

    I don't see any benefit to using |pci_dev_is_disconnected()|, other
    than your point that it makes the code clearer or more readable.

 2.

    I checked how we set |pci_dev_is_disconnected()| now, and it looks
    like failed reads don't guarantee that the device will immediately
    be marked as disconnected. I assume that eventually we will receive
    a link-status-changed interrupt and the device will then be marked
    as disconnected.

 3.

    On the other hand, |pcie_capability_read_word()| internally performs
    a |pci_dev_is_disconnected()| check. So in our context, validating
    the |pcie_capability_read_word()| return value is more robust,
    allowing us to react to device disappearance (for any reason) faster
    and more reliably.

  Thanks
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Yury M. 2 weeks, 3 days ago 
I sent v2 
(https://lore.kernel.org/linux-pci/20260907123853.1081635-1-yurypm@arista.com) 
of the patch for review, where I simply check pci_dev_is_disconnected().

Thanks,
Yury

On 9/5/26 11:06, Lukas Wunner wrote:
> It's repetitive because pcie_capability_read_word() already checks
> internally for pci_dev_is_disconnected():
>
>    pcie_capability_read_word()
>      pci_read_config_word()
>        pci_dev_is_disconnected()
>
> I just thought that since you specifically want to bail out in the
> hot-removal case, it might be clearer to check pci_dev_is_disconnected()
> in pcie_wait_for_link_status() before performing the config space read.
>
> But I don't feel strongly either way and checking the return value of
> pcie_capability_read_word() is a viable approach as well.
>
> Thanks,
>
> Lukas
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Ilpo Järvinen 3 weeks, 1 day ago 
On Fri, 4 Sep 2026, Yury M. wrote:

> what to you think about this check:
> if ((pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta) || 
> PCI_POSSIBLE_ERROR(lnksta)) && pci_dev_is_disconnected(pdev))
>     return -ENODEV;

Apparently you're ignoring my feedback. :-(

--
 i.
 
> On 9/4/26 13:20, Lukas Wunner wrote:
> > On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
> > > With a nested PCIe topology with multiple layers of hotplug, a link can go
> > > down near the bottom of the topology shortly before a link above it goes
> > > down. In that case, pcie_wait_for_link_status() can wait for the full
> > > timeout while every read of the link status register fails because the
> > > device has disappeared.
> > > 
> > > Return immediately when reading the link status fails so event processing
> > > can continue.
> > [...]
> > > +++ b/drivers/pci/pci.c
> > > @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev
> > > *pdev,
> > >     	end_jiffies = jiffies +
> > > msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
> > >   	do {
> > > -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> > > +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> > > +			return -ENODEV;
> > >   		if ((lnksta & lnksta_mask) == lnksta_match)
> > >   			return 0;
> > >   		msleep(1);
> > It might be clearer if you check for pci_dev_is_disconnected() directly
> > instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
> > generated as a side effect of the device being gone.
> > 
> > Thanks,
> > 
> > Lukas
> 
>
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Ilpo Järvinen 3 weeks, 1 day ago 
On Fri, 4 Sep 2026, Yury Murashka wrote:

> With a nested PCIe topology with multiple layers of hotplug, a link can go
> down near the bottom of the topology shortly before a link above it goes
> down. In that case, pcie_wait_for_link_status() can wait for the full
> timeout while every read of the link status register fails because the
> device has disappeared.
> 
> Return immediately when reading the link status fails so event processing
> can continue.
> 
> Signed-off-by: Yury Murashka <yurypm@arista.com>
> Co-authored-by: James Sewart <jamessewart@arista.com>
> ---
>  drivers/pci/pci.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be..a568d5ac1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4565,8 +4565,9 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
>   * @use_lt: Use the LT bit if TRUE, or the DLLLA bit if FALSE.
>   * @active: Waiting for active or inactive?
>   *
> - * Return 0 if successful, or -ETIMEDOUT if status has not changed within
> - * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
> + * Return 0 if successful, -ENODEV if the link status cannot be read, or
> + * -ETIMEDOUT if status has not changed within PCIE_LINK_RETRAIN_TIMEOUT_MS
> + * milliseconds.
>   */
>  static int pcie_wait_for_link_status(struct pci_dev *pdev,
>  				     bool use_lt, bool active)
> @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>  
>  	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
>  	do {
> -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> +			return -ENODEV;

Wouldn't it be better to base such checks on PCI_POSSIBLE_ERROR()?

If you keep the check for the case where pcie_capability_read_word() 
returns error, its return value should be converted with 
pcibios_err_to_errno(), not just return -ENODEV.

>  		if ((lnksta & lnksta_mask) == lnksta_match)
>  			return 0;
>  		msleep(1);
> @@ -4603,8 +4605,9 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>   * according to @use_lt.  It is not verified whether the use of the DLLLA
>   * bit is valid.
>   *
> - * Return 0 if successful, or -ETIMEDOUT if training has not completed
> - * within PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
> + * Return 0 if successful, -ENODEV if the link status cannot be read, or

Kerneldoc wants this formatting:

Return:

> + * -ETIMEDOUT if training has not completed within
> + * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
>   */
>  int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
>  {
> 
> base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3
> 

-- 
 i.
Re: [PATCH] PCI: Stop waiting for link status after config read failure
Posted by Yury M. 5 days, 12 hours ago 
On 9/4/26 12:59, Ilpo Järvinen wrote:
> Wouldn't it be better to base such checks on PCI_POSSIBLE_ERROR()?
>
> If you keep the check for the case where pcie_capability_read_word()
> returns error, its return value should be converted with
> pcibios_err_to_errno(), not just return -ENODEV.
1. There is no reason to use PCI_POSSIBLE_ERROR in this case, because 
pcie_capability_read_word() overrides the value returned by 
pci_read_config_word() and sets it to 0 in case of a read failure. So, 
if a read fails, pcie_capability_read_word() returns 0 instead of all Fs.
2. Thanks. I addressed your comment in the v3 patch and used 
pcibios_err_to_errno().

On 9/4/26 12:59, Ilpo Järvinen wrote:
> Kerneldoc wants this formatting:
>
> Return:
3. Addressed this comment as well in the v3 patch.

Thanks
[PATCH v3] PCI: Stop waiting for link status after config read failure
Posted by Yury Murashka 5 days, 12 hours ago 
With a nested PCIe topology with multiple layers of hotplug, a link can go
down near the bottom of the topology shortly before a link above it goes
down. In that case, pcie_wait_for_link_status() can wait for the full
timeout while every read of the link status register fails because the
device has disappeared.

Return immediately when reading link status fails so event processing can
continue.

Signed-off-by: Yury Murashka <yurypm@arista.com>
Co-authored-by: James Sewart <jamessewart@arista.com>
---
 drivers/pci/pci.c | 15 +++++++++++----
 1 file changed, 10 insertions(+), 5 deletions(-)

Changes in v3:
- Check the return value of pcie_capability_read_word() and stop retrying
  when the link status read fails.
- Do not use pci_dev_is_disconnected() because a read failure does not mean
  that the device will be marked as disconnected immediately.
- Update the kernel-doc return descriptions for errors from
  pcibios_err_to_errno() while retaining the -ETIMEDOUT description.

Changes in v2:
- Check pci_dev_is_disconnected() before reading link status instead of
  checking the return value of pcie_capability_read_word().

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f8080e949f7abfc4bab39d791735b8..430ff08f4f4b30f12b1190ea9c5408a3a8505a5d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4565,8 +4565,9 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
  * @use_lt: Use the LT bit if TRUE, or the DLLLA bit if FALSE.
  * @active: Waiting for active or inactive?
  *
- * Return 0 if successful, or -ETIMEDOUT if status has not changed within
- * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return: 0 if successful, -ETIMEDOUT if status has not changed within
+ * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds, or negative error code if
+ * reading of link status failed.
  */
 static int pcie_wait_for_link_status(struct pci_dev *pdev,
 				     bool use_lt, bool active)
@@ -4574,13 +4575,16 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
 	u16 lnksta_mask, lnksta_match;
 	unsigned long end_jiffies;
 	u16 lnksta;
+	int ret;
 
 	lnksta_mask = use_lt ? PCI_EXP_LNKSTA_LT : PCI_EXP_LNKSTA_DLLLA;
 	lnksta_match = active ? lnksta_mask : 0;
 
 	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
 	do {
-		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
+		ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
+		if (ret)
+			return pcibios_err_to_errno(ret);
 		if ((lnksta & lnksta_mask) == lnksta_match)
 			return 0;
 		msleep(1);
@@ -4603,8 +4607,9 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
  * according to @use_lt.  It is not verified whether the use of the DLLLA
  * bit is valid.
  *
- * Return 0 if successful, or -ETIMEDOUT if training has not completed
- * within PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return: 0 if successful, -ETIMEDOUT if training has not completed within
+ * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds, or negative error code if
+ * reading of link status failed.
  */
 int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 {

base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3
-- 
2.51.0
[PATCH v2] PCI: Stop waiting for link status after config read failure
Posted by Yury Murashka 2 weeks, 5 days ago 
With a nested PCIe topology with multiple layers of hotplug, a link can go
down near the bottom of the topology shortly before a link above it goes
down. In that case, pcie_wait_for_link_status() can wait for the full
timeout while every read of the link status register fails because the
device has disappeared.

Return immediately when the device is disconnected so event processing can
continue.

Signed-off-by: Yury Murashka <yurypm@arista.com>
Co-authored-by: James Sewart <jamessewart@arista.com>
---
 drivers/pci/pci.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Changes in v2:
- Check pci_dev_is_disconnected() before reading link status instead of
  checking the return value of pcie_capability_read_word().

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be..a568d5ac1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4565,8 +4565,9 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
  * @use_lt: Use the LT bit if TRUE, or the DLLLA bit if FALSE.
  * @active: Waiting for active or inactive?
  *
- * Return 0 if successful, or -ETIMEDOUT if status has not changed within
- * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return 0 if successful, -ENODEV if the device is disconnected, or
+ * -ETIMEDOUT if status has not changed within PCIE_LINK_RETRAIN_TIMEOUT_MS
+ * milliseconds.
  */
 static int pcie_wait_for_link_status(struct pci_dev *pdev,
 				     bool use_lt, bool active)
@@ -4580,6 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
 
 	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
 	do {
+		if (pci_dev_is_disconnected(pdev))
+			return -ENODEV;
 		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
 		if ((lnksta & lnksta_mask) == lnksta_match)
 			return 0;
@@ -4603,8 +4605,9 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
  * according to @use_lt.  It is not verified whether the use of the DLLLA
  * bit is valid.
  *
- * Return 0 if successful, or -ETIMEDOUT if training has not completed
- * within PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return 0 if successful, -ENODEV if the device is disconnected, or
+ * -ETIMEDOUT if training has not completed within PCIE_LINK_RETRAIN_TIMEOUT_MS
+ * milliseconds.
  */
 int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 {

base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3
-- 
2.51.0

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.