Page MenuHomeFreeBSD

bhyve: set lpc IDs to physical values
Needs ReviewPublic

Authored by c.koehne_beckhoff.com on Jan 22 2021, 10:59 AM.

Details

Reviewers
manu
markj
jhb
Group Reviewers
bhyve
Summary

The VID, DID, REVID, SUBVID and SUBDID of igd-lpc need aligned with physical one. Without these physical values, GVT-d GOP driver couldn't work.

Note: See https://github.com/Beckhoff/freebsd-src/commits/phab/corvink/lpc-id

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

markj added inline comments.
usr.sbin/bhyve/pci_lpc.c
504

Please limit capabilities on the descriptor, see the use of caph_rights_limit() and caph_ioctls_limit() in passthru_init(). Actually I do not see why the descriptor needs to be kept open indefinitely.

usr.sbin/bhyve/pci_lpc.c
93

I'd prefer to pass the descriptor as a parameter rather than introducing a global variable.

518

Sorry, I was not clear. It's not really worthwhile to limit rights if the descriptor is not kept open. Since it's closed right after this, the caph_* calls can be dropped IMO.

jhb added inline comments.
usr.sbin/bhyve/pci_lpc.c
527

In general I'd like some way to not force this on always perhaps? Or at least provide a configuration knob to allow it to be turned off in case it breaks other systems that don't need GVT-d.

Ideally I think what I would like is for the lpc device to honor config values to set these registers and have a hook here that sets the config variables if they aren't always set (so the user can always override them if needed).

c.koehne_beckhoff.com added reviewers: markj, jhb.
  • reuse read_config of pci_passthru
  • allow users to overwrite LPC ids

I've split my patch into multiple commits at https://github.com/Beckhoff/freebsd-src/commits/phab/corvink/lpc-id. It may be easier to review these small commits than the whole patch.

Btw: I can also create a pull request at https://github.com/Beckhoff/freebsd-src and we can talk in that pull request about the commits if you like to.

c.koehne_beckhoff.com added inline comments.
usr.sbin/bhyve/pci_lpc.c
527

What do you think about the current solution? It allows the user to override all values if he wants.