Hello Bjorn, Linas,
many thanks for clarifying this tricky topic! Hope this information can
serve a good starting point for Jinjian to develop a proper solution for
that case.
On 03.11.2024 03:24, Linas Vepstas wrote:
Top-post reply.
What Bjorn says is exactly right. Just one clarifying remark: When PCI
error recovery was designed, it was envisioned for high-end,
high-availability servers so that they could reset a failing device
without forcing a reboot of the OS. Concepts like suspend/resume did
not perturb even the unconscious sleep of the engineers. Thus,
stepping through error recovery during suspend would be novel,
untested, and perhaps prone to confusion. The error recovery procedure
is trying to reset the device into a fully-powered-on,
fully-functional and connected state. This might be problematic if the
suspend has already walked half-way through power-down. The fact that
error recovery might run a very long fraction of a second after the
error is detected further complicates things. The fact that error
recovery usually has the driver fully reinitializing the device,
including reloading the firmware, could be a problem if the firmware
is not in RAM or is sitting on a suspended block device. So it all
could be an adventure.
As to testing: I asked one of the guys in the lab how he did it, and
he said he would brush a metal wire against the PCI pins. I winced.
But I guess it's cleaner than pouring coffee on it. Later we developed
a software tool to artificially inject errors.
On Fri, Nov 1, 2024 at 8:52 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
On Thu, Oct 31, 2024 at 09:09:30PM +0800, Jinjian Song wrote:
From: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>
On 29.10.2024 05:46, Jinjian Song wrote:
From: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>
On 22.10.2024 11:43, Jinjian Song wrote:
If driver fails to set the device to suspend, it means that the
device is abnormal. In this case, reset the device to recover
when PCIe device is offline.
Is it a reproducible or a speculative issue? Does the fix
recover modem from a problematic state?
Anyway we need someone more familiar with this hardware (Intel
or MediaTek engineer) to Ack the change to make sure we are not
going to put a system in a more complicated state.
This is a very difficult issue to replicate onece occured and fixed.
The issue occured when driver and device lost the connection. I have
encountered this problem twice so far:
1. During suspend/resume stress test, there was a probabilistic D3L2
time sequence issue with the BIOS, result in PCIe link down, driver
read and write the register of device invalid, so suspend failed.
This issue was eventually fixed in the BIOS and I was able to restore
it through the reset module after reproducing the problem.
2. During idle test, the modem probabilistic hang up, result in PCIe
link down, driver read and write the register of device invalid, so
suspend failed. This issue was eventually fiex in device modem firmware
by adjust a certain power supply voltage, and reset modem as a workround
to restore when the MBIM port command timeout in userspace applycations.
Hardware reset modem to recover was discussed with MTK, and they said
that if we don't want to keep the on-site problem location in case of
suspend failure, we can use the recover solution.
Both the ocurred issues result in the PCIe link issue, driver can't
read and writer the register of WWAN device, so I want to add this
path
to restore, hardware reset modem can recover modem, but using the
pci_channle_offline() as the judgment is my inference.
Thank you for the clarification. Let me summarize what I've understood
from the explanation:
a) there were hardware (firmware) issues,
b) issues already were solved,
c) issues were not directly related to the device suspension procedure,
d) you want to implement a backup plan to make the modem support robust.
If got it right, then I would like to recommend to implement a generic
error handling solution for the PCIe interface. You can check this
document: Documentation/PCI/pci-error-recovery.rst
Yes, got it right.
I want to identify the scenario and then recover by reset device,
otherwise suspend failure will aways prevent the system from suspending
if it occurs.
If a PCIe link goes down here's my understanding of what happens:
- Writes to the device are silently dropped.
- Reads from the device return ~0 (PCI_POSSIBLE_ERROR()).
- If the device is in a slot and pciehp is enabled, a Data Link
Layer State Changed interrupt will cause pciehp_unconfigure_device()
to detach the driver and remove the pci_dev.
- If AER is enabled, a failed access to the device will cause an AER
interrupt. If the driver has registered pci_error_handlers, the
driver callbacks will be called, and based on what the driver
returns, the PCI core may reset the device.
The pciehp and AER interrupts are *asynchronous* to link down events
and to any driver access to the device, so they may be delayed an
arbitrary amount of time.
Both interrupt paths may lead to the device being marked as "offline".
Obviously this is asynchronous with respect to the driver.
+++ b/drivers/net/wwan/t7xx/t7xx_pci.c
@@ -427,6 +427,10 @@ static int __t7xx_pci_pm_suspend(struct
pci_dev *pdev)
iowrite32(T7XX_L1_BIT(0), IREG_BASE(t7xx_dev) +
ENABLE_ASPM_LOWPWR);
atomic_set(&t7xx_dev->md_pm_state, MTK_PM_RESUMED);
t7xx_pcie_mac_set_int(t7xx_dev, SAP_RGU_INT);
+ if (pci_channel_offline(pdev)) {
+ dev_err(&pdev->dev, "Device offline, reset to recover\n");
+ t7xx_reset_device(t7xx_dev, PLDR);
+ }
This looks like an unreliable way to detect issues. It only works if
AER is enabled, and the device is only marked "offline" some arbitrary
length of time *after* a driver access to the device has failed.
You can't reliably detect errors on writes to the device.
You can only reliably detect errors on reads from the device by
looking for PCI_POSSIBLE_ERROR(). Obviously ~0 might be a valid value
to read from some registers, so you need device-specific knowledge to
know whether ~0 is valid or indicates an error.
If AER or DPC are enabled, the driver can be *notified* about read
errors and some write errors via pci_error_handlers, but the
notification is long after the error.
--
Sergey
