Page MenuHomeFreeBSD

pci: Don't cache the count of MSI/MSI-X messages before allocation
ClosedPublic

Authored by jhb on Fri, Feb 7, 5:29 PM.

Details

Reviewers
imp
krzysztof.galazka_intel.com
erj
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rG346020138a0f: pci: Don't cache the count of MSI/MSI-X messages before allocation
Summary

A device can in theory change the read-only fields in the MSI/MSI-X
control registers that indicate the maximum number of supported
registers in response to changing other device registers. For
example, certain Intel networking VFs change the number of messages as
a result of changes in the PCI_IOV_ADD_VF callback.

To support this, always read the current value of the relevant control
register in the *_count and *_alloc methods. Once messages have been
allocated, the control register value remains cached.

Reported by: Krzysztof Galazka <krzysztof.galazka@intel.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Fri, Feb 7, 5:29 PM
sys/dev/pci/pci.c
1950

Is it possible that e.g. suspend modifies ctrl register between read and write here?

sys/dev/pci/pci.c
1950

It shouldn't be and is probably prevented by Giant atm. Eventually when we have proper locking for new-bus it will ensure that these can't race with suspend. Even now actually, suspend will only try to save and restore this register after a device driver's DEVICE_SUSPEND() method has returned, and a device driver shouldn't be allocating MSI messages while suspended.

erj added a subscriber: erj.
erj added inline comments.
sys/dev/iavf/iavf_lib.c
1478

Rest in well-deserved peace, my dumb workaround!

Ah, that makes sense. Thanks a lot for clarification!

This revision is now accepted and ready to land.Fri, Feb 7, 7:54 PM