From 4eb9b154478ac61e6433428f7db3ad66336ed538 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 8 May 2024 08:44:11 -0700 Subject: [PATCH] Add bootrom version to config files Presently, the BIOS version string in Propolis' SMBIOS tables is hardcoded to a default value. It would be nice to instead use the OVMF version for the BIOS version string. Because Propolis' understanding of bootroms is just a path on the filesystem to some kind of file, it's not aware of the OVMF version, or, indeed, that the bootrom even *is* OVMF (and it could conceivably be anything). Therefore, the bootrom version must be provided externally, such as by the Oxide control plane in the case of `propolis-server`, or by the user when running standalone. This PR adds a config field `bootrom_version` to the TOML config files for `propolis-server` and `propolis-standalone` which can be used to provide a value for the bootrom version string. If the version string is not provided, Propolis will continue to use the current default values. I considered changing the config format to move the `bootrom` path field and `bootrom_version` string field into a `bootrom` table, as in: ```toml [bootrom] path = "/path/to/OVMF_CODE.fd" version = "edk2-stable202402" ``` However, this would break existing configs, and I don't think it's so much nicer than ```toml bootrom = "/path/to/OVMF_CODE.fd" bootrom_version = "edk2-stable202402" ```` to justify breakage. I'm happy to change the format if others disagree. Along with #702, this branch implements the changes described in #701. --- bin/propolis-server/src/lib/initializer.rs | 10 +++++++++- bin/propolis-server/src/lib/server.rs | 2 +- bin/propolis-server/src/lib/vm/mod.rs | 1 + bin/propolis-standalone/src/config.rs | 1 + bin/propolis-standalone/src/main.rs | 12 ++++++++++-- crates/propolis-server-config/src/lib.rs | 3 +++ 6 files changed, 25 insertions(+), 4 deletions(-) diff --git a/bin/propolis-server/src/lib/initializer.rs b/bin/propolis-server/src/lib/initializer.rs index 41736ece9..13505f10b 100644 --- a/bin/propolis-server/src/lib/initializer.rs +++ b/bin/propolis-server/src/lib/initializer.rs @@ -115,6 +115,7 @@ pub struct MachineInitializer<'a> { pub(crate) crucible_backends: CrucibleBackendMap, pub(crate) spec: &'a InstanceSpecV0, pub(crate) properties: &'a InstanceProperties, + pub(crate) toml_config: &'a crate::server::VmTomlConfig, pub(crate) producer_registry: Option, pub(crate) state: MachineInitializerState, } @@ -857,9 +858,16 @@ impl<'a> MachineInitializer<'a> { let rom_size = self.state.rom_size_bytes.expect("ROM is already populated"); + let bios_version = self + .toml_config + .bootrom_version + .as_deref() + .unwrap_or("v0.8") + .try_into() + .expect("bootrom version string doesn't contain NUL bytes"); let smb_type0 = smbios::table::Type0 { vendor: "Oxide".try_into().unwrap(), - bios_version: "v0.8".try_into().unwrap(), + bios_version, bios_release_date: "The Aftermath 30, 3185 YOLD" .try_into() .unwrap(), diff --git a/bin/propolis-server/src/lib/server.rs b/bin/propolis-server/src/lib/server.rs index f42c199ad..772c93c54 100644 --- a/bin/propolis-server/src/lib/server.rs +++ b/bin/propolis-server/src/lib/server.rs @@ -34,7 +34,7 @@ use propolis_api_types::instance_spec::{ VersionedInstanceSpec, }; -use propolis_server_config::Config as VmTomlConfig; +pub use propolis_server_config::Config as VmTomlConfig; use rfb::server::VncServer; use slog::{error, info, o, warn, Logger}; use thiserror::Error; diff --git a/bin/propolis-server/src/lib/vm/mod.rs b/bin/propolis-server/src/lib/vm/mod.rs index cbaf9955a..97afb0b09 100644 --- a/bin/propolis-server/src/lib/vm/mod.rs +++ b/bin/propolis-server/src/lib/vm/mod.rs @@ -456,6 +456,7 @@ impl VmController { crucible_backends: CrucibleBackendMap::new(), spec: v0_spec, properties: &properties, + toml_config, producer_registry, state: MachineInitializerState::default(), }; diff --git a/bin/propolis-standalone/src/config.rs b/bin/propolis-standalone/src/config.rs index 0250cba0c..3d6758209 100644 --- a/bin/propolis-standalone/src/config.rs +++ b/bin/propolis-standalone/src/config.rs @@ -47,6 +47,7 @@ pub struct Main { pub name: String, pub cpus: u8, pub bootrom: String, + pub bootrom_version: Option, pub memory: usize, pub use_reservoir: Option, pub cpuid_profile: Option, diff --git a/bin/propolis-standalone/src/main.rs b/bin/propolis-standalone/src/main.rs index ee0b76803..3ca0da485 100644 --- a/bin/propolis-standalone/src/main.rs +++ b/bin/propolis-standalone/src/main.rs @@ -807,16 +807,20 @@ fn populate_rom( struct SmbiosParams { memory_size: usize, rom_size: usize, + rom_version: String, num_cpus: u8, cpuid_ident: Option, cpuid_procname: Option<[cpuid::Entry; 3]>, } fn generate_smbios(params: SmbiosParams) -> anyhow::Result { use smbios::table::{type0, type1, type16, type4}; - + let bios_version = params + .rom_version + .try_into() + .expect("bootrom version string doesn't contain NUL bytes"); let smb_type0 = smbios::table::Type0 { vendor: "Oxide".try_into().unwrap(), - bios_version: "v0.0.1 alpha1".try_into().unwrap(), + bios_version, bios_release_date: "Bureaucracy 41, 3186 YOLD".try_into().unwrap(), bios_rom_size: ((params.rom_size / (64 * 1024)) - 1) as u8, bios_characteristics: type0::BiosCharacteristics::UNSUPPORTED, @@ -1202,6 +1206,10 @@ fn setup_instance( generate_smbios(SmbiosParams { memory_size: memsize, rom_size: rom_len, + rom_version: config + .main + .bootrom_version + .unwrap_or_else(|| "v0.0.1-alpha 1".to_string()), num_cpus: cpus, cpuid_ident, cpuid_procname, diff --git a/crates/propolis-server-config/src/lib.rs b/crates/propolis-server-config/src/lib.rs index a9f50647a..b96989c59 100644 --- a/crates/propolis-server-config/src/lib.rs +++ b/crates/propolis-server-config/src/lib.rs @@ -18,6 +18,8 @@ pub use cpuid_profile_config::CpuidProfile; pub struct Config { pub bootrom: PathBuf, + pub bootrom_version: Option, + #[serde(default, rename = "pci_bridge")] pub pci_bridges: Vec, @@ -37,6 +39,7 @@ impl Default for Config { fn default() -> Self { Self { bootrom: PathBuf::new(), + bootrom_version: None, pci_bridges: Vec::new(), chipset: Chipset { options: BTreeMap::new() }, devices: BTreeMap::new(),