Page MenuHomeFreeBSD

Support MSI-X for passthrough devices with a separate PBA BAR.
ClosedPublic

Authored by jhb on Jun 5 2019, 4:53 PM.

Details

Summary

pci_alloc_msix() requires both the table and PBA BARs to be allocated
by the driver. ppt was only allocating the table BAR so would fail
for devices with the PBA in a separate BAR. Fix this by allocating
the PBA BAR before pci_alloc_msix() if it is stored in a separate BAR.

While here, release BARs after calling pci_release_msi() instead of
before. Also, don't call bus_teardown_intr() in error handling code
if bus_setup_intr() has just failed.

Reported by: gallatin

Test Plan
  • gallatin will test this

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 24702
Build 23468: arc lint + arc unit

Event Timeline

jhb created this revision.Jun 5 2019, 4:53 PM
rgrimes accepted this revision as: rgrimes.Jun 5 2019, 5:18 PM

Just questions really, no changes requested.

sys/amd64/vmm/io/ppt.c
110

I know it is outside the scope of this review, so this is more for my information, is the order of stuff in this struct wrong? Shouldnt the struct's be first, then the voids, then the ints? Or do we not follow the same rules for structs as we do for locals?

301

What is the reasoning for moving this code earlier? I can not see any functional difference.

This revision is now accepted and ready to land.Jun 5 2019, 5:18 PM

Just wanted to chime in and say that this, in combination with https://reviews.freebsd.org/D20523, allows me to pass an add-in USB XHCI controller to a guest. Before this patch, bhyve would abort when passing through the problematic controller.

markj accepted this revision as: markj.Jun 5 2019, 5:38 PM
jhb added inline comments.Jun 5 2019, 5:58 PM
sys/amd64/vmm/io/ppt.c
110

That rule isn't really followed that strictly by all developers. I tend to follow it a bit, though my practice is closer to "sort on length of typename" than "sort on size of type" which is what bde@ says the actual rule is.

301

pci_release_msi() may need to go write to the MSI-X table perhaps. I can't recall if it actually does, but part of the contract for the MSI API is that the driver should alloc the resources for the MSI-X related BARs so that the PCI code can use them to manage the table, and so the driver shouldn't release the resources until after pci_release_msi() so that the PCI bus can finish cleaning up MSI-X state which might include scribbling in the BARs.

This revision was automatically updated to reflect the committed changes.