On Sat, Sep 24, 2022 at 09:32:08AM +0200, Lukas Wunner wrote: > On Fri, Jul 08, 2022 at 10:35:15AM -0600, Jonathan Derrick wrote: > > On 9/20/2021 11:18 AM, Jon Derrick wrote: > > > On 9/14/21 9:46 AM, Lukas Wunner wrote: > > >> On Mon, Sep 13, 2021 at 04:07:22PM -0500, Jon Derrick wrote: > > >>> On 9/12/21 3:45 AM, Lukas Wunner wrote: > > >>>> On Mon, Aug 30, 2021 at 09:56:28AM -0600, Jon Derrick wrote: > > >>>>> When an Intel P5608 SSD is bifurcated into x4x4 mode, and the upstream > > >>>>> ports both support hotplugging on each respective x4 device, a slot > > >>>>> management system for the SSD requires both x4 slots to have power > > >>>>> removed via sysfs (echo 0 > slot/N/power), from the OS before it can > > >>>>> safely turn-off physical power for the whole x8 device. The implications > > >>>>> are that slot status will display powered off and link inactive statuses > > >>>>> for the x4 devices where the devices are actually powered until both > > >>>>> ports have powered off. > > >>>> > > >>>> Just to get a better understanding, does the P5608 have an internal > > >>>> PCIe switch with hotplug capability on the Downstream Ports or > > >>>> does it plug into two separate PCIe slots? I recall previous patches > > >>>> mentioned a CEM interposer? (An lspci listing might be helpful.) > > >>> > > >>> It looks like 2 NVMe endpoints plugged into two different root ports, ex, > > >>> 80:00.0 Root port to [81-86] > > >>> 80:01.0 Root port to [87-8b] > > >>> 81:00.0 NVMe > > >>> 87:00.0 NVMe > > >>> > > >>> The x8 is bifurcated to x4x4. Physically they share the same slot > > >>> power/clock/reset but are logically separate per root port. > > >> > > >> So are these two P5608 drives attached to a single Root Port with an > > >> interposer in-between? > > >> > > >> I assume the Root Port needs to know that it's bifurcated and has to > > >> appear as two slots on the bus. Is this configured with a BIOS setting? > > >> > > >> If these assumptions are true, the quirk isn't really specific to > > >> the P5608 but should rather apply to the bifurcation-capable Root Port > > >> and the quirk should set the flag if the Root Port is indeed configured > > >> for bifurcation. > > > It's a function of the slot + card combination, but we can't distinguish this > > > slot's special power handling behavior from the vanilla behavior. It's modified > > > to handle power on the logically bifurcated, singular physical device. > > > > Hi Bjorn, Lukas, > > > > I need to resubmit this. > > > > Besides the 'pdev->shared_pcc_and_link_slot = false', addition mentioned > > above, is there anything else that should be changed or any reason this > > wouldn't be accepted? > > Another report has cropped up of spurious DLLSC events: > https://bugzilla.kernel.org/show_bug.cgi?id=216511 > > That other case differs from yours in that a spurious DLLSC event > is seen on plugging *in* a card, whereas in your case the event > seems to occur on *removing* a card. In both cases, the spurious > event is seen on the hotplug port's sibling. > > I'm starting to think that we should probably disable DLLSC events > entirely if they're known to be unreliable. The hotplug port > solely relies on PDC events then. Otherwise we'd have to clutter But when we have ATTN, we don't subscribe to PDC events correct? Can we always rely on PDC changes? Unless you have a PowerControl, PDC can also be spurious correct? and so will be DLLSC.. But I agreem, the hotplug code is getting hairy complex and can help some simplification. A long time ago I remember I worked out a state diagram in google draw for how these states transition to guide the code, but I don't remember where they went. > the event handling with all sorts of special cases. The code would > become fairly difficult to follow. > > I've attached an experimental patch to the bug report which disables > DLLSC events on a hotplug port if a P5608 SSD is plugged into it: > https://bugzilla.kernel.org/attachment.cgi?id=301845 > > Would this approach work for you? > > One other question: What if the SSD is not bifurcated (i.e. x8 > instead of x4x4), don't we need avoid applying the quirk in that case? > Your patch doesn't seem to do that. Can we recognize somehow whether > the card is bifurcated or not? Is it sufficient to just look at the > Maximum Link Width in the Link Capabilities Register? Does the SSD > report x4 there if it's bifurcated? > > Thanks, > > Lukas
