Enable microdroid console output with debug level or debug policy
With this CL, pvmfw will add kernel.print.devkmsg and console to
bootargs with hardcoded value from microdroid's debuggable bootconfig.
Test: manually start protected/unprotected VMs \
with various debug levels and debug policies.
Change-Id: I31600f0d337ff47edfb5cfea60f5e6a010c85ab5
diff --git a/pvmfw/src/debug_policy.rs b/pvmfw/src/debug_policy.rs
index 37e2af8..4cb338d 100644
--- a/pvmfw/src/debug_policy.rs
+++ b/pvmfw/src/debug_policy.rs
@@ -14,7 +14,7 @@
//! Support for the debug policy overlay in pvmfw
-use alloc::vec;
+use alloc::{vec, vec::Vec};
use core::ffi::CStr;
use core::fmt;
use libfdt::FdtError;
@@ -63,12 +63,8 @@
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> {
+/// Disables ramdump by removing crashkernel from bootargs in /chosen.
+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();
@@ -129,6 +125,63 @@
}
}
+/// 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_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(()),
+ };
+
+ 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(chosen_path).unwrap().unwrap();
+ chosen_mut.setprop(bootargs_name, 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::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"log\0").unwrap())
+ .map_err(|e| DebugPolicyError::DebugPolicyFdt("Failed to find log prop", e))?
+ {
+ Some(1) => Ok(true),
+ _ => Ok(false),
+ }
+}
+
/// Handles debug policies.
///
/// # Safety
@@ -146,7 +199,14 @@
// Handles ramdump in the debug policy
if is_ramdump_enabled(fdt)? {
info!("ramdump is enabled by debug policy");
- return Ok(());
+ } else {
+ disable_ramdump(fdt)?;
}
- disable_ramdump(fdt)
+
+ // Handles conseole 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 f1e5054..d217c00 100644
--- a/tests/hostside/Android.bp
+++ b/tests/hostside/Android.bp
@@ -22,6 +22,20 @@
out: ["avf_debug_policy_without_ramdump.dtbo"],
}
+genrule {
+ name: "test_avf_debug_policy_with_console_output",
+ defaults: ["test_avf_debug_policy_overlay"],
+ srcs: ["assets/avf_debug_policy_with_console_output.dts"],
+ out: ["avf_debug_policy_with_console_output.dtbo"],
+}
+
+genrule {
+ name: "test_avf_debug_policy_without_console_output",
+ defaults: ["test_avf_debug_policy_overlay"],
+ srcs: ["assets/avf_debug_policy_without_console_output.dts"],
+ out: ["avf_debug_policy_without_console_output.dtbo"],
+}
+
java_test_host {
name: "MicrodroidHostTestCases",
srcs: ["java/**/*.java"],
@@ -48,6 +62,8 @@
":pvmfw_test",
":test_avf_debug_policy_with_ramdump",
":test_avf_debug_policy_without_ramdump",
+ ":test_avf_debug_policy_with_console_output",
+ ":test_avf_debug_policy_without_console_output",
"assets/bcc.dat",
],
data_native_bins: [
diff --git a/tests/hostside/assets/avf_debug_policy_with_console_output.dts b/tests/hostside/assets/avf_debug_policy_with_console_output.dts
new file mode 100644
index 0000000..8cf19d6
--- /dev/null
+++ b/tests/hostside/assets/avf_debug_policy_with_console_output.dts
@@ -0,0 +1,18 @@
+/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_console_output.dts b/tests/hostside/assets/avf_debug_policy_without_console_output.dts
new file mode 100644
index 0000000..da6400c
--- /dev/null
+++ b/tests/hostside/assets/avf_debug_policy_without_console_output.dts
@@ -0,0 +1,18 @@
+/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 bda52dd..1ada5a1 100644
--- a/tests/hostside/java/com/android/microdroid/test/PvmfwDebugPolicyHostTests.java
+++ b/tests/hostside/java/com/android/microdroid/test/PvmfwDebugPolicyHostTests.java
@@ -19,6 +19,7 @@
import static com.android.tradefed.device.TestDevice.MicrodroidBuilder;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assume.assumeTrue;
@@ -32,6 +33,7 @@
import com.android.tradefed.device.TestDevice;
import com.android.tradefed.device.ITestDevice;
import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
+import com.android.tradefed.util.CommandResult;
import com.android.tradefed.util.FileUtil;
import org.junit.After;
@@ -41,6 +43,7 @@
import java.io.File;
import java.util.Objects;
+import java.util.concurrent.TimeUnit;
import java.io.FileNotFoundException;
/** Tests debug policy of pvmfw.bin with custom debug policy */
@@ -50,13 +53,16 @@
@NonNull private static final String BCC_FILE_NAME = "bcc.dat";
@NonNull private static final String PACKAGE_FILE_NAME = "MicrodroidTestApp.apk";
@NonNull private static final String PACKAGE_NAME = "com.android.microdroid.test";
- @NonNull private static final String MICRODROID_DEBUG_LEVEL = "full";
+ @NonNull private static final String MICRODROID_DEBUG_FULL = "full";
@NonNull private static final String MICRODROID_CONFIG_PATH = "assets/vm_config_apex.json";
- private static final int BOOT_COMPLETE_TIMEOUT = 30000; // 30 seconds
+ @NonNull private static final String MICRODROID_LOG_PATH = TEST_ROOT + "log.txt";
+ @NonNull private static final String MICRODROID_CONSOLE_PATH = TEST_ROOT + "console.txt";
+ private static final int BOOT_COMPLETE_TIMEOUT_MS = 30000; // 30 seconds
+ private static final int CONSOLE_OUTPUT_WAIT_MS = 5000; // 5 seconds
@NonNull private static final String CUSTOM_PVMFW_FILE_PREFIX = "pvmfw";
@NonNull private static final String CUSTOM_PVMFW_FILE_SUFFIX = ".bin";
- @NonNull private static final String CUSTOM_PVMFW_IMG_PATH = TEST_ROOT + "/" + PVMFW_FILE_NAME;
+ @NonNull private static final String CUSTOM_PVMFW_IMG_PATH = TEST_ROOT + PVMFW_FILE_NAME;
@NonNull private static final String CUSTOM_PVMFW_IMG_PATH_PROP = "hypervisor.pvmfw.path";
@NonNull private static final String MICRODROID_CMDLINE_PATH = "/proc/cmdline";
@@ -162,6 +168,30 @@
.isEqualTo(HEX_STRING_ZERO);
}
+ @Test
+ public void testConsoleOutput() throws Exception {
+ Pvmfw pvmfw = createPvmfw("avf_debug_policy_with_console_output.dtbo");
+ pvmfw.serialize(mCustomPvmfwBinFileOnHost);
+
+ CommandResult result = tryLaunchProtectedNonDebuggableVm();
+
+ assertWithMessage("Microdroid's console message should have been enabled")
+ .that(hasConsoleOutput(result))
+ .isTrue();
+ }
+
+ @Test
+ public void testNoConsoleOutput() throws Exception {
+ Pvmfw pvmfw = createPvmfw("avf_debug_policy_without_console_output.dtbo");
+ pvmfw.serialize(mCustomPvmfwBinFileOnHost);
+
+ CommandResult result = tryLaunchProtectedNonDebuggableVm();
+
+ assertWithMessage("Microdroid's console message shouldn't have been disabled")
+ .that(hasConsoleOutput(result))
+ .isFalse();
+ }
+
@NonNull
private String readMicrodroidFileAsString(@NonNull String path)
throws DeviceNotAvailableException {
@@ -184,18 +214,50 @@
.build();
}
+ @NonNull
+ private boolean hasConsoleOutput(CommandResult result) throws DeviceNotAvailableException {
+ return result.getStdout().contains("Run /init as init process");
+ }
+
private ITestDevice launchProtectedVmAndWaitForBootCompleted()
throws DeviceNotAvailableException {
mMicrodroidDevice =
MicrodroidBuilder.fromDevicePath(
getPathForPackage(PACKAGE_NAME), MICRODROID_CONFIG_PATH)
- .debugLevel(MICRODROID_DEBUG_LEVEL)
+ .debugLevel(MICRODROID_DEBUG_FULL)
.protectedVm(/* protectedVm= */ true)
.addBootFile(mCustomPvmfwBinFileOnHost, PVMFW_FILE_NAME)
.build(mAndroidDevice);
- assertThat(mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT)).isTrue();
+ assertThat(mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT_MS)).isTrue();
assertThat(mMicrodroidDevice.enableAdbRoot()).isTrue();
-
return mMicrodroidDevice;
}
+
+ // Try to launch protected non-debuggable VM for a while and quit.
+ // Non-debuggable VM doesn't enable adb, so there's no ITestDevice instance of it.
+ private CommandResult tryLaunchProtectedNonDebuggableVm() throws DeviceNotAvailableException {
+ // Can't use MicrodroidBuilder because it expects adb connection
+ // but non-debuggable VM doesn't enable adb.
+ CommandRunner runner = new CommandRunner(mAndroidDevice);
+ runner.run("mkdir", "-p", TEST_ROOT);
+ mAndroidDevice.pushFile(mCustomPvmfwBinFileOnHost, TEST_ROOT + PVMFW_FILE_NAME);
+
+ // This will fail because app wouldn't finish itself.
+ // But let's run the app once and get logs.
+ String command =
+ String.join(
+ " ",
+ "/apex/com.android.virt/bin/vm",
+ "run-app",
+ "--log",
+ MICRODROID_LOG_PATH,
+ "--protected",
+ getPathForPackage(PACKAGE_NAME),
+ TEST_ROOT + "idsig",
+ TEST_ROOT + "instance.img",
+ "--config-path",
+ MICRODROID_CONFIG_PATH);
+ return mAndroidDevice.executeShellV2Command(
+ command, CONSOLE_OUTPUT_WAIT_MS, TimeUnit.MILLISECONDS, /* retryAttempts= */ 0);
+ }
}
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index aceb319..1781007 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -19,6 +19,7 @@
write_vm_booted_stats, write_vm_creation_stats};
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::payload::{add_microdroid_payload_images, add_microdroid_system_images};
use crate::selinux::{getfilecon, SeContext};
use android_os_permissions_aidl::aidl::android::os::IPermissionController;
@@ -978,13 +979,6 @@
})
}
-fn is_debuggable(config: &VirtualMachineConfig) -> bool {
- match config {
- VirtualMachineConfig::AppConfig(config) => config.debugLevel != DebugLevel::NONE,
- _ => false,
- }
-}
-
fn is_protected(config: &VirtualMachineConfig) -> bool {
match config {
VirtualMachineConfig::RawConfig(config) => config.protectedVm,
@@ -1031,7 +1025,11 @@
return Ok(Some(clone_file(fd)?));
}
- if !is_debuggable(config) {
+ if let VirtualMachineConfig::AppConfig(app_config) = config {
+ if !should_prepare_console_output(app_config.debugLevel) {
+ return Ok(None);
+ }
+ } else {
return Ok(None);
}
diff --git a/virtualizationmanager/src/debug_config.rs b/virtualizationmanager/src/debug_config.rs
new file mode 100644
index 0000000..332df08
--- /dev/null
+++ b/virtualizationmanager/src/debug_config.rs
@@ -0,0 +1,39 @@
+// 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.
+
+//! Functions for AVF debug policy and debug level
+
+use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
+ VirtualMachineAppConfig::DebugLevel::DebugLevel
+};
+use std::fs::File;
+use std::io::Read;
+
+/// Get debug policy value in bool. It's true iff the value is explicitly set to <1>.
+fn get_debug_policy_bool(path: &'static str) -> Option<bool> {
+ if let Ok(mut file) = File::open(path) {
+ let mut log: [u8; 4] = Default::default();
+ file.read_exact(&mut log).map_err(|_| false).unwrap();
+ // DT spec uses big endian although Android is always little endian.
+ return Some(u32::from_be_bytes(log) == 1);
+ }
+ None
+}
+
+/// Get whether console output should be configred for VM to leave console and adb log.
+/// Caller should create pipe and prepare for receiving VM log with it.
+pub fn should_prepare_console_output(debug_level: DebugLevel) -> bool {
+ debug_level != DebugLevel::NONE
+ || get_debug_policy_bool("/proc/device-tree/avf/guest/common/log").unwrap_or_default()
+}
diff --git a/virtualizationmanager/src/main.rs b/virtualizationmanager/src/main.rs
index 3f0b64b..bd7f8af 100644
--- a/virtualizationmanager/src/main.rs
+++ b/virtualizationmanager/src/main.rs
@@ -18,6 +18,7 @@
mod atom;
mod composite;
mod crosvm;
+mod debug_config;
mod payload;
mod selinux;