Actually make use of bhyve and viona API versions

Both bhyve (the kernel vmm portion) and viona expose version information
as it relates to their respective ioctl APIs.  Although there are few
(read: none) guarantees about one can depend on when it comes to
inferring compatibility from said versions, it does provide some modicum
of protection against weird behavior when expectations of those
interfaces differ between propolis and the underlying OS.

Author: pfmooney

Cargo.lock

@@ -2,7 +2,7 @@
 #![cfg_attr(usdt_need_asm, feature(asm))]
 #![cfg_attr(all(target_os = "macos", usdt_need_asm_sym), feature(asm_sym))]
 
-use anyhow::anyhow;
+use anyhow::{anyhow, Context};
 use clap::Parser;
 use dropshot::{
     ConfigDropshot, ConfigLogging, ConfigLoggingLevel, HttpServerStarter,
@@ -86,6 +86,9 @@ async fn main() -> anyhow::Result<()> {
                 |error| anyhow!("failed to create logger: {}", error),
             )?;
 
+            // Check that devices conform to expected API version
+            propolis::api_version::check().context("API version checks")?;
+
             let vnc_server = setup_vnc(&log, vnc_addr);
             let vnc_server_hdl = vnc_server.clone();
             let use_reservoir = config::reservoir_decide(&log);

bin/propolis-server/src/main.rs

@@ -863,6 +863,31 @@ fn setup_instance(
     Ok((inst, com1_sock))
 }
 
+/// Check bhyve and viona API versions, squawking if they do not meet
+/// expectations, but ultimately still allowing forward progress since
+/// propolis-standalone lives in the Thunderdome.
+fn api_version_checks(log: &slog::Logger) -> std::io::Result<()> {
+    match api_version::check() {
+        Err(api_version::Error::Io(e)) => {
+            // IO errors _are_ fatal
+            Err(e)
+        }
+        Err(api_version::Error::Mismatch(comp, act, exp)) => {
+            // Make noise about version mismatch, but soldier on and let the
+            // user decide if they want to quit
+            slog::error!(
+                log,
+                "{} API version mismatch {} != {}",
+                comp,
+                act,
+                exp
+            );
+            Ok(())
+        }
+        Ok(_) => Ok(()),
+    }
+}
+
 #[derive(clap::Parser)]
 /// Propolis command-line frontend for running a VM.
 struct Args {
@@ -887,6 +912,9 @@ fn main() -> anyhow::Result<()> {
 
     let (log, _log_async_guard) = build_log();
 
+    // Check that vmm and viona device version match what we expect
+    api_version_checks(&log).context("API version checks")?;
+
     // Create tokio runtime, we don't use the tokio::main macro
     // since we'll block in main when we call `Instance::wait_for_state`
     let rt =

bin/propolis-standalone/src/main.rs

@@ -9,3 +9,4 @@ edition = "2018"
 [dependencies]
 libc.workspace = true
 bhyve_api_sys.workspace = true
+num_enum.workspace = true

crates/bhyve-api/Cargo.toml

@@ -4,6 +4,8 @@ use std::os::fd::*;
 use std::os::unix::fs::OpenOptionsExt;
 use std::path::PathBuf;
 
+use num_enum::IntoPrimitive;
+
 pub use bhyve_api_sys::*;
 
 pub const VMM_PATH_PREFIX: &str = "/dev/vmm";
@@ -43,6 +45,16 @@ impl VmmCtlFd {
         unsafe { ioctl(self.as_raw_fd(), cmd, data as *mut libc::c_void) }
     }
 
+    /// Query the API version exposed by the kernel VMM.
+    pub fn api_version(&self) -> Result<u32> {
+        let vers = self.ioctl_usize(ioctls::VMM_INTERFACE_VERSION, 0)?;
+
+        // We expect and demand a positive version number from the
+        // VMM_INTERFACE_VERSION interface.
+        assert!(vers > 0);
+        Ok(vers as u32)
+    }
+
     /// Check VMM ioctl command against those known to not require any
     /// copyin/copyout to function.
     const fn ioctl_usize_safe(cmd: i32) -> bool {
@@ -151,3 +163,43 @@ unsafe fn ioctl(
 ) -> Result<i32> {
     Err(Error::new(ErrorKind::Other, "illumos required"))
 }
+
+/// Convenience constants to provide some documentation on what changes have
+/// been introduced in the various bhyve API versions.
+#[repr(u32)]
+#[derive(IntoPrimitive)]
+pub enum ApiVersion {
+    /// Revamps ioctls for administrating the VMM memory reservoir and adds
+    /// kstat for tracking its capacity and utilization.
+    V9 = 9,
+
+    /// Adds flag to enable dirty page tracking for VMs when running on hardware
+    /// with adequate support.
+    V8 = 8,
+
+    /// Adds pause/resume ioctls to assist with the ability to load or store a
+    /// consistent snapshot of VM state
+    V7 = 7,
+
+    /// Made hlt-on-exit a required CPU feature, and enabled by default in vmm
+    V6 = 6,
+
+    /// Adds ability to control CPUID results for guest vCPUs
+    V5 = 5,
+}
+impl ApiVersion {
+    pub const fn current() -> Self {
+        Self::V9
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn latest_api_version() {
+        let cur = ApiVersion::current();
+        assert_eq!(VMM_CURRENT_INTERFACE_VERSION, cur.into());
+    }
+}

crates/bhyve-api/src/lib.rs

@@ -13,8 +13,7 @@ pub const VMM_INTERFACE_VERSION: i32 = VMMCTL_IOC_BASE | 0x04;
 
 // VMM memory reservoir operations
 pub const VMM_RESV_QUERY: i32 = VMMCTL_IOC_BASE | 0x10;
-pub const VMM_RESV_ADD: i32 = VMMCTL_IOC_BASE | 0x11;
-pub const VMM_RESV_REMOVE: i32 = VMMCTL_IOC_BASE | 0x12;
+pub const VMM_RESV_SET_TARGET: i32 = VMMCTL_IOC_BASE | 0x11;
 
 // Operations performed in the context of a given vCPU
 pub const VM_RUN: i32 = VMM_CPU_IOC_BASE | 0x01;

crates/bhyve-api/sys/src/ioctls.rs

@@ -10,7 +10,7 @@ pub use vmm_data::*;
 
 pub const VM_MAXCPU: usize = 32;
 
-/// This is the VMM interface version against which bhyve_api expects to operate
+/// This is the VMM interface version which bhyve_api expects to operate
 /// against.  All constants and structs defined by the crate are done so in
 /// terms of that specific version.
-pub const VMM_CURRENT_INTERFACE_VERSION: u32 = 8;
+pub const VMM_CURRENT_INTERFACE_VERSION: u32 = 9;

crates/bhyve-api/sys/src/lib.rs

@@ -9,3 +9,4 @@ edition = "2018"
 [dependencies]
 libc.workspace = true
 viona_api_sys.workspace = true
+num_enum.workspace = true

crates/viona-api/Cargo.toml

@@ -2,23 +2,32 @@ use std::fs::{File, OpenOptions};
 use std::io::{Error, ErrorKind, Result};
 use std::os::fd::*;
 
+use num_enum::IntoPrimitive;
+
 pub use viona_api_sys::*;
 
 pub const VIONA_DEV_PATH: &str = "/dev/viona";
 
 pub struct VionaFd(File);
 impl VionaFd {
+    /// Open viona device and associate it with a given link and vmm instance,
+    /// provided in `link_id` and `vm_fd`, respectively.
     pub fn new(link_id: u32, vm_fd: RawFd) -> Result<Self> {
-        let fp =
-            OpenOptions::new().read(true).write(true).open(VIONA_DEV_PATH)?;
-
-        let this = Self(fp);
+        let this = Self::open()?;
 
         let mut vna_create = vioc_create { c_linkid: link_id, c_vmfd: vm_fd };
         let _ = unsafe { this.ioctl(ioctls::VNA_IOC_CREATE, &mut vna_create) }?;
         Ok(this)
     }
 
+    /// Open viona device instance without performing any other initialization
+    pub fn open() -> Result<Self> {
+        let fp =
+            OpenOptions::new().read(true).write(true).open(VIONA_DEV_PATH)?;
+
+        Ok(Self(fp))
+    }
+
     /// Issue ioctl against open viona instance
     ///
     /// # Safety
@@ -44,6 +53,16 @@ impl VionaFd {
         unsafe { ioctl(self.as_raw_fd(), cmd, data as *mut libc::c_void) }
     }
 
+    /// Query the API version exposed by the kernel VMM.
+    pub fn api_version(&self) -> Result<u32> {
+        let vers = self.ioctl_usize(ioctls::VNA_IOC_VERSION, 0)?;
+
+        // We expect and demand a positive version number from the
+        // VNA_IOC_VERSION interface.
+        assert!(vers > 0);
+        Ok(vers as u32)
+    }
+
     /// Check VMM ioctl command against those known to not require any
     /// copyin/copyout to function.
     const fn ioctl_usize_safe(cmd: i32) -> bool {
@@ -54,6 +73,7 @@ impl VionaFd {
                 | ioctls::VNA_IOC_RING_KICK
                 | ioctls::VNA_IOC_RING_PAUSE
                 | ioctls::VNA_IOC_RING_INTR_CLR
+                | ioctls::VNA_IOC_VERSION
         )
     }
 }
@@ -79,3 +99,31 @@ unsafe fn ioctl(
 ) -> Result<i32> {
     Err(Error::new(ErrorKind::Other, "illumos required"))
 }
+
+/// Convenience constants to provide some documentation on what changes have
+/// been introduced in the various viona API versions.
+#[repr(u32)]
+#[derive(IntoPrimitive)]
+pub enum ApiVersion {
+    /// Adds support for non-vnic datalink devices
+    V2 = 2,
+
+    /// Initial version available for query
+    V1 = 1,
+}
+impl ApiVersion {
+    pub const fn current() -> Self {
+        Self::V2
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn latest_api_version() {
+        let cur = ApiVersion::current();
+        assert_eq!(VIONA_CURRENT_INTERFACE_VERSION, cur.into());
+    }
+}

crates/viona-api/src/lib.rs

@@ -65,5 +65,10 @@ mod structs {
     }
 }
 
+/// This is the viona interface version which viona_api expects to operate
+/// against.  All constants and structs defined by the crate are done so in
+/// terms of that specific version.
+pub const VIONA_CURRENT_INTERFACE_VERSION: u32 = 2;
+
 pub use ioctls::*;
 pub use structs::*;

crates/viona-api/sys/src/lib.rs

@@ -0,0 +1,15 @@
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    #[error("IO Error")]
+    Io(#[from] std::io::Error),
+
+    #[error("{0} API version {1} did not match expectation {2}")]
+    Mismatch(&'static str, u32, u32),
+}
+
+pub fn check() -> Result<(), Error> {
+    crate::vmm::check_api_version()?;
+    crate::hw::virtio::viona::check_api_version()?;
+
+    Ok(())
+}

lib/propolis/src/api_version.rs

@@ -692,3 +692,18 @@ pub(crate) mod bits {
     pub const VIRTIO_NET_CFG_SIZE: usize = 0xc;
 }
 use bits::*;
+
+/// Check that available viona API matches expectations of propolis crate
+pub(crate) fn check_api_version() -> Result<(), crate::api_version::Error> {
+    let fd = viona_api::VionaFd::open()?;
+    let vers = fd.api_version()?;
+
+    // viona only requires the V2 bits for now
+    let compare = viona_api::ApiVersion::V2.into();
+
+    if vers < compare {
+        Err(crate::api_version::Error::Mismatch("viona", vers, compare))
+    } else {
+        Ok(())
+    }
+}

lib/propolis/src/hw/virtio/viona.rs

@@ -9,6 +9,7 @@ pub extern crate usdt;
 extern crate bitflags;
 
 pub mod accessors;
+pub mod api_version;
 pub mod block;
 pub mod chardev;
 pub mod common;

lib/propolis/src/lib.rs

@@ -8,3 +8,18 @@ pub mod mem;
 pub use hdl::*;
 pub use machine::*;
 pub use mem::*;
+
+/// Check that available vmm API matches expectations of propolis crate
+pub(crate) fn check_api_version() -> Result<(), crate::api_version::Error> {
+    let ctl = bhyve_api::VmmCtlFd::open()?;
+    let vers = ctl.api_version()?;
+
+    // propolis only requires the bits provided by V8, currently
+    let compare = bhyve_api::ApiVersion::V8.into();
+
+    if vers < compare {
+        return Err(crate::api_version::Error::Mismatch("vmm", vers, compare));
+    }
+
+    Ok(())
+}