Page MenuHomeFreeBSD

bhyve: enumerate BARs by size
ClosedPublic

Authored by corvink on Jan 22 2021, 9:34 AM.

Details

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

Diff Detail

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

Event Timeline

corvink retitled this revision from bhyve: order BARs by size to bhyve: enumerate BARs by size.
corvink edited the summary of this revision. (Show Details)
  • rebase and update

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 edited the summary of this revision. (Show Details)

Address feedback

Use TAILQ_INIT to delete pci_bar list

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.

In D28278#747302, @c.koehne_beckhoff.com 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().

This revision is now accepted and ready to land.Nov 22 2021, 2:06 PM
This revision now requires review to proceed.Nov 23 2021, 8:03 AM
corvink added a reviewer: jhb.

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.

usr.sbin/bhyve/pci_emul.c
675

Then the line doesn't need to be split.

708
733

Long line.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 3 2022, 4:02 PM
Closed by commit rG01f9362ef4eb: bhyve: enumerate BARs by size (authored by corvink, committed by manu). · Explain Why
This revision was automatically updated to reflect the committed changes.