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)
Mon, Nov 11, 6:08 PM
Unknown Object (File)
Sat, Nov 9, 8:41 AM
Unknown Object (File)
Fri, Nov 8, 8:53 AM
Unknown Object (File)
Thu, Oct 31, 5:25 PM
Unknown Object (File)
Sun, Oct 20, 5:44 PM
Unknown Object (File)
Oct 8 2024, 10:33 AM
Unknown Object (File)
Oct 7 2024, 10:29 PM
Unknown Object (File)
Oct 2 2024, 6:17 AM

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 51650
Build 48541: arc lint + arc unit

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.