server: read host CPUID values if the spec has none (#844)

The `cpuid` field in an instance spec's board is optional. If it is
`None`, have propolis-server read bhyve's default CPUID values and
insert them into the spec as though the caller had specified them. This
has two benefits:

1. propolis-server can set up CPUID values in the hypervisor leaf range
   without requiring callers to set CPUID values in the standard and
   extended ranges.
2. A migrating VM's CPUID configuration is now known up-front (during
   target initialization) and need not be transferred during the device
   state phase of migration (which happens while the VM is paused). This
   commit doesn't remove this migration payload, however; this will be
   addressed in a follow-up change.

bin/propolis-server/src/lib/initializer.rs

@@ -19,6 +19,7 @@ use crate::vm::{
     BlockBackendMap, CrucibleBackendMap, DeviceMap, NetworkInterfaceIds,
 };
 use anyhow::Context;
+use cpuid_utils::bits::{HYPERVISOR_BASE_LEAF, HYPERVISOR_BHYVE_VALUES};
 use cpuid_utils::CpuidValues;
 use crucible_client_types::VolumeConstructionRequest;
 pub use nexus_client::Client as NexusClient;
@@ -988,33 +989,19 @@ impl MachineInitializer<'_> {
             ..Default::default()
         };
 
-        fn get_spec_or_host_cpuid(
-            spec: &Spec,
-            leaf: u32,
-        ) -> Option<CpuidValues> {
-            let leaf = CpuidIdent::leaf(leaf);
-            let Some(cpuid) = &spec.cpuid else {
-                return Some(cpuid_utils::host::query(leaf));
-            };
-
-            cpuid.get(leaf).copied()
-        }
-
         // The processor vendor, family/model/stepping, and brand string should
-        // correspond to the values the guest will see if it queries CPUID. If
-        // the instance spec contains CPUID values, derive this information from
-        // those. Otherwise, derive them from the values on the host.
+        // correspond to the values the guest will see if it queries CPUID.
         //
         // Note that all these values are `Option`s, because the spec may
         // contain CPUID values that don't contain all of the input leaves.
-        let cpuid_vendor = get_spec_or_host_cpuid(self.spec, 0);
-        let cpuid_ident = get_spec_or_host_cpuid(self.spec, 1);
+        let cpuid_vendor = self.spec.cpuid.get(CpuidIdent::leaf(0)).copied();
+        let cpuid_ident = self.spec.cpuid.get(CpuidIdent::leaf(1)).copied();
 
         // Coerce the array-of-Options into an Option containing the array.
         let cpuid_procname: Option<[CpuidValues; 3]> = [
-            get_spec_or_host_cpuid(self.spec, 0x8000_0002),
-            get_spec_or_host_cpuid(self.spec, 0x8000_0003),
-            get_spec_or_host_cpuid(self.spec, 0x8000_0004),
+            self.spec.cpuid.get(CpuidIdent::leaf(0x8000_0002)).copied(),
+            self.spec.cpuid.get(CpuidIdent::leaf(0x8000_0003)).copied(),
+            self.spec.cpuid.get(CpuidIdent::leaf(0x8000_0004)).copied(),
         ]
         .into_iter()
         // This returns None if any of the input options were None (i.e. if any
@@ -1245,28 +1232,42 @@ impl MachineInitializer<'_> {
     /// tracking their kstats.
     pub async fn initialize_cpus(&mut self) -> Result<(), MachineInitError> {
         for vcpu in self.machine.vcpus.iter() {
-            if let Some(set) = &self.spec.cpuid {
-                let specialized = propolis::cpuid::Specializer::new()
-                    .with_vcpu_count(
-                        NonZeroU8::new(self.spec.board.cpus).unwrap(),
-                        true,
-                    )
-                    .with_vcpuid(vcpu.id)
-                    .with_cache_topo()
-                    .clear_cpu_topo(propolis::cpuid::TopoKind::iter())
-                    .execute(set.clone())
-                    .map_err(|e| {
-                        MachineInitError::CpuidSpecializationFailed(vcpu.id, e)
-                    })?;
-
-                info!(self.log, "setting CPUID for vCPU";
-                      "vcpu" => vcpu.id,
-                      "cpuid" => ?specialized);
+            // Report that the guest is running on bhyve.
+            //
+            // The CPUID set in the spec is not allowed to contain any leaves in
+            // the hypervisor leaf region (enforced at spec generation time).
+            let mut set = self.spec.cpuid.clone();
+            assert!(
+                set.insert(
+                    CpuidIdent::leaf(HYPERVISOR_BASE_LEAF),
+                    HYPERVISOR_BHYVE_VALUES
+                )
+                .expect("no hypervisor subleaves")
+                .is_none(),
+                "CPUID set should have no hypervisor leaves"
+            );
 
-                vcpu.set_cpuid(specialized).with_context(|| {
-                    format!("setting CPUID for vcpu {}", vcpu.id)
+            let specialized = propolis::cpuid::Specializer::new()
+                .with_vcpu_count(
+                    NonZeroU8::new(self.spec.board.cpus).unwrap(),
+                    true,
+                )
+                .with_vcpuid(vcpu.id)
+                .with_cache_topo()
+                .clear_cpu_topo(propolis::cpuid::TopoKind::iter())
+                .execute(set)
+                .map_err(|e| {
+                    MachineInitError::CpuidSpecializationFailed(vcpu.id, e)
                 })?;
-            }
+
+            info!(self.log, "setting CPUID for vCPU";
+                    "vcpu" => vcpu.id,
+                    "cpuid" => ?specialized);
+
+            vcpu.set_cpuid(specialized).with_context(|| {
+                format!("setting CPUID for vcpu {}", vcpu.id)
+            })?;
+
             vcpu.set_default_capabs()
                 .context("failed to set vcpu capabilities")?;
 

bin/propolis-server/src/lib/spec/api_spec_v0.rs

@@ -95,7 +95,7 @@ impl From<Spec> for InstanceSpecV0 {
             cpus: board.cpus,
             memory_mb: board.memory_mb,
             chipset: board.chipset,
-            cpuid: cpuid.map(|set| set.into_instance_spec_cpuid()),
+            cpuid: Some(cpuid.into_instance_spec_cpuid()),
         };
         let mut spec = InstanceSpecV0 { board, components: Default::default() };
 

bin/propolis-server/src/lib/spec/builder.rs

@@ -6,7 +6,6 @@
 
 use std::collections::{BTreeSet, HashSet};
 
-use cpuid_utils::CpuidMapConversionError;
 use propolis_api_types::instance_spec::{
     components::{
         board::Board as InstanceSpecBoard,
@@ -63,6 +62,9 @@ pub(crate) enum SpecBuilderError {
 
     #[error("instance spec's CPUID entries are invalid")]
     CpuidEntriesInvalid(#[from] cpuid_utils::CpuidMapConversionError),
+
+    #[error("failed to read default CPUID settings from the host")]
+    DefaultCpuidReadFailed(#[from] cpuid_utils::host::GetHostCpuidError),
 }
 
 #[derive(Debug, Default)]
@@ -77,24 +79,24 @@ impl SpecBuilder {
     pub(super) fn with_instance_spec_board(
         board: InstanceSpecBoard,
     ) -> Result<Self, SpecBuilderError> {
+        let cpuid = match board.cpuid {
+            Some(cpuid) => cpuid_utils::CpuidSet::from_map(
+                cpuid.entries.try_into()?,
+                cpuid.vendor,
+            ),
+            None => cpuid_utils::host::query_complete(
+                cpuid_utils::host::CpuidSource::BhyveDefault,
+            )?,
+        };
+
         Ok(Self {
             spec: super::Spec {
                 board: Board {
                     cpus: board.cpus,
                     memory_mb: board.memory_mb,
                     chipset: board.chipset,
                 },
-                cpuid: board
-                    .cpuid
-                    .map(|cpuid| -> Result<_, CpuidMapConversionError> {
-                        {
-                            Ok(cpuid_utils::CpuidSet::from_map(
-                                cpuid.entries.try_into()?,
-                                cpuid.vendor,
-                            )?)
-                        }
-                    })
-                    .transpose()?,
+                cpuid,
                 ..Default::default()
             },
             ..Default::default()

bin/propolis-server/src/lib/spec/mod.rs

@@ -63,7 +63,7 @@ pub struct ComponentTypeMismatch;
 #[derive(Clone, Debug, Default)]
 pub(crate) struct Spec {
     pub board: Board,
-    pub cpuid: Option<CpuidSet>,
+    pub cpuid: CpuidSet,
     pub disks: BTreeMap<SpecKey, Disk>,
     pub nics: BTreeMap<SpecKey, Nic>,
     pub boot_settings: Option<BootSettings>,

crates/cpuid-utils/src/bits.rs

@@ -8,9 +8,21 @@
 //! Definitions here are taken from the AMD Architecture Programmer's Manual,
 //! volume 3, appendix E (Publication 24594, revision 3.36, March 2024).
 
+use propolis_types::CpuidValues;
+
 pub const STANDARD_BASE_LEAF: u32 = 0;
+pub const HYPERVISOR_BASE_LEAF: u32 = 0x4000_0000;
 pub const EXTENDED_BASE_LEAF: u32 = 0x8000_0000;
 
+/// The bhyve default hypervisor identifier ("bhyve bhyve " in ebx/ecx/edx with
+/// no additional hypervisor CPUID leaves reported in eax).
+pub const HYPERVISOR_BHYVE_VALUES: CpuidValues = CpuidValues {
+    eax: HYPERVISOR_BASE_LEAF,
+    ebx: 0x76796862,
+    ecx: 0x68622065,
+    edx: 0x20657679,
+};
+
 bitflags::bitflags! {
     /// Leaf 1 ecx: instruction feature identifiers.
     #[derive(Clone, Copy, Debug, PartialEq, Eq)]

crates/cpuid-utils/src/host.rs

@@ -11,13 +11,13 @@ use crate::{
         AmdExtLeaf1DCacheType, AmdExtLeaf1DEax, Leaf1Ecx, EXTENDED_BASE_LEAF,
         STANDARD_BASE_LEAF,
     },
-    CpuidSet, SubleafInsertConflict,
+    CpuidMapInsertError, CpuidSet,
 };
 
 #[derive(Debug, Error)]
 pub enum GetHostCpuidError {
     #[error("failed to insert into the CPUID map")]
-    CpuidInsertFailed(#[from] SubleafInsertConflict),
+    CpuidInsertFailed(#[from] CpuidMapInsertError),
 
     #[error("CPUID vendor not recognized: {0}")]
     VendorNotRecognized(&'static str),

crates/cpuid-utils/src/lib.rs

@@ -65,7 +65,7 @@ pub mod host;
 mod instance_spec;
 
 type CpuidSubleafMap = BTreeMap<u32, CpuidValues>;
-type CpuidMapInsertResult = Result<Option<CpuidValues>, SubleafInsertConflict>;
+type CpuidMapInsertResult = Result<Option<CpuidValues>, CpuidMapInsertError>;
 
 /// Denotes the presence or absence of subleaves for a given CPUID leaf.
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -82,7 +82,7 @@ enum Subleaves {
 /// new value would produce a leaf that has both per-subleaf values and a
 /// no-subleaf value.
 #[derive(Debug, Error)]
-pub enum SubleafInsertConflict {
+pub enum CpuidMapInsertError {
     #[error("leaf {0:x} has entries with subleaves")]
     SubleavesAlreadyPresent(u32),
 
@@ -161,7 +161,7 @@ impl CpuidMap {
             }
             Entry::Occupied(mut e) => match e.get_mut() {
                 Subleaves::Present(_) => {
-                    Err(SubleafInsertConflict::SubleavesAlreadyPresent(leaf))
+                    Err(CpuidMapInsertError::SubleavesAlreadyPresent(leaf))
                 }
                 Subleaves::Absent(v) => Ok(Some(std::mem::replace(v, values))),
             },
@@ -183,7 +183,7 @@ impl CpuidMap {
             }
             Entry::Occupied(mut e) => match e.get_mut() {
                 Subleaves::Absent(_) => {
-                    Err(SubleafInsertConflict::SubleavesAlreadyAbsent(leaf))
+                    Err(CpuidMapInsertError::SubleavesAlreadyAbsent(leaf))
                 }
                 Subleaves::Present(sl_map) => {
                     Ok(sl_map.insert(subleaf, values))
@@ -410,11 +410,8 @@ impl CpuidSet {
 
     /// Creates a new `CpuidSet` with the supplied initial leaf/value `map` and
     /// `vendor`.
-    pub fn from_map(
-        map: CpuidMap,
-        vendor: CpuidVendor,
-    ) -> Result<Self, SubleafInsertConflict> {
-        Ok(Self { map, vendor })
+    pub fn from_map(map: CpuidMap, vendor: CpuidVendor) -> Self {
+        Self { map, vendor }
     }
 
     /// Yields this set's vendor.
@@ -556,7 +553,7 @@ pub enum CpuidMapConversionError {
     LeafOutOfRange(u32),
 
     #[error(transparent)]
-    SubleafConflict(#[from] SubleafInsertConflict),
+    SubleafConflict(#[from] CpuidMapInsertError),
 }
 
 /// The range of standard, architecturally-defined CPUID leaves.