archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <>
To: Yicong Yang <>
Cc: Bjorn Helgaas <>,
	Sathyanarayanan Kuppuswamy 
	Dan Williams <>,
	Ethan Zhao <>,
	Sinan Kaya <>, Ashok Raj <>,
	Keith Busch <>,, Russell Currey <>,
	Oliver O'Halloran <>,
	Stuart Hayes <>,
	Mika Westerberg <>,
	Linuxarm <>
Subject: Re: [PATCH] PCI: pciehp: Ignore Link Down/Up caused by DPC
Date: Fri, 30 Apr 2021 14:15:49 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Fri, Apr 30, 2021 at 04:47:54PM +0800, Yicong Yang wrote:
> On 2021/4/30 3:42, Lukas Wunner wrote:
> > The 3000 msec were chosen arbitrarily.  I couldn't imagine that
> > it would ever take longer than that.  The spec does not seem to
> > mandate a time limit for DPC recovery.  But we do need a timeout
> > because the DPC Trigger Status bit may never clear and then pciehp
> > would wait indefinitely.  This can happen if dpc_wait_rp_inactive()
> > fails or perhaps because the hardware is buggy.
> The DPC recovery process will not be blocked indefinitely. What about
> wait until the DPC process is finished or until the dpc_reset_link()
> is finished? We can try to up the link if the DPC recovery failed.

As I've indicated above, there's a condition when DPC never completes:

According to the PCIe Base Spec, sec. 6.2.10,

   "After DPC has been triggered in a Root Port that supports RP Extensions
    for DPC, the Root Port may require some time to quiesce and clean up
    its internal activities, such as those associated with DMA read Requests.
    When the DPC Trigger Status bit is Set and the DPC RP Busy bit is Set,
    software must leave the Root Port in DPC until the DPC RP Busy bit
    reads 0b."

The spec does not mandate a time limit until DPC RP Busy clears:

   "The DPC RP Busy bit is a means for hardware to indicate to software
    that the RP needs to remain in DPC containment while the RP does
    some internal cleanup and quiescing activities. While the details
    of these activities are implementation specific, the activities will
    typically complete within a few microseconds or less. However, under
    worst-case conditions such as those that might occur with certain
    internal errors in large systems, the busy period might extend
    substantially, possibly into multiple seconds."

Thus, dpc_wait_rp_inactive() polls every 10 msec (for up to HZ, i.e. 1 sec)
whether PCI_EXP_DPC_RP_BUSY has been cleared.  If it does not clear
within 1 sec, the function returns -EBUSY to its caller dpc_reset_link().
Note that according to the spec, we're not allowed to clear the
Trigger Status bit as long as the DPC RP Busy bit is set, hence
dpc_reset_link() errors out without clearing PCI_EXP_DPC_STATUS_TRIGGER.

Because pciehp waits for that bit to clear, it may end up waiting
indefinitely for DPC to complete.  That's not acceptable, so we do
need a timeout to allow pciehp to progress.

> I noticed the hotplug interrupt arrives prior to the DPC and then the
> wait begins. DPC will clear the Trigger Status in its irq thread.
> So not all the time is elapsed by the hardware recovery, but also by
> the software process. Considering it's in the irq thread, if we are
> preempted and clear the status slower, then we may be timed out.

That is correct.  If the system is super busy then there's a chance
that pciehp may time out because the DPC IRQ thread wasn't scheduled.
So the timeout needs to be long enough to accommodate for that.



  reply	other threads:[~2021-04-30 12:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-28  8:52 Lukas Wunner
2021-03-30 20:53 ` Kuppuswamy, Sathyanarayanan
2021-04-28  0:39   ` Kuppuswamy, Sathyanarayanan
2021-04-28  1:42     ` Zhao, Haifeng
2021-04-28 10:08 ` Yicong Yang
2021-04-28 14:40   ` Lukas Wunner
2021-04-29 11:29     ` Yicong Yang
2021-04-29 12:40       ` Zhao, Haifeng
2021-04-29 19:42       ` Lukas Wunner
2021-04-30  8:47         ` Yicong Yang
2021-04-30 12:15           ` Lukas Wunner [this message]
2021-04-29 19:36 ` Keith Busch
2021-04-29 20:16   ` Lukas Wunner
2021-04-29 21:16     ` Keith Busch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] PCI: pciehp: Ignore Link Down/Up caused by DPC' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).