Differential D20499

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

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

Details

Reviewers

Commits

rS348681: Only respond to the PCIe Attention Button if a device is already plugged in.

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 - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 24671

Event Timeline

cperciva created this revision.Jun 3 2019, 12:37 AM

Herald added a subscriber: imp. · View Herald TranscriptJun 3 2019, 12:37 AM

Harbormaster completed remote builds in B24660: Diff 58184.Jun 3 2019, 12:37 AM

jhb added inline comments.Jun 3 2019, 6:26 PM

sys/dev/pci/pci_pci.c
1187	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.

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

Harbormaster completed remote builds in B24671: Diff 58207.Jun 3 2019, 9:39 PM

cperciva added inline comments.Jun 3 2019, 9:40 PM

sys/dev/pci/pci_pci.c
1187	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	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

Closed by commit rS348681: Only respond to the PCIe Attention Button if a device is already plugged in. (authored by cperciva). · Explain WhyJun 5 2019, 4:58 AM

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.

mqudsi_neosmart.net mentioned this in D18613: Add driver for the VMware Paravirtual SCSI (pvscsi) controller.Nov 30 2019, 5:58 AM