Re: PCI: hotplug: Erroneous removal of hotplug PCI devices

[Keith Busch <keith.busch@intel.com>]

On Wed, Jan 23, 2019 at 06:20:57PM +0000, Alex_Gagniuc@Dellteam.com wrote:
> Hi all,
> 
> This may be a mind-twisting explanation, so pleas bear with me.
> 
> In PCIe, the presence detect bit (PD) in the slot status register should 
> be a logical OR of in-band and out-of band presence. In-band presence is 
> the data link layer status. So one would expect that a link up event, 
> would be accompanied by a PD changed event with PD set. Not everyone 
> follows that.
>
> I have a system here with the following order of events:
>   *   0 ms : Link up
>   * 400 ms : Presence detect up
> On the first event, the device is probed as expected, and on the second 
> event, the device is removed as a SURPRISE!!!_REMOVAL. This is a bug.
> 
> The logic is that on every change of presence detect:
> /* Even if [the slot]'s occupied again, we cannot assume the card is the 
> same. */
> Reasonable, but the resulting behavior is a bug.
> 
> Solution 1 is to say it's a spec violation, so ignore it. They'll change 
> the "logical OR" thing in the next PCIe spec, so we still will have to 
> worry about this.

When's that changing? 5.0 is the next spec, and it still says:

  Presence Detect State - This bit indicates the presence of an adapter
  in the slot, reflected by the logical “OR” of the Physical Layer
  in-band presence detect mechanism and, if present, any out-of-band
  presence detect mechanism defined for the slot’s corresponding
  form factor.

> It's obvious that just relying on presence detect state is prone to race 
> conditions. However, if a device is replaced, we'd expect the data link 
> layer state to change as well. So I think the best way to proceed is to 
> skip the SURPRISE!!!_REMOVAL if the following are true:
>   * presence detect is set
>   * DLL changed is not set
>   * presence detect was not previously set
> 
> Thoughts?

What is the value of PDS on the Link up event? If it's still "Slot
Empty", could we just ignore the Link event instead and wait for the PDC
event?