Page MenuHomeFreeBSD

bhyve/pci_emul: Use vmem to track BAR allocations
Needs ReviewPublic

Authored by bnovkov on Sun, Jan 11, 1:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jan 12, 9:14 AM
Unknown Object (File)
Sun, Jan 11, 10:03 PM
Unknown Object (File)
Sun, Jan 11, 5:48 PM
Unknown Object (File)
Sun, Jan 11, 5:06 PM
Unknown Object (File)
Sun, Jan 11, 4:56 PM

Details

Reviewers
None
Group Reviewers
bhyve
Summary

This patch replaces bhyve's PCI BAR bump allocator with libuvmem(3).
This allows us to allocate PCI BARs during runtime, which is a
prerequisite for PCI device hotplugging.

Under this resource management scheme, each virtual PCI bus manages its
IO, MEM32, and MEM64 BARs using separate vmem_t arenas, with each BAR
type's address space evenly distributed across all virtual PCI buses.
For example, consider a hypothetical virtual machine with two PCI buses whose
PCIBAR_IO BARs can be allocated from [0x1000, 0x3000). Under this BAR
management scheme, a vmem_t arena for PCI bus 0 manages the
[0x1000, 0x2000) range, while the PCI bus 1 arena manages the
[0x2000, 0x3000) range.

All BARs are allocated using vmem's M_BESTFIT flag to match the
previous allocator's fragmentation prevention policy.

update_bar_address was also changed to use vmem_free and
vmem_xalloc to handle guest PCI BAR reprogramming.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

markj added inline comments.
usr.sbin/bhyve/pci_emul.c
749

alloc should be a bool.

792

Why do we need to specify M_NOWAIT?

935

Does this comment get addressed somehow later in the series?

948

Same here.

948

Shouldn't the upper limit be ~0ul? The value you passed there gives a 32-bit limit, no?

1547

Do these pci_emul_* variables still need to be global?

1551

Where do these values come from? It looks like you're partitioning the address space based on the number of pci buses, but why are you using the size of each partition as the vmem quantum?

Oh, these aren't the quantum sizes, it's just the size of the initial address range. These variables are misnamed I think.

bnovkov marked 5 inline comments as done.

Address @markj 's comments.

usr.sbin/bhyve/pci_emul.c
792

Thanks for catching this, it was not intentional.

935

Ah, I knew I'd forgotten address something.

I wasn't sure how PCIBAR_ROM behaved initially, but looking at the code now I think we can just remove that comment. It's effectively a noop since the actual allocation is handled by pci_emul_alloc_rom.

948

I'm not sure how vmem_addr_t behaves here, but I'll throw in ul to be safe.

1547

Thanks for pointing this out, only the rombase variables should remain global since they're handled by pci_emul_alloc_rom.

1551

Sorry for the awkward phrasing, I forgot that 'quantum' vmem(9) jargon. I've changed the names of the variables, hopefully it's a bit clearer now.

rew added inline comments.
usr.sbin/bhyve/pci_emul.c
100

PCIBAR_MAX isn't declared as an enum member in this review

i see it's added later on in D54645 but I'm guessing you meant to include it in this review/commit