On Mon, Feb 11, 2019 at 11:48:12PM +0000, Alex_Gagniuc@xxxxxxxxxxxx wrote: > On 2/9/19 5:58 AM, Lukas Wunner wrote: > ECN [1] was approved last November, so it's normative spec text. Sorry > if the Ukranians didn't get ahold of it yet. I'll try to explain the delta. > There's an in-band PD supported/disable set of bits. If 'supported' is > set, then you can set 'disable', which removes the 'logical OR" and now > PD is only out-of-band presence. I see, thanks a lot for relaying this information. The PCI SIG should probably consider granting access to specs to open source developers to ease adoption of new features in Linux et al. > > Table 4-14 in sec 4.2.6 is also important because it shows the difference > > between Link Up and in-band presence. > > So, we'd expect PD to come up at the same time or before DLL? Per table 4-14 and figure 4-23 (immediately below the table) in r4.0, PDC ought to be signaled before DLLSC. As said, Linux can handle PDC after DLLSC if they're not too far apart (100 ms, see pcie_wait_for_link()). > I'd like a generic solution. I suspect supporting 'in-band PD disabled' > and quirking for that would be a saner way to do things. If we support > it, then we need to handle PDSC after DLLSC scenarios anyway. I agree, having support for this new ECN would be great. If you look at struct controller in drivers/pci/hotplug/pciehp.h, there's a section labelled /* capabilities and quirks */. An idea would be to add an unsigned int there to indicate whether in-band presence is disabled. An alternative would be to add a flag to struct pci_dev. Instead of modifying the logic in pciehp_handle_presence_or_link_change(), you could amend pcie_wait_for_link() to poll PDS until it's set, in addition to DLLLA. The rationale would be that although the link is up, the hot-added device cannot really be considered accessible until PDS is also set. Unfortunately we cannot do this for all devices because (as I've said before), some broken devices hardwire PDS to zero. But it should be safe to constrain it to those which can disable in-band presence. Be mindful however that pcie_wait_for_link() is also called from the DPC driver. Keith should be able to judge whether a change to that function breaks DPC. You'll probably also need to add code to disable in-band presence if that is supported, either in pcie_init() in pciehp_hpc.c or in generic PCI code. Thanks, Lukas
