Enhancing pciehp (was: pcielw An alternate pcie hotplug driver)

Enhancing pciehp (was: pcielw An alternate pcie hotplug driver)

[Rajat Jain]

Hello Bjorn / Eric / Folks,

This is to seek suggestions about a problem I'm trying to solve.

The problem
=====1) My company makes routers that have hot-pluggable cards with PCI express interfaces on them. We need to be able to hot-plug those cards, however the cards or the slots do not have the fancy bells & whistles (hot plug elements like MRL, sensor, attention button etc), and hence the hot-plug signals from the PCIe switch aren't really connected.

2) In addition to the above, we have onboard ASICs that are reachable via the PCIe. However, as part of regular operation of a router (e.g. user wants to switch off some ports), the ASICs can be off-lined / on-lined via out-of-band HW pins. The result is that we could see the PCIe link go down or up to that ASIC. This can be thought of as a "virtual" hot-plug of ASIC devices. Since the ASICs are themselves on the board, there is really no slot, and the HP signals again do not make much sense in this case.

What I found
======What I was looking for, was a way to hot-plug and un-plug based on link state changes (since that is pretty much what is available in my case). And I found this:
http://www.spinics.net/lists/linux-pci/msg05783.html
http://www.spinics.net/lists/linux-pci/msg05880.html

My questions
======
a) I was wanting to know if what is the latest on this, or if any progress was made on this? If useful, I am willing to volunteer to work on this. Do you think there is interest in getting this to work? I feel there may be many such systems that are in the same situation.

b) If I wanted to make use of Eric's patch in a way that is useful to the community, what is the best way to move forward? Do you still want to enhance the pciehp to include this functionality? Or a separate service driver (what Eric already has) seems a better idea? I'd appreciate if you could please provide any guidance here.

TIA,

Rajat

Re: Enhancing pciehp (was: pcielw An alternate pcie hotplug driver)

[Bjorn Helgaas]

On Mon, Oct 7, 2013 at 12:58 PM, Rajat Jain <rajatjain@juniper.net> wrote:
> Hello Bjorn / Eric / Folks,
>
> This is to seek suggestions about a problem I'm trying to solve.
>
> The problem
> =====> 1) My company makes routers that have hot-pluggable cards with PCI express interfaces on them. We need to be able to hot-plug those cards, however the cards or the slots do not have the fancy bells & whistles (hot plug elements like MRL, sensor, attention button etc), and hence the hot-plug signals from the PCIe switch aren't really connected.

The elements you mention are optional per the spec (PCIe r3.0, sec
6.7.1).  The lack of them, by itself, should not be enough to force
you to write a new hotplug driver.  pciehp is used for ExpressCard
hotplug, and that's a similar situation where there's no MRL, no MRL
sensor, no interlock, no attention button, etc.

What connects the hot-pluggable card to the system?  PCI?  PCIe?
Something else?  If it's PCIe, it seems like you'd make your life
simpler by taking advantage of some or all of the hotplug support that
is likely in the PCIe Root Port or Downstream Port leading to the
slot.

> 2) In addition to the above, we have onboard ASICs that are reachable via the PCIe. However, as part of regular operation of a router (e.g. user wants to switch off some ports), the ASICs can be off-lined / on-lined via out-of-band HW pins. The result is that we could see the PCIe link go down or up to that ASIC. This can be thought of as a "virtual" hot-plug of ASIC devices. Since the ASICs are themselves on the board, there is really no slot, and the HP signals again do not make much sense in this case.

It doesn't really matter that there's no physical slot and the user
can't replace an ASIC with a different one.  You can still use the
hotplug signals that *are* relevant, e.g., it sounds like things
related to Hot-Plug Surprise, Presence Detect, and Data Link Layer
State would still make sense.

The current pciehp driver doesn't really do anything with the Data
Link Layer State Changed Enable bit (it looks like we *disable* that
notification, but never *enable* it).  It sounds like a reasonable
thing to add to pciehp, though.

Bjorn

RE: Enhancing pciehp (was: pcielw An alternate pcie hotplug driver)

[Rajat Jain]

Hello,

I'm working on enhancing the pciehp driver to allow link state changes to drive the hot-plug and un-plug. The idea was discussed here:

http://www.spinics.net/lists/linux-pci/msg25733.html

However, this is to invite ideas on how to handle a particular corner case I see. Let's say a PCIe link comes up and the pciehp driver calls pciehp_enable_slot() to start scanning and adding the devices on the slot. However, what to do if before that completes, the link goes down due to some reason? Can the pciehp driver call pciehp_disable_slot() before the call to pciehp_enable_slot() returns (with or without error)?

Essentially even without link state to driver the hot-plug, this problem can also be thought of as a problem that exists today. For e.g. for the slots that support surprise removal, the pciehp will initiate hot-plug and unplug based on the presence detection (as soon as card is inserted or removed). Now if a user puts in a card very momentarily, long enough for the hot-add process to get started (pciehp_enable_slot()) but removes it before it can be completed, the same problem can be envisioned. As the code exists today, I believe it would call pciehp_disable_slot() even before pciehp_enable_slot() returns. Is that the designed / correct behavior? 

Should I be following the same for link state change driven hot-plug?

Thanks,

Rajat

Re: Enhancing pciehp

[Eric W. Biederman]

Rajat Jain <rajatjain@juniper.net> writes:

> Hello,
>
> I'm working on enhancing the pciehp driver to allow link state changes to drive the hot-plug and un-plug. The idea was discussed here:
>
> http://www.spinics.net/lists/linux-pci/msg25733.html
>
> However, this is to invite ideas on how to handle a particular corner case I see. Let's say a PCIe link comes up and the pciehp driver calls pciehp_enable_slot() to start scanning and adding the devices on the slot. However, what to do if before that completes, the link goes down due to some reason? Can the pciehp driver call pciehp_disable_slot() before the call to pciehp_enable_slot() returns (with or without error)?
>
> Essentially even without link state to driver the hot-plug, this problem can also be thought of as a problem that exists today. For e.g. for the slots that support surprise removal, the pciehp will initiate hot-plug and unplug based on the presence detection (as soon as card is inserted or removed). Now if a user puts in a card very momentarily, long enough for the hot-add process to get started (pciehp_enable_slot()) but removes it before it can be completed, the same problem can be envisioned. As the code exists today, I believe it would call pciehp_disable_slot() even before pciehp_enable_slot() returns. Is that the designed / correct behavior? 
>
> Should I be following the same for link state change driven hot-plug?

The way I handled this problem is made the enable and disable all run
through the same single threaded work queue.

The code does need to be single threaded on a per hotplug slot basis
(aka enable and disable concurrently on the same slot is nonsense).

I think there is some locking in the device tree that you might be able
to depend on to help single thread this.  At the time I was working on
this I did not see any locking or anything else that would serialize the
access so I went for the very cheap solution of an interrupt handler
that queued work in a single threaded work queue.

The code to be correct and robuse must handle the case of where what it
thought was going on add/remove is time late information and is actually
incorrect at the time of the hotplug scan.

This all gets even more interesting when you are plugging in a tree of
hotplug devices with hotplug slots of their own.

From an implementation point of view I have observed devices in the wild
with imperfect PCIe busses that due to bus noise would occassionally
flap their presence bit for no particularly good reason.  It is a
hardware but so you don't need to handle it beyond not letting that kind
of activity crash the kernel but it does happen.

Eric

Re: Enhancing pciehp

[Eric W. Biederman]

Rajat Jain <rajatjain@juniper.net> writes:

> Hello Bjorn / Eric / Folks,
>
> This is to seek suggestions about a problem I'm trying to solve.
>
> The problem
> =====> 1) My company makes routers that have hot-pluggable cards with PCI express interfaces on them. We need to be able to hot-plug those cards, however the cards or the slots do not have the fancy bells & whistles (hot plug elements like MRL, sensor, attention button etc), and hence the hot-plug signals from the PCIe switch aren't really connected.
>
> 2) In addition to the above, we have onboard ASICs that are reachable via the PCIe. However, as part of regular operation of a router (e.g. user wants to switch off some ports), the ASICs can be off-lined / on-lined via out-of-band HW pins. The result is that we could see the PCIe link go down or up to that ASIC. This can be thought of as a "virtual" hot-plug of ASIC devices. Since the ASICs are themselves on the board, there is really no slot, and the HP signals again do not make much sense in this case.
>
> What I found
> ======> What I was looking for, was a way to hot-plug and un-plug based on link state changes (since that is pretty much what is available in my case). And I found this:
> http://www.spinics.net/lists/linux-pci/msg05783.html
> http://www.spinics.net/lists/linux-pci/msg05880.html
>
> My questions
> ======

> a) I was wanting to know if what is the latest on this, or if any
> progress was made on this? If useful, I am willing to volunteer to
> work on this. Do you think there is interest in getting this to work?
> I feel there may be many such systems that are in the same situation.

Agreed.  It appears this is a design that is likely to come up more than
once.

> b) If I wanted to make use of Eric's patch in a way that is useful to
> the community, what is the best way to move forward? Do you still want
> to enhance the pciehp to include this functionality? Or a separate
> service driver (what Eric already has) seems a better idea? I'd
> appreciate if you could please provide any guidance here.

Where I wound up was I unfortunately had not left time in my schedule to
completely rewrite what I had done and to merge that into pciehp.

I wrote pcielw simply because the pciehp driver did not and may still
not work with multiple layers of pci hotplug so I needed to do some deep
digging.  Dealing with all of the cases of physical buttons when I did
not have those was prohibitive for my poor schedule.

I work somewhere else now and I don't have this problem so I will not be
looking at this problem again any time soon.

I do agree with Bjorn that one driver that can handle everything is
doable and ideal.  So if you are willing to do that work it would be
great.  pcielw should certainly work as a proof of concept solution to
any of the problems, and time permitting I am willing to answer
questions of the what was I thinking variety.

Eric