Project

General

Profile

Feature #12871

bhyve ioctls needn't the struct size

Added by Patrick Mooney 6 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Category:
bhyve
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
bhyve
Gerrit CR:

Description

The bhyve ioctl numbers, as defined from the FreeBSD sources, communicate the size of the underlying struct in the upper bits of the ID. FreeBSD's ioctl() plumbing uses this for automatic copyin/copyout logic, but with our artisanal hand-crafted handling for that in vmm_sol_dev.c, we have no such need to couple the ioctl IDs to struct sizing. Defining them with a more simple expression would make it easier for potential non-C consumers to utilize the ioctls directly.

#1

Updated by Patrick Mooney 6 months ago

I banged together a test program to emit the ioctl names and numbers as they currently exist in the header to cross-check between old and new code.

#include <stdio.h>
#include <sys/ioccom.h>
#include <sys/types.h>
#include <sys/param.h>
#include <sys/module.h>
#include <sys/_types.h>
#include <sys/cpuset.h>

#include <machine/specialreg.h>
#include <machine/vmm.h>
#include <machine/vmm_dev.h>
#include <sys/vmm_impl.h>

int main()
{
        printf("VM_RUN %x\n", VM_RUN);
        printf("VM_SUSPEND %x\n", VM_SUSPEND);
        printf("VM_REINIT %x\n", VM_REINIT);
        printf("VM_ALLOC_MEMSEG_FBSD12 %x\n", VM_ALLOC_MEMSEG_FBSD12);
        printf("VM_ALLOC_MEMSEG %x\n", VM_ALLOC_MEMSEG);
        printf("VM_GET_MEMSEG_FBSD12 %x\n", VM_GET_MEMSEG_FBSD12);
        printf("VM_GET_MEMSEG %x\n", VM_GET_MEMSEG);
        printf("VM_MMAP_MEMSEG %x\n", VM_MMAP_MEMSEG);
        printf("VM_MMAP_GETNEXT %x\n", VM_MMAP_GETNEXT);
        printf("VM_SET_REGISTER %x\n", VM_SET_REGISTER);
        printf("VM_GET_REGISTER %x\n", VM_GET_REGISTER);
        printf("VM_SET_SEGMENT_DESCRIPTOR %x\n", VM_SET_SEGMENT_DESCRIPTOR);
        printf("VM_GET_SEGMENT_DESCRIPTOR %x\n", VM_GET_SEGMENT_DESCRIPTOR);
        printf("VM_SET_REGISTER_SET %x\n", VM_SET_REGISTER_SET);
        printf("VM_GET_REGISTER_SET %x\n", VM_GET_REGISTER_SET);
        printf("VM_SET_KERNEMU_DEV %x\n", VM_SET_KERNEMU_DEV);
        printf("VM_GET_KERNEMU_DEV %x\n", VM_GET_KERNEMU_DEV);
        printf("VM_INJECT_EXCEPTION %x\n", VM_INJECT_EXCEPTION);
        printf("VM_LAPIC_IRQ %x\n", VM_LAPIC_IRQ);
        printf("VM_LAPIC_LOCAL_IRQ %x\n", VM_LAPIC_LOCAL_IRQ);
        printf("VM_LAPIC_MSI %x\n", VM_LAPIC_MSI);
        printf("VM_IOAPIC_ASSERT_IRQ %x\n", VM_IOAPIC_ASSERT_IRQ);
        printf("VM_IOAPIC_DEASSERT_IRQ %x\n", VM_IOAPIC_DEASSERT_IRQ);
        printf("VM_IOAPIC_PULSE_IRQ %x\n", VM_IOAPIC_PULSE_IRQ);
        printf("VM_IOAPIC_PINCOUNT %x\n", VM_IOAPIC_PINCOUNT);
        printf("VM_ISA_ASSERT_IRQ %x\n", VM_ISA_ASSERT_IRQ);
        printf("VM_ISA_DEASSERT_IRQ %x\n", VM_ISA_DEASSERT_IRQ);
        printf("VM_ISA_PULSE_IRQ %x\n", VM_ISA_PULSE_IRQ);
        printf("VM_ISA_SET_IRQ_TRIGGER %x\n", VM_ISA_SET_IRQ_TRIGGER);
        printf("VM_SET_CAPABILITY %x\n", VM_SET_CAPABILITY);
        printf("VM_GET_CAPABILITY %x\n", VM_GET_CAPABILITY);
        printf("VM_BIND_PPTDEV %x\n", VM_BIND_PPTDEV);
        printf("VM_UNBIND_PPTDEV %x\n", VM_UNBIND_PPTDEV);
        printf("VM_MAP_PPTDEV_MMIO %x\n", VM_MAP_PPTDEV_MMIO);
        printf("VM_PPTDEV_MSI %x\n", VM_PPTDEV_MSI);
        printf("VM_PPTDEV_MSIX %x\n", VM_PPTDEV_MSIX);
        printf("VM_GET_PPTDEV_LIMITS %x\n", VM_GET_PPTDEV_LIMITS);
        printf("VM_INJECT_NMI %x\n", VM_INJECT_NMI);
        printf("VM_STATS_IOC %x\n", VM_STATS_IOC);
        printf("VM_STAT_DESC %x\n", VM_STAT_DESC);
        printf("VM_SET_X2APIC_STATE %x\n", VM_SET_X2APIC_STATE);
        printf("VM_GET_X2APIC_STATE %x\n", VM_GET_X2APIC_STATE);
        printf("VM_GET_HPET_CAPABILITIES %x\n", VM_GET_HPET_CAPABILITIES);
        printf("VM_SET_TOPOLOGY %x\n", VM_SET_TOPOLOGY);
        printf("VM_GET_TOPOLOGY %x\n", VM_GET_TOPOLOGY);
        printf("VM_GET_GPA_PMAP %x\n", VM_GET_GPA_PMAP);
        printf("VM_GLA2GPA %x\n", VM_GLA2GPA);
        printf("VM_GLA2GPA_NOFAULT %x\n", VM_GLA2GPA_NOFAULT);
        printf("VM_ACTIVATE_CPU %x\n", VM_ACTIVATE_CPU);
        printf("VM_GET_CPUS %x\n", VM_GET_CPUS);
        printf("VM_SUSPEND_CPU %x\n", VM_SUSPEND_CPU);
        printf("VM_RESUME_CPU %x\n", VM_RESUME_CPU);
        printf("VM_SET_INTINFO %x\n", VM_SET_INTINFO);
        printf("VM_GET_INTINFO %x\n", VM_GET_INTINFO);
        printf("VM_RTC_WRITE %x\n", VM_RTC_WRITE);
        printf("VM_RTC_READ %x\n", VM_RTC_READ);
        printf("VM_RTC_SETTIME %x\n", VM_RTC_SETTIME);
        printf("VM_RTC_GETTIME %x\n", VM_RTC_GETTIME);
        printf("VM_RESTART_INSTRUCTION %x\n", VM_RESTART_INSTRUCTION);
        printf("VM_DEVMEM_GETOFFSET %x\n", VM_DEVMEM_GETOFFSET);
        printf("VM_WRLOCK_CYCLE %x\n", VM_WRLOCK_CYCLE);
        printf("VMM_CREATE_VM %x\n", VMM_CREATE_VM);
        printf("VMM_DESTROY_VM %x\n", VMM_DESTROY_VM);
        printf("VMM_VM_SUPPORTED %x\n", VMM_VM_SUPPORTED);

        return (0);
}

Which, on the old code, I compiled like this:

gcc -std=c99 -nostdinc -I/$WS/usr/src/uts/i86pc -I$WS/usr/src/compat/bhyve/amd64 -I$WS/usr/src/compat/bhyve -I/$WS/usr/src/contrib/bhyve/amd64 -I$WS/usr/src/contrib/bhyve -I/usr/include -m64 test_bhyve_ioctls.c

getting, on this machine:

VM_RUN c0907601
VM_SUSPEND 80047604
VM_REINIT 20007605
VM_ALLOC_MEMSEG_FBSD12 8050760e
VM_ALLOC_MEMSEG 8010760e
VM_GET_MEMSEG_FBSD12 c050760f
VM_GET_MEMSEG c010760f
VM_MMAP_MEMSEG 80287610
VM_MMAP_GETNEXT c0287611
VM_SET_REGISTER 80107614
VM_GET_REGISTER c0107615
VM_SET_SEGMENT_DESCRIPTOR 80187616
VM_GET_SEGMENT_DESCRIPTOR c0187617
VM_SET_REGISTER_SET 80187618
VM_GET_REGISTER_SET c0187619
VM_SET_KERNEMU_DEV 8018761b
VM_GET_KERNEMU_DEV c018761a
VM_INJECT_EXCEPTION 8014761e
VM_LAPIC_IRQ 8008761f
VM_LAPIC_LOCAL_IRQ 80087625
VM_LAPIC_MSI 80107624
VM_IOAPIC_ASSERT_IRQ 80047621
VM_IOAPIC_DEASSERT_IRQ 80047622
VM_IOAPIC_PULSE_IRQ 80047623
VM_IOAPIC_PINCOUNT 40047626
VM_ISA_ASSERT_IRQ 80087650
VM_ISA_DEASSERT_IRQ 80087651
VM_ISA_PULSE_IRQ 80087652
VM_ISA_SET_IRQ_TRIGGER 80087653
VM_SET_CAPABILITY 80107602
VM_GET_CAPABILITY c0107603
VM_BIND_PPTDEV 80047628
VM_UNBIND_PPTDEV 80047629
VM_MAP_PPTDEV_MMIO 8020762a
VM_PPTDEV_MSI 8020762b
VM_PPTDEV_MSIX 8028762c
VM_GET_PPTDEV_LIMITS 400c762d
VM_INJECT_NMI 80047620
VM_STATS_IOC c0187632
VM_STAT_DESC c0847633
VM_SET_X2APIC_STATE 8008763c
VM_GET_X2APIC_STATE c008763d
VM_GET_HPET_CAPABILITIES 4004763e
VM_SET_TOPOLOGY 8008763f
VM_GET_TOPOLOGY 40087640
VM_GET_GPA_PMAP c030760c
VM_GLA2GPA c038760d
VM_GLA2GPA_NOFAULT c0387612
VM_ACTIVATE_CPU 8004765a
VM_GET_CPUS 8010765b
VM_SUSPEND_CPU 8004765c
VM_RESUME_CPU 8004765d
VM_SET_INTINFO 8018761d
VM_GET_INTINFO c018761c
VM_RTC_WRITE 80087665
VM_RTC_READ c0087664
VM_RTC_SETTIME 80087666
VM_RTC_GETTIME 40087667
VM_RESTART_INSTRUCTION 80047627
VM_DEVMEM_GETOFFSET 80107700
VM_WRLOCK_CYCLE 20007701
VMM_CREATE_VM 564d01
VMM_DESTROY_VM 564d02
VMM_VM_SUPPORTED 564d03

... when awk(1)ed/uniq(1)ed, confirming 64 distinct ioctl IDs.

#2

Updated by Patrick Mooney 6 months ago

When done with the proposed wad, 62 unique IDs are emitted (taking into account the removal of the FreeBSD 12 ABI entities which do not apply on illumos)

VM_RUN 767001
VM_SUSPEND 766d12
VM_REINIT 766c01
VM_ALLOC_MEMSEG 766c05
VM_GET_MEMSEG 766d02
VM_MMAP_MEMSEG 766c06
VM_MMAP_GETNEXT 766d03
VM_SET_REGISTER 767002
VM_GET_REGISTER 767003
VM_SET_SEGMENT_DESCRIPTOR 767004
VM_GET_SEGMENT_DESCRIPTOR 767005
VM_SET_REGISTER_SET 767006
VM_GET_REGISTER_SET 767007
VM_SET_KERNEMU_DEV 767014
VM_GET_KERNEMU_DEV 767015
VM_INJECT_EXCEPTION 767008
VM_LAPIC_IRQ 766d04
VM_LAPIC_LOCAL_IRQ 766d05
VM_LAPIC_MSI 766d06
VM_IOAPIC_ASSERT_IRQ 766d07
VM_IOAPIC_DEASSERT_IRQ 766d08
VM_IOAPIC_PULSE_IRQ 766d09
VM_IOAPIC_PINCOUNT 766d13
VM_ISA_ASSERT_IRQ 766d0a
VM_ISA_DEASSERT_IRQ 766d0b
VM_ISA_PULSE_IRQ 766d0c
VM_ISA_SET_IRQ_TRIGGER 766d0d
VM_SET_CAPABILITY 767009
VM_GET_CAPABILITY 76700a
VM_BIND_PPTDEV 766c02
VM_UNBIND_PPTDEV 766c03
VM_MAP_PPTDEV_MMIO 766c04
VM_PPTDEV_MSI 76700b
VM_PPTDEV_MSIX 76700c
VM_GET_PPTDEV_LIMITS 766d14
VM_INJECT_NMI 766d18
VM_STATS_IOC 766d16
VM_STAT_DESC 766d17
VM_SET_X2APIC_STATE 76700d
VM_GET_X2APIC_STATE 766d19
VM_GET_HPET_CAPABILITIES 766d15
VM_SET_TOPOLOGY 766d1a
VM_GET_TOPOLOGY 766d1b
VM_GET_GPA_PMAP 766d01
VM_GLA2GPA 76700e
VM_GLA2GPA_NOFAULT 76700f
VM_ACTIVATE_CPU 767010
VM_GET_CPUS 766d1c
VM_SUSPEND_CPU 766d1d
VM_RESUME_CPU 766d1e
VM_SET_INTINFO 767011
VM_GET_INTINFO 767012
VM_RTC_WRITE 766d0e
VM_RTC_READ 766d0f
VM_RTC_SETTIME 766d10
VM_RTC_GETTIME 766d11
VM_RESTART_INSTRUCTION 767013
VM_DEVMEM_GETOFFSET 766dff
VM_WRLOCK_CYCLE 766cff
VMM_CREATE_VM 564d01
VMM_DESTROY_VM 564d02
VMM_VM_SUPPORTED 564d03

#3

Updated by Patrick Mooney 6 months ago

I fired up a machine on a platform featuring this patch. There, I smoke tested bhyve to see that a guest could start and run. bhyvectl was also able to query details just fine. To verify that the new ioctl() IDs were in use, I truss(1)@ed the @bhyvectl operation and observed the expected "base" IDs:

...
ioctl(3, (('v'<<16)|('p'<<8)|5), 0xFFFFFC7FFFDF8BD0) = 0
cs desc[0]      0x0000000000000000/0xffffffff/0x0000a09b
write(1, " c s   d e s c [ 0 ]\t 0".., 52)      = 52
ioctl(3, (('v'<<16)|('p'<<8)|5), 0xFFFFFC7FFFDF8BD0) = 0
tr desc[0]      0xffff8e04ffc04000/0x00002087/0x0000008b
write(1, " t r   d e s c [ 0 ]\t 0".., 52)      = 52
ioctl(3, (('v'<<16)|('p'<<8)|5), 0xFFFFFC7FFFDF8BD0) = 0
ldtr desc[0]    0x0000000000000000/0xffffffff/0x0001c000
write(1, " l d t r   d e s c [ 0 ]".., 54)      = 54
ioctl(3, (('v'<<16)|('p'<<8)|5), 0xFFFFFC7FFFDF8BD0) = 0
gdtr[0]         0xffff8e04ffc0c000/0x0000007f
write(1, " g d t r [ 0 ]\t\t 0 x f".., 39)      = 39
ioctl(3, (('v'<<16)|('p'<<8)|5), 0xFFFFFC7FFFDF8BD0) = 0
idtr[0]         0xffffffffff528000/0x00000fff
write(1, " i d t r [ 0 ]\t\t 0 x f".., 39)      = 39
ioctl(3, (('v'<<16)|('p'<<8)|3), 0xFFFFFC7FFFDF8BF0) = 0
cs[0]           0x0010
write(1, " c s [ 0 ]\t\t 0 x 0 0 1".., 14)      = 14
ioctl(3, (('v'<<16)|('p'<<8)|3), 0xFFFFFC7FFFDF8BF0) = 0
ds[0]           0x0000
write(1, " d s [ 0 ]\t\t 0 x 0 0 0".., 14)      = 14
ioctl(3, (('v'<<16)|('p'<<8)|3), 0xFFFFFC7FFFDF8BF0) = 0
es[0]           0x0000
...
write(1, " i p i s   s e n t   t o".., 43)      = 43
ioctl(3, (('v'<<16)|('m'<<8)|23), 0xFFFFFC7FEF2874C0) = 0
ipis sent to vcpu[31]                           0
write(1, " i p i s   s e n t   t o".., 43)      = 43
ioctl(3, (('v'<<16)|('m'<<8)|27), 0xFFFFFC7FFFDF8C58) = 0
cpu_topology:   sockets=4, cores=1, threads=1, maxcpus=32
write(1, " c p u _ t o p o l o g y".., 56)      = 56
...

#4

Updated by Patrick Mooney 5 months ago

Mike Zeller tested in his setup with this (and #12869) applied on top of SmartOS. He booted up an Ubuntu 18.04 VM and exercised various bits like disk and network IO.

#5

Updated by Electric Monk 5 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit e156a47b6ebcf79f0515365c32a22c7ed38dc19b

commit  e156a47b6ebcf79f0515365c32a22c7ed38dc19b
Author: Patrick Mooney <pmooney@pfmooney.com>
Date:   2020-06-23T16:08:41.000Z

    12871 bhyve ioctls needn't the struct size
    Reviewed by: Mike Zeller <mike.zeller@joyent.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF