Page MenuHomeFreeBSD

pci: loosen PCIe hot-plug requirements
AcceptedPublic

Authored by chuck on Mon, May 18, 1:27 PM.

Details

Reviewers
jhb
scottl
mav
Summary

The original PCIe hot-plug code required a couple of things which cause
PCI probing errors on the qemu q35 system and possibly physical systems
(Dell R6515).

First, the code required the hot-plug IRQ's to be unique (i.e. not
shared). On qemu q35, all PCIe hot-plug IRQ's are identical. Fix by
allocating the IRQ as RF_SHAREABLE.

Second, the code required the Electromechanical Interlock (Slot Status
EIS) to be engaged if present (Slot Capability EIP). Several platforms
including qemu q35 set EIP but not EIS. Fix by deleting the check.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 31147

Event Timeline

chuck created this revision.Mon, May 18, 1:27 PM
chuck requested review of this revision.Mon, May 18, 1:27 PM
mav added inline comments.Tue, May 26, 4:57 PM
sys/dev/pci/pci_pci.c
1083

I've never had hardware with interlock myself, but this seems like just removing the interlock support, that can make somebody unhappy.

1333

It seems the code in pcib_pcie_intr_hotplug() does not expect to be called for unrelated reasons, that are normal in case of shared interrupts.

imp added inline comments.Tue, May 26, 8:45 PM
sys/dev/pci/pci_pci.c
1083

If I were doing it, I'd park this behind a sysctl that defaulted to 'don't block', but I don't know how relevant this actually is....

jhb accepted this revision.Tue, May 26, 10:05 PM

I would perhaps say "some" rather than several platforms. Lying about an EI being present doesn't appear to be super prevalent. The interrupt issue is also only seen on QEMU. I doubt it exists on any real hardware. I would perhaps word the paragraph about shared interrupts more like:

Allocate the hot plug interrupt shared to support INTx interrupts.  The hot plug
interrupt should normally be an MSI interrupt since PCI-e mandates MSI interrupt
support, but QEMU's Q35 bridge only provides INTx interrupts.
sys/dev/pci/pci_pci.c
1083

This doesn't remove the code to engage / disengage the EI, it simply doesn't require the EI to report a status of engaged before trying to use the card. Linux doesn't do this check, and the spec doesn't really say that you have to do this. I think this change is ok.

1333

This is only needed for the !MSI case, but I must have made that change in my branch before it was merged to head and missed it when adding the !MSI fallback. I'm really surprised that you have a bridge without MSI. The fact that QEMU doesn't do MSI is mind boggling. MSI is so much nicer for emulators than INTx, and PCI-e hot plug is PCI-e only, and PCI-e mandates MSI anyway, so the q35 bridge is arguably violating the spec. The interrupt should be fine though as the handler just inspects whatever the current state is and drives the state machine from there. It might reset a timer and miss a timeout due to spurious interrupts but probably work fine otherwise.

This revision is now accepted and ready to land.Tue, May 26, 10:05 PM