Skip to content

Commit

Permalink
Add bootrom version to config files (#703)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hawkw authored May 8, 2024
1 parent 6d4fe85 commit 4591a60
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 4 deletions.
10 changes: 9 additions & 1 deletion bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProducerRegistry>,
pub(crate) state: MachineInitializerState,
}
Expand Down Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion bin/propolis-server/src/lib/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions bin/propolis-server/src/lib/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ impl VmController {
crucible_backends: CrucibleBackendMap::new(),
spec: v0_spec,
properties: &properties,
toml_config,
producer_registry,
state: MachineInitializerState::default(),
};
Expand Down
1 change: 1 addition & 0 deletions bin/propolis-standalone/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub struct Main {
pub name: String,
pub cpus: u8,
pub bootrom: String,
pub bootrom_version: Option<String>,
pub memory: usize,
pub use_reservoir: Option<bool>,
pub cpuid_profile: Option<String>,
Expand Down
12 changes: 10 additions & 2 deletions bin/propolis-standalone/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::Entry>,
cpuid_procname: Option<[cpuid::Entry; 3]>,
}
fn generate_smbios(params: SmbiosParams) -> anyhow::Result<smbios::TableBytes> {
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,
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions crates/propolis-server-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ pub use cpuid_profile_config::CpuidProfile;
pub struct Config {
pub bootrom: PathBuf,

pub bootrom_version: Option<String>,

#[serde(default, rename = "pci_bridge")]
pub pci_bridges: Vec<PciBridge>,

Expand All @@ -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(),
Expand Down

0 comments on commit 4591a60

Please sign in to comment.