bhyve: Refactor initial PCI BAR setup
ClosedPublic
Actions

Authored by jhb on Mon, Aug 17, 5:55 PM.

Details

Reviewers

bnovkov

Group Reviewers

bhyve

Commits

rGb00bb87a6142: bhyve: Refactor initial PCI BAR setup

Summary

Fully initialize BARs with an address of 0 in pci_emul_alloc_bar()
instead of deferring some of that initialization to
pci_emul_assign_bar(). Now, the latter is only used to allocate an
initial address range for PCI BARs.

Note that this means that the pci_passthru model now overrides the
initial lobits after they are set removing the need for a workaround
in pci_emul_assign_bar().

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Mon, Aug 17, 5:55 PM

Herald added subscribers: bcran, rgrimes, imp. · View Herald TranscriptMon, Aug 17, 5:55 PM

jhb requested review of this revision.Mon, Aug 17, 5:55 PM

Harbormaster completed remote builds in B75653: Diff 184208.Mon, Aug 17, 5:55 PM

jhb added a child revision: D58894: bhyve: Tidy lobits handling in pci_passthru.Mon, Aug 17, 5:55 PM

jhb added a child revision: D58579: bhyve: Return void from pci_emul_alloc_bar.Mon, Aug 17, 5:56 PM

jhb removed a child revision: D58579: bhyve: Return void from pci_emul_alloc_bar.

jhb added a parent revision: D58579: bhyve: Return void from pci_emul_alloc_bar.

jhb added inline comments.

usr.sbin/bhyve/pci_emul.c
869	@bnovkov I think once this change is in place, all you would need to do here for the HotPlug case is return here before adding the BAR to the linked list or enabling decoding in the command register.

jhb mentioned this in D54641: bhyve/pci_emul: Use vmem to track BAR allocations.Wed, Aug 19, 5:42 PM

bnovkov added inline comments.Fri, Aug 21, 1:51 PM

usr.sbin/bhyve/pci_emul.c
921	I can't find the `PCIM_BAR_MEM_PREFETCH` flag in the reworked code, is this intentional?

jhb added inline comments.Fri, Aug 21, 6:57 PM

usr.sbin/bhyve/pci_emul.c
921	Bleh, I think this is a bug in the old code. If a device model needs this, it should request it explicitly (and we should update the API of pci_emul_alloc_bar() to take a bool or flag to request prefetch, or maybe define a new PCI_MEM64_PREFETCH BAR type or some such). However, I don't think any of the software device models request a large enough BAR. Only nvme(4) allocates a MEM64 BAR, and the size of that BAR is capped to (0x1008 + (8 * (16 + 1)) which is far less than 128 MB. The PCI pass through device model overwrites the lobits value with the "real" value (precisely to preserve the PREFETCH flag), so it would never use the flag set here.

bnovkov accepted this revision.Sun, Aug 23, 1:07 PM

bnovkov added inline comments.

usr.sbin/bhyve/pci_emul.c
921	Bleh, I think this is a bug in the old code. If a device model needs this, it should request it explicitly (and we should update the API of pci_emul_alloc_bar() to take a bool or flag to request prefetch, or maybe define a new PCI_MEM64_PREFETCH BAR type or some such). However, I don't think any of the software device models request a large enough BAR. Only nvme(4) allocates a MEM64 BAR, and the size of that BAR is capped to (0x1008 + (8 * (16 + 1)) which is far less than 128 MB. That makes sense, I just wanted to make sure that it's safe to remove this.