Don't emulate anything yet. Just check if the user would like to pass an
Intel GPU to the guest.
Details
- Reviewers
jhb markj - Group Reviewers
bhyve - Commits
- rG3b81aa26ab4c: bhyve: add empty GVT-d emulation
rG90c3a1b6629c: bhyve: add empty GVT-d emulation
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
usr.sbin/bhyve/pci_passthru.c | ||
---|---|---|
868 | Missing parens around the return value. | |
889 | This function has no return value. | |
891 | No need for a return statement here. | |
usr.sbin/bhyve/pci_passthru.h | ||
51 | These should probably go in their own (amd64-specific) header. Mixing them in like this makes it hard to maintain and integrate the arm64 bhyve port. |
usr.sbin/bhyve/pci_passthru.c | ||
---|---|---|
856 | Why "quirks"? This looks like device-specific initialization. Usually, "quirks" are behaviours of certain implementations of a device that require special handling in drivers. passthru_init_dev() or some such seems like a better name. | |
878 | How can sc be NULL? This is not an external function, so this kind of defensive checking is not worth it IMHO. |
It might be nicer to have the logic of which devices pci-gvt_d.c supports be a bit more self-contained, e.g.:
pci_passthru.h:
struct passthru_devinit { int (*init)(struct pci_devinst *const pi, nvlist_t *const nvl); void (*deinit)(struct pci_devinst *const pi); };
Then in passthru_init:
struct passthru_devinit *device_init; device_init = NULL; ... device_init = gvt_d_match(pi); if (device_init != NULL) { error = device_init->init(pi, nvl); if (error != 0) goto done; } return (0); done: if (error) { if (device_init != NULL) device_init->deinit(pi); ...
In pci_gvt-d.c you would make the two new functions static and add the following:
static struct passthru_devinit gvt_d_devinit = { .init = gvt_d_init, .deinit = gvt_d_deinit, }; struct passthru_init * gvt_d_match(struct pci_devinst *const pi) { struct passthru_softc *const sc = pi->pi_arg; const uint16_t vendor = read_config(&sc->psc_sel, PCIR_VENDOR, 0x02); const uint8_t class = read_config(&sc->psc_sel, PCIR_CLASS, 0x01); if (class == PCIC_DISPLAY && vendor == PCI_VENDOR_INTEL) return (&gvt_d_devinit); return (NULL); }
If you ever needed to support more than one of these in the future it would be easy to add a linker-set for the "match" functions and convert the call to gvt_d_match into a walk of the linker set. If you know for certain you will have more than one of these (e.g. if GPU pass through of other devices will need different init/deinit handlers), then I would encourage you go to ahead and do the linker-set approach now.