Re: [RFC PATCH net-next 05/12] PCI: Add device-specific reset for NVIDIA Spectrum devices

[Lukas Wunner <lukas@wunner.de>]

On Tue, Oct 17, 2023 at 10:42:50AM +0300, Ido Schimmel wrote:
> The PCIe specification defines two methods to trigger a hot reset across
> a link: Bus reset and link disablement (r6.0.1, sec 7.1, sec 6.6.1). In
> the first method, the Secondary Bus Reset (SBR) bit in the Bridge
> Control Register of the Downstream Port is asserted for at least 1ms
> (r6.0.1, sec 7.5.1.3.13). In the second method, the Link Disable bit in
> the Link Control Register of the Downstream Port is asserted and then
> cleared to disable and enable the link (r6.0.1, sec 7.5.3.7).
> 
> While the two methods are identical from the perspective of the
> Downstream device, they are different as far as the host is concerned.
> In the first method, the Link Training and Status State Machine (LTSSM)
> of the Downstream Port is expected to be in the Hot Reset state as long
> as the SBR bit is asserted. In the second method, the LTSSM of the
> Downstream Port is expected to be in the Disabled state as long as the
> Link Disable bit is asserted.
> 
> This above difference is of importance because the specification
> requires the LTTSM to exit from the Hot Reset state to the Detect state
> within a 2ms timeout (r6.0.1, sec 4.2.7.11). NVIDIA Spectrum devices
> cannot guarantee it and a host enforcing such a behavior might fail to
> communicate with the device after issuing a Secondary Bus Reset.

How does that failure manifest itself exactly?  Is the problem that
the Vendor ID register in config space is read too early and the
device doesn't like that?

It is possible to increase the d3cold_delay in struct pci_dev to
lengthen the delay until the Vendor ID is read.  Have you considered
that instead of using the Link Disable method?

The following commit queued for v6.7 introduces a quirk for a 1 second
d3cold_delay, perhaps you can take advantage of it?

https://git.kernel.org/pci/pci/c/c9260693aa0c

Thanks,

Lukas