Page MenuHomeFreeBSD

pci: loosen PCIe hot-plug requirements
ClosedPublic

Authored by chuck on May 18 2020, 1:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 8, 12:43 AM
Unknown Object (File)
Tue, Mar 19, 3:31 PM
Unknown Object (File)
Tue, Mar 19, 3:20 PM
Unknown Object (File)
Mon, Mar 18, 8:07 PM
Unknown Object (File)
Dec 20 2023, 7:38 AM
Unknown Object (File)
Nov 10 2023, 1:35 PM
Unknown Object (File)
Nov 10 2023, 7:05 AM
Unknown Object (File)
Nov 7 2023, 3:26 PM
Subscribers

Details

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 - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 31147

Event Timeline

chuck requested review of this revision.May 18 2020, 1:27 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.

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

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.May 26 2020, 10:05 PM

Changes made to the comments. Thanks for the reviews!

This revision was automatically updated to reflect the committed changes.