Merge changes from topics "microdroid_early_kernel_log", "no_logcat_on_microdroid_tests"

* changes:
  Stop VM immediately with stop() call
  Implement microdroid boot time test
  Add a helper for device tests to remove duplicates
  Use log file instead of micrdroid logcat for tests
  Start seriallogging eariler on microdroid
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 8d74f5e..0ce7991 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -484,8 +484,13 @@
      * #run()}.
      */
     public void stop() throws VirtualMachineException {
-        // Dropping the IVirtualMachine handle stops the VM
-        mVirtualMachine = null;
+        if (mVirtualMachine == null) return;
+        try {
+            mVirtualMachine.stop();
+            mVirtualMachine = null;
+        } catch (RemoteException e) {
+            throw new VirtualMachineException(e);
+        }
     }
 
     /**
diff --git a/microdroid/Android.bp b/microdroid/Android.bp
index 3bbab13..65aeb07 100644
--- a/microdroid/Android.bp
+++ b/microdroid/Android.bp
@@ -57,8 +57,8 @@
         "libbinder",
         "libbinder_ndk",
         "libstdc++",
-        "logcat",
-        "logd",
+        "logcat.microdroid",
+        "logd.microdroid",
         "secilc",
 
         // "com.android.adbd" requires these,
diff --git a/microdroid/init.rc b/microdroid/init.rc
index ff3d68e..be08bbd 100644
--- a/microdroid/init.rc
+++ b/microdroid/init.rc
@@ -62,6 +62,13 @@
     start vendor.dice-microdroid
     start diced
 
+on init && property:ro.boot.logd.enabled=1
+    # Start logd before any other services run to ensure we capture all of their logs.
+    # TODO(b/217796229) set filterspec if debug_level is app_only
+    start logd
+    start seriallogging
+
+on init
     mkdir /mnt/apk 0755 system system
     mkdir /mnt/extra-apk 0755 root root
     # Microdroid_manager starts apkdmverity/zipfuse/apexd
@@ -85,10 +92,6 @@
     # Mount tracefs (with GID=AID_READTRACEFS)
     mount tracefs tracefs /sys/kernel/tracing gid=3012
 
-on init && property:ro.boot.logd.enabled=1
-    # Start logd before any other services run to ensure we capture all of their logs.
-    start logd
-
 on init && property:ro.boot.adb.enabled=1
     start adbd
 
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 350fbc5..ae26787 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -69,7 +69,6 @@
 const VMADDR_CID_HOST: u32 = 2;
 
 const APEX_CONFIG_DONE_PROP: &str = "apex_config.done";
-const LOGD_ENABLED_PROP: &str = "ro.boot.logd.enabled";
 const APP_DEBUGGABLE_PROP: &str = "ro.boot.microdroid.app_debuggable";
 
 // SYNC WITH virtualizationservice/src/crosvm.rs
@@ -599,12 +598,6 @@
     info!("notifying payload started");
     service.notifyPayloadStarted()?;
 
-    // Start logging if enabled
-    // TODO(b/200914564) set filterspec if debug_level is app_only
-    if system_properties::read_bool(LOGD_ENABLED_PROP, false)? {
-        system_properties::write("ctl.start", "seriallogging")?;
-    }
-
     let exit_status = command.spawn()?.wait()?;
     exit_status.code().ok_or_else(|| anyhow!("Failed to get exit_code from the paylaod."))
 }
diff --git a/tests/benchmark/Android.bp b/tests/benchmark/Android.bp
index f333d03..e6d5b83 100644
--- a/tests/benchmark/Android.bp
+++ b/tests/benchmark/Android.bp
@@ -9,6 +9,7 @@
     ],
     srcs: ["src/java/**/*.java"],
     static_libs: [
+        "MicroroidDeviceTestHelper",
         "androidx.test.runner",
         "androidx.test.ext.junit",
         "truth-prebuilt",
diff --git a/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java b/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
index 864d2d5..e96f58b 100644
--- a/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
+++ b/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
@@ -20,25 +20,14 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 
-import static org.junit.Assume.assumeNoException;
-
 import android.app.Instrumentation;
-import android.content.Context;
 import android.os.Bundle;
-import android.os.ParcelFileDescriptor;
 import android.os.SystemProperties;
-import android.sysprop.HypervisorProperties;
-import android.system.virtualizationservice.DeathReason;
-import android.system.virtualmachine.VirtualMachine;
-import android.system.virtualmachine.VirtualMachineCallback;
 import android.system.virtualmachine.VirtualMachineConfig;
 import android.system.virtualmachine.VirtualMachineConfig.DebugLevel;
 import android.system.virtualmachine.VirtualMachineException;
-import android.system.virtualmachine.VirtualMachineManager;
-import android.util.Log;
 
-import androidx.annotation.CallSuper;
-import androidx.test.core.app.ApplicationProvider;
+import com.android.microdroid.test.MicrodroidDeviceTestBase;
 
 import org.junit.After;
 import org.junit.Before;
@@ -48,17 +37,10 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
-import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 @RunWith(Parameterized.class)
-public class MicrodroidBenchmarks {
+public class MicrodroidBenchmarks extends MicrodroidDeviceTestBase {
     private static final String TAG = "MicrodroidBenchmarks";
 
     @Rule public Timeout globalTimeout = Timeout.seconds(300);
@@ -74,41 +56,6 @@
                         || productName.startsWith("cf_arm"));
     }
 
-    /** Copy output from the VM to logcat. This is helpful when things go wrong. */
-    private static void logVmOutput(InputStream vmOutputStream, String name) {
-        new Thread(
-                () -> {
-                    try {
-                        BufferedReader reader =
-                                new BufferedReader(new InputStreamReader(vmOutputStream));
-                        String line;
-                        while ((line = reader.readLine()) != null
-                                && !Thread.interrupted()) {
-                            Log.i(TAG, name + ": " + line);
-                        }
-                    } catch (Exception e) {
-                        Log.w(TAG, name, e);
-                    }
-                }).start();
-    }
-
-    private static class Inner {
-        public boolean mProtectedVm;
-        public Context mContext;
-        public VirtualMachineManager mVmm;
-        public VirtualMachine mVm;
-
-        Inner(boolean protectedVm) {
-            mProtectedVm = protectedVm;
-        }
-
-        /** Create a new VirtualMachineConfig.Builder with the parameterized protection mode. */
-        public VirtualMachineConfig.Builder newVmConfigBuilder(String payloadConfigPath) {
-            return new VirtualMachineConfig.Builder(mContext, payloadConfigPath)
-                    .protectedVm(mProtectedVm);
-        }
-    }
-
     @Parameterized.Parameters(name = "protectedVm={0}")
     public static Object[] protectedVmConfigs() {
         return new Object[] {false, true};
@@ -116,128 +63,17 @@
 
     @Parameterized.Parameter public boolean mProtectedVm;
 
-    private boolean mPkvmSupported = false;
-    private Inner mInner;
-
     private Instrumentation mInstrumentation;
 
     @Before
     public void setup() {
-        // In case when the virt APEX doesn't exist on the device, classes in the
-        // android.system.virtualmachine package can't be loaded. Therefore, before using the
-        // classes, check the existence of a class in the package and skip this test if not exist.
-        try {
-            Class.forName("android.system.virtualmachine.VirtualMachineManager");
-            mPkvmSupported = true;
-        } catch (ClassNotFoundException e) {
-            assumeNoException(e);
-            return;
-        }
-        if (mProtectedVm) {
-            assume().withMessage("Skip where protected VMs aren't support")
-                    .that(HypervisorProperties.hypervisor_protected_vm_supported().orElse(false))
-                    .isTrue();
-        } else {
-            assume().withMessage("Skip where VMs aren't support")
-                    .that(HypervisorProperties.hypervisor_vm_supported().orElse(false))
-                    .isTrue();
-        }
-        mInner = new Inner(mProtectedVm);
-        mInner.mContext = ApplicationProvider.getApplicationContext();
-        mInner.mVmm = VirtualMachineManager.getInstance(mInner.mContext);
+        prepareTestSetup(mProtectedVm);
         mInstrumentation = getInstrumentation();
     }
 
     @After
     public void cleanup() throws VirtualMachineException {
-        if (!mPkvmSupported) {
-            return;
-        }
-        if (mInner == null) {
-            return;
-        }
-        if (mInner.mVm == null) {
-            return;
-        }
-        mInner.mVm.stop();
-        mInner.mVm.delete();
-    }
-
-    private abstract static class VmEventListener implements VirtualMachineCallback {
-        private ExecutorService mExecutorService = Executors.newSingleThreadExecutor();
-
-        void runToFinish(VirtualMachine vm) throws VirtualMachineException, InterruptedException {
-            vm.setCallback(mExecutorService, this);
-            vm.run();
-            logVmOutput(vm.getConsoleOutputStream(), "Console");
-            logVmOutput(vm.getLogOutputStream(), "Log");
-            mExecutorService.awaitTermination(300, TimeUnit.SECONDS);
-        }
-
-        void forceStop(VirtualMachine vm) {
-            try {
-                vm.clearCallback();
-                vm.stop();
-                mExecutorService.shutdown();
-            } catch (VirtualMachineException e) {
-                throw new RuntimeException(e);
-            }
-        }
-
-        @Override
-        public void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stream) {}
-
-        @Override
-        public void onPayloadReady(VirtualMachine vm) {}
-
-        @Override
-        public void onPayloadFinished(VirtualMachine vm, int exitCode) {}
-
-        @Override
-        public void onError(VirtualMachine vm, int errorCode, String message) {}
-
-        @Override
-        @CallSuper
-        public void onDied(VirtualMachine vm, @DeathReason int reason) {
-            mExecutorService.shutdown();
-        }
-
-        @Override
-        public void onRamdump(VirtualMachine vm, ParcelFileDescriptor ramdump) {}
-    }
-
-    private static class BootResult {
-        public final boolean payloadStarted;
-        public final int deathReason;
-
-        BootResult(boolean payloadStarted, int deathReason) {
-            this.payloadStarted = payloadStarted;
-            this.deathReason = deathReason;
-        }
-    }
-
-    private BootResult tryBootVm(String vmName)
-            throws VirtualMachineException, InterruptedException {
-        mInner.mVm = mInner.mVmm.get(vmName); // re-load the vm before running tests
-        final CompletableFuture<Boolean> payloadStarted = new CompletableFuture<>();
-        final CompletableFuture<Integer> deathReason = new CompletableFuture<>();
-        VmEventListener listener =
-                new VmEventListener() {
-                    @Override
-                    public void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stream) {
-                        payloadStarted.complete(true);
-                        forceStop(vm);
-                    }
-
-                    @Override
-                    public void onDied(VirtualMachine vm, int reason) {
-                        deathReason.complete(reason);
-                        super.onDied(vm, reason);
-                    }
-                };
-        listener.runToFinish(mInner.mVm);
-        return new BootResult(
-                payloadStarted.getNow(false), deathReason.getNow(DeathReason.INFRASTRUCTURE_ERROR));
+        cleanupTestSetup();
     }
 
     private boolean canBootMicrodroidWithMemory(int mem)
@@ -246,18 +82,13 @@
 
         // returns true if succeeded at least once.
         for (int i = 0; i < trialCount; i++) {
-            VirtualMachine existingVm = mInner.mVmm.get("test_vm_minimum_memory");
-            if (existingVm != null) {
-                existingVm.delete();
-            }
-
             VirtualMachineConfig.Builder builder =
                     mInner.newVmConfigBuilder("assets/vm_config.json");
             VirtualMachineConfig normalConfig =
                     builder.debugLevel(DebugLevel.FULL).memoryMib(mem).build();
-            mInner.mVmm.create("test_vm_minimum_memory", normalConfig);
+            mInner.forceCreateNewVirtualMachine("test_vm_minimum_memory", normalConfig);
 
-            if (tryBootVm("test_vm_minimum_memory").payloadStarted) return true;
+            if (tryBootVm(TAG, "test_vm_minimum_memory").payloadStarted) return true;
         }
 
         return false;
@@ -288,4 +119,44 @@
         bundle.putInt("avf_perf/microdroid/minimum_required_memory", minimum);
         mInstrumentation.sendStatus(0, bundle);
     }
+
+    @Test
+    public void testMicrodroidBootTime()
+            throws VirtualMachineException, InterruptedException, IOException {
+        assume().withMessage("Skip on CF; too slow").that(isCuttlefish()).isFalse();
+
+        final int trialCount = 10;
+
+        double sum = 0;
+        double squareSum = 0;
+        double min = Double.MAX_VALUE;
+        double max = Double.MIN_VALUE;
+        for (int i = 0; i < trialCount; i++) {
+            VirtualMachineConfig.Builder builder =
+                    mInner.newVmConfigBuilder("assets/vm_config.json");
+            VirtualMachineConfig normalConfig =
+                    builder.debugLevel(DebugLevel.NONE).memoryMib(256).build();
+            mInner.forceCreateNewVirtualMachine("test_vm_boot_time", normalConfig);
+
+            BootResult result = tryBootVm(TAG, "test_vm_boot_time");
+            assertThat(result.payloadStarted).isTrue();
+
+            double elapsedMilliseconds = result.elapsedNanoTime / 1000000.0;
+
+            sum += elapsedMilliseconds;
+            squareSum += elapsedMilliseconds * elapsedMilliseconds;
+            if (min > elapsedMilliseconds) min = elapsedMilliseconds;
+            if (max < elapsedMilliseconds) max = elapsedMilliseconds;
+        }
+
+        Bundle bundle = new Bundle();
+        double average = sum / trialCount;
+        double variance = squareSum / trialCount - average * average;
+        double stdev = Math.sqrt(variance);
+        bundle.putDouble("avf_perf/microdroid/boot_time_average_ms", average);
+        bundle.putDouble("avf_perf/microdroid/boot_time_min_ms", min);
+        bundle.putDouble("avf_perf/microdroid/boot_time_max_ms", max);
+        bundle.putDouble("avf_perf/microdroid/boot_time_stdev_ms", stdev);
+        mInstrumentation.sendStatus(0, bundle);
+    }
 }
diff --git a/tests/helper/Android.bp b/tests/helper/Android.bp
new file mode 100644
index 0000000..679fbfe
--- /dev/null
+++ b/tests/helper/Android.bp
@@ -0,0 +1,15 @@
+package {
+    default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+java_library_static {
+    name: "MicroroidDeviceTestHelper",
+    srcs: ["src/java/**/*.java"],
+    static_libs: [
+        "androidx.test.runner",
+        "androidx.test.ext.junit",
+        "truth-prebuilt",
+    ],
+    libs: ["android.system.virtualmachine"],
+    platform_apis: true,
+}
diff --git a/tests/helper/src/java/com/android/microdroid/test/MicrodroidDeviceTestBase.java b/tests/helper/src/java/com/android/microdroid/test/MicrodroidDeviceTestBase.java
new file mode 100644
index 0000000..87c53a7
--- /dev/null
+++ b/tests/helper/src/java/com/android/microdroid/test/MicrodroidDeviceTestBase.java
@@ -0,0 +1,225 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+package com.android.microdroid.test;
+
+import static com.google.common.truth.TruthJUnit.assume;
+
+import static org.junit.Assume.assumeNoException;
+
+import android.content.Context;
+import android.os.ParcelFileDescriptor;
+import android.sysprop.HypervisorProperties;
+import android.system.virtualizationservice.DeathReason;
+import android.system.virtualmachine.VirtualMachine;
+import android.system.virtualmachine.VirtualMachineCallback;
+import android.system.virtualmachine.VirtualMachineConfig;
+import android.system.virtualmachine.VirtualMachineException;
+import android.system.virtualmachine.VirtualMachineManager;
+import android.util.Log;
+
+import androidx.annotation.CallSuper;
+import androidx.test.core.app.ApplicationProvider;
+
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+public abstract class MicrodroidDeviceTestBase {
+    /** Copy output from the VM to logcat. This is helpful when things go wrong. */
+    protected static void logVmOutput(String tag, InputStream vmOutputStream, String name) {
+        new Thread(
+                () -> {
+                    try {
+                        BufferedReader reader =
+                                new BufferedReader(new InputStreamReader(vmOutputStream));
+                        String line;
+                        while ((line = reader.readLine()) != null
+                                && !Thread.interrupted()) {
+                            Log.i(tag, name + ": " + line);
+                        }
+                    } catch (Exception e) {
+                        Log.w(tag, name, e);
+                    }
+                }).start();
+    }
+
+    private boolean mPkvmSupported;
+
+    // TODO(b/220920264): remove Inner class; this is a hack to hide virt APEX types
+    protected static class Inner {
+        private boolean mProtectedVm;
+        private Context mContext;
+        private VirtualMachineManager mVmm;
+
+        public Inner(Context context, boolean protectedVm, VirtualMachineManager vmm) {
+            mProtectedVm = protectedVm;
+            mVmm = vmm;
+            mContext = context;
+        }
+
+        public VirtualMachineManager getVirtualMachineManager() {
+            return mVmm;
+        }
+
+        public Context getContext() {
+            return mContext;
+        }
+
+        /** Create a new VirtualMachineConfig.Builder with the parameterized protection mode. */
+        public VirtualMachineConfig.Builder newVmConfigBuilder(String payloadConfigPath) {
+            return new VirtualMachineConfig.Builder(mContext, payloadConfigPath)
+                        .protectedVm(mProtectedVm);
+        }
+
+        /**
+         * Creates a new virtual machine, potentially removing an existing virtual machine with
+         * given name.
+         */
+        public VirtualMachine forceCreateNewVirtualMachine(String name, VirtualMachineConfig config)
+                throws VirtualMachineException {
+            VirtualMachine existingVm = mVmm.get(name);
+            if (existingVm != null) {
+                existingVm.delete();
+            }
+            return mVmm.create(name, config);
+        }
+    }
+
+    protected Inner mInner;
+
+    protected Context getContext() {
+        return mInner.getContext();
+    }
+
+    public void prepareTestSetup(boolean protectedVm) {
+        // In case when the virt APEX doesn't exist on the device, classes in the
+        // android.system.virtualmachine package can't be loaded. Therefore, before using the
+        // classes, check the existence of a class in the package and skip this test if not exist.
+        try {
+            Class.forName("android.system.virtualmachine.VirtualMachineManager");
+            mPkvmSupported = true;
+        } catch (ClassNotFoundException e) {
+            assumeNoException(e);
+            return;
+        }
+        if (protectedVm) {
+            assume().withMessage("Skip where protected VMs aren't support")
+                    .that(HypervisorProperties.hypervisor_protected_vm_supported().orElse(false))
+                    .isTrue();
+        } else {
+            assume().withMessage("Skip where VMs aren't support")
+                    .that(HypervisorProperties.hypervisor_vm_supported().orElse(false))
+                    .isTrue();
+        }
+        Context context = ApplicationProvider.getApplicationContext();
+        mInner = new Inner(context, protectedVm, VirtualMachineManager.getInstance(context));
+    }
+
+    public void cleanupTestSetup() throws VirtualMachineException {
+        if (!mPkvmSupported) {
+            return;
+        }
+    }
+
+    protected abstract static class VmEventListener implements VirtualMachineCallback {
+        private ExecutorService mExecutorService = Executors.newSingleThreadExecutor();
+
+        void runToFinish(String logTag, VirtualMachine vm)
+                throws VirtualMachineException, InterruptedException {
+            vm.setCallback(mExecutorService, this);
+            vm.run();
+            logVmOutput(logTag, vm.getConsoleOutputStream(), "Console");
+            logVmOutput(logTag, vm.getLogOutputStream(), "Log");
+            mExecutorService.awaitTermination(300, TimeUnit.SECONDS);
+        }
+
+        void forceStop(VirtualMachine vm) {
+            try {
+                vm.clearCallback();
+                vm.stop();
+                mExecutorService.shutdown();
+            } catch (VirtualMachineException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        @Override
+        public void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stream) {}
+
+        @Override
+        public void onPayloadReady(VirtualMachine vm) {}
+
+        @Override
+        public void onPayloadFinished(VirtualMachine vm, int exitCode) {}
+
+        @Override
+        public void onError(VirtualMachine vm, int errorCode, String message) {}
+
+        @Override
+        @CallSuper
+        public void onDied(VirtualMachine vm, @DeathReason int reason) {
+            mExecutorService.shutdown();
+        }
+
+        @Override
+        public void onRamdump(VirtualMachine vm, ParcelFileDescriptor ramdump) {}
+    }
+
+    public static class BootResult {
+        public final boolean payloadStarted;
+        public final int deathReason;
+        public final long elapsedNanoTime;
+
+        BootResult(boolean payloadStarted, int deathReason, long elapsedNanoTime) {
+            this.payloadStarted = payloadStarted;
+            this.deathReason = deathReason;
+            this.elapsedNanoTime = elapsedNanoTime;
+        }
+    }
+
+    public BootResult tryBootVm(String logTag, String vmName)
+            throws VirtualMachineException, InterruptedException {
+        VirtualMachine vm = mInner.getVirtualMachineManager().get(vmName);
+        final CompletableFuture<Boolean> payloadStarted = new CompletableFuture<>();
+        final CompletableFuture<Integer> deathReason = new CompletableFuture<>();
+        final CompletableFuture<Long> endTime = new CompletableFuture<>();
+        VmEventListener listener =
+                new VmEventListener() {
+                    @Override
+                    public void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stream) {
+                        endTime.complete(System.nanoTime());
+                        payloadStarted.complete(true);
+                        forceStop(vm);
+                    }
+
+                    @Override
+                    public void onDied(VirtualMachine vm, int reason) {
+                        deathReason.complete(reason);
+                        super.onDied(vm, reason);
+                    }
+                };
+        long beginTime = System.nanoTime();
+        listener.runToFinish(logTag, vm);
+        return new BootResult(
+                payloadStarted.getNow(false),
+                deathReason.getNow(DeathReason.INFRASTRUCTURE_ERROR),
+                endTime.getNow(beginTime) - beginTime);
+    }
+}
diff --git a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
index 56829fc..afccef6 100644
--- a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
+++ b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
@@ -25,6 +25,7 @@
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertThat;
@@ -384,9 +385,8 @@
                         Optional.of(CPU_AFFINITY));
         adbConnectToMicrodroid(getDevice(), cid);
         waitForBootComplete();
-        runOnMicrodroidRetryingOnFailure(MICRODROID_COMMAND_TIMEOUT_MILLIS,
-                        MICRODROID_ADB_CONNECT_MAX_ATTEMPTS,
-                        "logcat -c");
+        runOnMicrodroidRetryingOnFailure(
+                MICRODROID_COMMAND_TIMEOUT_MILLIS, MICRODROID_ADB_CONNECT_MAX_ATTEMPTS, "true");
         // We need root permission to write to /data/tombstones/
         rootMicrodroid();
         // Write a test tombstone file in /data/tombstones
@@ -394,9 +394,15 @@
                     + "> /data/tombstones/transmit.txt");
         // check if the tombstone have been tranferred from VM. This is a bit flaky - increasing
         // timeout to 30s can result in SIGKILL inside microdroid due to logcat memory issue
-        assertNotEquals(runOnMicrodroid("timeout 15s logcat | grep -m 1 "
-                            + "'tombstone_transmit.microdroid:.*data/tombstones/transmit.txt'"),
-                "");
+        CommandRunner android = new CommandRunner(getDevice());
+        android.runWithTimeout(
+                15000,
+                "grep",
+                "-m",
+                "1",
+                "'tombstone_transmit.microdroid:.*data/tombstones/transmit.txt'",
+                LOG_PATH);
+
         // Confirm that tombstone is received (from host logcat)
         assertNotEquals(runOnHost("adb", "-s", getDevice().getSerialNumber(),
                             "logcat", "-d", "-e",
@@ -441,7 +447,9 @@
         assertThat(runOnMicrodroid("ls", "-Z", testLib), is(label + " " + testLib));
 
         // Check that no denials have happened so far
-        assertThat(runOnMicrodroid("logcat -d -e 'avc:[[:space:]]{1,2}denied'"), is(""));
+        CommandRunner android = new CommandRunner(getDevice());
+        assertThat(android.tryRun("egrep", "'avc:[[:space:]]{1,2}denied'", LOG_PATH),
+                is(nullValue()));
 
         assertThat(runOnMicrodroid("cat /proc/cpuinfo | grep processor | wc -l"),
                 is(Integer.toString(NUM_VCPUS)));
diff --git a/tests/testapk/Android.bp b/tests/testapk/Android.bp
index b3b0808..d468d76 100644
--- a/tests/testapk/Android.bp
+++ b/tests/testapk/Android.bp
@@ -10,6 +10,7 @@
     ],
     srcs: ["src/java/**/*.java"],
     static_libs: [
+        "MicroroidDeviceTestHelper",
         "androidx.test.runner",
         "androidx.test.ext.junit",
         "authfs_test_apk_assets",
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index 3a874c4..e7e4647 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -18,27 +18,18 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 
-import static org.junit.Assume.assumeNoException;
-
 import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 
-import android.content.Context;
 import android.os.Build;
 import android.os.ParcelFileDescriptor;
 import android.os.SystemProperties;
-import android.sysprop.HypervisorProperties;
 import android.system.virtualizationservice.DeathReason;
 import android.system.virtualmachine.VirtualMachine;
-import android.system.virtualmachine.VirtualMachineCallback;
 import android.system.virtualmachine.VirtualMachineConfig;
 import android.system.virtualmachine.VirtualMachineConfig.DebugLevel;
 import android.system.virtualmachine.VirtualMachineException;
-import android.system.virtualmachine.VirtualMachineManager;
 import android.util.Log;
 
-import androidx.annotation.CallSuper;
-import androidx.test.core.app.ApplicationProvider;
-
 import com.android.microdroid.testservice.ITestService;
 
 import org.junit.After;
@@ -49,22 +40,16 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
-import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
 import java.io.RandomAccessFile;
 import java.nio.file.Files;
 import java.util.List;
 import java.util.OptionalLong;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 import co.nstant.in.cbor.CborDecoder;
 import co.nstant.in.cbor.CborException;
@@ -73,140 +58,28 @@
 import co.nstant.in.cbor.model.MajorType;
 
 @RunWith(Parameterized.class)
-public class MicrodroidTests {
+public class MicrodroidTests extends MicrodroidDeviceTestBase {
     private static final String TAG = "MicrodroidTests";
 
     @Rule public Timeout globalTimeout = Timeout.seconds(300);
 
     private static final String KERNEL_VERSION = SystemProperties.get("ro.kernel.version");
 
-    /** Copy output from the VM to logcat. This is helpful when things go wrong. */
-    private static void logVmOutput(InputStream vmOutputStream, String name) {
-        new Thread(() -> {
-            try {
-                BufferedReader reader = new BufferedReader(new InputStreamReader(vmOutputStream));
-                String line;
-                while ((line = reader.readLine()) != null && !Thread.interrupted()) {
-                    Log.i(TAG, name + ": " + line);
-                }
-            } catch (Exception e) {
-                Log.w(TAG, name, e);
-            }
-        }).start();
-    }
-
-    private static class Inner {
-        public boolean mProtectedVm;
-        public Context mContext;
-        public VirtualMachineManager mVmm;
-        public VirtualMachine mVm;
-
-        Inner(boolean protectedVm) {
-            mProtectedVm = protectedVm;
-        }
-
-        /** Create a new VirtualMachineConfig.Builder with the parameterized protection mode. */
-        public VirtualMachineConfig.Builder newVmConfigBuilder(String payloadConfigPath) {
-            return new VirtualMachineConfig.Builder(mContext, payloadConfigPath)
-                            .protectedVm(mProtectedVm);
-        }
-    }
-
     @Parameterized.Parameters(name = "protectedVm={0}")
     public static Object[] protectedVmConfigs() {
         return new Object[] { false, true };
     }
 
-    @Parameterized.Parameter
-    public boolean mProtectedVm;
-
-    private boolean mPkvmSupported = false;
-    private Inner mInner;
+    @Parameterized.Parameter public boolean mProtectedVm;
 
     @Before
     public void setup() {
-        // In case when the virt APEX doesn't exist on the device, classes in the
-        // android.system.virtualmachine package can't be loaded. Therefore, before using the
-        // classes, check the existence of a class in the package and skip this test if not exist.
-        try {
-            Class.forName("android.system.virtualmachine.VirtualMachineManager");
-            mPkvmSupported = true;
-        } catch (ClassNotFoundException e) {
-            assumeNoException(e);
-            return;
-        }
-        if (mProtectedVm) {
-            assume()
-                .withMessage("Skip where protected VMs aren't support")
-                .that(HypervisorProperties.hypervisor_protected_vm_supported().orElse(false))
-                .isTrue();
-        } else {
-            assume()
-                .withMessage("Skip where VMs aren't support")
-                .that(HypervisorProperties.hypervisor_vm_supported().orElse(false))
-                .isTrue();
-        }
-        mInner = new Inner(mProtectedVm);
-        mInner.mContext = ApplicationProvider.getApplicationContext();
-        mInner.mVmm = VirtualMachineManager.getInstance(mInner.mContext);
+        prepareTestSetup(mProtectedVm);
     }
 
     @After
     public void cleanup() throws VirtualMachineException {
-        if (!mPkvmSupported) {
-            return;
-        }
-        if (mInner == null) {
-            return;
-        }
-        if (mInner.mVm == null) {
-            return;
-        }
-        mInner.mVm.stop();
-        mInner.mVm.delete();
-    }
-
-    private abstract static class VmEventListener implements VirtualMachineCallback {
-        private ExecutorService mExecutorService = Executors.newSingleThreadExecutor();
-
-        void runToFinish(VirtualMachine vm) throws VirtualMachineException, InterruptedException {
-            vm.setCallback(mExecutorService, this);
-            vm.run();
-            logVmOutput(vm.getConsoleOutputStream(), "Console");
-            logVmOutput(vm.getLogOutputStream(), "Log");
-            mExecutorService.awaitTermination(300, TimeUnit.SECONDS);
-        }
-
-        void forceStop(VirtualMachine vm) {
-            try {
-                vm.clearCallback();
-                vm.stop();
-                mExecutorService.shutdown();
-            } catch (VirtualMachineException e) {
-                throw new RuntimeException(e);
-            }
-        }
-
-        @Override
-        public void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stream) {}
-
-        @Override
-        public void onPayloadReady(VirtualMachine vm) {}
-
-        @Override
-        public void onPayloadFinished(VirtualMachine vm, int exitCode) {}
-
-        @Override
-        public void onError(VirtualMachine vm, int errorCode, String message) {}
-
-        @Override
-        @CallSuper
-        public void onDied(VirtualMachine vm, @DeathReason int reason) {
-            mExecutorService.shutdown();
-        }
-
-        @Override
-        public void onRamdump(VirtualMachine vm, ParcelFileDescriptor ramdump) {}
+        cleanupTestSetup();
     }
 
     private static final int MIN_MEM_ARM64 = 150;
@@ -233,8 +106,7 @@
             }
         }
         VirtualMachineConfig config = builder.build();
-
-        mInner.mVm = mInner.mVmm.getOrCreate("test_vm_extra_apk", config);
+        VirtualMachine vm = mInner.forceCreateNewVirtualMachine("test_vm_extra_apk", config);
 
         class TestResults {
             Exception mException;
@@ -276,10 +148,11 @@
                     public void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stream) {
                         Log.i(TAG, "onPayloadStarted");
                         payloadStarted.complete(true);
-                        logVmOutput(new FileInputStream(stream.getFileDescriptor()), "Payload");
+                        logVmOutput(TAG, new FileInputStream(stream.getFileDescriptor()),
+                                "Payload");
                     }
                 };
-        listener.runToFinish(mInner.mVm);
+        listener.runToFinish(TAG, vm);
         assertThat(payloadStarted.getNow(false)).isTrue();
         assertThat(payloadReady.getNow(false)).isTrue();
         assertThat(testResults.mException).isNull();
@@ -299,7 +172,7 @@
 
         VirtualMachineConfig.Builder builder = mInner.newVmConfigBuilder("assets/vm_config.json");
         VirtualMachineConfig normalConfig = builder.debugLevel(DebugLevel.NONE).build();
-        mInner.mVm = mInner.mVmm.getOrCreate("test_vm", normalConfig);
+        VirtualMachine vm = mInner.forceCreateNewVirtualMachine("test_vm", normalConfig);
         VmEventListener listener =
                 new VmEventListener() {
                     @Override
@@ -307,19 +180,20 @@
                         forceStop(vm);
                     }
                 };
-        listener.runToFinish(mInner.mVm);
+        listener.runToFinish(TAG, vm);
 
         // Launch the same VM with different debug level. The Java API prohibits this (thankfully).
         // For testing, we do that by creating another VM with debug level, and copy the config file
         // from the new VM directory to the old VM directory.
         VirtualMachineConfig debugConfig = builder.debugLevel(DebugLevel.FULL).build();
-        VirtualMachine newVm  = mInner.mVmm.getOrCreate("test_debug_vm", debugConfig);
-        File vmRoot = new File(mInner.mContext.getFilesDir(), "vm");
+        VirtualMachine newVm = mInner.forceCreateNewVirtualMachine("test_debug_vm", debugConfig);
+        File vmRoot = new File(getContext().getFilesDir(), "vm");
         File newVmConfig = new File(new File(vmRoot, "test_debug_vm"), "config.xml");
         File oldVmConfig = new File(new File(vmRoot, "test_vm"), "config.xml");
         Files.copy(newVmConfig.toPath(), oldVmConfig.toPath(), REPLACE_EXISTING);
         newVm.delete();
-        mInner.mVm = mInner.mVmm.get("test_vm"); // re-load with the copied-in config file.
+        // re-load with the copied-in config file.
+        vm = mInner.getVirtualMachineManager().get("test_vm");
         final CompletableFuture<Boolean> payloadStarted = new CompletableFuture<>();
         listener =
                 new VmEventListener() {
@@ -329,7 +203,7 @@
                         forceStop(vm);
                     }
                 };
-        listener.runToFinish(mInner.mVm);
+        listener.runToFinish(TAG, vm);
         assertThat(payloadStarted.getNow(false)).isFalse();
     }
 
@@ -340,10 +214,7 @@
 
     private VmCdis launchVmAndGetCdis(String instanceName)
             throws VirtualMachineException, InterruptedException {
-        VirtualMachineConfig normalConfig = mInner.newVmConfigBuilder("assets/vm_config.json")
-                .debugLevel(DebugLevel.NONE)
-                .build();
-        mInner.mVm = mInner.mVmm.getOrCreate(instanceName, normalConfig);
+        VirtualMachine vm = mInner.getVirtualMachineManager().get(instanceName);
         final VmCdis vmCdis = new VmCdis();
         final CompletableFuture<Exception> exception = new CompletableFuture<>();
         VmEventListener listener =
@@ -361,7 +232,7 @@
                         }
                     }
                 };
-        listener.runToFinish(mInner.mVm);
+        listener.runToFinish(TAG, vm);
         assertThat(exception.getNow(null)).isNull();
         return vmCdis;
     }
@@ -374,6 +245,11 @@
             .that(KERNEL_VERSION)
             .isNotEqualTo("5.4");
 
+        VirtualMachineConfig normalConfig = mInner.newVmConfigBuilder("assets/vm_config.json")
+                .debugLevel(DebugLevel.NONE)
+                .build();
+        mInner.forceCreateNewVirtualMachine("test_vm_a", normalConfig);
+        mInner.forceCreateNewVirtualMachine("test_vm_b", normalConfig);
         VmCdis vm_a_cdis = launchVmAndGetCdis("test_vm_a");
         VmCdis vm_b_cdis = launchVmAndGetCdis("test_vm_b");
         assertThat(vm_a_cdis.cdiAttest).isNotNull();
@@ -393,6 +269,11 @@
             .that(KERNEL_VERSION)
             .isNotEqualTo("5.4");
 
+        VirtualMachineConfig normalConfig = mInner.newVmConfigBuilder("assets/vm_config.json")
+                .debugLevel(DebugLevel.NONE)
+                .build();
+        mInner.forceCreateNewVirtualMachine("test_vm", normalConfig);
+
         VmCdis first_boot_cdis = launchVmAndGetCdis("test_vm");
         VmCdis second_boot_cdis = launchVmAndGetCdis("test_vm");
         // The attestation CDI isn't specified to be stable, though it might be
@@ -412,7 +293,7 @@
         VirtualMachineConfig normalConfig = mInner.newVmConfigBuilder("assets/vm_config.json")
                 .debugLevel(DebugLevel.NONE)
                 .build();
-        mInner.mVm = mInner.mVmm.getOrCreate("bcc_vm", normalConfig);
+        VirtualMachine vm = mInner.forceCreateNewVirtualMachine("bcc_vm", normalConfig);
         final VmCdis vmCdis = new VmCdis();
         final CompletableFuture<byte[]> bcc = new CompletableFuture<>();
         final CompletableFuture<Exception> exception = new CompletableFuture<>();
@@ -430,7 +311,7 @@
                         }
                     }
                 };
-        listener.runToFinish(mInner.mVm);
+        listener.runToFinish(TAG, vm);
         byte[] bccBytes = bcc.getNow(null);
         assertThat(exception.getNow(null)).isNull();
         assertThat(bccBytes).isNotNull();
@@ -483,57 +364,19 @@
         file.writeByte(b ^ 1);
     }
 
-    private static class BootResult {
-        public final boolean payloadStarted;
-        public final int deathReason;
-
-        BootResult(boolean payloadStarted, int deathReason) {
-            this.payloadStarted = payloadStarted;
-            this.deathReason = deathReason;
-        }
-    }
-
-    private BootResult tryBootVm(String vmName)
-            throws VirtualMachineException, InterruptedException {
-        mInner.mVm = mInner.mVmm.get(vmName); // re-load the vm before running tests
-        final CompletableFuture<Boolean> payloadStarted = new CompletableFuture<>();
-        final CompletableFuture<Integer> deathReason = new CompletableFuture<>();
-        VmEventListener listener =
-                new VmEventListener() {
-                    @Override
-                    public void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stream) {
-                        payloadStarted.complete(true);
-                        forceStop(vm);
-                    }
-                    @Override
-                    public void onDied(VirtualMachine vm, int reason) {
-                        deathReason.complete(reason);
-                        super.onDied(vm, reason);
-                    }
-                };
-        listener.runToFinish(mInner.mVm);
-        return new BootResult(
-                payloadStarted.getNow(false), deathReason.getNow(DeathReason.INFRASTRUCTURE_ERROR));
-    }
-
     private RandomAccessFile prepareInstanceImage(String vmName)
             throws VirtualMachineException, InterruptedException, IOException {
         VirtualMachineConfig config = mInner.newVmConfigBuilder("assets/vm_config.json")
                 .debugLevel(DebugLevel.FULL)
                 .build();
 
-        // Remove any existing VM so we can start from scratch
-        VirtualMachine oldVm = mInner.mVmm.getOrCreate(vmName, config);
-        oldVm.delete();
-        mInner.mVmm.getOrCreate(vmName, config);
+        mInner.forceCreateNewVirtualMachine(vmName, config);
+        assertThat(tryBootVm(TAG, vmName).payloadStarted).isTrue();
 
-        assertThat(tryBootVm(vmName).payloadStarted).isTrue();
-
-        File vmRoot = new File(mInner.mContext.getFilesDir(), "vm");
+        File vmRoot = new File(getContext().getFilesDir(), "vm");
         File vmDir = new File(vmRoot, vmName);
         File instanceImgPath = new File(vmDir, "instance.img");
         return new RandomAccessFile(instanceImgPath, "rw");
-
     }
 
     private void assertThatPartitionIsMissing(UUID partitionUuid)
@@ -551,7 +394,8 @@
         assertThat(offset.isPresent()).isTrue();
 
         flipBit(instanceFile, offset.getAsLong());
-        assertThat(tryBootVm("test_vm_integrity").payloadStarted).isFalse();
+
+        assertThat(tryBootVm(TAG, "test_vm_integrity").payloadStarted).isFalse();
     }
 
     @Test
@@ -596,17 +440,12 @@
     @Test
     public void bootFailsWhenConfigIsInvalid()
             throws VirtualMachineException, InterruptedException, IOException {
-        VirtualMachine existingVm = mInner.mVmm.get("test_vm_invalid_config");
-        if (existingVm != null) {
-            existingVm.delete();
-        }
-
         VirtualMachineConfig.Builder builder =
                 mInner.newVmConfigBuilder("assets/vm_config_no_task.json");
         VirtualMachineConfig normalConfig = builder.debugLevel(DebugLevel.NONE).build();
-        mInner.mVmm.create("test_vm_invalid_config", normalConfig);
+        mInner.forceCreateNewVirtualMachine("test_vm_invalid_config", normalConfig);
 
-        BootResult bootResult = tryBootVm("test_vm_invalid_config");
+        BootResult bootResult = tryBootVm(TAG, "test_vm_invalid_config");
         assertThat(bootResult.payloadStarted).isFalse();
         assertThat(bootResult.deathReason).isEqualTo(DeathReason.MICRODROID_INVALID_PAYLOAD_CONFIG);
     }
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
index 6f3d4f0..d9d9a61 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
@@ -34,6 +34,13 @@
     /** Starts running the VM. */
     void start();
 
+    /**
+     * Stops this virtual machine. Stopping a virtual machine is like pulling the plug on a real
+     * computer; the machine halts immediately. Software running on the virtual machine is not
+     * notified with the event.
+     */
+    void stop();
+
     /** Open a vsock connection to the CID of the VM on the given port. */
     ParcelFileDescriptor connectVsock(int port);
 }
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 4135253..0078736 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -823,6 +823,13 @@
         })
     }
 
+    fn stop(&self) -> binder::Result<()> {
+        self.instance.kill().map_err(|e| {
+            error!("Error stopping VM with CID {}: {:?}", self.instance.cid, e);
+            new_binder_exception(ExceptionCode::SERVICE_SPECIFIC, e.to_string())
+        })
+    }
+
     fn connectVsock(&self, port: i32) -> binder::Result<ParcelFileDescriptor> {
         if !matches!(&*self.instance.vm_state.lock().unwrap(), VmState::Running { .. }) {
             return Err(new_binder_exception(ExceptionCode::SERVICE_SPECIFIC, "VM is not running"));
@@ -841,7 +848,9 @@
 impl Drop for VirtualMachine {
     fn drop(&mut self) {
         debug!("Dropping {:?}", self);
-        self.instance.kill();
+        if let Err(e) = self.instance.kill() {
+            debug!("Error stopping dropped VM with CID {}: {:?}", self.instance.cid, e);
+        }
     }
 }
 
diff --git a/virtualizationservice/src/crosvm.rs b/virtualizationservice/src/crosvm.rs
index 46ad6b3..b6e62fe 100644
--- a/virtualizationservice/src/crosvm.rs
+++ b/virtualizationservice/src/crosvm.rs
@@ -241,7 +241,9 @@
                     "Microdroid failed to start payload within {} secs timeout. Shutting down",
                     BOOT_HANGUP_TIMEOUT.as_secs()
                 );
-                self.kill();
+                if let Err(e) = self.kill() {
+                    error!("Error stopping timed-out VM with CID {}: {:?}", self.cid, e);
+                }
                 true
             } else {
                 false
@@ -304,15 +306,19 @@
     }
 
     /// Kills the crosvm instance, if it is running.
-    pub fn kill(&self) {
+    pub fn kill(&self) -> Result<(), Error> {
         let vm_state = &*self.vm_state.lock().unwrap();
         if let VmState::Running { child } = vm_state {
             let id = child.id();
             debug!("Killing crosvm({})", id);
             // TODO: Talk to crosvm to shutdown cleanly.
             if let Err(e) = child.kill() {
-                error!("Error killing crosvm({}) instance: {}", id, e);
+                bail!("Error killing crosvm({}) instance: {}", id, e);
+            } else {
+                Ok(())
             }
+        } else {
+            bail!("VM is not running")
         }
     }