From: Ethan Zhao <xerces.zhao@gmail.com>
To: "Kuppuswamy, Sathyanarayanan" <sathyanarayanan.kuppuswamy@intel.com>
Cc: Ethan Zhao <haifeng.zhao@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>, Oliver <oohall@gmail.com>,
ruscur@russell.cc, Lukas Wunner <lukas@wunner.de>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Stuart Hayes <stuart.w.hayes@gmail.com>,
Alexandru Gagniuc <mr.nuke.me@gmail.com>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
linux-pci <linux-pci@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"Raj, Ashok" <ashok.raj@linux.intel.com>
Subject: Re: [PATCH v8 2/6] PCI/DPC: define a function to check and wait till port finish DPC handling
Date: Thu, 8 Oct 2020 13:49:05 +0800 [thread overview]
Message-ID: <CAKF3qh3KqgJmbS6v=O_Z6+1_k7vAN2kv=_YGUcsJNjqKKi3NSg@mail.gmail.com> (raw)
In-Reply-To: <4bedeb35-942e-5ad3-9721-62495af1f09a@intel.com>
On Thu, Oct 8, 2020 at 2:16 AM Kuppuswamy, Sathyanarayanan
<sathyanarayanan.kuppuswamy@intel.com> wrote:
>
>
> On 10/7/20 4:31 AM, Ethan Zhao wrote:
> > Once root port DPC capability is enabled and triggered, at the beginning
> > of DPC is triggered, the DPC status bits are set by hardware and then
> > sends DPC/DLLSC/PDC interrupts to OS DPC and pciehp drivers, it will
> > take the port and software DPC interrupt handler 10ms to 50ms (test data
> > on ICS(Ice Lake SP platform, see
> > https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server)
> > & stable 5.9-rc6) to complete the DPC containment procedure
> This data is based on one particular architecture. So using this
> to create a timed loop in pci_wait_port_outdpc() looks incorrect.
>
> I still recommend looking for some locking model to fix this
> issue (may be atomic state flag or lock).
It is actually a device semaphore. DLLSC/PDC handler needs to wait
for the critical
area is clear to enter by monitoring the DPC triggered status is
cleaned or not.
Another problem is, DPC reset/interrupt is initiated by hardware,
you couldn't place
a software lock between interrupt handler and device resetting.
While device semaphore--- DPC triggered status is the right one to wait for.
Better idea ?
Thanks,
Ethan
> > till the DPC status is cleared at the end of the DPC interrupt handler.
> >
> > We use this function to check if the root port is in DPC handling status
> > and wait till the hardware and software completed the procedure.
> >
> > Signed-off-by: Ethan Zhao <haifeng.zhao@intel.com>
> > Tested-by: Wen Jin <wen.jin@intel.com>
> > Tested-by: Shanshan Zhang <ShanshanX.Zhang@intel.com>
> > ---
> > changes:
> > v2:align ICS code name to public doc.
> > v3: no change.
> > v4: response to Christoph's (Christoph Hellwig <hch@infradead.org>)
> > tip, move pci_wait_port_outdpc() to DPC driver and its declaration
> > to pci.h.
> > v5: fix building issue reported by lkp@intel.com with some config.
> > v6: move from [1/5] to [2/5].
> > v7: no change.
> > v8: no change.
> >
> > drivers/pci/pci.h | 2 ++
> > drivers/pci/pcie/dpc.c | 27 +++++++++++++++++++++++++++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index fa12f7cbc1a0..455b32187abd 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -455,10 +455,12 @@ void pci_restore_dpc_state(struct pci_dev *dev);
> > void pci_dpc_init(struct pci_dev *pdev);
> > void dpc_process_error(struct pci_dev *pdev);
> > pci_ers_result_t dpc_reset_link(struct pci_dev *pdev);
> > +bool pci_wait_port_outdpc(struct pci_dev *pdev);
> > #else
> > static inline void pci_save_dpc_state(struct pci_dev *dev) {}
> > static inline void pci_restore_dpc_state(struct pci_dev *dev) {}
> > static inline void pci_dpc_init(struct pci_dev *pdev) {}
> > +static inline bool pci_wait_port_outdpc(struct pci_dev *pdev) { return false; }
> > #endif
> >
> > #ifdef CONFIG_PCI_ATS
> > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> > index daa9a4153776..2e0e091ce923 100644
> > --- a/drivers/pci/pcie/dpc.c
> > +++ b/drivers/pci/pcie/dpc.c
> > @@ -71,6 +71,33 @@ void pci_restore_dpc_state(struct pci_dev *dev)
> > pci_write_config_word(dev, dev->dpc_cap + PCI_EXP_DPC_CTL, *cap);
> > }
> >
> > +bool pci_wait_port_outdpc(struct pci_dev *pdev)
> > +{
> > + u16 cap = pdev->dpc_cap, status;
> > + u16 loop = 0;
> > +
> > + if (!cap) {
> > + pci_WARN_ONCE(pdev, !cap, "No DPC capability initiated\n");
> > + return false;
> > + }
> > + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
> > + pci_dbg(pdev, "DPC status %x, cap %x\n", status, cap);
> > +
> > + while (status & PCI_EXP_DPC_STATUS_TRIGGER && loop < 100) {
> > + msleep(10);
> > + loop++;
> > + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
> > + }
> > +
> > + if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) {
> > + pci_dbg(pdev, "Out of DPC %x, cost %d ms\n", status, loop*10);
> > + return true;
> > + }
> > +
> > + pci_dbg(pdev, "Timeout to wait port out of DPC status\n");
> > + return false;
> > +}
> > +
> > static int dpc_wait_rp_inactive(struct pci_dev *pdev)
> > {
> > unsigned long timeout = jiffies + HZ;
>
> --
> Sathyanarayanan Kuppuswamy
> Linux Kernel Developer
>
next prev parent reply other threads:[~2020-10-08 5:49 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-07 11:31 [PATCH v8 0/6] Fix DPC hotplug race and enhance error handling Ethan Zhao
2020-10-07 11:31 ` [PATCH v8 1/6] PCI/ERR: get device before call device driver to avoid NULL pointer dereference Ethan Zhao
2020-10-07 17:24 ` Kuppuswamy, Sathyanarayanan
2020-10-08 5:38 ` Ethan Zhao
2020-10-07 11:31 ` [PATCH v8 2/6] PCI/DPC: define a function to check and wait till port finish DPC handling Ethan Zhao
2020-10-07 17:28 ` Kuppuswamy, Sathyanarayanan
2020-10-08 5:49 ` Ethan Zhao [this message]
2020-10-09 3:16 ` Ethan Zhao
2020-10-07 11:31 ` [PATCH v8 3/6] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC Ethan Zhao
2020-10-07 11:31 ` [PATCH v8 4/6] PCI/ERR: simplify function pci_dev_set_io_state() with if Ethan Zhao
2020-10-07 11:31 ` [PATCH v8 5/6] PCI/ERR: only return true when dev io state is really changed Ethan Zhao
2020-10-07 11:31 ` [PATCH v8 6/6] PCI/ERR: don't mix io state not changed and no driver together Ethan Zhao
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKF3qh3KqgJmbS6v=O_Z6+1_k7vAN2kv=_YGUcsJNjqKKi3NSg@mail.gmail.com' \
--to=xerces.zhao@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=ashok.raj@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=haifeng.zhao@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=mika.westerberg@linux.intel.com \
--cc=mr.nuke.me@gmail.com \
--cc=oohall@gmail.com \
--cc=ruscur@russell.cc \
--cc=sathyanarayanan.kuppuswamy@intel.com \
--cc=stuart.w.hayes@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).