Re: [PATCH v3 1/2] PCI: Add D3cold as general reset method

[Jose Ignacio Tornos Martinez <jtornosm@redhat.com>]

Hello Alex,

Thank you again for your feedbak.

> This fall through is invalid, if D3cold can't be reached the reset should
> be handled by pci_pm_reset() where NoSoftRst is honored.  pci_pr3_preset()
> should be tested in all cases, not just probe.  I think we also need to
> test device flags that would prevent a D3cold transition,
> like pci_pm_reset().  Thanks,
Ok, I've implemented it as a strict method (only when _PR3 is present) as
you suggested for the next version. I think it can be very helpful.

However, I'm facing a challenge with a part of our deployment scenario and
if possible I would appreciate your guidance (spoiler: d3hot possibility
would be very helpful).

Let me comment on about this setup in our lab:
- Qualcomm WiFi cards (ath11k 17cb:1103, ath12k 17cb:1107) and modems
  (SDX62/SDX65 17cb:0308)
- M.2 format devices using passive PCIe adapters (not native M.2 slots)
Problem:
- No FLR capability on devices
- NoSoftRst+ set (blocks PM reset)
- SBR broken through passive adapters (PERST# signal not properly wired)
- No _PR3 ACPI resources for standard PCIe slots (only for native M.2/
  Thunderbolt)
- d3cold_allowed=1 but pci_pr3_present() returns false
Result:
Default kernel has no working reset method (reset_methods shows only "bus"
which doesn't work).

I've tested the D3hot transition behavior and this is what I see before/
after:
- Command register: Cleared to 0x0000 (device disabled)
- BARs: Preserved (0x84200004 unchanged)
- Device successfully reinitializes and works in VFIO passthrough, so we
  can use it.

I don't dare to generalize D3hot in pci_pm_reset() with something like:
if (csr & PCI_PM_CTRL_NO_SOFT_RESET) {
  pci_read_config_word(dev, dev->pm_cap + PCI_PM_PMC, &pmc);

  /* If device supports D3hot, try reset despite NoSoftRst */
  if (!pmc & PCI_PM_CAP_D3HOT_MASK)
    return -ENOTTY;
}
because some devices could truly advertise NoSoftRst+ and d3hot could not be
usable as a reset.

I don't know if some device-specific quirk could be acceptable to bypass
NoSoftRst check for these Qualcomm devices (similar to
PCI_DEV_FLAGS_FORCE_PM_RESET from v2), although d3hot is usable in these
cases.

Or maybe the generalization could be to create a user parameter to bypass
PCI_PM_CTRL_NO_SOFT_RESET for the configured device. This would be manual but
at least it would allow the device to work on VMs.

And finally, what I would choose: move pm to the end of the resets methods
ignoring NoSoftRst, prioritizing hardware resets first (FLR, bus, d3cold)
and using d3hot as the last resource if none of the others work. But this is
an important change and there could be some reasons for pm positioning that
I ignore, so if you don't mind I would like to consult you first.

What do you think? Could you help me?
I would like to address these devices on VMs in some way. 

Thanks

Best regards
José Ignacio