Refactor to handle log output outside of pvmfw
This unifies the components to handle debug policies,
and also make pvmfw to depend less on the host's input.
This also limits the number of components to handle debug policy
to make further debug policy changes easier.
Bug: 272496125
Test: atest MicrodroidTestApp MicrodroidHostTestCases
Change-Id: I4404240c1b9ce381be93b0d935c4d29643a06973
diff --git a/pvmfw/src/debug_policy.rs b/pvmfw/src/debug_policy.rs
index f4b99a6..bbf7e04 100644
--- a/pvmfw/src/debug_policy.rs
+++ b/pvmfw/src/debug_policy.rs
@@ -14,12 +14,8 @@
//! Support for the debug policy overlay in pvmfw
-use crate::cstr;
-use alloc::vec::Vec;
-use core::ffi::CStr;
use core::fmt;
use libfdt::FdtError;
-use log::info;
#[derive(Debug, Clone)]
pub enum DebugPolicyError {
@@ -64,60 +60,6 @@
fdt.pack().map_err(|e| DebugPolicyError::OverlaidFdt("Failed to re-pack", e))
}
-/// Enables console output by adding kernel.printk.devkmsg and kernel.console to bootargs.
-/// This uses hardcoded console name 'hvc0' and it should be match with microdroid's bootconfig.debuggable.
-fn enable_console_output(fdt: &mut libfdt::Fdt) -> Result<(), DebugPolicyError> {
- let chosen = match fdt
- .node(cstr!("/chosen"))
- .map_err(|e| DebugPolicyError::Fdt("Failed to find /chosen", e))?
- {
- Some(node) => node,
- None => return Ok(()),
- };
-
- let bootargs = match chosen
- .getprop_str(cstr!("bootargs"))
- .map_err(|e| DebugPolicyError::Fdt("Failed to find bootargs prop", e))?
- {
- Some(value) if !value.to_bytes().is_empty() => value,
- _ => return Ok(()),
- };
-
- let mut new_bootargs = Vec::from(bootargs.to_bytes());
- new_bootargs.extend_from_slice(b" printk.devkmsg=on console=hvc0\0");
-
- // We'll set larger prop, and need to prepare some room first.
- fdt.unpack().map_err(|e| DebugPolicyError::OverlaidFdt("Failed to unpack", e))?;
-
- // We've checked existence of /chosen node at the beginning.
- let mut chosen_mut = fdt.node_mut(cstr!("/chosen")).unwrap().unwrap();
- chosen_mut.setprop(cstr!("bootargs"), new_bootargs.as_slice()).map_err(|e| {
- DebugPolicyError::OverlaidFdt("Failed to enabled console output. FDT might be corrupted", e)
- })?;
-
- fdt.pack().map_err(|e| DebugPolicyError::OverlaidFdt("Failed to pack", e))?;
- Ok(())
-}
-
-/// Returns true only if fdt has log prop in the /avf/guest/common node with value <1>
-fn is_console_output_enabled(fdt: &libfdt::Fdt) -> Result<bool, DebugPolicyError> {
- let common = match fdt
- .node(cstr!("/avf/guest/common"))
- .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!("log"))
- .map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find log prop", e))?
- {
- Some(1) => Ok(true),
- _ => Ok(false),
- }
-}
-
/// Handles debug policies.
///
/// # Safety
@@ -132,10 +74,5 @@
apply_debug_policy(fdt, dp)?;
}
- // Handles console output in the debug policy
- if is_console_output_enabled(fdt)? {
- enable_console_output(fdt)?;
- info!("console output is enabled by debug policy");
- }
Ok(())
}
diff --git a/tests/hostside/Android.bp b/tests/hostside/Android.bp
index 4b5cbda..248755f 100644
--- a/tests/hostside/Android.bp
+++ b/tests/hostside/Android.bp
@@ -9,20 +9,6 @@
}
genrule {
- name: "test_avf_debug_policy_with_log.dtbo",
- defaults: ["test_avf_debug_policy_overlay"],
- srcs: ["assets/avf_debug_policy_with_log.dts"],
- out: ["avf_debug_policy_with_log.dtbo"],
-}
-
-genrule {
- name: "test_avf_debug_policy_without_log.dtbo",
- defaults: ["test_avf_debug_policy_overlay"],
- srcs: ["assets/avf_debug_policy_without_log.dts"],
- out: ["avf_debug_policy_without_log.dtbo"],
-}
-
-genrule {
name: "test_avf_debug_policy_with_adb",
defaults: ["test_avf_debug_policy_overlay"],
srcs: ["assets/avf_debug_policy_with_adb.dts"],
@@ -60,8 +46,6 @@
":test.com.android.virt.pem",
":test2.com.android.virt.pem",
":pvmfw_test",
- ":test_avf_debug_policy_with_log.dtbo",
- ":test_avf_debug_policy_without_log.dtbo",
":test_avf_debug_policy_with_adb",
":test_avf_debug_policy_without_adb",
"assets/bcc.dat",
diff --git a/tests/hostside/assets/avf_debug_policy_with_log.dts b/tests/hostside/assets/avf_debug_policy_with_log.dts
deleted file mode 100644
index 8cf19d6..0000000
--- a/tests/hostside/assets/avf_debug_policy_with_log.dts
+++ /dev/null
@@ -1,18 +0,0 @@
-/dts-v1/;
-/plugin/;
-
-/ {
- fragment@avf {
- target-path = "/";
-
- __overlay__ {
- avf {
- guest {
- common {
- log = <1>;
- };
- };
- };
- };
- };
-};
\ No newline at end of file
diff --git a/tests/hostside/assets/avf_debug_policy_without_log.dts b/tests/hostside/assets/avf_debug_policy_without_log.dts
deleted file mode 100644
index da6400c..0000000
--- a/tests/hostside/assets/avf_debug_policy_without_log.dts
+++ /dev/null
@@ -1,18 +0,0 @@
-/dts-v1/;
-/plugin/;
-
-/ {
- fragment@avf {
- target-path = "/";
-
- __overlay__ {
- avf {
- guest {
- common {
- log = <0>;
- };
- };
- };
- };
- };
-};
\ No newline at end of file
diff --git a/tests/hostside/java/com/android/microdroid/test/PvmfwDebugPolicyHostTests.java b/tests/hostside/java/com/android/microdroid/test/PvmfwDebugPolicyHostTests.java
index 18aa273..ddcc866 100644
--- a/tests/hostside/java/com/android/microdroid/test/PvmfwDebugPolicyHostTests.java
+++ b/tests/hostside/java/com/android/microdroid/test/PvmfwDebugPolicyHostTests.java
@@ -151,56 +151,6 @@
}
@Test
- public void testLog_consoleOutput() throws Exception {
- Pvmfw pvmfw = createPvmfw("avf_debug_policy_with_log.dtbo");
- pvmfw.serialize(mCustomPvmfwBinFileOnHost);
-
- CommandResult result = tryLaunchProtectedNonDebuggableVm();
-
- assertWithMessage("Microdroid's console message should have been enabled")
- .that(hasConsoleOutput(result))
- .isTrue();
- }
-
- @Test
- public void testLog_logcat() throws Exception {
- Pvmfw pvmfw = createPvmfw("avf_debug_policy_with_log.dtbo");
- pvmfw.serialize(mCustomPvmfwBinFileOnHost);
-
- tryLaunchProtectedNonDebuggableVm();
-
- assertWithMessage("Microdroid's logcat should have been enabled")
- .that(hasMicrodroidLogcatOutput())
- .isTrue();
- }
-
- @Test
- public void testNoLog_noConsoleOutput() throws Exception {
- Pvmfw pvmfw = createPvmfw("avf_debug_policy_without_log.dtbo");
- pvmfw.serialize(mCustomPvmfwBinFileOnHost);
-
- CommandResult result = tryLaunchProtectedNonDebuggableVm();
-
- assertWithMessage("Microdroid's console message shouldn't have been disabled")
- .that(hasConsoleOutput(result))
- .isFalse();
- }
-
- @Test
- public void testNoLog_noLogcat() throws Exception {
- Pvmfw pvmfw = createPvmfw("avf_debug_policy_without_log.dtbo");
- pvmfw.serialize(mCustomPvmfwBinFileOnHost);
-
- assertThrows(
- "Microdroid shouldn't be recognized because of missing adb connection",
- DeviceRuntimeException.class,
- () ->
- launchProtectedVmAndWaitForBootCompleted(
- MICRODROID_DEBUG_NONE, BOOT_FAILURE_WAIT_TIME_MS));
- assertThat(hasMicrodroidLogcatOutput()).isFalse();
- }
-
- @Test
public void testAdb_boots() throws Exception {
assumeTrue(
"Skip if host wouldn't install adbd",
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 7201670..8497564 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -732,18 +732,13 @@
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"));
- }
- if config.debug_level == DebugLevel::NONE
- && should_prepare_console_output(config.debug_level)
- {
- // bootconfig.normal will be used, but we need log.
- // pvmfw will add following commands by itself, but non-protected VM should do so here.
- command.arg("--params").arg("printk.devkmsg=on");
- command.arg("--params").arg("console=hvc0");
- }
+ } else if config.ramdump.is_some() {
+ command.arg("--params").arg(format!("crashkernel={RAMDUMP_RESERVED_MIB}M"));
+ }
+ if config.debug_level == DebugLevel::NONE && should_prepare_console_output(config.debug_level) {
+ // bootconfig.normal will be used, but we need log.
+ command.arg("--params").arg("printk.devkmsg=on");
+ command.arg("--params").arg("console=hvc0");
}
if let Some(memory_mib) = config.memory_mib {