Differential D20525

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

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

Details

Reviewers

rgrimes
markj

Group Reviewers

bhyve

Commits

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

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

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 24702
Build 23468: arc lint + arc unit

Event Timeline

jhb created this revision.Jun 5 2019, 4:53 PM

Herald added subscribers: bcran, rgrimes, imp. · View Herald TranscriptJun 5 2019, 4:53 PM

Harbormaster completed remote builds in B24702: Diff 58271.Jun 5 2019, 4:53 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.

Closed by commit rS348695: Support MSI-X for passthrough devices with a separate PBA BAR. (authored by jhb). · Explain WhyJun 5 2019, 7:30 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

sys/

amd64/

vmm/

io/

ppt.c

35 lines

Diff 58271

View Options

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