Page MenuHomeFreeBSD

Honor the disabled setting for MSI-X interrupts for passthrough devices.
ClosedPublic

Authored by jhb on Fri, Nov 13, 11:21 PM.

Details

Summary

Add a new ioctl to disable all MSI-X interrupts for a PCI passthrough
device and invoke it if a write to the MSI-X capability registers
disables MSI-X. This avoids leaving MSI-X interrupts enabled on the
host if a guest device driver has disabled them (e.g. as part of
detaching a guest device driver).

This was found by Chelsio QA when testing that a Linux guest could
switch from MSI-X to MSI interrupts when using the cxgb4vf driver.

While here, explicitly fail requests to enable MSI on a passthrough
device if MSI-X is enabled and vice versa.

Test Plan
  • tested by detaching the cxgb4vf driver in a Linux guest and reattaching it with MSI enabled (instead of MSI-X). Previously bhyve exited with an error as the ioctl to enable MSI failed, now it properly disables MSI-X (as verified via pciconf -lc on the host) when the guest driver is unloaded

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb requested review of this revision.Fri, Nov 13, 11:21 PM
markj added inline comments.
sys/amd64/vmm/io/ppt.c
522 ↗(On Diff #79530)

Why bother handling numvec == 0 specially? Doesn't that case imply that we have both MSI and MSI-X configured on the device?

sys/amd64/vmm/io/ppt.c
522 ↗(On Diff #79530)

No. The MSI ioctl is invoked anytime any MSI cap register is written to. Even if the effect is no-op that means it was disabled before and remains disabled now. Allowing 'numvec == 0' ensures any of those writes do not result in an error that causes bhyve to exit. IOW, it is ok to disable MSI while MSI-X is active, it is only invalid to try to enable MSI while MSI-X is active.

sys/amd64/vmm/io/ppt.c
522 ↗(On Diff #79530)

Thanks.

721 ↗(On Diff #79530)

It might be nice to put this logic in a subroutine. All but one caller of ppt_find() implements identical logic.

sys/amd64/vmm/io/ppt.c
721 ↗(On Diff #79530)

It might be nice to put this logic in a subroutine. All but one caller of ppt_find() implements identical logic.

I can add a followup as a separate change. I think even the one odd use is a bug as it should not allow re-assignment (or if it does, it should be a no-op, you don't want to add it to the iommu twice, etc.)

This revision is now accepted and ready to land.Sat, Nov 21, 9:10 AM