Page MenuHomeFreeBSD

vmm: Avoid embedding cpuset_t ioctl ABIs
ClosedPublic

Authored by markj on May 15 2023, 9:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 12 2024, 5:00 PM
Unknown Object (File)
Mar 12 2024, 5:00 PM
Unknown Object (File)
Mar 8 2024, 11:05 AM
Unknown Object (File)
Mar 8 2024, 11:04 AM
Unknown Object (File)
Mar 8 2024, 8:34 AM
Unknown Object (File)
Jan 4 2024, 7:05 AM
Unknown Object (File)
Dec 20 2023, 1:16 AM
Unknown Object (File)
Dec 12 2023, 5:49 AM

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.May 15 2023, 9:04 PM
This revision is now accepted and ready to land.May 17 2023, 1:36 PM
jhb added inline comments.
sys/amd64/include/vmm.h
758

It's really tempting to remove the dmask pointer entirely. My one suggestion here removes the remaining in-kernel use. For userspace you might just need to pass the 'struct vm_run` to the exit handlers instead of vm_exit and then you could kill this entirely.

sys/amd64/vmm/io/vlapic.c
1149–1150
sys/amd64/vmm/vmm_dev.c
97

Maybe 's/old/13'? E.g. vm_exit_ipi_13 and VM_RUN_13?

620

I would do this fixup in libvmmapi perhaps rather than in the kernel.

usr.sbin/bhyve/bhyverun.c
1008

I don't think you need the zero? We don't memset vme to 0 here either.

markj marked 3 inline comments as done.
  • Move cpuset_t entirely out of struct vm_exit.
  • Modify bhyve exitcode handlers to take a vmrun structure.
  • Rename compat ioctls/structures to use a more precise suffix.
This revision now requires review to proceed.May 23 2023, 7:00 PM

Thanks for doing the other changes.

usr.sbin/bhyve/bhyverun.c
1008

So I think I was wrong, you do need the CPU_ZERO, but maybe with a comment to explain why. Doing the zero here ensures that if the kernel is using a smaller cpuset_t size than userspace, that the kernel doesn't have to explicitly zero out the extra bits each time. Otherwise you will need to patch the VM_RUN ioctl in the kernel to copy out zeroes to the "extra" cpuset_t bits.

This revision is now accepted and ready to land.May 23 2023, 9:15 PM
sys/amd64/include/vmm.h
758

Yeah, that's a better idea. I didn't do that initially just out of laziness.

sys/amd64/vmm/vmm_dev.c
97

"old" is what we say elsewhere, but yeah it's better to be specific.

usr.sbin/bhyve/bhyverun.c
1008

Since userspace is doing the work to provide us with its view of the cpusetsize anyway, I think it's a bit tidier if the kernel takes take of zeroing the rest. That's what cpuset syscalls do too.

This revision was automatically updated to reflect the committed changes.