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

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

Details

Reviewers

grehan
markj

Group Reviewers

bhyve

Commits

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

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Nov 13 2020, 11:21 PM

Herald added a reviewer: bhyve. · View Herald TranscriptNov 13 2020, 11:21 PM

Herald added subscribers: bcran, rgrimes. · View Herald Transcript

jhb requested review of this revision.Nov 13 2020, 11:21 PM

Harbormaster completed remote builds in B34792: Diff 79530.Nov 13 2020, 11:21 PM

markj added a subscriber: markj.Nov 17 2020, 4:10 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?

jhb added inline comments.Nov 17 2020, 4:27 PM

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.

markj added inline comments.Nov 19 2020, 3:52 PM

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.

jhb added inline comments.Nov 20 2020, 8:05 PM

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