Page MenuHomeFreeBSD

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

Authored by jhb on Feb 7 2025, 5:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 5, 12:34 PM
Unknown Object (File)
Thu, Feb 27, 6:05 PM
Unknown Object (File)
Mon, Feb 24, 11:25 AM
Unknown Object (File)
Sun, Feb 23, 8:25 AM
Unknown Object (File)
Feb 13 2025, 12:46 PM
Unknown Object (File)
Feb 13 2025, 12:15 PM
Unknown Object (File)
Feb 13 2025, 9:40 AM
Unknown Object (File)
Feb 7 2025, 6:32 PM
Subscribers

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.Feb 7 2025, 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.Feb 7 2025, 7:54 PM