uefi_nvram_storage: save restore nvram contents (#1556)

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",
 ]
@@ -7276,10 +7277,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

openhcl/underhill_core/src/worker.rs

@@ -2064,16 +2064,20 @@ async fn new_underhill_vm(
                 logger: Box::new(UnderhillLogger {
                     get: get_client.clone(),
                 }),
-                nvram_storage: Box::new(HclCompatNvram::new(
-                    VmgsStorageBackendAdapter(
-                        vmgs_client
-                            .as_non_volatile_store(vmgs::FileId::BIOS_NVRAM, true)
-                            .context("failed to instantiate UEFI NVRAM store")?,
-                    ),
-                    Some(HclCompatNvramQuirks {
-                        skip_corrupt_vars_with_missing_null_term: true,
-                    }),
-                )),
+                nvram_storage: Box::new(
+                    HclCompatNvram::new(
+                        VmgsStorageBackendAdapter(
+                            vmgs_client
+                                .as_non_volatile_store(vmgs::FileId::BIOS_NVRAM, true)
+                                .context("failed to instantiate UEFI NVRAM store")?,
+                        ),
+                        Some(HclCompatNvramQuirks {
+                            skip_corrupt_vars_with_missing_null_term: true,
+                        }),
+                        is_restoring,
+                    )
+                    .await?,
+                ),
                 generation_id_recv: get_client
                     .take_generation_id_recv()
                     .await

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

openvmm/hvlite_core/src/worker/dispatch.rs

@@ -1067,13 +1067,17 @@ impl InitializedVm {
                         use vmm_core::emuplat::hcl_compat_uefi_nvram_storage::VmgsStorageBackendAdapter;
 
                         match vmgs_client {
-                            Some(vmgs) => Box::new(HclCompatNvram::new(
-                                VmgsStorageBackendAdapter(
-                                    vmgs.as_non_volatile_store(vmgs::FileId::BIOS_NVRAM, true)
-                                        .context("failed to instantiate UEFI NVRAM store")?,
-                                ),
-                                None,
-                            )),
+                            Some(vmgs) => Box::new(
+                                HclCompatNvram::new(
+                                    VmgsStorageBackendAdapter(
+                                        vmgs.as_non_volatile_store(vmgs::FileId::BIOS_NVRAM, true)
+                                            .context("failed to instantiate UEFI NVRAM store")?,
+                                    ),
+                                    None,
+                                    false,
+                                )
+                                .await?,
+                            ),
                             None => Box::new(InMemoryNvram::new()),
                         }
                     },

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

@@ -74,7 +74,7 @@ use std::convert::TryInto;
 use std::ops::RangeInclusive;
 use std::task::Context;
 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;
@@ -129,7 +129,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,