bhyve: enumerate BARs by size

[corvink]

Summary

E.g. Framebuffers can require large space and BARs need to be aligned
by their size. If BARs aren't allocated by size, it'll cause much
fragmentation of the MMIO space. Reduce fragmentation by ordering
the BAR allocation on their size to reduce the risk of
OUT_OF_MMIO_SPACE issues.

Note: You can take a look at https://github.com/Beckhoff/freebsd-src/commits/phab/corvink/bar-enumeration

[corvink]

• rebase and update

[markj]

I think D27220 was intended to solve the same problem? I'm not sure if a bhyve patch to use libuvmem is available.

This seems like a reasonable interim solution.

usr.sbin/bhyve/pci_emul.c

676

682

Rather than having comments like this, I think it'd be more useful to say that this code is creating a list of BARs sorted by size, with larger BARs appearing first, to reduce fragmentation in the MMIO space.

1306

1314

Seems it would be simpler to write:

TAILQ_FOREACH_SAFE(bar, &pci_bars, chain, tmp) {
    pci_emul_assign_bar(bar);
    free(bar);
}
TAILQ_INIT(&pci_bars);

[corvink]

Address feedback

[corvink]

Use TAILQ_INIT to delete pci_bar list

[corvink]

In D28278#747021, @markj wrote:

I think D27220 was intended to solve the same problem? I'm not sure if a bhyve patch to use libuvmem is available.

This seems like a reasonable interim solution.

I'm unfamiliar with libuvmem. I'm unsure whether libuvmem is able to solve the problem too or not.

[markj]

In D28278#747302, @c.koehne_beckhoff.com wrote:

In D28278#747021, @markj wrote:

I think D27220 was intended to solve the same problem? I'm not sure if a bhyve patch to use libuvmem is available.

This seems like a reasonable interim solution.

I'm unfamiliar with libuvmem. I'm unsure whether libuvmem is able to solve the problem too or not.

It's an address space allocator which tries to minimize fragmentation over time. Currently libuvmem doesn't exist in FreeBSD, there is just vmem(9).

The change seems ok to me, I am just nitpicking a bit.

usr.sbin/bhyve/pci_emul.c

667

716

IMO it'd be cleaner to pass the parameters individually. That would also be consistent with pci_emul_alloc_bar().

[corvink]

pci_passthru synchronizes his physical cmd reg with it's virtual cmd reg on init. For that reason, we can't update the virtual cmd reg in pci_emul_assign_bar. It would be after pci_passthru synchronization. The virtual cmd reg needs to be updated earlier in pci_emul_alloc_bar.

[markj]

usr.sbin/bhyve/pci_emul.c

675

Then the line doesn't need to be split.

708

733

Long line.

[corvink]

prettify code style