-
Notifications
You must be signed in to change notification settings - Fork 39
feat: enable basic systemd-boot support #978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ authors = ["Colin Walters <[email protected]>"] | |
edition = "2021" | ||
rust-version = "1.84.1" | ||
homepage = "https://github.com/coreos/bootupd" | ||
build = "build.rs" | ||
|
||
include = ["src", "LICENSE", "Makefile", "systemd"] | ||
|
||
|
@@ -15,6 +16,10 @@ include = ["src", "LICENSE", "Makefile", "systemd"] | |
platforms = ["*-unknown-linux-gnu"] | ||
tier = "2" | ||
|
||
[features] | ||
default = [] | ||
systemd-boot = [] | ||
|
||
[[bin]] | ||
name = "bootupd" | ||
path = "src/main.rs" | ||
|
@@ -27,7 +32,14 @@ bootc-internal-utils = "0.0.0" | |
cap-std-ext = "4.0.6" | ||
camino = "1.1.11" | ||
chrono = { version = "0.4.41", features = ["serde"] } | ||
clap = { version = "4.5", default-features = false, features = ["cargo", "derive", "std", "help", "usage", "suggestions"] } | ||
clap = { version = "4.5", default-features = false, features = [ | ||
"cargo", | ||
"derive", | ||
"std", | ||
"help", | ||
"usage", | ||
"suggestions", | ||
] } | ||
env_logger = "0.11" | ||
fail = { version = "0.5", features = ["failpoints"] } | ||
fn-error-context = "0.2.1" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
fn main() { | ||
if std::env::var("CARGO_FEATURE_SYSTEMD_BOOT").is_ok() { | ||
if let Ok(arch) = std::env::var("CARGO_CFG_TARGET_ARCH") { | ||
if arch.starts_with("riscv") { | ||
panic!("The systemd-boot feature is not supported on RISC-V."); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,13 @@ use std::fs::{self, File}; | |
use std::io::{BufRead, BufReader, BufWriter, Write}; | ||
use std::path::{Path, PathBuf}; | ||
|
||
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
pub(crate) enum Bootloader { | ||
Grub2, | ||
#[cfg(feature = "systemd-boot")] | ||
SystemdBoot, | ||
} | ||
|
||
pub(crate) enum ConfigMode { | ||
None, | ||
Static, | ||
|
@@ -81,6 +88,8 @@ pub(crate) fn install( | |
anyhow::bail!("No components specified"); | ||
} | ||
|
||
let bootloader = select_bootloader(&source_root); | ||
|
||
let mut state = SavedState::default(); | ||
let mut installed_efi_vendor = None; | ||
for &component in target_components.iter() { | ||
|
@@ -93,20 +102,51 @@ pub(crate) fn install( | |
continue; | ||
} | ||
|
||
#[cfg(feature = "systemd-boot")] | ||
if bootloader == Bootloader::SystemdBoot && component.name() == "BIOS" { | ||
log::warn!("Skip installing BIOS component when using systemd-boot"); | ||
continue; | ||
} | ||
|
||
let update_firmware = match bootloader { | ||
Bootloader::Grub2 => update_firmware, | ||
#[cfg(feature = "systemd-boot")] | ||
Bootloader::SystemdBoot => false, | ||
}; | ||
|
||
let meta = component | ||
.install(&source_root, dest_root, device, update_firmware) | ||
.install( | ||
&source_root, | ||
dest_root, | ||
device, | ||
update_firmware, | ||
&bootloader, | ||
) | ||
.with_context(|| format!("installing component {}", component.name()))?; | ||
log::info!("Installed {} {}", component.name(), meta.meta.version); | ||
state.installed.insert(component.name().into(), meta); | ||
// Yes this is a hack...the Component thing just turns out to be too generic. | ||
if let Some(vendor) = component.get_efi_vendor(&source_root)? { | ||
assert!(installed_efi_vendor.is_none()); | ||
installed_efi_vendor = Some(vendor); | ||
|
||
match bootloader { | ||
Bootloader::Grub2 => { | ||
// Yes this is a hack...the Component thing just turns out to be too generic. | ||
if let Some(vendor) = component.get_efi_vendor(&source_root)? { | ||
assert!(installed_efi_vendor.is_none()); | ||
installed_efi_vendor = Some(vendor); | ||
} | ||
} | ||
#[cfg(feature = "systemd-boot")] | ||
_ => {} | ||
} | ||
} | ||
let sysroot = &openat::Dir::open(dest_root)?; | ||
|
||
match configs.enabled_with_uuid() { | ||
// If systemd-boot is enabled, do not run grubconfigs::install | ||
let configs_with_uuid = match bootloader { | ||
Bootloader::Grub2 => configs.enabled_with_uuid(), | ||
#[cfg(feature = "systemd-boot")] | ||
_ => None, | ||
}; | ||
match configs_with_uuid { | ||
Some(uuid) => { | ||
let meta = get_static_config_meta()?; | ||
state.static_configs = Some(meta); | ||
|
@@ -715,6 +755,41 @@ fn strip_grub_config_file( | |
Ok(()) | ||
} | ||
|
||
/// Determine whether the necessary bootloader files are present for GRUB. | ||
fn has_grub(source_root: &openat::Dir) -> bool { | ||
source_root.open_file("usr/sbin/grub2-install").is_ok() | ||
} | ||
|
||
/// Determine whether the necessary bootloader files are present for systemd-boot. | ||
#[cfg(feature = "systemd-boot")] | ||
fn has_systemd_boot(source_root: &openat::Dir) -> bool { | ||
source_root.open_file(efi::SYSTEMD_BOOT_EFI).is_ok() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Super minor but doing a |
||
} | ||
|
||
/// Select the bootloader based on available binaries and feature flags. | ||
fn select_bootloader(source_root: &openat::Dir) -> Bootloader { | ||
#[cfg(not(feature = "systemd-boot"))] | ||
{ | ||
if has_grub(source_root) { | ||
Bootloader::Grub2 | ||
} else { | ||
log::warn!("No bootloader binaries found, defaulting to Grub2"); | ||
Bootloader::Grub2 | ||
} | ||
} | ||
#[cfg(feature = "systemd-boot")] | ||
{ | ||
if has_grub(source_root) { | ||
Bootloader::Grub2 | ||
} else if has_systemd_boot(source_root) { | ||
Bootloader::SystemdBoot | ||
} else { | ||
log::warn!("No bootloader binaries found, defaulting to Grub2"); | ||
Bootloader::Grub2 | ||
} | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ use rustix::fd::BorrowedFd; | |
use walkdir::WalkDir; | ||
use widestring::U16CString; | ||
|
||
use crate::bootupd::RootContext; | ||
use crate::bootupd::{Bootloader, RootContext}; | ||
use crate::freezethaw::fsfreeze_thaw_cycle; | ||
use crate::model::*; | ||
use crate::ostreeutil; | ||
|
@@ -50,6 +50,10 @@ pub(crate) const SHIM: &str = "shimriscv64.efi"; | |
/// Systemd boot loader info EFI variable names | ||
const LOADER_INFO_VAR_STR: &str = "LoaderInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"; | ||
const STUB_INFO_VAR_STR: &str = "StubInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"; | ||
#[cfg(all(feature = "systemd-boot", target_arch = "aarch64"))] | ||
pub(crate) const SYSTEMD_BOOT_EFI: &str = "usr/lib/systemd/boot/efi/systemd-bootaarch64.efi"; | ||
#[cfg(all(feature = "systemd-boot", target_arch = "x86_64"))] | ||
pub(crate) const SYSTEMD_BOOT_EFI: &str = "usr/lib/systemd/boot/efi/systemd-bootx64.efi"; | ||
|
||
/// Return `true` if the system is booted via EFI | ||
pub(crate) fn is_efi_booted() -> Result<bool> { | ||
|
@@ -342,14 +346,8 @@ impl Component for Efi { | |
dest_root: &str, | ||
device: &str, | ||
update_firmware: bool, | ||
bootloader: &Bootloader, | ||
) -> Result<InstalledContent> { | ||
let Some(meta) = get_component_update(src_root, self)? else { | ||
anyhow::bail!("No update metadata for component {} found", self.name()); | ||
}; | ||
log::debug!("Found metadata {}", meta.version); | ||
let srcdir_name = component_updatedirname(self); | ||
let ft = crate::filetree::FileTree::new_from_dir(&src_root.sub_dir(&srcdir_name)?)?; | ||
|
||
// Let's attempt to use an already mounted ESP at the target | ||
// dest_root if one is already mounted there in a known ESP location. | ||
let destpath = if let Some(destdir) = self.get_mounted_esp(Path::new(dest_root))? { | ||
|
@@ -365,6 +363,31 @@ impl Component for Efi { | |
self.mount_esp_device(Path::new(dest_root), Path::new(&esp_device))? | ||
}; | ||
|
||
match bootloader { | ||
#[cfg(feature = "systemd-boot")] | ||
Bootloader::SystemdBoot => { | ||
log::info!("Installing systemd-boot via bootctl"); | ||
let esp_dir = openat::Dir::open(&destpath)?; | ||
crate::systemdbootconfigs::install(src_root, &esp_dir)?; | ||
return Ok(InstalledContent { | ||
meta: ContentMetadata { | ||
timestamp: Utc::now(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This timestamp should be sourced from the |
||
version: "systemd-boot".to_string(), | ||
}, | ||
filetree: None, | ||
adopted_from: None, | ||
}); | ||
} | ||
_ => {} | ||
} | ||
|
||
let Some(meta) = get_component_update(src_root, self)? else { | ||
anyhow::bail!("No update metadata for component {} found", self.name()); | ||
}; | ||
log::debug!("Found metadata {}", meta.version); | ||
let srcdir_name = component_updatedirname(self); | ||
let ft = crate::filetree::FileTree::new_from_dir(&src_root.sub_dir(&srcdir_name)?)?; | ||
|
||
let destd = &openat::Dir::open(&destpath) | ||
.with_context(|| format!("opening dest dir {}", destpath.display()))?; | ||
validate_esp_fstype(destd)?; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
use std::path::Path; | ||
|
||
use anyhow::{Context, Result}; | ||
use fn_error_context::context; | ||
|
||
const CONFIG_DIR: &str = "usr/lib/bootupd/systemd-boot"; | ||
|
||
/// Install files required for systemd-boot | ||
/// This mostly proxies the bootctl install command | ||
#[context("Installing systemd-boot")] | ||
pub(crate) fn install(src_root: &openat::Dir, esp_path: &openat::Dir) -> Result<()> { | ||
let esp_path_buf = esp_path.recover_path().context("ESP path is not valid")?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using |
||
let esp_path_str = esp_path_buf | ||
.to_str() | ||
.context("ESP path is not valid UTF-8")?; | ||
let status = std::process::Command::new("bootctl") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We won't be able to use
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, understood. Would you suggest doing something like this (but pointing to the right paths)?
Will need to figure out a way to identify the shim without reintroducing the RPM code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah! I did not know that we could pass arguments to shim! If this works then this is great. We should probably install the systemd-boot binary under the os path: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For sealed images we do want to skip shim though right?
I think it should be the same as detecting grub ideally. I'm not sure we care about grub-but-not-shim (hopefully?) but we do care about the matrix of:
So...I think we can reasonably do this by detecting if we have
Hmm AFAIK efibootmgr is primarily for manipulating the EFI firmware, which is distinct from the files on disk. We already have code which...currently hardcodes targeting shim, which would definitely have to change for sealed images. |
||
.args(["install", "--esp-path", esp_path_str]) | ||
.status() | ||
.context("Failed to execute bootctl")?; | ||
|
||
if !status.success() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can use our helper trait |
||
anyhow::bail!( | ||
"bootctl install failed with status code {}", | ||
status.code().unwrap_or(-1) | ||
); | ||
} | ||
|
||
// If loader.conf is present in the bootupd configuration, replace the original config with it | ||
let configdir_path = Path::new(CONFIG_DIR); | ||
if let Err(e) = try_copy_loader_conf(src_root, configdir_path, esp_path_str) { | ||
log::debug!("Optional loader.conf copy skipped: {}", e); | ||
} | ||
|
||
Ok(()) | ||
} | ||
|
||
/// Try to copy loader.conf from configdir to ESP, returns error if not present or copy fails | ||
fn try_copy_loader_conf( | ||
src_root: &openat::Dir, | ||
configdir_path: &Path, | ||
esp_path_str: &str, | ||
) -> Result<()> { | ||
let configdir = src_root | ||
.sub_dir(configdir_path) | ||
.context(format!("Config directory '{}' not found", CONFIG_DIR))?; | ||
let dst_loader_conf = Path::new(esp_path_str).join("loader/loader.conf"); | ||
match configdir.open_file("loader.conf") { | ||
Ok(mut src_file) => { | ||
let mut dst_file = std::fs::File::create(&dst_loader_conf) | ||
.context("Failed to create loader.conf in ESP")?; | ||
std::io::copy(&mut src_file, &mut dst_file) | ||
.context("Failed to copy loader.conf to ESP")?; | ||
log::info!("loader.conf copied to {}", dst_loader_conf.display()); | ||
Ok(()) | ||
} | ||
Err(e) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should only match if this is For cap-std-ext we have https://docs.rs/cap-std-ext/latest/cap_std_ext/dirext/trait.CapStdExtDirExt.html#tymethod.open_optional but we're not yet using cap-std here... |
||
log::debug!("loader.conf not found in configdir, skipping: {}", e); | ||
Err(anyhow::anyhow!(e)) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, this whole build.rs seems like it could use a comment for why; I'm sure some RISC-V systems aren't UEFI but it looks like at least some are working on it e.g. https://archive.fosdem.org/2021/schedule/event/firmware_uor/#:~:text=Online%20%2F%206%20%26%207%20February%202021,EDK2%20UEFI%20on%20RISC%2DV
Do we actually fail to build on RISC-V in this case?