On Wed, 7 Aug 2024 22:29:35 +1000 Oliver O'Halloran Wrote > My read was that Matt is essentially doing a surprise hot-unplug by > removing power to the card without notifying the OS. I thought the > LBMS bit wouldn't be set in that case since the link goes down rather > than changes speed, but the spec is a little vague and that appears to > be happening in Matt's testing. It might be worth disabling the > workaround if the port has the surprise hotplug capability bit set. Most of the systems I have are using downstream port containment which does not recommend setting the Hot-Plug Surprise in Slot Capabilities & therefore we do not. The first time we noticed an issue with this patch was in test automation which was power cycling the endpoints & injecting uncorrectable errors to ensure our hosts are robust in the face of PCIe chaos & that they will recover. Later we started to see other teams on other products encountering the same bug in simpler cases where humans turn on and off EP power for development purposes. Although we are not using Hot-Plug Surprise we are often triggering DPC on the Surprise Down Uncorrectable Error when we hit this Gen1 issue. On Wed, 7 Aug 2024 12:14:13 +0100 Maciej W. Rozycki Wrote > For the quirk to trigger, the link has to be down and there has to be the > LBMS Link Status bit set from link management events as per the PCIe spec > while the link was previously up, and then both of that while rescanning > the PCIe device in question, so there's a lot of conditions to meet. If there is nothing clearing the bit then why is there any expectation that it wouldn't be set? We have 20/30/40 endpoints in a host that can be hot-removed, hot-added at any point in time in any combination & its often the case that the system uptime be hundreds of days. Eventually the bit will just become set as a result of time and scale. On Wed, 7 Aug 2024 12:14:13 +0100 Maciej W. Rozycki Wrote > The reason for this is safety: it's better to have a link run at 2.5GT/s > than not at all, and from the nature of the issue there is no guarantee > that if you remove the speed clamp, then the link won't go back into the > infinite retraining loop that the workaround is supposed to break. I guess I don't really understand why it wouldn't be safe for every device other than the ASMedia since they aren't the device that had the issue in the first place. The main problem in my mind is the system doesn't actually have to be retraining at all, it can occur the first time you replace a device or power cycle the device or the first time the device goes into DPC & comes back. If the quirk simply returned without doing anything when the ASMedia is not in the allow-list it would make me more at ease. I can also imagine some other implementations that would work well, but it doesn't seem correct that we could only give a single opportunity to a device before forcing it to live at Gen1. Perhaps it should be aware of the rate or a count or something... I can only assume that there will be more systems that start to run into issues with the patch as they strive to keep up with LTS & they exercise the hot-plug path.
