On 9/13/20 7:22 PM, Bjorn Helgaas wrote: [...] >> The R-Car PCIe controller is capable of handling L0s/L1 link states. >> While the controller can enter and exit L0s link state, and exit L1 >> link state, without any additional action from the driver, to enter >> L1 link state, the driver must complete the link state transition by >> issuing additional commands to the controller. > > So IIUC an R-Car Root Port can't enter L1 without help from the > driver? That *sounds* like a hardware defect, since ASPM is supposed > to be "hardware-autonomous" once configured. The driver needs to complete the switch from L0 to L1 link state, yes. >> The problem is, this transition is not atomic. The controller sets >> PMEL1RX bit in PMSR register upon reception of PM_ENTER_L1 DLLP from >> the PCIe card, but then the controller enters some sort of inbetween >> state. The driver must detect this condition and complete the link >> state transition, by setting L1IATN bit in PMCTLR and waiting for >> the link state transition to complete. >> >> If a PCIe access happens inside of this window, where the controller >> is inbetween L0 and L1 link states, the access generates a fault and >> the ARM 'imprecise external abort' handler is invoked. > > Let me see if I understand this. > > 1) Endpoint sends PM_ENTER_L1 > 2) R-Car Root Port receives PM_ENTER_L1, sets PMEL1RX > 3) R-Car driver does something to complete transition to L1 > > And if the Endpoint driver does an MMIO access between 2) and 3), it > causes an external abort? Yes, because the link is in this inbetween state. So you need 3) to complete that transition. > I couldn't find anything in the R-Car driver today that completes the > transition to L1. That's because this patch adds it. > And the MMIO access will immediately bring the link back to L0, won't > it? Yes > Is there any benefit to L1 at all for this Root Port? If it can't > enter L1 and just stays in the in-between state until some event that > will bring it *out* of L1, maybe we just need a quirk to stop > advertising support for L1 in the first place. Power saving I would say. >> Just like other PCI controller drivers, here we hook the fault handler, >> perform the fixup to help the controller enter L1 link state, and then >> restart the instruction which triggered the fault. Since the controller >> is in L1 link state now, the link can exit from L1 link state to L0 and >> successfully complete the access. >> >> Note that this fixup is applicable only to Aarch32 R-Car controllers, >> the Aarch64 R-Car perform the same fixup in TFA, see TFA commit >> 0969397f2 ("rcar_gen3: plat: Prevent PCIe hang during L1X config access") > > TFA? This doesn't seem to be an upstream Linux commit; can you give a > more specific reference? This is the Trusted Firmware from ARM, in this case it behaves similar to ACPI. See e.g.: https://github.com/ARM-software/arm-trusted-firmware/commit/0969397f295621aa26b3d14b76dd397d22be58bf