Page MenuHomeFreeBSD

Only respond to Attention Button if device is already plugged in
ClosedPublic

Authored by cperciva on Jun 3 2019, 12:37 AM.

Details

Summary

Currently if PCIEM_SLOT_STA_ABP and PCIEM_SLOT_STA_PDC are asserted simultaneously, FreeBSD sets a 5 second "hardware going away" timer and then processes the "presence detect" change. In the (physically challenging) case that someone presses the "attention button" and inserts a new PCIe device at exactly the same moment, this results in FreeBSD recognizing that the device is present, attaching it, and then detaching it 5 seconds later.

On EC2 "bare metal" hardware this is the precise sequence of events which takes place when a new EBS volume is attached; virtual machines have no difficulty effecting physically implausible simultaneity.

This patch changes the handling of PCIEM_SLOT_STA_ABP to only detach a device (or cancel the detach of a device) if the presence of a device was detected *before* the interrupt.

Test Plan

Works in EC2. Not sure if it's possible to test this fix anywhere else.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cperciva created this revision.Jun 3 2019, 12:37 AM
jhb added inline comments.Jun 3 2019, 6:26 PM
sys/dev/pci/pci_pci.c
1187 ↗(On Diff #58184)

Maybe add a comment here, something like:

/* Ignore attention button presses if no device was previously present. */

A valid test even on real hardware would be if someone pressed the attention button when the slot was empty. We would currently queue a spurious timer instead of ignoring the button press. I think we should perhaps push the PDS check down into the "arming case" though so that we always honor a cancel button press.

cperciva updated this revision to Diff 58207.Jun 3 2019, 9:39 PM

Moved test to the !PCIB_DETACH_PENDING case and added an explanatory comment.

cperciva added inline comments.Jun 3 2019, 9:40 PM
sys/dev/pci/pci_pci.c
1187 ↗(On Diff #58184)

Good points, is this revised patch ok?

jhb accepted this revision.Jun 4 2019, 3:57 PM
jhb added inline comments.
sys/dev/pci/pci_pci.c
1194 ↗(On Diff #58207)

Maybe add 'device was previously present' since we are using 'old_slot_sta', but looks good to me.

This revision is now accepted and ready to land.Jun 4 2019, 3:57 PM
This revision was automatically updated to reflect the committed changes.

This works for VMware ESXi as well, where network card add/remove is directly mapped to PCIe hotplug.

It's a pretty clean (and reasonable) fix; the only other obvious workaround would be to cancel extant device removal timers upon insert (if it were desirable to preserve the "attention button" signal for some odd reason), but I see no reason to complicate things and go that route.