This is a partial solution, but to fix it properly we need a new version of the VM_RUN ioctl.

One way to do it is to modify struct vm_exit:

struct {
    uint32_t mode;
    uint8_t vector;
    size_t cpusetsize;
    cpuset_t *dmaskp;
} ipi;

(Or perhaps the pointer/cpusetsize should be outside of the union.)

Then, userspace sets dmaskp to point to a locally allocated cpuset before invoking VM_RUN, and vmm has to copyout the IPI mask before returning. In the kernel, you could do something like this:

case VM_RUN: {
    cpuset_t ipimask, *uipimask;
    uipimask = vmrun->ipi.dmaskp;
    vmrun->ipi.dmaskp = &ipimask;
    error = vm_run();
    if (error == 0)
        copyout(&vmrun->ipi.dmaskp, uipimask, min(vmrun->ipi.cpusetsize, sizeof(cpuset_t));

I can split this patch into multiple commits (e.g. one commit which introduces the dmask member to the vcpu struct) if you like to.

In D40056#912304, @markj wrote:

This is a partial solution, but to fix it properly we need a new version of the VM_RUN ioctl.

One way to do it is to modify struct vm_exit:

struct {
    uint32_t mode;
    uint8_t vector;
    size_t cpusetsize;
    cpuset_t *dmaskp;
} ipi;

The union is large enough to hold the additional size_t and cpuset_t. So, we could avoid a new VM_RUN ioctl by extending the struct to:

struct {
  uint32_t mode;
  uint8_t vector;
  __BITSET_DEFINE(, 256) dmask;
  size_t cpusetsize;
  cpuset_t *dmaskp;
} ipi;

I don't like it but it could be worth to avoid the new ioctl. What do you think?

In D40056#912799, @corvink wrote:

The union is large enough to hold the additional size_t and cpuset_t. So, we could avoid a new VM_RUN ioctl by extending the struct to:

struct {
  uint32_t mode;
  uint8_t vector;
  __BITSET_DEFINE(, 256) dmask;
  size_t cpusetsize;
  cpuset_t *dmaskp;
} ipi;

I don't like it but it could be worth to avoid the new ioctl. What do you think?

I don't really like it either. :) I think it is best to hide the compat bits as much as possible, and keep all extra complexity in the ioctl handler. In particular, things like struct vm_run and struct vm_exitinfo should not contain fields that are only used for compat.

Looking at VM_RUN a bit more, it is also a bit messy in general: libvmmapi zeros the exitinfo structure; the exitinfo structure is copied into the kernel but never read; at the end of vm_run() the kernel overwrites the copy with the one in vcpu->exitinfo; the kernel copies the copy back into userspace; libvmmapi copies the exitinfo into a caller-provided buffer. That's a lot of unnecessary copying.

I tried to write a patch to address both issues, since I think a new VM_RUN ioctl is needed anyway. The idea:

• change ipi.dmask to a pointer
• let userspace provide the cpuset buffer and buffer size in the vm_run structure
• add a cpuset to struct vcpu in the kernel, and use that when populating IPI exitinfo
• split struct vm_run so that struct vm_exit is not embedded directly
• avoid unnecessary copying of exit info: vcpu->exitinfo is copied directly out to userspace, and libvmmapi does no copying or zeroing

The diff is bigger but I think it's a bit cleaner. Any thoughts? https://reviews.freebsd.org/D40113