Page MenuHomeFreeBSD

bhyve: passthru: enable BARs before possibly mmap(2)ing them
ClosedPublic

Authored by bz on Dec 23 2021, 3:17 PM.
Tags
None
Referenced Files
F83066989: D33628.id100708.diff
Sun, May 5, 8:18 PM
F83044340: D33628.diff
Sun, May 5, 11:56 AM
Unknown Object (File)
Feb 20 2024, 5:26 PM
Unknown Object (File)
Feb 4 2024, 6:54 AM
Unknown Object (File)
Jan 31 2024, 2:07 PM
Unknown Object (File)
Jan 26 2024, 6:45 PM
Unknown Object (File)
Jan 19 2024, 7:30 AM
Unknown Object (File)
Dec 20 2023, 3:24 AM

Details

Summary

The first time we start bhyve with a passthru device everything is fine
as on boot we do enable BARs. If a driver (unload) inside bhyve disables
the BAR(s) as some Linux drivers do, we need to make sure we re-enable
them on next bhyve start.

If we are trying to mmap a disabled BAR for MSI-X (PCIOCBARMMAP)
the kernel will give us an EBUSY.
While we were re-enabling the BAR(s) in the current code loop
cfginit() was writing the changes out too late to the real hardware.

Move the call to init_msix_table() after the register on the real
hardware was updated. That way the kernel will be happy and the
mmap will succeed and bhyve will start.

PR: 260148
Sponsored by: The FreeBSD Foundation

Test Plan

start bhyve with iwlwifi on passthrough
kldload if_iwlwifi
kldunload if_iwlwifi
reboot
start bhyve again (fails or works again with the change)

See also PR.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 43558
Build 40446: arc lint + arc unit

Event Timeline

bz requested review of this revision.Dec 23 2021, 3:17 PM

I'd love https://reviews.freebsd.org/D20623 to also go in at some point.

Seems ok to me other than the nit, thanks.

usr.sbin/bhyve/pci_passthru.c
570

Why do we need the loop? Can this just be

if ((i = pci_msix_table_bar(sc->psc_pi)) != -1) {
    assert(i >= 0 && i <= PCI_BARMAX);
    error = init_msix_table(ctx, sc, sc->psc_bar[i].addr);

?

In fact, the third parameter to init_msix_table() is unused and could be dropped, so the BAR index i is not needed.

This revision is now accepted and ready to land.Dec 29 2021, 2:28 PM

Also simplify the code given the last argument to init_msix_table()
is unused we do not need to do checks for each bar. [1]

This revision now requires review to proceed.Dec 29 2021, 4:24 PM
bz marked an inline comment as done.Dec 29 2021, 4:25 PM
bz added inline comments.
usr.sbin/bhyve/pci_passthru.c
570

Good spot. Makes the code a lot simpler. Thank you!

This revision is now accepted and ready to land.Dec 29 2021, 4:27 PM
This revision was automatically updated to reflect the committed changes.
bz marked an inline comment as done.
corvink added a subscriber: corvink.

This patch unconditionally calls init_msix_table. This breaks passthru for devices which only support MSI. I created a patch for it D33728