diff --git a/bin/propolis-cli/src/main.rs b/bin/propolis-cli/src/main.rs index 10a649986..72ba175b4 100644 --- a/bin/propolis-cli/src/main.rs +++ b/bin/propolis-cli/src/main.rs @@ -18,10 +18,9 @@ use futures::{future, SinkExt}; use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag}; use propolis_client::instance_spec::{ BlobStorageBackend, Board, Chipset, Component, CrucibleStorageBackend, - GuestHypervisorInterface, HyperVFeatureFlag, I440Fx, InstanceMetadata, - InstanceProperties, InstanceSpec, InstanceSpecGetResponse, NvmeDisk, - PciPath, QemuPvpanic, ReplacementComponent, SerialPort, SerialPortNumber, - SpecKey, VirtioDisk, + I440Fx, InstanceMetadata, InstanceProperties, InstanceSpec, + InstanceSpecGetResponse, NvmeDisk, PciPath, QemuPvpanic, + ReplacementComponent, SerialPort, SerialPortNumber, SpecKey, VirtioDisk, }; use propolis_client::support::nvme_serial_from_str; use propolis_client::types::{ @@ -198,10 +197,6 @@ struct VmConfig { // cloud_init ISO file #[clap(long, action, conflicts_with = "spec")] cloud_init: Option, - - /// enable Hyper-V compatible enlightenments for this VM - #[clap(long, action)] - hyperv: bool, } fn add_component_to_spec( @@ -330,21 +325,18 @@ impl VmConfig { }) .transpose()?; + let guest_hv_interface = from_toml + .as_ref() + .map(|cfg| cfg.hv_interface.clone()) + .unwrap_or_default(); + let mut spec = InstanceSpec { board: Board { chipset: Chipset::I440Fx(I440Fx { enable_pcie }), cpuid: cpuid_profile, cpus: self.vcpus, memory_mb: self.memory, - guest_hv_interface: if self.hyperv { - GuestHypervisorInterface::HyperV { - features: [HyperVFeatureFlag::ReferenceTsc] - .into_iter() - .collect(), - } - } else { - Default::default() - }, + guest_hv_interface, }, components: Default::default(), smbios: None, diff --git a/bin/propolis-server/src/lib/vm/ensure.rs b/bin/propolis-server/src/lib/vm/ensure.rs index 67612cc5c..3f7d50709 100644 --- a/bin/propolis-server/src/lib/vm/ensure.rs +++ b/bin/propolis-server/src/lib/vm/ensure.rs @@ -95,6 +95,7 @@ use oximeter::types::ProducerRegistry; use oximeter_instruments::kstat::KstatSampler; use propolis::common::DeviceMetadataMap; use propolis::enlightenment::{ + self, bhyve::BhyveGuestInterface, hyperv::{Features as HyperVFeatures, HyperV}, Enlightenment, @@ -496,28 +497,26 @@ async fn initialize_vm_objects( let vmm_log = log.new(slog::o!("component" => "vmm")); - let (guest_hv_interface, guest_hv_lifecycle) = - match &spec.board.guest_hv_interface { - GuestHypervisorInterface::Bhyve => { - let bhyve = Arc::new(BhyveGuestInterface); - let lifecycle = bhyve.clone(); - (bhyve as Arc, lifecycle.as_lifecycle()) - } - GuestHypervisorInterface::HyperV { features } => { - let mut hv_features = HyperVFeatures::default(); - for f in features { - match f { - HyperVFeatureFlag::ReferenceTsc => { - hv_features.reference_tsc = true - } + let guest_hv_interface = match &spec.board.guest_hv_interface { + GuestHypervisorInterface::Bhyve => { + Arc::new(BhyveGuestInterface) as Arc + } + GuestHypervisorInterface::HyperV { features } => { + let mut hv_features = HyperVFeatures::default(); + for f in features { + match f { + HyperVFeatureFlag::ReferenceTsc => { + hv_features.reference_tsc = true } } - - let hyperv = Arc::new(HyperV::new(&vmm_log, hv_features)); - let lifecycle = hyperv.clone(); - (hyperv as Arc, lifecycle.as_lifecycle()) } - }; + + let hv = HyperV::new(&vmm_log, hv_features); + Arc::new(hv) as Arc + } + }; + let guest_hv_lifecycle = + enlightenment::as_lifecycle(Arc::clone(&guest_hv_interface)); // Set up the 'shell' instance into which the rest of this routine will // add components. diff --git a/bin/propolis-standalone/src/config.rs b/bin/propolis-standalone/src/config.rs index 2280a6399..6a0d24353 100644 --- a/bin/propolis-standalone/src/config.rs +++ b/bin/propolis-standalone/src/config.rs @@ -78,6 +78,29 @@ pub struct Main { /// Default: V0 #[serde(default)] pub acpi_variant: AcpiVariant, + + /// The kind of hypervisor interface to present to guests + /// + /// Default: Bhyve + #[serde(default)] + pub hv_interface: HypervisorInterface, +} + +/// The hypervisor interface to present to guest OSes. +/// +/// The variants here correspond to implementations of `Enlightenment`, which +/// may influence many aspects of a VM. Most immediately, different interfaces +/// have different CPUID leaves, but can also support para-virtualized features +/// such as additional hypercalls and MSRs. +#[derive(Clone, Default, Serialize, Deserialize, Debug, PartialEq)] +#[serde(tag = "type", rename_all = "lowercase")] +pub enum HypervisorInterface { + #[default] + Bhyve, + + HyperV { + reference_tsc: bool, + }, } #[derive(Copy, Clone, Debug, Deserialize, Serialize)] diff --git a/bin/propolis-standalone/src/main.rs b/bin/propolis-standalone/src/main.rs index 9e61cca10..721190338 100644 --- a/bin/propolis-standalone/src/main.rs +++ b/bin/propolis-standalone/src/main.rs @@ -24,6 +24,7 @@ use tokio::runtime; use propolis::chardev::{BlockingSource, Sink, Source, UDSock}; use propolis::common::{DeviceMetadataMap, GB, MB}; +use propolis::enlightenment::{bhyve, hyperv, Enlightenment}; use propolis::firmware::{acpi, smbios}; use propolis::hw::chipset::{i440fx, Chipset}; use propolis::hw::ps2::ctrl::PS2Ctrl; @@ -217,6 +218,9 @@ struct Inventory { block: BTreeMap>, } impl Inventory { + fn register_dyn(&mut self, dev: Arc) { + self.devs.insert(dev.type_name().into(), dev); + } fn register(&mut self, dev: &Arc) { self.devs.insert( dev.type_name().into(), @@ -298,6 +302,10 @@ impl Instance { let state = &mut *state_guard; let machine = state.machine.as_ref().unwrap(); + state.inventory.register_dyn(enlightenment::as_lifecycle(Arc::clone( + &machine.guest_hv_interface, + ))); + let bind_cpus = match this.0.config.main.cpu_binding { Some(config::BindingStrategy::UpperHalf) => { let total_cpus = @@ -768,11 +776,13 @@ impl Instance { } fn build_machine( + log: &slog::Logger, name: &str, max_cpu: u8, lowmem: usize, highmem: usize, use_reservoir: bool, + hv_interface: &config::HypervisorInterface, ) -> Result { let mut builder = Builder::new( name, @@ -800,6 +810,18 @@ fn build_machine( "dev64", )?; + let hv = match hv_interface { + config::HypervisorInterface::Bhyve => { + Arc::new(bhyve::BhyveGuestInterface) as Arc + } + config::HypervisorInterface::HyperV { reference_tsc } => { + let hv_feats = hyperv::Features { reference_tsc: *reference_tsc }; + Arc::new(hyperv::HyperV::new(log, hv_feats)) + as Arc + } + }; + builder = builder.guest_hypervisor_interface(hv); + builder.finalize() } @@ -1152,8 +1174,16 @@ fn setup_instance( slog::info!(log, "Creating VM with {} vCPUs, {} lowmem, {} highmem", cpus, lowmem, highmem;); - let machine = build_machine(vm_name, cpus, lowmem, highmem, use_reservoir) - .context("Failed to create VM Machine")?; + let machine = build_machine( + log, + vm_name, + cpus, + lowmem, + highmem, + use_reservoir, + &config.main.hv_interface, + ) + .context("Failed to create VM Machine")?; let com1_sock = UDSock::bind(Path::new("./ttya")).context("Cannot open UD socket")?; let inst = Instance::new( @@ -1537,7 +1567,7 @@ fn setup_instance( guard.inventory.register(&ramfb); for vcpu in machine.vcpus.iter() { - let vcpu_profile = if let Some(profile) = cpuid_profile.as_ref() { + let mut vcpu_profile = if let Some(profile) = cpuid_profile.as_ref() { propolis::cpuid::Specializer::new() .with_vcpu_count( std::num::NonZeroU8::new(config.main.cpus).unwrap(), @@ -1554,6 +1584,11 @@ fn setup_instance( // fallback behavior cpuid_utils::CpuidSet::new_host() }; + machine + .guest_hv_interface + .add_cpuid(&mut vcpu_profile) + .context("failed to add hypervisor cpuid leaves")?; + vcpu.set_cpuid(vcpu_profile)?; vcpu.set_default_capabs()?; } diff --git a/crates/propolis-config-toml/src/lib.rs b/crates/propolis-config-toml/src/lib.rs index 737f97278..f98820ff6 100644 --- a/crates/propolis-config-toml/src/lib.rs +++ b/crates/propolis-config-toml/src/lib.rs @@ -52,6 +52,23 @@ impl Default for Config { } } +/// The hypervisor interface to present to guest OSes. +/// +/// The variants here correspond to implementations of `Enlightenment`, which +/// may influence many aspects of a VM. Most immediately, different interfaces +/// have different CPUID leaves, but can also support para-virtualized features +/// such as additional hypercalls and MSRs. +#[derive(Clone, Default, Serialize, Deserialize, Debug, PartialEq)] +#[serde(tag = "type", rename_all = "lowercase")] +pub enum HypervisorInterface { + #[default] + Bhyve, + + HyperV { + reference_tsc: bool, + }, +} + /// Settings covering the VM "at large". /// /// This corresponds to (and is a subset of) the `main` block understood by @@ -59,6 +76,48 @@ impl Default for Config { #[derive(Serialize, Deserialize, Debug, PartialEq, Default)] pub struct MachineSettings { pub boot_order: Option>, + // In the fullness of time, one could imagine a list of hypervisor + // interfaces for Propolis to offer up. The premise here would be that each + // hypervisor interface's CPUID leaves are offered up at different offsets + // of 0x100 with the "first" hypervisor being at 0x4000_0000 like normal. + // + // There is some background here: + // * Linux detects hypervisors like above, see `hypervisor_cpuid_bsae`, + // which does this and returns the leaf that matched a given brand string. + // * FreeBSD does the same basic approach at + // `identify_hypervisor_cpuid_base()`, looking for the first hypervisor + // brand string that matches one of the signatures in `vm_cpuids`. + // * illumos in particular takes the above approach to look for the Xen + // signature. + // + // The history seems to be that in 2008, Xen added support for presenting + // Hyper-V enlightenments in support of Windows guests. That's Xen commit + // 39f97ffa2. This also placed the "normal" Xen hypervisor leaves at + // 0x40000100 to query Xen rather than querying the reading back the Hyper- + // leaves lower down. Intel reserves all of 0x4xxxxxxx for hypervisors, + // TODO: AMD may have only reserved 0x400000xx, or maybe it has since become + // more. + // + // In 2010, to detect Xen vs Hyper-V correctly, Linux got a a "check every + // 0x100'th leaf" loop in `xen_cpuid_base()` with commit bee6ab53e6. + // Allegedly a VMWare knowledge base item about detecting virtualization + // platforms also included a mention of checking every 0x100'th leaf for + // hypervisor signatures, but the knowledge base entry has since been lost + // to the sands of time; this is only inference from FreeBSD and mailing + // list posts. + // + // Later, this loop was generalized to all hypervisor detection in Linux and + // things start looking much closer to how they are today. illumos, for its + // part, currently only looks for Xen in the upper hypervisor leaves. + // + // So: one could imagine Hyper-V-then-KVM brand strings, such as QEMU + // presumably offers when providing Hyper-V enlightenments. Should we be + // Hyper-V-then-byhve? Hyper-V-then-KVM-then-bhyve may be technically viable + // too. Other orderings are mostly a test of guest tolerance for *weird + // things*. + // + // That all to say: this could be a list. + pub hv_interface: Option, } /// The instance's chipset. diff --git a/crates/propolis-config-toml/src/spec.rs b/crates/propolis-config-toml/src/spec.rs index 85527fe2e..34ea865e6 100644 --- a/crates/propolis-config-toml/src/spec.rs +++ b/crates/propolis-config-toml/src/spec.rs @@ -4,15 +4,19 @@ //! Functions for converting a [`super::Config`] into instance spec elements. -use std::{collections::BTreeMap, str::FromStr}; +use std::{ + collections::{BTreeMap, BTreeSet}, + str::FromStr, +}; +use crate::HypervisorInterface; use propolis_client::{ instance_spec::{ BootOrderEntry, BootSettings, Component, Cpuid, CpuidVendor, - DlpiNetworkBackend, FileStorageBackend, MigrationFailureInjector, - NvmeDisk, P9fs, PciPath, PciPciBridge, SoftNpuP9, SoftNpuPciPort, - SoftNpuPort, SpecKey, VirtioDisk, VirtioNetworkBackend, VirtioNic, - VirtioSocket, + DlpiNetworkBackend, FileStorageBackend, GuestHypervisorInterface, + HyperVFeatureFlag, MigrationFailureInjector, NvmeDisk, P9fs, PciPath, + PciPciBridge, SoftNpuP9, SoftNpuPciPort, SoftNpuPort, SpecKey, + VirtioDisk, VirtioNetworkBackend, VirtioNic, VirtioSocket, }, support::nvme_serial_from_str, }; @@ -77,6 +81,7 @@ pub enum TomlToSpecError { #[derive(Clone, Debug, Default)] pub struct SpecConfig { pub enable_pcie: bool, + pub hv_interface: GuestHypervisorInterface, pub components: BTreeMap, } @@ -105,6 +110,22 @@ impl TryFrom<&super::Config> for SpecConfig { type Error = TomlToSpecError; fn try_from(config: &super::Config) -> Result { + let hv_interface = config + .machine_settings + .hv_interface + .as_ref() + .map(|hv| match hv { + HypervisorInterface::Bhyve => GuestHypervisorInterface::Bhyve, + HypervisorInterface::HyperV { reference_tsc } => { + let mut features = BTreeSet::new(); + if *reference_tsc { + features.insert(HyperVFeatureFlag::ReferenceTsc); + } + GuestHypervisorInterface::HyperV { features } + } + }) + .unwrap_or_default(); + let mut spec = SpecConfig { enable_pcie: config .chipset @@ -117,6 +138,7 @@ impl TryFrom<&super::Config> for SpecConfig { }) .transpose()? .unwrap_or(false), + hv_interface, ..Default::default() }; diff --git a/lib/propolis/src/enlightenment/hyperv/mod.rs b/lib/propolis/src/enlightenment/hyperv/mod.rs index a67d8b898..fcaa7f144 100644 --- a/lib/propolis/src/enlightenment/hyperv/mod.rs +++ b/lib/propolis/src/enlightenment/hyperv/mod.rs @@ -54,7 +54,7 @@ mod probes { fn hyperv_rdmsr_reference_time(time_units: u64) {} } -const TYPE_NAME: &str = "guest-hyperv-interface"; +const TYPE_NAME: &str = "hyperv-guest-interface"; /// A set of features that can be enabled for a given Hyper-V instance. #[derive(Clone, Copy, Debug, Default)] diff --git a/lib/propolis/src/enlightenment/mod.rs b/lib/propolis/src/enlightenment/mod.rs index 6cf6b01a5..87c6a6bda 100644 --- a/lib/propolis/src/enlightenment/mod.rs +++ b/lib/propolis/src/enlightenment/mod.rs @@ -70,15 +70,31 @@ use crate::{ pub mod bhyve; pub mod hyperv; +// This is a freestanding function, rather than part of `trait Enlightenment` +// below, for boring Rust reasons. If you're inclined to try moving it, I +// commend you! It sure feels out of place here. Below is the "why", and how it +// ended up here: +// +// In some cases (propolis-standalone) we get an +// `Arc` well before registering all `Lifecycle`s in a VM. In +// that case there is no concrete `T: Enlightenment` for even an `Arc` +// receiver to fit. So we take `Arc` specifically, as in all +// cases at some point the callers setting up enlightenments will unify the +// impls into some singular `dyn Enlightenment` for actual operations. +// +// Then, if you try to make this an associated function taking +// `Arc`, you realize this is "non-dispatchable" as +// `Arc` which is *not* `Arc` aka a concrete impl of +// Enlightenment. Being non-dispatchable, it would require `Self: Sized`, but we +// don't even have a `Self` to work with here. So this function would make +// `Enlightenment` not dyn-compatible and defeat every use - and the point of! - +// this trait.. +pub fn as_lifecycle(me: Arc) -> Arc { + me +} + /// Functionality provided by all enlightenment interfaces. pub trait Enlightenment: Lifecycle + Send + Sync { - fn as_lifecycle(self: Arc) -> Arc - where - Self: Sized, - { - self - } - /// Attaches this enlightenment stack to a VM. /// /// Users of an enlightenment stack must guarantee that this function is