On Thu, Oct 8, 2020 at 2:16 AM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxx> 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@xxxxxxxxx> > > Tested-by: Wen Jin <wen.jin@xxxxxxxxx> > > Tested-by: Shanshan Zhang <ShanshanX.Zhang@xxxxxxxxx> > > --- > > changes: > > v2：align ICS code name to public doc. > > v3: no change. > > v4: response to Christoph's (Christoph Hellwig <hch@xxxxxxxxxxxxx>) > > tip, move pci_wait_port_outdpc() to DPC driver and its declaration > > to pci.h. > > v5: fix building issue reported by lkp@xxxxxxxxx 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 >
