Page MenuHomeFreeBSD

Native PCI-express HotPlug support.
ClosedPublic

Authored by jhb on Apr 28 2016, 12:46 AM.
Tags
None
Referenced Files
F105940447: D6136.id15683.diff
Sun, Dec 22, 8:46 PM
Unknown Object (File)
Wed, Dec 11, 2:13 PM
Unknown Object (File)
Tue, Dec 10, 10:42 PM
Unknown Object (File)
Nov 17 2024, 10:46 PM
Unknown Object (File)
Oct 31 2024, 12:28 AM
Unknown Object (File)
Oct 29 2024, 3:06 PM
Unknown Object (File)
Oct 27 2024, 1:40 AM
Unknown Object (File)
Oct 27 2024, 1:40 AM

Details

Summary

Native PCI-express HotPlug support.

PCI-express HotPlug support is implemented via bits in the slot
registers of the PCI-express capability of the downstream port along
with an interrupt that triggers when bits in the slot status register
change.

This is implemented for FreeBSD by adding HotPlug support to the
PCI-PCI bridge driver which attaches to the virtual PCI-PCI bridges
representing downstream ports on HotPlug slots. The PCI-PCI bridge
driver registers an interrupt handler to receive HotPlug events. It
also uses the slot registers to determine the current HotPlug state
and drive an internal HotPlug state machine. For simplicty of
implementation, the PCI-PCI bridge device detaches and deletes the
child PCI device when a card is removed from a slot and creates and
attaches a PCI child device when a card is inserted into the slot.

The PCI-PCI bridge driver provides a bus_child_present which claims
that child devices are present on HotPlug-capable slots only when a
card is inserted. Rather than requiring a timeout in the RC for
config accesses to not-present children, the pcib_read/write_config
methods fail all requests when a card is not present (or not yet
ready).

These changes include support for various optional HotPlug
capabilities such as a power controller, mechanical latch,
electro-mechanical interlock, indicators, and an attention button.
It also includes support for devices which require waiting for
command completion events before initiating a subsequent HotPlug
command. However, it has only been tested on ExpressCard systems
which support surprise removal and have none of these optional
capabilities.

At the moment this includes some extra debugging printfs for
HotPlug events I have not yet tested. These could be moved under
bootverbose if not removed altogether.

Also, this code is currently always enabled and is always included.
It could be moved under a PCI_HP option similar to PCI_IOV if desired.

The _OSC handling is also not quite right. Ideally the PCI-PCI bridge
driver would pass a request of "can I do HotPlug" up to the ACPI PCI
bridge driver which would only return true if _OSC consents. For now
this just always requests it but doesn't do anything except whine if
_OSC refuses.

Test Plan
  • Tested on a Lenovo x220 and T400. The T400 exercised the DLL timer which fires whining about a missed HotPlug interrupt to note that the Data Layer became active without an associated interrupt.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb retitled this revision from to Native PCI-express HotPlug support..
jhb updated this object.
jhb edited the test plan for this revision. (Show Details)
jhb added a reviewer: imp.
jhb added subscribers: vangyzen, adrian.
adrian added a reviewer: adrian.

would you mind putting this behind a PCIE_HOTPLUG define? We don't need this for embedded platforms and I'd like to minimise footprint.

Otherwise, looks good! Thanks!

This revision is now accepted and ready to land.Apr 28 2016, 9:01 PM
jhb edited edge metadata.
  • Move hotplug under a PCI_HP option.
This revision now requires review to proceed.Apr 30 2016, 12:53 AM
sys/dev/pci/pci_pci.c
1099 ↗(On Diff #15752)

why 5 seconds?

1624–1634 ↗(On Diff #15752)

Why have two different implementations?

sys/dev/pci/pcib_private.h
133–144 ↗(On Diff #15752)

ifdef PCI_HP?

sys/dev/pci/pci_pci.c
1099 ↗(On Diff #15752)

It's in the spec. The power light is supposed to blink for 5 seconds so that the operator has the chance to push it again to cancel an eject request. If they don't cancel it, then the device is supposed to be detached (powered down, EI released, etc.) after the 5 seconds expires.

1624–1634 ↗(On Diff #15752)

pcib_hotplug_present is only available under #ifdef PCI_HP.

sys/dev/pci/pcib_private.h
133–144 ↗(On Diff #15752)

I'm a bit leery to have variant-sized softc's since subclass drivers may want to append items to the structure. NEW_PCIB is a bit different in my mind since it should be a platform-wide decision whereas individual kernels may choose to leave out PCI_HP to save space (e.g. embedded systems with stripped-down kernels). However, if a subclass doesn't include "opt_pci.h" it wouldn't get a compile error or warning, just corruption at runtime.

sys/dev/pci/pci_pci.c
890 ↗(On Diff #15752)

Why not return here?

971 ↗(On Diff #15752)

Passing observation: -1 seems like a strange way to indicate truth.

sys/dev/pci/pci_pci.c
1201 ↗(On Diff #15752)

Should this be "else if"? I don't see a reason to simulate the interrupt if the DLL is inactive.

sys/dev/pci/pci_pci.c
1224 ↗(On Diff #15752)

What did you intend here? (...by essentially assigning count to itself)

This revision is now accepted and ready to land.May 2 2016, 8:18 PM
sys/dev/pci/pci_pci.c
890 ↗(On Diff #15752)

Oh, that's missing. It should return.

jhb marked 3 inline comments as done.May 2 2016, 10:46 PM
jhb added inline comments.
sys/dev/pci/pci_pci.c
971 ↗(On Diff #15752)

It is because it is used to implement bus_child_present() which has this semantic. (-1 means device present, 0 means absent, > 0 means error trying to determine and/or not sure)

1201 ↗(On Diff #15752)

Hmm, probably, yes. I should perhaps force a detach in this case which will power down the card as well.

1224 ↗(On Diff #15752)

Originally I was going to support arbitrary counts but only request 1 and then "move" the 1 interrupt to the right slot. However, for now I've decided to punt until somehow shows up with such a beast (slot with multiple MSI-X messages). The assignment is leftover from the previous code.

jhb edited edge metadata.
jhb marked 2 inline comments as done.
  • Return if the slot accepts direct hotplug commands.
  • Fix issues in DLL Active timeout.
  • Remove redundant assignment.
This revision now requires review to proceed.May 3 2016, 12:52 AM
vangyzen edited edge metadata.
This revision is now accepted and ready to land.May 3 2016, 2:02 PM
imp edited edge metadata.
jhb edited edge metadata.
  • Stop the DLL timer if the link becomes active.
  • Move some debug printfs under bootverbose.
This revision now requires review to proceed.May 5 2016, 10:02 PM
This revision was automatically updated to reflect the committed changes.