Skip to content

Commit

Permalink
Add bootrom version to config files
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 committed May 8, 2024
1 parent 0d8efa1 commit 4eb9b15
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 4eb9b15

Please sign in to comment.