Do not require OS for BootOptions.

veluca93 · veluca93 · commit 7bf0c6a57df1 · 2025-11-30T23:18:17.000+01:00
diff --git a/pixie-uefi/src/flash.rs b/pixie-uefi/src/flash.rs
@@ -14,6 +14,7 @@ use pixie_shared::util::BytesFmt;
 use pixie_shared::{ChunkHash, Image, TcpRequest, UdpRequest, CHUNKS_PORT, MAX_CHUNK_SIZE};
 use uefi::proto::console::text::Color;
 
+use crate::os::boot_options::BootOptions;
 use crate::os::error::{Error, Result};
 use crate::os::{memory, TcpStream, UefiOS, PACKET_SIZE};
 use crate::MIN_MEMORY;
@@ -230,9 +231,8 @@ pub async fn flash(os: UefiOS, server_addr: SocketAddrV4) -> Result<()> {
 
     info!("Fetch complete, updating boot options");
 
-    let bo = os.boot_options();
-    let mut order = bo.order();
-    let reboot_target = bo.reboot_target();
+    let mut order = BootOptions::order();
+    let reboot_target = BootOptions::reboot_target();
     if let Some(target) = reboot_target {
         order = order
             .into_iter()
@@ -241,8 +241,8 @@ pub async fn flash(os: UefiOS, server_addr: SocketAddrV4) -> Result<()> {
     } else {
         order.push(image.boot_option_id);
     };
-    bo.set_order(&order);
-    bo.set(image.boot_option_id, &image.boot_entry);
+    BootOptions::set_order(&order);
+    BootOptions::set(image.boot_option_id, &image.boot_entry);
 
     Ok(())
 }
diff --git a/pixie-uefi/src/os/boot_options.rs b/pixie-uefi/src/os/boot_options.rs
@@ -1,100 +1,120 @@
+use alloc::boxed::Box;
 use alloc::string::{String, ToString};
 use alloc::vec::Vec;
 
 use uefi::proto::device_path::DevicePath;
 use uefi::runtime::{VariableAttributes, VariableVendor};
-use uefi::CString16;
+use uefi::{CStr16, CString16};
 
-use super::UefiOS;
+use crate::os::error::{Error, Result};
 
-pub struct BootOptions {
-    pub(super) os: UefiOS,
+#[derive(Debug)]
+pub struct Variable {
+    name: CString16,
+    vendor: VariableVendor,
 }
 
+impl Variable {
+    pub fn new(name: &str, vendor: VariableVendor) -> Self {
+        // name.len() should be enough, but...
+        let mut name_buf = vec![0u16; name.len() * 2 + 16];
+        let name = CStr16::from_str_with_buf(name, &mut name_buf).unwrap();
+        Self {
+            name: name.into(),
+            vendor,
+        }
+    }
+
+    pub fn get(&self) -> Result<(Box<[u8]>, VariableAttributes)> {
+        uefi::runtime::get_variable_boxed(&self.name, &self.vendor)
+            .map_err(|e| Error(format!("Error getting variable: {e:?}")))
+    }
+
+    pub fn set(&self, data: &[u8], attrs: VariableAttributes) -> Result<()> {
+        uefi::runtime::set_variable(&self.name, &self.vendor, attrs, data)
+            .map_err(|e| Error(format!("Error setting variable: {e:?}")))
+    }
+}
+
+pub struct BootOptions;
+
 impl BootOptions {
-    pub fn current(&self) -> u16 {
-        let cur = self
-            .os
-            .get_variable("BootCurrent", &VariableVendor::GLOBAL_VARIABLE)
+    pub fn current() -> u16 {
+        let cur = Variable::new("BootCurrent", VariableVendor::GLOBAL_VARIABLE)
+            .get()
             .unwrap()
             .0;
-        u16::from_le_bytes(cur.try_into().unwrap())
+        u16::from_le_bytes((*cur).try_into().unwrap())
     }
 
-    pub fn order(&self) -> Vec<u16> {
-        self.os
-            .get_variable("BootOrder", &VariableVendor::GLOBAL_VARIABLE)
+    pub fn order() -> Vec<u16> {
+        Variable::new("BootOrder", VariableVendor::GLOBAL_VARIABLE)
+            .get()
             .unwrap()
             .0
             .chunks(2)
             .map(|x| u16::from_le_bytes(x.try_into().unwrap()))
             .collect()
     }
 
-    pub fn set_order(&self, order: &[u16]) {
+    pub fn set_order(order: &[u16]) {
         let order: Vec<_> = order
             .iter()
             .flat_map(|x| x.to_le_bytes().into_iter())
             .collect();
-        self.os
-            .set_variable(
-                "BootOrder",
-                &VariableVendor::GLOBAL_VARIABLE,
+        Variable::new("BootOrder", VariableVendor::GLOBAL_VARIABLE)
+            .set(
+                &order[..],
                 VariableAttributes::NON_VOLATILE
                     | VariableAttributes::BOOTSERVICE_ACCESS
                     | VariableAttributes::RUNTIME_ACCESS,
-                &order[..],
             )
             .unwrap()
     }
 
-    pub fn set_next(&self, next: u16) {
-        self.os
-            .set_variable(
-                "BootNext",
-                &VariableVendor::GLOBAL_VARIABLE,
+    pub fn set_next(next: u16) {
+        Variable::new("BootNext", VariableVendor::GLOBAL_VARIABLE)
+            .set(
+                &next.to_le_bytes(),
                 VariableAttributes::NON_VOLATILE
                     | VariableAttributes::BOOTSERVICE_ACCESS
                     | VariableAttributes::RUNTIME_ACCESS,
-                &next.to_le_bytes(),
             )
             .unwrap();
     }
 
-    pub fn reboot_target(&self) -> Option<u16> {
-        let order = self.order();
+    pub fn reboot_target() -> Option<u16> {
+        let order = Self::order();
         let num_skip = order
             .iter()
             .cloned()
-            .position(|x| x == self.current())
+            .position(|x| x == Self::current())
             .map(|x| x + 1)
             .unwrap_or(0);
         order.iter().cloned().skip(num_skip).find(|x| *x < 0x2000)
     }
 
-    pub fn get(&self, id: u16) -> Vec<u8> {
-        self.os
-            .get_variable(&format!("Boot{id:04X}"), &VariableVendor::GLOBAL_VARIABLE)
+    pub fn get(id: u16) -> Box<[u8]> {
+        Variable::new(&format!("Boot{id:04X}"), VariableVendor::GLOBAL_VARIABLE)
+            .get()
             .unwrap()
             .0
     }
 
-    pub fn set(&self, id: u16, data: &[u8]) {
-        self.os
-            .set_variable(
-                &format!("Boot{id:04X}"),
-                &VariableVendor::GLOBAL_VARIABLE,
+    pub fn set(id: u16, data: &[u8]) {
+        Variable::new(&format!("Boot{id:04X}"), VariableVendor::GLOBAL_VARIABLE)
+            .set(
+                data,
                 VariableAttributes::NON_VOLATILE
                     | VariableAttributes::BOOTSERVICE_ACCESS
                     | VariableAttributes::RUNTIME_ACCESS,
-                data,
             )
             .unwrap();
     }
 
     /// Boot entry *must* be valid (TODO).
     /// Returns boot option description and device path of the option.
-    pub fn boot_entry_info<'a>(&self, entry: &'a [u8]) -> (String, &'a DevicePath) {
+    pub fn boot_entry_info(entry: &[u8]) -> (String, &DevicePath) {
         let skip_attropt = &entry[6..];
         let end_of_description = skip_attropt
             .chunks(2)
diff --git a/pixie-uefi/src/os/mod.rs b/pixie-uefi/src/os/mod.rs
@@ -19,18 +19,16 @@ use uefi::proto::device_path::build::DevicePathBuilder;
 use uefi::proto::device_path::text::{AllowShortcuts, DevicePathToText, DisplayOnly};
 use uefi::proto::device_path::DevicePath;
 use uefi::proto::Protocol;
-use uefi::runtime::{VariableAttributes, VariableVendor};
-use uefi::{CStr16, Event, Handle, Status};
+use uefi::{Event, Handle, Status};
 
-use self::boot_options::BootOptions;
 use self::disk::Disk;
-use self::error::{Error, Result};
+use self::error::Result;
 use self::executor::{Executor, Task};
 use self::net::NetworkInterface;
 use self::sync::SyncRefCell;
 use self::timer::Timer;
 
-mod boot_options;
+pub mod boot_options;
 pub mod disk;
 pub mod error;
 mod executor;
@@ -310,38 +308,6 @@ impl UefiOS {
         uefi::boot::wait_for_event(&mut [e]).unwrap();
     }
 
-    pub fn get_variable(
-        &self,
-        name: &str,
-        vendor: &VariableVendor,
-    ) -> Result<(Vec<u8>, VariableAttributes)> {
-        // name.len() should be enough, but...
-        let mut name_buf = vec![0u16; name.len() * 2 + 16];
-        let name = CStr16::from_str_with_buf(name, &mut name_buf).unwrap();
-        let (var, attrs) = uefi::runtime::get_variable_boxed(name, vendor)
-            .map_err(|e| Error(format!("Error getting variable: {e:?}")))?;
-        Ok((var.to_vec(), attrs))
-    }
-
-    pub fn set_variable(
-        &self,
-        name: &str,
-        vendor: &VariableVendor,
-        attrs: VariableAttributes,
-        data: &[u8],
-    ) -> Result<()> {
-        // name.len() should be enough, but...
-        let mut name_buf = vec![0u16; name.len() * 2 + 16];
-        let name = CStr16::from_str_with_buf(name, &mut name_buf).unwrap();
-        uefi::runtime::set_variable(name, vendor, attrs, data)
-            .map_err(|e| Error(format!("Error setting variable: {e:?}")))?;
-        Ok(())
-    }
-
-    pub fn boot_options(&self) -> BootOptions {
-        BootOptions { os: *self }
-    }
-
     pub fn device_path_to_string(&self, device: &DevicePath) -> String {
         let handle = uefi::boot::get_handle_for_protocol::<DevicePathToText>().unwrap();
         let device_path_to_text =
diff --git a/pixie-uefi/src/os/net.rs b/pixie-uefi/src/os/net.rs
@@ -19,6 +19,7 @@ use uefi::Status;
 use super::error::{Error, Result};
 use super::timer::Timer;
 use super::UefiOS;
+use crate::os::boot_options::BootOptions;
 use crate::os::timer::rdtsc;
 
 pub const PACKET_SIZE: usize = 1514;
@@ -151,9 +152,8 @@ pub struct NetworkInterface {
 
 impl NetworkInterface {
     pub fn new(os: UefiOS) -> NetworkInterface {
-        let bo = os.boot_options();
-        let curopt = bo.get(bo.current());
-        let (descr, device) = bo.boot_entry_info(&curopt[..]);
+        let curopt = BootOptions::get(BootOptions::current());
+        let (descr, device) = BootOptions::boot_entry_info(&curopt[..]);
         log::info!(
             "Configuring network on interface used for booting ({} -- {})",
             descr,
diff --git a/pixie-uefi/src/reboot_to_os.rs b/pixie-uefi/src/reboot_to_os.rs
@@ -1,17 +1,17 @@
+use crate::os::boot_options::BootOptions;
 use crate::os::UefiOS;
 
 pub async fn reboot_to_os(os: UefiOS) -> ! {
-    let bo = os.boot_options();
-    let next = bo.reboot_target();
+    let next = BootOptions::reboot_target();
     if let Some(next) = next {
         // Reboot to next boot option.
-        bo.set_next(next);
+        BootOptions::set_next(next);
     } else {
         log::warn!(
             "Did not find a valid boot order entry! current: {}",
-            bo.current()
+            BootOptions::current()
         );
-        log::warn!("{:?}", bo.order());
+        log::warn!("{:?}", BootOptions::order());
         os.sleep_us(100_000_000).await;
     }
     os.reset();
diff --git a/pixie-uefi/src/store.rs b/pixie-uefi/src/store.rs
@@ -9,6 +9,7 @@ use pixie_shared::util::BytesFmt;
 use pixie_shared::{Chunk, Image, Offset, TcpRequest, UdpRequest, MAX_CHUNK_SIZE};
 use uefi::proto::console::text::Color;
 
+use crate::os::boot_options::BootOptions;
 use crate::os::error::{Error, Result};
 use crate::os::{memory, TcpStream, UefiOS};
 use crate::{parse_disk, MIN_MEMORY};
@@ -65,9 +66,8 @@ pub async fn store(os: UefiOS, server_address: SocketAddrV4) -> Result<()> {
         }
     });
 
-    let bo = os.boot_options();
-    let boid = bo.reboot_target().expect("Could not find reboot target");
-    let bo_command = bo.get(boid);
+    let boid = BootOptions::reboot_target().expect("Could not find reboot target");
+    let bo_command = BootOptions::get(boid);
 
     let mut disk = os.open_first_disk();
     let chunks = parse_disk::parse_disk(&mut disk).await?;
@@ -198,7 +198,7 @@ pub async fn store(os: UefiOS, server_address: SocketAddrV4) -> Result<()> {
         &stream_upload_chunk,
         Image {
             boot_option_id: boid,
-            boot_entry: bo_command,
+            boot_entry: bo_command.to_vec(),
             disk: chunk_hashes,
         },
     )
