Page MenuHomeFreeBSD

bhyve: add empty GVT-d emulation
ClosedPublic

Authored by corvink on May 10 2023, 12:45 PM.
Tags
None
Referenced Files
F99351789: D40038.diff
Tue, Oct 8, 5:04 PM
Unknown Object (File)
Mon, Oct 7, 2:29 PM
Unknown Object (File)
Mon, Oct 7, 2:29 PM
Unknown Object (File)
Sun, Oct 6, 2:37 AM
Unknown Object (File)
Sat, Oct 5, 1:56 PM
Unknown Object (File)
Sat, Oct 5, 11:36 AM
Unknown Object (File)
Fri, Oct 4, 8:55 PM
Unknown Object (File)
Fri, Oct 4, 12:13 PM
Subscribers

Details

Summary

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

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.

  • use own header for GVT-d
  • fix style issues
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.

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.

This revision is now accepted and ready to land.Jun 14 2023, 1:15 PM
  • use linker set for probing passthru dev
This revision now requires review to proceed.Jun 15 2023, 6:34 AM
  • remove spurious gvt_d_match definition from passthru.h
This revision is now accepted and ready to land.Jun 15 2023, 3:59 PM
This revision was automatically updated to reflect the committed changes.