Bug 271330 - struct vm_exit requires userland cpuset_t to be MAXCPU-sized
Summary: struct vm_exit requires userland cpuset_t to be MAXCPU-sized
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Mark Johnston
URL:
Keywords:
Depends on:
Blocks: 269572
  Show dependency treegraph
 
Reported: 2023-05-09 13:53 UTC by Ed Maste
Modified: 2023-05-24 01:19 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Maste freebsd_committer freebsd_triage 2023-05-09 13:53:13 UTC
In 76887e84be97 I increased the userland maximum size for cpuset_t to 1024 cores, but corvink reported it broke bhyve

> vmm kernel module and bhyve userland are sharing a cpuset_t. So, this
> commit introduces a mismatch in the ioctl signature of VM_RUN.

mjg adds:

> On one hand this really should not be a part of the abi (you could
> have a pointer to cpuset and a a var stating the size).
>
> On the other hand, i suspect the least problematic long term solution
> is to introduce usercpuset_t (or whatever the name) and have a
> conversion func on import. This would still mean you need a kernel and
> user variant of the vm_exit struct.
Comment 1 commit-hook freebsd_committer freebsd_triage 2023-05-09 17:05:17 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ea6dd3d1d4519c2798f7417c826afd6739b84383

commit ea6dd3d1d4519c2798f7417c826afd6739b84383
Author:     Ed Maste <emaste@FreeBSD.org>
AuthorDate: 2023-05-09 13:40:27 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-05-09 17:05:00 +0000

    Revert "cpuset: increase userland maximum size to 1024"

    This reverts commit 76887e84be975698b14699d7d0dfb157d73e9990.

    struct vm_exit currently requires that cpuset_t be identical in userland
    and kernel.  This will be recommitted after these are decoupled.

    PR:             271330, 269572
    Reported by:    corvink
    Sponsored by:   The FreeBSD Foundation

 sys/sys/_cpuset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2023-05-23 20:03:48 UTC
https://reviews.freebsd.org/D40113
Comment 3 commit-hook freebsd_committer freebsd_triage 2023-05-24 01:18:12 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e17eca327633efc511450310afb5e4a662724e3d

commit e17eca327633efc511450310afb5e4a662724e3d
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-05-24 01:13:33 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-05-24 01:15:59 +0000

    vmm: Avoid embedding cpuset_t ioctl ABIs

    Commit 0bda8d3e9f7a ("vmm: permit some IPIs to be handled by userspace")
    embedded cpuset_t into the vmm(4) ioctl ABI.  This was a mistake since
    we otherwise have some leeway to change the cpuset_t for the whole
    system, but we want to keep the vmm ioctl ABI stable.

    Rework IPI reporting to avoid this problem.  Along the way, make VM_RUN
    a bit more efficient:
    - Split vmexit metadata out of the main VM_RUN structure.  This data is
      only written by the kernel.
    - Have userspace pass a cpuset_t pointer and cpusetsize in the VM_RUN
      structure, as is done for cpuset syscalls.
    - Have the destination CPU mask for VM_EXITCODE_IPIs live outside the
      vmexit info structure, and make VM_RUN copy it out separately.  Zero
      out any extra bytes in the CPU mask, like cpuset syscalls do.
    - Modify the vmexit handler prototype to take a full VM_RUN structure.

    PR:             271330
    Reviewed by:    corvink, jhb (previous versions)
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D40113

 lib/libvmmapi/vmmapi.c       | 11 +----
 lib/libvmmapi/vmmapi.h       |  2 +-
 sys/amd64/include/vmm.h      |  9 +++-
 sys/amd64/include/vmm_dev.h  |  6 ++-
 sys/amd64/vmm/io/vlapic.c    |  4 +-
 sys/amd64/vmm/vmm.c          | 11 +++--
 sys/amd64/vmm/vmm_dev.c      | 98 ++++++++++++++++++++++++++++++++++++++++++--
 usr.sbin/bhyve/bhyverun.c    | 91 +++++++++++++++++++++++++---------------
 usr.sbin/bhyve/bhyverun.h    |  4 +-
 usr.sbin/bhyve/task_switch.c |  4 +-
 10 files changed, 182 insertions(+), 58 deletions(-)