bhyve: add empty GVT-d emulation

[corvink]

Summary

Don't emulate anything yet. Just check if the user would like to pass an
Intel GPU to the guest.

[markj]

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.

[corvink]

• use own header for GVT-d
• fix style issues

[markj]

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.

[corvink]

• rename quirks to dev

[corvink]

@jhb @markj Any additional comments?

[jhb]

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.

[corvink]

• address feedback of jhb

[corvink]

Any additional comments?

[markj]

In D40038#921193, @jhb wrote:

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.

If I understand the suggestion clearly, this will also be nicer when arm64 bhyve supports PCI passthrough, since it will not implement GVT-d.

usr.sbin/bhyve/pci_passthru.c

926

Extra newline here.

usr.sbin/bhyve/pci_passthru.h

29

The indentation here looks wrong.

[corvink]

• use linker set for probing passthru dev

[corvink]

• remove spurious gvt_d_match definition from passthru.h

[jhb]

Thanks.