uefi_nvram_storage: save restore nvram contents (#1556) (#1675)

Implement save/restore for HclCompatNvram/NvramStorage/InMemoryNvram so
that OpenHCL doesn't need to re-read from the VMGS file after a
servicing or other save-restore operation. This will help avoid panics
if the VMGS file is not accessible in certain VM teardown cases.

Author: tjones60

Cargo.lock

@@ -2799,6 +2799,7 @@ dependencies = [
  "tracing",
  "ucs2 0.0.0",
  "uefi_nvram_storage",
+ "vmcore",
  "wchar",
  "zerocopy 0.8.24",
 ]
@@ -7275,10 +7276,12 @@ dependencies = [
  "async-trait",
  "guid",
  "inspect",
+ "mesh_protobuf",
  "pal_async",
  "thiserror 2.0.12",
  "ucs2 0.0.0",
  "uefi_specs",
+ "vmcore",
  "wchar",
  "zerocopy 0.8.24",
 ]

openhcl/underhill_core/Cargo.toml

@@ -56,7 +56,7 @@ hyperv_ic_resources.workspace = true
 hyperv_secure_boot_templates.workspace = true
 hyperv_uefi_custom_vars_json.workspace = true
 framebuffer.workspace = true
-hcl_compat_uefi_nvram_storage = { workspace = true, features = ["inspect"] }
+hcl_compat_uefi_nvram_storage = { workspace = true, features = ["inspect", "save_restore"] }
 get_helpers.workspace = true
 get_protocol.workspace = true
 guest_emulation_transport.workspace = true

openvmm/hvlite_core/Cargo.toml

@@ -52,10 +52,10 @@ disk_backend.workspace = true
 firmware_pcat.workspace = true
 firmware_uefi_custom_vars.workspace = true
 firmware_uefi.workspace = true
-uefi_nvram_storage.workspace = true
+uefi_nvram_storage = { workspace = true, features = ["save_restore"] }
 framebuffer.workspace = true
 get_resources.workspace = true
-hcl_compat_uefi_nvram_storage = { workspace = true, features = ["inspect"] }
+hcl_compat_uefi_nvram_storage = { workspace = true, features = ["inspect", "save_restore"] }
 ide.workspace = true
 floppy.workspace = true
 input_core.workspace = true

vm/devices/firmware/firmware_uefi/Cargo.toml

@@ -22,7 +22,7 @@ fuzzing = []
 
 [dependencies]
 firmware_uefi_custom_vars.workspace = true
-uefi_nvram_storage = { workspace = true, features = ["inspect"] }
+uefi_nvram_storage = { workspace = true, features = ["inspect", "save_restore"] }
 uefi_specs.workspace = true
 uefi_nvram_specvars.workspace = true
 generation_id.workspace = true

vm/devices/firmware/firmware_uefi/src/lib.rs

@@ -76,7 +76,7 @@ use std::ops::RangeInclusive;
 use std::task::Context;
 use std::task::Poll;
 use thiserror::Error;
-use uefi_nvram_storage::InspectableNvramStorage;
+use uefi_nvram_storage::VmmNvramStorage;
 use vmcore::device_state::ChangeDeviceState;
 use vmcore::vmtime::VmTimeSource;
 use watchdog_core::platform::WatchdogPlatform;
@@ -131,7 +131,7 @@ pub struct UefiConfig {
 /// Various runtime objects used by the UEFI device + underlying services.
 pub struct UefiRuntimeDeps<'a> {
     pub gm: GuestMemory,
-    pub nvram_storage: Box<dyn InspectableNvramStorage>,
+    pub nvram_storage: Box<dyn VmmNvramStorage>,
     pub logger: Box<dyn UefiLogger>,
     pub vmtime: &'a VmTimeSource,
     pub watchdog_platform: Box<dyn WatchdogPlatform>,

vm/devices/firmware/firmware_uefi/src/service/nvram/mod.rs

@@ -24,7 +24,7 @@ use inspect::Inspect;
 use std::borrow::Cow;
 use std::fmt::Debug;
 use thiserror::Error;
-use uefi_nvram_storage::InspectableNvramStorage;
+use uefi_nvram_storage::VmmNvramStorage;
 use uefi_specs::uefi::common::EfiStatus;
 use uefi_specs::uefi::nvram::EfiVariableAttributes;
 use zerocopy::IntoBytes;
@@ -67,12 +67,12 @@ pub struct NvramServices {
 
     // Sub-emulators
     #[inspect(flatten)]
-    services: NvramSpecServices<Box<dyn InspectableNvramStorage>>,
+    services: NvramSpecServices<Box<dyn VmmNvramStorage>>,
 }
 
 impl NvramServices {
     pub async fn new(
-        nvram_storage: Box<dyn InspectableNvramStorage>,
+        nvram_storage: Box<dyn VmmNvramStorage>,
         custom_vars: CustomVars,
         secure_boot_enabled: bool,
         vsm_config: Option<Box<dyn VsmConfig>>,
@@ -622,15 +622,14 @@ mod save_restore {
     mod state {
         use crate::service::nvram::NvramSpecServices;
         use mesh::payload::Protobuf;
-        use uefi_nvram_storage::InspectableNvramStorage;
+        use uefi_nvram_storage::VmmNvramStorage;
         use vmcore::save_restore::SaveRestore;
 
         #[derive(Protobuf)]
         #[mesh(package = "firmware.uefi.nvram")]
         pub struct SavedState {
             #[mesh(1)]
-            pub services:
-                <NvramSpecServices<Box<dyn InspectableNvramStorage>> as SaveRestore>::SavedState,
+            pub services: <NvramSpecServices<Box<dyn VmmNvramStorage>> as SaveRestore>::SavedState,
         }
     }
 

vm/devices/firmware/firmware_uefi/src/service/nvram/spec_services/mod.rs

@@ -23,9 +23,9 @@ use ucs2::Ucs2LeSlice;
 use ucs2::Ucs2ParseError;
 use uefi_nvram_specvars::signature_list;
 use uefi_nvram_specvars::signature_list::ParseSignatureLists;
-use uefi_nvram_storage::InspectableNvramStorage;
 use uefi_nvram_storage::NextVariable;
 use uefi_nvram_storage::NvramStorageError;
+use uefi_nvram_storage::VmmNvramStorage;
 use uefi_specs::uefi::common::EfiStatus;
 use uefi_specs::uefi::nvram::EfiVariableAttributes;
 use uefi_specs::uefi::time::EFI_TIME;
@@ -224,12 +224,12 @@ impl RuntimeState {
 /// `NvramError` type provides additional context as to what error occurred in
 /// OpenVMM (i.e: for logging purposes).
 #[derive(Debug, Inspect)]
-pub struct NvramSpecServices<S: InspectableNvramStorage> {
+pub struct NvramSpecServices<S: VmmNvramStorage> {
     storage: S,
     runtime_state: RuntimeState,
 }
 
-impl<S: InspectableNvramStorage> NvramSpecServices<S> {
+impl<S: VmmNvramStorage> NvramSpecServices<S> {
     /// Construct a new NvramServices instance from an existing storage backend.
     pub fn new(storage: S) -> NvramSpecServices<S> {
         NvramSpecServices {
@@ -1453,6 +1453,8 @@ mod save_restore {
 
     mod state {
         use mesh::payload::Protobuf;
+        use uefi_nvram_storage::in_memory::InMemoryNvram;
+        use vmcore::save_restore::SaveRestore;
 
         #[derive(Protobuf)]
         #[mesh(package = "firmware.uefi.nvram.spec")]
@@ -1470,10 +1472,12 @@ mod save_restore {
         pub struct SavedState {
             #[mesh(1)]
             pub runtime_state: SavedRuntimeState,
+            #[mesh(2)]
+            pub storage: <InMemoryNvram as SaveRestore>::SavedState,
         }
     }
 
-    impl<S: InspectableNvramStorage> SaveRestore for NvramSpecServices<S> {
+    impl<S: VmmNvramStorage> SaveRestore for NvramSpecServices<S> {
         type SavedState = state::SavedState;
 
         fn save(&mut self) -> Result<Self::SavedState, SaveError> {
@@ -1483,17 +1487,22 @@ mod save_restore {
                     RuntimeState::Boot => state::SavedRuntimeState::Boot,
                     RuntimeState::Runtime => state::SavedRuntimeState::Runtime,
                 },
+                storage: self.storage.save()?,
             })
         }
 
         fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> {
-            let state::SavedState { runtime_state } = state;
+            let state::SavedState {
+                runtime_state,
+                storage,
+            } = state;
 
             self.runtime_state = match runtime_state {
                 state::SavedRuntimeState::PreBoot => RuntimeState::PreBoot,
                 state::SavedRuntimeState::Boot => RuntimeState::Boot,
                 state::SavedRuntimeState::Runtime => RuntimeState::Runtime,
             };
+            self.storage.restore(storage)?;
 
             Ok(())
         }
@@ -1526,7 +1535,7 @@ mod test {
     }
 
     #[async_trait::async_trait]
-    impl<S: InspectableNvramStorage> NvramServicesTestExt for NvramSpecServices<S> {
+    impl<S: VmmNvramStorage> NvramServicesTestExt for NvramSpecServices<S> {
         async fn set_test_var(&mut self, name: &[u8], attr: u32, data: &[u8]) -> NvramResult<()> {
             let vendor = Guid::default();
 

vm/devices/firmware/firmware_uefi/src/service/nvram/spec_services/nvram_services_ext.rs

@@ -6,7 +6,7 @@ use super::NvramResult;
 use super::NvramSpecServices;
 use guid::Guid;
 use ucs2::Ucs2LeSlice;
-use uefi_nvram_storage::InspectableNvramStorage;
+use uefi_nvram_storage::VmmNvramStorage;
 use uefi_specs::uefi::common::EfiStatus;
 
 /// Extension trait around `NvramServices` that makes it easier to use the API
@@ -56,7 +56,7 @@ pub trait NvramServicesExt {
 }
 
 #[async_trait::async_trait]
-impl<S: InspectableNvramStorage> NvramServicesExt for NvramSpecServices<S> {
+impl<S: VmmNvramStorage> NvramServicesExt for NvramSpecServices<S> {
     async fn get_variable(
         &mut self,
         vendor: Guid,

vm/devices/firmware/hcl_compat_uefi_nvram_storage/Cargo.toml

@@ -10,9 +10,11 @@ rust-version.workspace = true
 default = []
 
 inspect = ["dep:inspect", "uefi_nvram_storage/inspect"]
+save_restore = [ "dep:vmcore", "uefi_nvram_storage/save_restore"]
 
 [dependencies]
 uefi_nvram_storage.workspace = true
+vmcore = { workspace = true, optional = true }
 
 cvm_tracing.workspace = true
 guid.workspace = true

vm/devices/firmware/hcl_compat_uefi_nvram_storage/src/lib.rs

@@ -94,6 +94,9 @@ pub struct HclCompatNvram<S> {
     // reduced allocator pressure
     #[cfg_attr(feature = "inspect", inspect(skip))] // internal bookkeeping - not worth inspecting
     nvram_buf: Vec<u8>,
+
+    // whether the NVRAM has been loaded, either from storage or saved state
+    loaded: bool,
 }
 
 /// "Quirks" to take into account when loading/storing nvram blob data.
@@ -129,6 +132,8 @@ impl<S: StorageBackend> HclCompatNvram<S> {
             in_memory: in_memory::InMemoryNvram::new(),
 
             nvram_buf: Vec::new(),
+
+            loaded: false,
         }
     }
 
@@ -146,10 +151,12 @@ impl<S: StorageBackend> HclCompatNvram<S> {
     }
 
     async fn lazy_load_from_storage_inner(&mut self) -> Result<(), NvramStorageError> {
-        if !self.nvram_buf.is_empty() {
+        if self.loaded {
             return Ok(());
         }
 
+        tracing::info!("loading uefi nvram from storage");
+
         let nvram_buf = self
             .storage
             .restore()
@@ -288,11 +295,13 @@ impl<S: StorageBackend> HclCompatNvram<S> {
             ));
         }
 
+        self.loaded = true;
         Ok(())
     }
 
     /// Dump in-memory nvram to the underlying storage device.
     async fn flush_storage(&mut self) -> Result<(), NvramStorageError> {
+        tracing::info!("flushing uefi nvram to storage");
         self.nvram_buf.clear();
 
         for in_memory::VariableEntry {
@@ -473,6 +482,30 @@ impl<S: StorageBackend> NvramStorage for HclCompatNvram<S> {
     }
 }
 
+#[cfg(feature = "save_restore")]
+mod save_restore {
+    use super::*;
+    use vmcore::save_restore::RestoreError;
+    use vmcore::save_restore::SaveError;
+    use vmcore::save_restore::SaveRestore;
+
+    impl<S: StorageBackend> SaveRestore for HclCompatNvram<S> {
+        type SavedState = <in_memory::InMemoryNvram as SaveRestore>::SavedState;
+
+        fn save(&mut self) -> Result<Self::SavedState, SaveError> {
+            self.in_memory.save()
+        }
+
+        fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> {
+            if state.nvram.is_some() {
+                self.in_memory.restore(state)?;
+                self.loaded = true;
+            }
+            Ok(())
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::storage_backend::StorageBackend;