XXX: Specializer should more gracefully handle errors

bin/propolis-standalone/src/main.rs

@@ -926,24 +926,22 @@ fn setup_instance(
     let cpuid_profile = config::parse_cpuid(&config)?;
 
     for vcpu in machine.vcpus.iter() {
-        let vcpu_profile = cpuid_profile
-            .as_ref()
-            .map(|profile| {
-                propolis::cpuid::Specializer::new()
-                    .with_vcpu_count(
-                        std::num::NonZeroU8::new(config.main.cpus).unwrap(),
-                        true,
-                    )
-                    .with_vcpuid(vcpu.id)
-                    .with_cache_topo()
-                    .clear_cpu_topo(cpuid::TopoKind::all())
-                    .execute(profile.clone())
-            })
-            .unwrap_or_else(|| {
-                // An empty set will instruct the kernel to use the legacy
-                // fallback behavior
-                propolis::cpuid::Set::new_host()
-            });
+        let 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(),
+                    true,
+                )
+                .with_vcpuid(vcpu.id)
+                .with_cache_topo()
+                .clear_cpu_topo(cpuid::TopoKind::all())
+                .execute(profile.clone())
+                .context("failed to specialize cpuid profile")?
+        } else {
+            // An empty set will instruct the kernel to use the legacy
+            // fallback behavior
+            propolis::cpuid::Set::new_host()
+        };
         vcpu.set_cpuid(vcpu_profile)?;
         vcpu.set_default_capabs()?;
     }

lib/propolis/src/cpuid.rs

@@ -148,6 +148,14 @@ impl<'a> Iterator for Iter<'a> {
     }
 }
 
+#[derive(Debug, thiserror::Error)]
+pub enum SpecializeError {
+    #[error("unsupported cache level")]
+    UnsupportedCacheLevel,
+    #[error("missing vcpu count")]
+    MissingVcpuCount,
+}
+
 /// Specialize a set of cpuid leafs for provided attributes.
 ///
 /// This includes things such as a CPU topology (cores/threads/etc), a given
@@ -223,22 +231,22 @@ impl Specializer {
 
     /// Given the attributes and modifiers specified in this [Specializer],
     /// render an updated [Set] reflecting those data.
-    pub fn execute(self, mut set: Set) -> Set {
+    pub fn execute(self, mut set: Set) -> Result<Set, SpecializeError> {
         // Use vendor override if provided, or else the existing one
         if let Some(vendor) = self.vendor_kind {
             set.vendor = vendor;
         }
         match set.vendor {
             VendorKind::Amd => {
                 if self.do_cache_topo && self.num_vcpu.is_some() {
-                    self.fix_amd_cache_topo(&mut set);
+                    self.fix_amd_cache_topo(&mut set)?;
                 }
             }
             _ => {}
         }
 
         // apply any requested topo info fixups
-        self.fix_cpu_topo(&mut set);
+        self.fix_cpu_topo(&mut set)?;
 
         // APIC ID based on vcpuid
         if let Some(vcpuid) = self.vcpuid.as_ref() {
@@ -261,10 +269,10 @@ impl Specializer {
             }
         }
 
-        set
+        Ok(set)
     }
 
-    fn fix_amd_cache_topo(&self, set: &mut Set) {
+    fn fix_amd_cache_topo(&self, set: &mut Set) -> Result<(), SpecializeError> {
         assert!(self.do_cache_topo);
         let num = self.num_vcpu.unwrap().get();
         for ecx in 0..u32::MAX {
@@ -288,7 +296,7 @@ impl Specializer {
                         }
                         _ => {
                             // unceremonious handling of unexpected cache levels
-                            panic!("unsupported cache level");
+                            return Err(SpecializeError::UnsupportedCacheLevel);
                         }
                     };
                     // the number of logical CPUs (minus 1) sharing this cache
@@ -298,8 +306,9 @@ impl Specializer {
                 }
             }
         }
+        Ok(())
     }
-    fn fix_cpu_topo(&self, set: &mut Set) {
+    fn fix_cpu_topo(&self, set: &mut Set) -> Result<(), SpecializeError> {
         for topo in self.cpu_topo_populate.union(&self.cpu_topo_clear) {
             // Nuke any existing info in order to potentially override it
             let leaf = *topo as u32;
@@ -309,13 +318,10 @@ impl Specializer {
                 continue;
             }
 
-            let num_vcpu = match self.num_vcpu {
-                Some(n) => n.get() as u32,
-                None => {
-                    // Skip adding topo info if we lack vcpu count
-                    continue;
-                }
-            };
+            let num_vcpu = self
+                .num_vcpu
+                .ok_or(SpecializeError::MissingVcpuCount)
+                .map(|n| n.get() as u32)?;
 
             match topo {
                 TopoKind::StdB => {
@@ -378,6 +384,7 @@ impl Specializer {
                 }
             }
         }
+        Ok(())
     }
 }