Page MenuHomeFreeBSD

bhyve: add ioctl to query infos about special memory regions
Needs ReviewPublic

Authored by c.koehne_beckhoff.com on Nov 16 2021, 1:39 PM.

Details

Reviewers
manu
markj
jhb
Group Reviewers
bhyve
Summary

To passthrough some devices like Intel GPUs or TPM2 devices, bhyve needs to map some special memory regions into the guest memory space. Add an ioctl to get informations about those memory regions. These information are used by bhyve to properly pass those regions to the guest.

See also:
D26209
D32961

Diff Detail

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

Event Timeline

Order SRC in Makefile alphabetically

I don't really like this interface. The name is too general and the interface is kind of simplistic, for example because it can only describe a single contiguous memory region and can't refer to different instances of a device. I'm not what a better interface would look like, I haven't yet read more of the patch series. But I think this deserves some more thought.

lib/libvmmapi/vmmapi.h
184

The whitespace looks wrong here.

sys/amd64/include/vmm_dev.h
158

Why does it need to be here?

sys/amd64/vmm/io/acpi.c
9

Don't need to add this to new files anymore.

33

Why is the size always zero?

sys/amd64/vmm/vmm_dev.c
551

This should be handled in intelgpu.c.

sys/dev/pci/pcireg.h
1111

I don't think this is the right place for these. AFAIK we don't have a centralized PCI ID like Linux does, which is a bit unfortunate, but I don't think stashing a few random IDs here is really helpful.

Maybe we could start with sys/dev/pci/pciids.h or so and deduplicate existing definitions of PCI_VENDOR_INTEL in the tree. But that should be a separate change and I'd defer to @jhb on the idea.

bz added inline comments.
sys/dev/pci/pcireg.h
1111

I'd love to see that happen; this is outside this change really; for USB we do generate them from usbdevs; for PCI the file lives solely in user space...

I don't really like this interface. The name is too general and the interface is kind of simplistic, for example because it can only describe a single contiguous memory region and can't refer to different instances of a device. I'm not what a better interface would look like, I haven't yet read more of the patch series. But I think this deserves some more thought.

MEMORY_REGION_INTEL_GSM and MEMORY_REGION_INTEL_OPREGION could be calculated in user space too. However, sys/x86/pci/pci_early_quirks.c already calculates the GSM address and size. I wanna avoid reimplementing it. For MEMORY_REGION_TPM_CONTROL_ADDRESS ACPI tables needs to be read. I'm unsure whether it's possible to do it in user space or not.

It would be ok for me to implement multiple different ioctls if that's a better interface.