Disable ramdump with debug policy
This CL disables ramdump if debug policy doesn't explicitly
enables ramdump via ramdump prop in the /avf/guest/common node.
The virtualization service always provides crashkernel=17M via
kernel command, so this CL removes crashkernel=17MB in the
microdroid's bootargs in the /chosen node.
Here's the test result on my machine with MicrodroidTestApp.
- Before the CL (or this CL + ramdump is enabled)
$ adb shell /proc/meminfo
MemTotal: 212968 kB
MemFree: 138336 kB
MemAvailable: 171980 kB
- With this CL + ramdump is disabled
$ adb shell /proc/meminfo
MemTotal: 230372 kB
MemFree: 165240 kB
MemAvailable: 192648 kB
Bug: 243630590
Test: Boot microdroid with following AVF debug policies \
- AVF debug policy exists, and ramdump=<1> \
- AVF debug policy exists, and ramdump=<0> \
- No AVF debug policy
Change-Id: Ia486448b5513c2d5662a4f16ddb3334b20913329
diff --git a/libs/libfdt/src/lib.rs b/libs/libfdt/src/lib.rs
index 8fd1879..29d7abe 100644
--- a/libs/libfdt/src/lib.rs
+++ b/libs/libfdt/src/lib.rs
@@ -388,6 +388,23 @@
fdt_err_expect_zero(ret)
}
+ /// Create or change a property name-value pair to the given node.
+ pub fn setprop(&mut self, name: &CStr, value: &[u8]) -> Result<()> {
+ // SAFETY - New value size is constrained to the DT totalsize
+ // (validated by underlying libfdt).
+ let ret = unsafe {
+ libfdt_bindgen::fdt_setprop(
+ self.fdt.as_mut_ptr(),
+ self.offset,
+ name.as_ptr(),
+ value.as_ptr().cast::<c_void>(),
+ value.len().try_into().map_err(|_| FdtError::BadValue)?,
+ )
+ };
+
+ fdt_err_expect_zero(ret)
+ }
+
/// Get reference to the containing device tree.
pub fn fdt(&mut self) -> &mut Fdt {
self.fdt
diff --git a/pvmfw/src/debug_policy.rs b/pvmfw/src/debug_policy.rs
new file mode 100644
index 0000000..37e2af8
--- /dev/null
+++ b/pvmfw/src/debug_policy.rs
@@ -0,0 +1,152 @@
+// Copyright 2023, The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Support for the debug policy overlay in pvmfw
+
+use alloc::vec;
+use core::ffi::CStr;
+use core::fmt;
+use libfdt::FdtError;
+use log::info;
+
+#[derive(Debug, Clone)]
+pub enum DebugPolicyError {
+ /// The provided baseline FDT was invalid or malformed, so cannot access certain node/prop
+ Fdt(&'static str, FdtError),
+ /// The provided debug policy FDT was invalid or malformed.
+ DebugPolicyFdt(&'static str, FdtError),
+ /// The overlaid result FDT is invalid or malformed, and may be corrupted.
+ OverlaidFdt(&'static str, FdtError),
+}
+
+impl fmt::Display for DebugPolicyError {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ match self {
+ Self::Fdt(s, e) => write!(f, "Invalid baseline FDT. {s}: {e}"),
+ Self::DebugPolicyFdt(s, e) => write!(f, "Invalid overlay FDT. {s}: {e}"),
+ Self::OverlaidFdt(s, e) => write!(f, "Invalid overlaid FDT. {s}: {e}"),
+ }
+ }
+}
+
+/// Applies the debug policy device tree overlay to the pVM DT.
+///
+/// # Safety
+///
+/// When an error is returned by this function, the input `Fdt` should be
+/// discarded as it may have have been partially corrupted during the overlay
+/// application process.
+unsafe fn apply_debug_policy(
+ fdt: &mut libfdt::Fdt,
+ debug_policy: &mut [u8],
+) -> Result<(), DebugPolicyError> {
+ let overlay = libfdt::Fdt::from_mut_slice(debug_policy)
+ .map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to load debug policy overlay", e))?;
+
+ fdt.unpack().map_err(|e| DebugPolicyError::Fdt("Failed to unpack", e))?;
+
+ let fdt = fdt
+ .apply_overlay(overlay)
+ .map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to apply overlay", e))?;
+
+ fdt.pack().map_err(|e| DebugPolicyError::OverlaidFdt("Failed to re-pack", e))
+}
+
+/// Dsiables ramdump by removing crashkernel from bootargs in /chosen.
+///
+/// # Safety
+///
+/// This may corrupt the input `Fdt` when error happens while editing prop value.
+unsafe fn disable_ramdump(fdt: &mut libfdt::Fdt) -> Result<(), DebugPolicyError> {
+ let chosen_path = CStr::from_bytes_with_nul(b"/chosen\0").unwrap();
+ let bootargs_name = CStr::from_bytes_with_nul(b"bootargs\0").unwrap();
+
+ let chosen = match fdt
+ .node(chosen_path)
+ .map_err(|e| DebugPolicyError::Fdt("Failed to find /chosen", e))?
+ {
+ Some(node) => node,
+ None => return Ok(()),
+ };
+
+ let bootargs = match chosen
+ .getprop_str(bootargs_name)
+ .map_err(|e| DebugPolicyError::Fdt("Failed to find bootargs prop", e))?
+ {
+ Some(value) if !value.to_bytes().is_empty() => value,
+ _ => return Ok(()),
+ };
+
+ // TODO: Improve add 'crashkernel=17MB' only when it's unnecessary.
+ // Currently 'crashkernel=17MB' in virtualizationservice and passed by
+ // chosen node, because it's not exactly a debug policy but a
+ // configuration. However, it's actually microdroid specific
+ // so we need a way to generalize it.
+ let mut args = vec![];
+ for arg in bootargs.to_bytes().split(|byte| byte.is_ascii_whitespace()) {
+ if arg.is_empty() || arg.starts_with(b"crashkernel=") {
+ continue;
+ }
+ args.push(arg);
+ }
+ let mut new_bootargs = args.as_slice().join(&b" "[..]);
+ new_bootargs.push(b'\0');
+
+ // We've checked existence of /chosen node at the beginning.
+ let mut chosen_mut = fdt.node_mut(chosen_path).unwrap().unwrap();
+ chosen_mut.setprop(bootargs_name, new_bootargs.as_slice()).map_err(|e| {
+ DebugPolicyError::OverlaidFdt("Failed to remove crashkernel. FDT might be corrupted", e)
+ })
+}
+
+/// Returns true only if fdt has ramdump prop in the /avf/guest/common node with value <1>
+fn is_ramdump_enabled(fdt: &libfdt::Fdt) -> Result<bool, DebugPolicyError> {
+ let common = match fdt
+ .node(CStr::from_bytes_with_nul(b"/avf/guest/common\0").unwrap())
+ .map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find /avf/guest/common node", e))?
+ {
+ Some(node) => node,
+ None => return Ok(false),
+ };
+
+ match common
+ .getprop_u32(CStr::from_bytes_with_nul(b"ramdump\0").unwrap())
+ .map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find ramdump prop", e))?
+ {
+ Some(1) => Ok(true),
+ _ => Ok(false),
+ }
+}
+
+/// Handles debug policies.
+///
+/// # Safety
+///
+/// This may corrupt the input `Fdt` when overlaying debug policy or applying
+/// ramdump configuration.
+pub unsafe fn handle_debug_policy(
+ fdt: &mut libfdt::Fdt,
+ debug_policy: Option<&mut [u8]>,
+) -> Result<(), DebugPolicyError> {
+ if let Some(dp) = debug_policy {
+ apply_debug_policy(fdt, dp)?;
+ }
+
+ // Handles ramdump in the debug policy
+ if is_ramdump_enabled(fdt)? {
+ info!("ramdump is enabled by debug policy");
+ return Ok(());
+ }
+ disable_ramdump(fdt)
+}
diff --git a/pvmfw/src/entry.rs b/pvmfw/src/entry.rs
index 4f30902..c7ae011 100644
--- a/pvmfw/src/entry.rs
+++ b/pvmfw/src/entry.rs
@@ -15,6 +15,7 @@
//! Low-level entry and exit points of pvmfw.
use crate::config;
+use crate::debug_policy::{handle_debug_policy, DebugPolicyError};
use crate::fdt;
use crate::heap;
use crate::helpers;
@@ -52,6 +53,16 @@
SecretDerivationError,
}
+impl From<DebugPolicyError> for RebootReason {
+ fn from(error: DebugPolicyError) -> Self {
+ match error {
+ DebugPolicyError::Fdt(_, _) => RebootReason::InvalidFdt,
+ DebugPolicyError::DebugPolicyFdt(_, _) => RebootReason::InvalidConfig,
+ DebugPolicyError::OverlaidFdt(_, _) => RebootReason::InternalError,
+ }
+ }
+}
+
main!(start);
/// Entry point for pVM firmware.
@@ -178,37 +189,6 @@
}
}
-/// Applies the debug policy device tree overlay to the pVM DT.
-///
-/// # Safety
-///
-/// When an error is returned by this function, the input `Fdt` should be discarded as it may have
-/// have been partially corrupted during the overlay application process.
-unsafe fn apply_debug_policy(
- fdt: &mut libfdt::Fdt,
- debug_policy: &mut [u8],
-) -> Result<(), RebootReason> {
- let overlay = libfdt::Fdt::from_mut_slice(debug_policy).map_err(|e| {
- error!("Failed to load the debug policy overlay: {e}");
- RebootReason::InvalidConfig
- })?;
-
- fdt.unpack().map_err(|e| {
- error!("Failed to unpack DT for debug policy: {e}");
- RebootReason::InternalError
- })?;
-
- let fdt = fdt.apply_overlay(overlay).map_err(|e| {
- error!("Failed to apply the debug policy overlay: {e}");
- RebootReason::InvalidConfig
- })?;
-
- fdt.pack().map_err(|e| {
- error!("Failed to re-pack DT after debug policy: {e}");
- RebootReason::InternalError
- })
-}
-
/// Sets up the environment for main() and wraps its result for start().
///
/// Provide the abstractions necessary for start() to abort the pVM boot and for main() to run with
@@ -283,9 +263,12 @@
helpers::flushed_zeroize(bcc_slice);
helpers::flush(slices.fdt.as_slice());
- if let Some(debug_policy) = appended.get_debug_policy() {
- // SAFETY - As we `?` the result, there is no risk of re-using a bad `slices.fdt`.
- unsafe { apply_debug_policy(slices.fdt, debug_policy) }?;
+ // SAFETY - As we `?` the result, there is no risk of using a bad `slices.fdt`.
+ unsafe {
+ handle_debug_policy(slices.fdt, appended.get_debug_policy()).map_err(|e| {
+ error!("Unexpected error when handling debug policy: {e:?}");
+ RebootReason::from(e)
+ })?;
}
info!("Expecting a bug making MMIO_GUARD_UNMAP return NOT_SUPPORTED on success");
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index 24c36b3..faadd37 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -22,6 +22,7 @@
mod avb;
mod config;
+mod debug_policy;
mod dice;
mod entry;
mod exceptions;