Change the condition for preparing for ramdump on the crosvm side
Preparing for ramdump on the crosvm side means two activities:
* creating an empty file for storing the ramdump (and making it as the
backing file for the /dev/hvc1 virtio console)
* adding `crashkernel=xxM` parameter
For protected VMs: ramdump is prepared if debug policy (accessed by
reading the device tree) says ramdump is enabled. Note that we can do
this unconditionally for protected VMs because pvmfw will anyway kick in
and remove the parameter if ramdump is enabled. We do this conditionally
on the host just because we don't want to create the ramdump file
unnecessarily.
For non-protected VMs: ramdump is prepared if debug policy says so OR
the debugLevel is full. This allows developers to do the ramdump even on
a device where debug policy is not provisioed.
Bug: 270657304
Test: test ramdump on the following combinations:
* dp provisioned + pVM + debugLevel=none: ramdump enabled
* dp provisioned + pVM + debugLevel=full: ramdump enabled
* dp provisioned + non-pVM + debugLevel=none: ramdump enabled
* dp provisioned + non-pVM + debugLevel=full: ramdump enabled
* dp empty + pVM + debugLevel=none: ramdump disabled
* dp empty + pVM + debugLevel=full: ramdump disabled [note]
* dp empty + non-pVM + debugLevel=none: ramdump disabled
* dp empty + non-pVM + debugLevel=full: ramdump enabled
[note] `crashkernel=xx` is sent from the host. However, pvmfw
unconditionally removes the parameter from the cmdline because dp is not
provisioned. It could have checked the debug level by looking into the
bootconfig in the initrd, but it's too dirty.
Change-Id: I52ce34e0f9ef6404e482abec728c8108341a9fd6
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index 1781007..25516bb 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -20,6 +20,7 @@
use crate::composite::make_composite_image;
use crate::crosvm::{CrosvmConfig, DiskFile, PayloadState, VmContext, VmInstance, VmState};
use crate::debug_config::should_prepare_console_output;
+use crate::debug_config::is_ramdump_needed;
use crate::payload::{add_microdroid_payload_images, add_microdroid_system_images};
use crate::selinux::{getfilecon, SeContext};
use android_os_permissions_aidl::aidl::android::os::IPermissionController;
@@ -318,6 +319,12 @@
check_gdb_allowed(config)?;
}
+ let ramdump = if is_ramdump_needed(config) {
+ Some(prepare_ramdump_file(&temporary_directory)?)
+ } else {
+ None
+ };
+
let state = &mut *self.state.lock().unwrap();
let console_fd =
clone_or_prepare_logger_fd(config, console_fd, format!("Console({})", cid))?;
@@ -408,19 +415,6 @@
}
};
- // Creating this ramdump file unconditionally is not harmful as ramdump will be created
- // only when the VM is configured as such. `ramdump_write` is sent to crosvm and will
- // be the backing store for the /dev/hvc1 where VM will emit ramdump to. `ramdump_read`
- // will be sent back to the client (i.e. the VM owner) for readout.
- let ramdump_path = temporary_directory.join("ramdump");
- let ramdump = prepare_ramdump_file(&ramdump_path).map_err(|e| {
- error!("Failed to prepare ramdump file: {:?}", e);
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to prepare ramdump file: {:?}", e)),
- )
- })?;
-
// Actually start the VM.
let crosvm_config = CrosvmConfig {
cid,
@@ -437,7 +431,7 @@
task_profiles: config.taskProfiles.clone(),
console_fd,
log_fd,
- ramdump: Some(ramdump),
+ ramdump,
indirect_files,
platform_version: parse_platform_version_req(&config.platformVersion)?,
detect_hangup: is_app_config,
@@ -486,10 +480,6 @@
part.flush()
}
-fn prepare_ramdump_file(ramdump_path: &Path) -> Result<File> {
- File::create(ramdump_path).context(format!("Failed to create ramdump file {:?}", &ramdump_path))
-}
-
fn round_up(input: u64, granularity: u64) -> u64 {
if granularity == 0 {
return input;
@@ -979,6 +969,22 @@
})
}
+/// Create the empty ramdump file
+fn prepare_ramdump_file(temporary_directory: &Path) -> binder::Result<File> {
+ // `ramdump_write` is sent to crosvm and will be the backing store for the /dev/hvc1 where
+ // VM will emit ramdump to. `ramdump_read` will be sent back to the client (i.e. the VM
+ // owner) for readout.
+ let ramdump_path = temporary_directory.join("ramdump");
+ let ramdump = File::create(ramdump_path).map_err(|e| {
+ error!("Failed to prepare ramdump file: {:?}", e);
+ Status::new_service_specific_error_str(
+ -1,
+ Some(format!("Failed to prepare ramdump file: {:?}", e)),
+ )
+ })?;
+ Ok(ramdump)
+}
+
fn is_protected(config: &VirtualMachineConfig) -> bool {
match config {
VirtualMachineConfig::RawConfig(config) => config.protectedVm,
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 1b52507..6ac1658 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -667,16 +667,6 @@
}
}
-fn ramdump_enabled() -> bool {
- if let Ok(mut file) = File::open("/proc/device-tree/avf/guest/common/ramdump") {
- let mut ramdump: [u8; 4] = Default::default();
- file.read_exact(&mut ramdump).map_err(|_| false).unwrap();
- // DT spec uses big endian although Android is always little endian.
- return u32::from_be_bytes(ramdump) == 1;
- }
- false
-}
-
/// Starts an instance of `crosvm` to manage a new VM.
fn run_vm(
config: CrosvmConfig,
@@ -723,12 +713,14 @@
// Context in b/238324526.
command.arg("--unmap-guest-memory-on-fork");
- // Protected VM needs to reserve memory for ramdump here. pvmfw will drop This
- // if ramdump should be disabled (via debug policy). Note that we reserve more
- // memory for the restricted dma pool.
- let ramdump_reserve = RAMDUMP_RESERVED_MIB + swiotlb_size_mib;
- command.arg("--params").arg(format!("crashkernel={ramdump_reserve}M"));
- } else if ramdump_enabled() {
+ if config.ramdump.is_some() {
+ // Protected VM needs to reserve memory for ramdump here. pvmfw will drop This
+ // if ramdump should be disabled (via debug policy). Note that we reserve more
+ // memory for the restricted dma pool.
+ let ramdump_reserve = RAMDUMP_RESERVED_MIB + swiotlb_size_mib;
+ command.arg("--params").arg(format!("crashkernel={ramdump_reserve}M"));
+ }
+ } else if config.ramdump.is_some() {
command.arg("--params").arg(format!("crashkernel={RAMDUMP_RESERVED_MIB}M"));
}
diff --git a/virtualizationmanager/src/debug_config.rs b/virtualizationmanager/src/debug_config.rs
index 332df08..d564770 100644
--- a/virtualizationmanager/src/debug_config.rs
+++ b/virtualizationmanager/src/debug_config.rs
@@ -15,7 +15,7 @@
//! Functions for AVF debug policy and debug level
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
- VirtualMachineAppConfig::DebugLevel::DebugLevel
+ VirtualMachineAppConfig::DebugLevel::DebugLevel, VirtualMachineConfig::VirtualMachineConfig,
};
use std::fs::File;
use std::io::Read;
@@ -37,3 +37,24 @@
debug_level != DebugLevel::NONE
|| get_debug_policy_bool("/proc/device-tree/avf/guest/common/log").unwrap_or_default()
}
+
+/// Decision to support ramdump
+pub fn is_ramdump_needed(config: &VirtualMachineConfig) -> bool {
+ let enabled_in_dp =
+ get_debug_policy_bool("/proc/device-tree/avf/guest/common/ramdump").unwrap_or_default();
+ let (protected, debuggable) = match config {
+ VirtualMachineConfig::RawConfig(config) => {
+ // custom VMs are considered debuggable for flexibility
+ (config.protectedVm, true)
+ }
+ VirtualMachineConfig::AppConfig(config) => {
+ (config.protectedVm, config.debugLevel == DebugLevel::FULL)
+ }
+ };
+
+ if protected {
+ enabled_in_dp
+ } else {
+ enabled_in_dp || debuggable
+ }
+}