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)
Tue, Feb 3, 4:23 PM
Unknown Object (File)
Tue, Feb 3, 4:22 PM
Unknown Object (File)
Tue, Feb 3, 4:18 PM
Unknown Object (File)
Tue, Feb 3, 4:17 PM
Unknown Object (File)
Tue, Jan 20, 7:54 PM
Unknown Object (File)
Tue, Jan 20, 1:08 PM
Unknown Object (File)
Tue, Jan 20, 12:25 PM
Unknown Object (File)
Mon, Jan 19, 8:50 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

Address @rew 's comments.

usr.sbin/bhyve/pci_emul.c
100

Yes, I don't how PCIBAR_MAX managed to slip into D54645, thanks for catching this!

jhb added inline comments.
usr.sbin/bhyve/pci_emul.c
948

In the kernel we just use '~0' for rman_res_t (e.g. for bus_alloc_resource_any()) and the compiler DTRT based on the argument type from the function prototype.

Have you tested with PCI passthru configured? With just this patch applied, I get an assertion failure in update_bar_address(). Same result if I test your github branch.

usr.sbin/bhyve/pci_emul.c
907

These assignments can be removed.

948