Skip to content

hcl_compat_uefi_nvram_storage: load from storage if no saved state #1697

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 17, 2025
Merged

hcl_compat_uefi_nvram_storage: load from storage if no saved state #1697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a735995
load uefi nvram from storage if no saved state
tjones60 Jul 15, 2025
0473eac
fix + lazy load
tjones60 Jul 15, 2025
e7a1747
no restoreerror change
tjones60 Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion openhcl/underhill_core/src/worker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2098,7 +2098,6 @@ async fn new_underhill_vm(
Some(HclCompatNvramQuirks {
skip_corrupt_vars_with_missing_null_term: true,
}),
is_restoring,
)
.await?,
)
Expand Down
1 change: 0 additions & 1 deletion openvmm/hvlite_core/src/worker/dispatch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,6 @@ impl InitializedVm {
.context("failed to instantiate UEFI NVRAM store")?,
),
None,
false,
)
.await?,
),
Expand Down
73 changes: 42 additions & 31 deletions vm/devices/firmware/hcl_compat_uefi_nvram_storage/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -121,9 +124,8 @@ impl<S: StorageBackend> HclCompatNvram<S> {
pub async fn new(
storage: S,
quirks: Option<HclCompatNvramQuirks>,
is_restoring: bool,
) -> Result<Self, NvramStorageError> {
let mut nvram = Self {
let nvram = Self {
quirks: quirks.unwrap_or(HclCompatNvramQuirks {
skip_corrupt_vars_with_missing_null_term: false,
}),
Expand All @@ -133,16 +135,14 @@ impl<S: StorageBackend> HclCompatNvram<S> {
in_memory: in_memory::InMemoryNvram::new(),

nvram_buf: Vec::new(),

loaded: false,
};
if !is_restoring {
nvram.load_from_storage().await?;
}
Ok(nvram)
}

async fn load_from_storage(&mut self) -> Result<(), NvramStorageError> {
tracing::info!("loading uefi nvram from storage");
let res = self.load_from_storage_inner().await;
async fn lazy_load_from_storage(&mut self) -> Result<(), NvramStorageError> {
let res = self.lazy_load_from_storage_inner().await;
if let Err(e) = &res {
tracing::error!(CVM_ALLOWED, "storage contains corrupt nvram state");
tracing::error!(
Expand All @@ -154,7 +154,13 @@ impl<S: StorageBackend> HclCompatNvram<S> {
res
}

async fn load_from_storage_inner(&mut self) -> Result<(), NvramStorageError> {
async fn lazy_load_from_storage_inner(&mut self) -> Result<(), NvramStorageError> {
if self.loaded {
return Ok(());
}

tracing::info!("loading uefi nvram from storage");

let nvram_buf = self
.storage
.restore()
Expand Down Expand Up @@ -293,6 +299,7 @@ impl<S: StorageBackend> HclCompatNvram<S> {
));
}

self.loaded = true;
Ok(())
}

Expand Down Expand Up @@ -343,8 +350,11 @@ impl<S: StorageBackend> HclCompatNvram<S> {

/// Iterate over the NVRAM entries. This function asynchronously loads the
/// NVRAM contents into memory from the backing storage if necessary.
pub fn iter(&mut self) -> impl Iterator<Item = in_memory::VariableEntry<'_>> {
self.in_memory.iter()
pub async fn iter(
&mut self,
) -> Result<impl Iterator<Item = in_memory::VariableEntry<'_>>, NvramStorageError> {
self.lazy_load_from_storage().await?;
Ok(self.in_memory.iter())
}
}

Expand All @@ -355,6 +365,8 @@ impl<S: StorageBackend> NvramStorage for HclCompatNvram<S> {
name: &Ucs2LeSlice,
vendor: Guid,
) -> Result<Option<(u32, Vec<u8>, EFI_TIME)>, NvramStorageError> {
self.lazy_load_from_storage().await?;

if name.as_bytes().len() > EFI_MAX_VARIABLE_NAME_SIZE {
return Err(NvramStorageError::VariableNameTooLong);
}
Expand All @@ -370,6 +382,8 @@ impl<S: StorageBackend> NvramStorage for HclCompatNvram<S> {
data: Vec<u8>,
timestamp: EFI_TIME,
) -> Result<(), NvramStorageError> {
self.lazy_load_from_storage().await?;

if name.as_bytes().len() > EFI_MAX_VARIABLE_NAME_SIZE {
return Err(NvramStorageError::VariableNameTooLong);
}
Expand Down Expand Up @@ -404,14 +418,15 @@ impl<S: StorageBackend> NvramStorage for HclCompatNvram<S> {

Ok(())
}

async fn append_variable(
&mut self,
name: &Ucs2LeSlice,
vendor: Guid,
data: Vec<u8>,
timestamp: EFI_TIME,
) -> Result<bool, NvramStorageError> {
self.lazy_load_from_storage().await?;

if name.as_bytes().len() > EFI_MAX_VARIABLE_NAME_SIZE {
return Err(NvramStorageError::VariableNameTooLong);
}
Expand Down Expand Up @@ -442,6 +457,8 @@ impl<S: StorageBackend> NvramStorage for HclCompatNvram<S> {
name: &Ucs2LeSlice,
vendor: Guid,
) -> Result<bool, NvramStorageError> {
self.lazy_load_from_storage().await?;

if name.as_bytes().len() > EFI_MAX_VARIABLE_NAME_SIZE {
return Err(NvramStorageError::VariableNameTooLong);
}
Expand All @@ -456,6 +473,8 @@ impl<S: StorageBackend> NvramStorage for HclCompatNvram<S> {
&mut self,
name_vendor: Option<(&Ucs2LeSlice, Guid)>,
) -> Result<NextVariable, NvramStorageError> {
self.lazy_load_from_storage().await?;

if let Some((name, _)) = name_vendor {
if name.as_bytes().len() > EFI_MAX_VARIABLE_NAME_SIZE {
return Err(NvramStorageError::VariableNameTooLong);
Expand All @@ -481,7 +500,11 @@ mod save_restore {
}

fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> {
self.in_memory.restore(state)
if state.nvram.is_some() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have tests below that cover both cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, but we still need to add cross version open source servicing tests.

self.in_memory.restore(state)?;
self.loaded = true;
}
Ok(())
}
}
}
Expand Down Expand Up @@ -516,36 +539,28 @@ mod test {
#[async_test]
async fn test_single_variable() {
let mut storage = EphemeralStorageBackend::default();
let mut nvram = HclCompatNvram::new(&mut storage, None, false)
.await
.unwrap();
let mut nvram = HclCompatNvram::new(&mut storage, None).await.unwrap();
impl_agnostic_tests::test_single_variable(&mut nvram).await;
}

#[async_test]
async fn test_multiple_variable() {
let mut storage = EphemeralStorageBackend::default();
let mut nvram = HclCompatNvram::new(&mut storage, None, false)
.await
.unwrap();
let mut nvram = HclCompatNvram::new(&mut storage, None).await.unwrap();
impl_agnostic_tests::test_multiple_variable(&mut nvram).await;
}

#[async_test]
async fn test_next() {
let mut storage = EphemeralStorageBackend::default();
let mut nvram = HclCompatNvram::new(&mut storage, None, false)
.await
.unwrap();
let mut nvram = HclCompatNvram::new(&mut storage, None).await.unwrap();
impl_agnostic_tests::test_next(&mut nvram).await;
}

#[async_test]
async fn boundary_conditions() {
let mut storage = EphemeralStorageBackend::default();
let mut nvram = HclCompatNvram::new(&mut storage, None, false)
.await
.unwrap();
let mut nvram = HclCompatNvram::new(&mut storage, None).await.unwrap();

let vendor = Guid::new_random();
let attr = 0x1234;
Expand Down Expand Up @@ -633,9 +648,7 @@ mod test {
let data = vec![0x1, 0x2, 0x3, 0x4, 0x5];
let timestamp = EFI_TIME::default();

let mut nvram = HclCompatNvram::new(&mut storage, None, false)
.await
.unwrap();
let mut nvram = HclCompatNvram::new(&mut storage, None).await.unwrap();
nvram
.set_variable(name1, vendor1, attr, data.clone(), timestamp)
.await
Expand All @@ -652,9 +665,7 @@ mod test {
drop(nvram);

// reload
let mut nvram = HclCompatNvram::new(&mut storage, None, false)
.await
.unwrap();
let mut nvram = HclCompatNvram::new(&mut storage, None).await.unwrap();

let (result_attr, result_data, result_timestamp) =
nvram.get_variable(name1, vendor1).await.unwrap().unwrap();
Expand Down
62 changes: 32 additions & 30 deletions vm/devices/firmware/uefi_nvram_storage/src/in_memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ mod save_restore {
#[mesh(package = "firmware.in_memory_nvram")]
pub struct SavedState {
#[mesh(1)]
pub nvram: Vec<NvramEntry>,
pub nvram: Option<Vec<NvramEntry>>,
}
}

Expand All @@ -225,39 +225,41 @@ mod save_restore {

fn save(&mut self) -> Result<Self::SavedState, SaveError> {
Ok(state::SavedState {
nvram: self
.nvram
.iter()
.map(|(k, v)| state::NvramEntry {
vendor: k.vendor,
name: k.name.clone().into_inner(),
data: v.data.clone(),
timestamp: v.timestamp,
attr: v.attr,
})
.collect(),
nvram: Some(
self.nvram
.iter()
.map(|(k, v)| state::NvramEntry {
vendor: k.vendor,
name: k.name.clone().into_inner(),
data: v.data.clone(),
timestamp: v.timestamp,
attr: v.attr,
})
.collect(),
),
})
}

fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> {
let state::SavedState { nvram } = state;
self.nvram = nvram
.into_iter()
.map::<Result<(VariableKey, Variable), RestoreError>, _>(|e| {
Ok((
VariableKey {
vendor: e.vendor,
name: Ucs2LeVec::from_vec_with_nul(e.name)
.map_err(|e| RestoreError::InvalidSavedState(e.into()))?,
},
Variable {
data: e.data,
timestamp: e.timestamp,
attr: e.attr,
},
))
})
.collect::<Result<BTreeMap<_, _>, _>>()?;
if let state::SavedState { nvram: Some(nvram) } = state {
self.nvram = nvram
.into_iter()
.map::<Result<(VariableKey, Variable), RestoreError>, _>(|e| {
Ok((
VariableKey {
vendor: e.vendor,
name: Ucs2LeVec::from_vec_with_nul(e.name)
.map_err(|e| RestoreError::InvalidSavedState(e.into()))?,
},
Variable {
data: e.data,
timestamp: e.timestamp,
attr: e.attr,
},
))
})
.collect::<Result<BTreeMap<_, _>, _>>()?;
}
Ok(())
}
}
Expand Down
3 changes: 1 addition & 2 deletions vm/vmgs/vmgstool/src/uefi_nvram.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ async fn dump_nvram(
truncate: bool,
) -> Result<(), Error> {
let mut count = 0;
for entry in nvram_storage.iter() {
for entry in nvram_storage.iter().await? {
let meta = NvramEntryMetadata {
vendor: entry.vendor.to_string(),
name: entry.name.to_string(),
Expand Down Expand Up @@ -354,7 +354,6 @@ async fn open_nvram(
VmgsStorageBackend::new(vmgs, vmgs::FileId::BIOS_NVRAM, encrypted)
.map_err(Error::VmgsStorageBackend)?,
None,
false,
)
.await?)
}
Expand Down