Merge "Use ignorabletest for devicemapper tests."
diff --git a/TEST_MAPPING b/TEST_MAPPING
index d17b434..3217ee1 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -4,9 +4,6 @@
"name": "MicrodroidHostTestCases"
},
{
- "name": "ComposHostTestCases"
- },
- {
"name": "MicrodroidTestApp"
},
{
@@ -36,6 +33,9 @@
"name": "ComposBenchmarkApp"
},
{
+ "name": "ComposHostTestCases"
+ },
+ {
"name": "AVFHostTestCases"
}
],
diff --git a/authfs/service/src/authfs.rs b/authfs/service/src/authfs.rs
index 3e8e0e0..cfd5766 100644
--- a/authfs/service/src/authfs.rs
+++ b/authfs/service/src/authfs.rs
@@ -176,7 +176,7 @@
bail!("Child has exited: {}", exit_status);
}
if start_time.elapsed() > AUTHFS_SETUP_TIMEOUT_SEC {
- let _ = child.kill();
+ let _ignored = child.kill();
bail!("Time out mounting authfs");
}
sleep(AUTHFS_SETUP_POLL_INTERVAL_MS);
diff --git a/authfs/src/fusefs.rs b/authfs/src/fusefs.rs
index 544a94e..87bdffc 100644
--- a/authfs/src/fusefs.rs
+++ b/authfs/src/fusefs.rs
@@ -587,7 +587,7 @@
match delete_now {
Ok(true) => {
- let _ = inode_table.remove(&inode).expect("Removed an existing entry");
+ let _ignored = inode_table.remove(&inode).expect("Removed an existing entry");
}
Ok(false) => { /* Let the inode stay */ }
Err(e) => {
diff --git a/compos/common/compos_client.rs b/compos/common/compos_client.rs
index bc6ab25..232485a 100644
--- a/compos/common/compos_client.rs
+++ b/compos/common/compos_client.rs
@@ -164,7 +164,7 @@
/// relevant logs to be written.
pub fn shutdown(self, service: Strong<dyn ICompOsService>) {
info!("Requesting CompOS VM to shutdown");
- let _ = service.quit(); // If this fails, the VM is probably dying anyway
+ let _ignored = service.quit(); // If this fails, the VM is probably dying anyway
self.wait_for_shutdown();
}
diff --git a/compos/composd/src/instance_starter.rs b/compos/composd/src/instance_starter.rs
index fc4c58b..457520f 100644
--- a/compos/composd/src/instance_starter.rs
+++ b/compos/composd/src/instance_starter.rs
@@ -98,17 +98,16 @@
) -> Result<CompOsInstance> {
info!("Creating {} CompOs instance", self.instance_name);
- // Ignore failure here - the directory may already exist.
- let _ = fs::create_dir(&self.instance_root);
+ fs::create_dir_all(&self.instance_root)?;
// Overwrite any existing instance - it's unlikely to be valid with the current set
// of APEXes, and finding out it isn't is much more expensive than creating a new one.
self.create_instance_image(virtualization_service)?;
// Delete existing idsig files. Ignore error in case idsig doesn't exist.
- let _ = fs::remove_file(&self.idsig);
- let _ = fs::remove_file(&self.idsig_manifest_apk);
- let _ = fs::remove_file(&self.idsig_manifest_ext_apk);
+ let _ignored1 = fs::remove_file(&self.idsig);
+ let _ignored2 = fs::remove_file(&self.idsig_manifest_apk);
+ let _ignored3 = fs::remove_file(&self.idsig_manifest_ext_apk);
let instance = self.start_vm(virtualization_service)?;
diff --git a/compos/tests/java/android/compos/test/ComposTestCase.java b/compos/tests/java/android/compos/test/ComposTestCase.java
index 3a09475..1cebd1a 100644
--- a/compos/tests/java/android/compos/test/ComposTestCase.java
+++ b/compos/tests/java/android/compos/test/ComposTestCase.java
@@ -23,8 +23,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static org.junit.Assume.assumeFalse;
-
import android.platform.test.annotations.RootPermissionTest;
import com.android.microdroid.test.host.CommandRunner;
@@ -83,8 +81,6 @@
@Before
public void setUp() throws Exception {
assumeDeviceIsCapable(getDevice());
- // We get a very high level of (apparently bogus) OOM errors on Cuttlefish (b/264496291).
- assumeFalse("Skipping test on Cuttlefish", isCuttlefish());
String value = getDevice().getProperty(SYSTEM_SERVER_COMPILER_FILTER_PROP_NAME);
if (value == null) {
diff --git a/demo_native/Android.bp b/demo_native/Android.bp
new file mode 100644
index 0000000..901f829
--- /dev/null
+++ b/demo_native/Android.bp
@@ -0,0 +1,18 @@
+package {
+ default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+cc_binary {
+ name: "vm_demo_native",
+ srcs: ["main.cpp"],
+ static_libs: [
+ "libbase",
+ "android.system.virtualizationservice-ndk",
+ "com.android.microdroid.testservice-ndk",
+ ],
+ shared_libs: [
+ "libbinder_ndk",
+ "libbinder_rpc_unstable",
+ "liblog",
+ ],
+}
diff --git a/demo_native/README.md b/demo_native/README.md
new file mode 100644
index 0000000..700ca83
--- /dev/null
+++ b/demo_native/README.md
@@ -0,0 +1,61 @@
+# Microdroid Demo app in C++
+
+This app is a demonstration of how to create a VM and run payload in it, in C++.
+
+## Restriction
+
+This is for VMs that are part of the platform itself. Specifically, this C++ example is for cases
+like creating and using a VM from a HAL process.
+
+For non-system-level VMs, you must use the Java APIs from an Android app. See the [Java demo
+app](../demo/README.md).
+
+## Building
+
+```sh
+source build/envsetup.sh
+choosecombo 1 aosp_arm64 userdebug
+m MicrodroidTestApp
+m vm_demo_native
+```
+
+`MicrodroidTestApp` is the application what will be running in the VM. Actually, we will run a
+native shared library `MicrodroidTestNativeLib.so` from the APK.
+
+`vm_demo_native` runs on the host (i.e. Android). Its job is to start the VM and connect to the
+native shared lib and do some work using the lib. Specifically, we will call an AIDL method
+`addInteger` which adds two integers and returns the sum. The computation will be done in the VM.
+
+## Installing
+
+```sh
+adb push out/target/product/generic_arm64/testcases/MicrodroidTestApp/arm64/MicrodroidTestApp.apk \
+ /data/local/tmp/
+adb push out/target/product/generic_arm64/system/bin/vm_demo_native /data/local/tmp/
+```
+
+## Running
+
+```sh
+adb root
+adb shell setenforce 0
+adb shell /data/local/tmp/vm_demo_native
+```
+
+Rooting and selinux disabling are required just because there's no sepolicy configured for this demo
+application. For production, you need to set the sepolicy up correctly. You may use
+`system/sepolicy/private/composd.te` (specifically, the macro `virtualizationservice_use`) as a
+reference.
+
+## Expected output
+
+```sh
+[2023-05-10T23:45:54.904181191+09:00 INFO crosvm] crosvm started.
+[2023-05-10T23:45:54.906048663+09:00 INFO crosvm] CLI arguments parsed.
+...
+The answer from VM is 30
+[ 1.996707][ T57] microdroid_manager[57]: task successfully finished
+...
+[2023-05-10T23:45:58.263614461+09:00 INFO crosvm] exiting with success
+Done
+```
diff --git a/demo_native/main.cpp b/demo_native/main.cpp
new file mode 100644
index 0000000..fa87549
--- /dev/null
+++ b/demo_native/main.cpp
@@ -0,0 +1,401 @@
+/*
+ * 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.
+ */
+#include <aidl/android/system/virtualizationcommon/DeathReason.h>
+#include <aidl/android/system/virtualizationcommon/ErrorCode.h>
+#include <aidl/android/system/virtualizationservice/BnVirtualMachineCallback.h>
+#include <aidl/android/system/virtualizationservice/IVirtualMachine.h>
+#include <aidl/android/system/virtualizationservice/IVirtualMachineCallback.h>
+#include <aidl/android/system/virtualizationservice/IVirtualizationService.h>
+#include <aidl/android/system/virtualizationservice/VirtualMachineConfig.h>
+#include <aidl/android/system/virtualizationservice/VirtualMachineState.h>
+#include <aidl/com/android/microdroid/testservice/ITestService.h>
+#include <android-base/errors.h>
+#include <android-base/file.h>
+#include <android-base/result.h>
+#include <android-base/unique_fd.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#include <binder_rpc_unstable.hpp>
+#include <chrono>
+#include <condition_variable>
+#include <cstddef>
+#include <memory>
+#include <mutex>
+#include <thread>
+
+using namespace std::chrono_literals;
+
+using android::base::ErrnoError;
+using android::base::Error;
+using android::base::Pipe;
+using android::base::Result;
+using android::base::Socketpair;
+using android::base::unique_fd;
+
+using ndk::ScopedAStatus;
+using ndk::ScopedFileDescriptor;
+using ndk::SharedRefBase;
+using ndk::SpAIBinder;
+
+using aidl::android::system::virtualizationcommon::DeathReason;
+using aidl::android::system::virtualizationcommon::ErrorCode;
+using aidl::android::system::virtualizationservice::BnVirtualMachineCallback;
+using aidl::android::system::virtualizationservice::IVirtualizationService;
+using aidl::android::system::virtualizationservice::IVirtualMachine;
+using aidl::android::system::virtualizationservice::PartitionType;
+using aidl::android::system::virtualizationservice::toString;
+using aidl::android::system::virtualizationservice::VirtualMachineAppConfig;
+using aidl::android::system::virtualizationservice::VirtualMachineConfig;
+using aidl::android::system::virtualizationservice::VirtualMachinePayloadConfig;
+using aidl::android::system::virtualizationservice::VirtualMachineState;
+
+using aidl::com::android::microdroid::testservice::ITestService;
+
+// This program demonstrates a way to run a VM and do something in the VM using AVF in the C++
+// language. Instructions for building and running this demo can be found in `README.md` in this
+// directory.
+
+//--------------------------------------------------------------------------------------------------
+// Step 1: connect to IVirtualizationService
+//--------------------------------------------------------------------------------------------------
+static constexpr const char VIRTMGR_PATH[] = "/apex/com.android.virt/bin/virtmgr";
+static constexpr size_t VIRTMGR_THREADS = 2;
+
+// Start IVirtualizationService instance and get FD for the unix domain socket that is connected to
+// the service. The returned FD should be kept open until the service is no longer needed.
+Result<unique_fd> get_service_fd() {
+ unique_fd server_fd, client_fd;
+ if (!Socketpair(SOCK_STREAM, &server_fd, &client_fd)) {
+ return ErrnoError() << "Failed to create socketpair";
+ }
+
+ unique_fd wait_fd, ready_fd;
+ if (!Pipe(&wait_fd, &ready_fd, 0)) {
+ return ErrnoError() << "Failed to create pipe";
+ }
+
+ if (fork() == 0) {
+ client_fd.reset();
+ wait_fd.reset();
+
+ auto server_fd_str = std::to_string(server_fd.get());
+ auto ready_fd_str = std::to_string(ready_fd.get());
+
+ if (execl(VIRTMGR_PATH, VIRTMGR_PATH, "--rpc-server-fd", server_fd_str.c_str(),
+ "--ready-fd", ready_fd_str.c_str(), nullptr) == -1) {
+ return ErrnoError() << "Failed to execute virtmgr";
+ }
+ }
+
+ server_fd.reset();
+ ready_fd.reset();
+
+ char buf;
+ if (read(wait_fd.get(), &buf, sizeof(buf)) < 0) {
+ return ErrnoError() << "Failed to wait for VirtualizationService to be ready";
+ }
+
+ return client_fd;
+}
+
+// Establish a binder communication channel over the unix domain socket and returns the remote
+// IVirtualizationService.
+Result<std::shared_ptr<IVirtualizationService>> connect_service(int fd) {
+ std::unique_ptr<ARpcSession, decltype(&ARpcSession_free)> session(ARpcSession_new(),
+ &ARpcSession_free);
+ ARpcSession_setFileDescriptorTransportMode(session.get(),
+ ARpcSession_FileDescriptorTransportMode::Unix);
+ ARpcSession_setMaxIncomingThreads(session.get(), VIRTMGR_THREADS);
+ ARpcSession_setMaxOutgoingConnections(session.get(), VIRTMGR_THREADS);
+ AIBinder* binder = ARpcSession_setupUnixDomainBootstrapClient(session.get(), fd);
+ if (binder == nullptr) {
+ return Error() << "Failed to connect to VirtualizationService";
+ }
+ return IVirtualizationService::fromBinder(SpAIBinder{binder});
+}
+
+//--------------------------------------------------------------------------------------------------
+// Step 2: construct VirtualMachineAppConfig
+//--------------------------------------------------------------------------------------------------
+
+// Utility function for opening a file at a given path and wrap the resulting FD in
+// ScopedFileDescriptor so that it can be passed to the service.
+Result<ScopedFileDescriptor> open_file(const std::string& path, int flags) {
+ int fd = open(path.c_str(), flags, S_IWUSR);
+ if (fd == -1) {
+ return ErrnoError() << "Failed to open " << path;
+ }
+ return ScopedFileDescriptor(fd);
+}
+
+// Create or update idsig file for the given APK file. The idsig is essentially a hashtree of the
+// APK file's content
+Result<ScopedFileDescriptor> create_or_update_idsig_file(IVirtualizationService& service,
+ const std::string& work_dir,
+ ScopedFileDescriptor& main_apk) {
+ std::string path = work_dir + "/apk.idsig";
+ ScopedFileDescriptor idsig = OR_RETURN(open_file(path, O_CREAT | O_RDWR));
+ ScopedAStatus ret = service.createOrUpdateIdsigFile(main_apk, idsig);
+ if (!ret.isOk()) {
+ return Error() << "Failed to create or update idsig file: " << path;
+ }
+ return idsig;
+}
+
+// Get or create the instance disk image file, if it doesn't exist. The VM will fill this disk with
+// its own identity information in an encrypted form.
+Result<ScopedFileDescriptor> create_instance_image_file_if_needed(IVirtualizationService& service,
+ const std::string& work_dir) {
+ std::string path = work_dir + "/instance.img";
+
+ // If instance.img already exists, use it.
+ if (access(path.c_str(), F_OK) == 0) {
+ return open_file(path, O_RDWR);
+ }
+
+ // If not, create a new one.
+ ScopedFileDescriptor instance = OR_RETURN(open_file(path, O_CREAT | O_RDWR));
+ long size = 10 * 1024 * 1024; // 10MB, but could be smaller.
+ ScopedAStatus ret =
+ service.initializeWritablePartition(instance, size, PartitionType::ANDROID_VM_INSTANCE);
+ if (!ret.isOk()) {
+ return Error() << "Failed to create instance disk image: " << path;
+ }
+ return instance;
+}
+
+// Construct VirtualMachineAppConfig for a Microdroid-based VM named `vm_name` that executes a
+// shared library named `paylaod_binary_name` in the apk `main_apk_path`.
+Result<VirtualMachineAppConfig> create_vm_config(
+ IVirtualizationService& service, const std::string& work_dir, const std::string& vm_name,
+ const std::string& main_apk_path, const std::string& payload_binary_name, bool debuggable,
+ bool protected_vm, int32_t memory_mib) {
+ ScopedFileDescriptor main_apk = OR_RETURN(open_file(main_apk_path, O_RDONLY));
+ ScopedFileDescriptor idsig =
+ OR_RETURN(create_or_update_idsig_file(service, work_dir, main_apk));
+ ScopedFileDescriptor instance =
+ OR_RETURN(create_instance_image_file_if_needed(service, work_dir));
+
+ // There are two ways to specify the payload. The simpler way is by specifying the name of the
+ // payload binary as shown below. The other way (which is allowed only to system-level VMs) is
+ // by passing the path to the JSON file in the main APK which has detailed specification about
+ // what to load in Microdroid. See packages/modules/Virtualization/compos/apk/assets/*.json as
+ // examples.
+ VirtualMachinePayloadConfig payload;
+ payload.payloadBinaryName = payload_binary_name;
+
+ VirtualMachineAppConfig app_config;
+ app_config.name = vm_name;
+ app_config.apk = std::move(main_apk);
+ app_config.idsig = std::move(idsig);
+ app_config.instanceImage = std::move(instance);
+ app_config.payload = std::move(payload);
+ if (debuggable) {
+ app_config.debugLevel = VirtualMachineAppConfig::DebugLevel::FULL;
+ }
+ app_config.protectedVm = protected_vm;
+ app_config.memoryMib = memory_mib;
+
+ return app_config;
+}
+
+//--------------------------------------------------------------------------------------------------
+// Step 3: create a VM and start it
+//--------------------------------------------------------------------------------------------------
+
+// Create a virtual machine with the config, but doesn't start it yet.
+Result<std::shared_ptr<IVirtualMachine>> create_virtual_machine(
+ IVirtualizationService& service, VirtualMachineAppConfig& app_config) {
+ std::shared_ptr<IVirtualMachine> vm;
+
+ VirtualMachineConfig config = std::move(app_config);
+ ScopedFileDescriptor console_fd(fcntl(fileno(stdout), F_DUPFD_CLOEXEC));
+ ScopedFileDescriptor log_fd(fcntl(fileno(stdout), F_DUPFD_CLOEXEC));
+
+ ScopedAStatus ret = service.createVm(config, console_fd, log_fd, &vm);
+ if (!ret.isOk()) {
+ return Error() << "Failed to create VM";
+ }
+ return vm;
+}
+
+// When a VM lifecycle changes, a corresponding method in this class is called. This also provides
+// methods for blocking the current thread until the VM reaches a specific state.
+class Callback : public BnVirtualMachineCallback {
+public:
+ Callback(const std::shared_ptr<IVirtualMachine>& vm) : mVm(vm) {}
+
+ ScopedAStatus onPayloadStarted(int32_t) {
+ std::unique_lock lock(mMutex);
+ mCv.notify_all();
+ return ScopedAStatus::ok();
+ }
+
+ ScopedAStatus onPayloadReady(int32_t) {
+ std::unique_lock lock(mMutex);
+ mCv.notify_all();
+ return ScopedAStatus::ok();
+ }
+
+ ScopedAStatus onPayloadFinished(int32_t, int32_t) {
+ std::unique_lock lock(mMutex);
+ mCv.notify_all();
+ return ScopedAStatus::ok();
+ }
+
+ ScopedAStatus onError(int32_t, ErrorCode, const std::string&) {
+ std::unique_lock lock(mMutex);
+ mCv.notify_all();
+ return ScopedAStatus::ok();
+ }
+
+ ScopedAStatus onDied(int32_t, DeathReason) {
+ std::unique_lock lock(mMutex);
+ mCv.notify_all();
+ return ScopedAStatus::ok();
+ }
+
+ Result<void> wait_for_state(VirtualMachineState state) {
+ std::unique_lock lock(mMutex);
+ mCv.wait_for(lock, 5s, [this, &state] {
+ auto cur_state = get_vm_state();
+ return cur_state.ok() && *cur_state == state;
+ });
+ auto cur_state = get_vm_state();
+ if (cur_state.ok()) {
+ if (*cur_state == state) {
+ return {};
+ } else {
+ return Error() << "Timeout waiting for state becomes " << toString(state);
+ }
+ }
+ return cur_state.error();
+ }
+
+private:
+ std::shared_ptr<IVirtualMachine> mVm;
+ std::condition_variable mCv;
+ std::mutex mMutex;
+
+ Result<VirtualMachineState> get_vm_state() {
+ VirtualMachineState state;
+ ScopedAStatus ret = mVm->getState(&state);
+ if (!ret.isOk()) {
+ return Error() << "Failed to get state of virtual machine";
+ }
+ return state;
+ }
+};
+
+// Start (i.e. boot) the virtual machine and return Callback monitoring the lifecycle event of the
+// VM.
+Result<std::shared_ptr<Callback>> start_virtual_machine(std::shared_ptr<IVirtualMachine> vm) {
+ std::shared_ptr<Callback> cb = SharedRefBase::make<Callback>(vm);
+ ScopedAStatus ret = vm->registerCallback(cb);
+ if (!ret.isOk()) {
+ return Error() << "Failed to register callback to virtual machine";
+ }
+ ret = vm->start();
+ if (!ret.isOk()) {
+ return Error() << "Failed to start virtual machine";
+ }
+ return cb;
+}
+
+//--------------------------------------------------------------------------------------------------
+// Step 4: connect to the payload and communicate with it over binder/vsock
+//--------------------------------------------------------------------------------------------------
+
+// Connect to the binder service running in the payload.
+Result<std::shared_ptr<ITestService>> connect_to_vm_payload(std::shared_ptr<IVirtualMachine> vm) {
+ std::unique_ptr<ARpcSession, decltype(&ARpcSession_free)> session(ARpcSession_new(),
+ &ARpcSession_free);
+ ARpcSession_setMaxIncomingThreads(session.get(), 1);
+
+ AIBinder* binder = ARpcSession_setupPreconnectedClient(
+ session.get(),
+ [](void* param) {
+ std::shared_ptr<IVirtualMachine> vm =
+ *static_cast<std::shared_ptr<IVirtualMachine>*>(param);
+ ScopedFileDescriptor sock_fd;
+ ScopedAStatus ret = vm->connectVsock(ITestService::PORT, &sock_fd);
+ if (!ret.isOk()) {
+ return -1;
+ }
+ return sock_fd.release();
+ },
+ &vm);
+ if (binder == nullptr) {
+ return Error() << "Failed to connect to vm payload";
+ }
+ return ITestService::fromBinder(SpAIBinder{binder});
+}
+
+// Do something with the service in the VM
+Result<void> do_something(ITestService& payload) {
+ int32_t result;
+ ScopedAStatus ret = payload.addInteger(10, 20, &result);
+ if (!ret.isOk()) {
+ return Error() << "Failed to call addInteger";
+ }
+ std::cout << "The answer from VM is " << result << std::endl;
+ return {};
+}
+
+// This is the main routine that follows the steps in order
+Result<void> inner_main() {
+ TemporaryDir work_dir;
+ std::string work_dir_path(work_dir.path);
+
+ // Step 1: connect to the virtualizationservice
+ unique_fd fd = OR_RETURN(get_service_fd());
+ std::shared_ptr<IVirtualizationService> service = OR_RETURN(connect_service(fd.get()));
+
+ // Step 2: create vm config
+ VirtualMachineAppConfig app_config = OR_RETURN(
+ create_vm_config(*service, work_dir_path, "my_vm",
+ "/data/local/tmp/MicrodroidTestApp.apk", "MicrodroidTestNativeLib.so",
+ /* debuggable = */ true, // should be false for production VMs
+ /* protected_vm = */ true, 150));
+
+ // Step 3: start vm
+ std::shared_ptr<IVirtualMachine> vm = OR_RETURN(create_virtual_machine(*service, app_config));
+ std::shared_ptr<Callback> cb = OR_RETURN(start_virtual_machine(vm));
+ OR_RETURN(cb->wait_for_state(VirtualMachineState::READY));
+
+ // Step 4: do something in the vm
+ std::shared_ptr<ITestService> payload = OR_RETURN(connect_to_vm_payload(vm));
+ OR_RETURN(do_something(*payload));
+
+ // Step 5: let VM quit by itself, and wait for the graceful shutdown
+ ScopedAStatus ret = payload->quit();
+ if (!ret.isOk()) {
+ return Error() << "Failed to command quit to the VM";
+ }
+ OR_RETURN(cb->wait_for_state(VirtualMachineState::DEAD));
+
+ return {};
+}
+
+int main() {
+ if (auto ret = inner_main(); !ret.ok()) {
+ std::cerr << ret.error() << std::endl;
+ return EXIT_FAILURE;
+ }
+ std::cout << "Done" << std::endl;
+ return EXIT_SUCCESS;
+}
diff --git a/libs/devicemapper/src/lib.rs b/libs/devicemapper/src/lib.rs
index 984ceda..1b18b49 100644
--- a/libs/devicemapper/src/lib.rs
+++ b/libs/devicemapper/src/lib.rs
@@ -344,8 +344,8 @@
scopeguard::defer! {
loopdevice::detach(&data_device).unwrap();
- _ = delete_device(&dm, device);
- _ = delete_device(&dm, &device_diff);
+ let _ignored1 = delete_device(&dm, device);
+ let _ignored2 = delete_device(&dm, &device_diff);
}
let target = DmCryptTargetBuilder::default()
@@ -386,8 +386,8 @@
let device_diff = device.to_owned() + "_diff";
scopeguard::defer! {
loopdevice::detach(&data_device).unwrap();
- _ = delete_device(&dm, device);
- _ = delete_device(&dm, &device_diff);
+ let _ignored1 = delete_device(&dm, device);
+ let _ignored2 = delete_device(&dm, &device_diff);
}
let target = DmCryptTargetBuilder::default()
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index c78b20f..09b23a1 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -200,7 +200,7 @@
}
fn try_main() -> Result<()> {
- let _ = kernlog::init();
+ let _ignored = kernlog::init();
info!("started.");
if let Err(e) = set_cloexec_on_vm_payload_service_socket() {
diff --git a/pvmfw/src/memory.rs b/pvmfw/src/memory.rs
index cfd6b0a..0656321 100644
--- a/pvmfw/src/memory.rs
+++ b/pvmfw/src/memory.rs
@@ -23,8 +23,7 @@
use alloc::alloc::handle_alloc_error;
use alloc::boxed::Box;
use alloc::vec::Vec;
-use buddy_system_allocator::Heap;
-use buddy_system_allocator::LockedHeap;
+use buddy_system_allocator::{FrameAllocator, LockedFrameAllocator};
use core::alloc::Layout;
use core::cmp::max;
use core::cmp::min;
@@ -144,7 +143,7 @@
type Result<T> = result::Result<T, MemoryTrackerError>;
-static SHARED_POOL: OnceBox<LockedHeap<32>> = OnceBox::new();
+static SHARED_POOL: OnceBox<LockedFrameAllocator<32>> = OnceBox::new();
static SHARED_MEMORY: SpinMutex<Option<MemorySharer>> = SpinMutex::new(None);
/// Allocates memory on the heap and shares it with the host.
@@ -164,7 +163,7 @@
}
/// Get from the global allocator a granule-aligned region that suits `hint` and share it.
- pub fn refill(&mut self, pool: &mut Heap<32>, hint: Layout) {
+ pub fn refill(&mut self, pool: &mut FrameAllocator<32>, hint: Layout) {
let layout = hint.align_to(self.granule).unwrap().pad_to_align();
assert_ne!(layout.size(), 0);
// SAFETY - layout has non-zero size.
@@ -181,8 +180,7 @@
}
self.shared_regions.push((base, layout));
- // SAFETY - The underlying memory range is owned by self and reserved for this pool.
- unsafe { pool.add_to_heap(base, end) };
+ pool.add_frame(base, end);
}
}
@@ -343,7 +341,7 @@
}
SHARED_POOL
- .set(Box::new(LockedHeap::empty()))
+ .set(Box::new(LockedFrameAllocator::new()))
.map_err(|_| MemoryTrackerError::SharedPoolSetFailure)?;
Ok(())
@@ -359,13 +357,9 @@
pub fn init_static_shared_pool(&mut self, range: Range<usize>) -> Result<()> {
let size = NonZeroUsize::new(range.len()).unwrap();
let range = self.alloc_mut(range.start, size)?;
- let shared_pool = LockedHeap::<32>::new();
+ let shared_pool = LockedFrameAllocator::<32>::new();
- // SAFETY - `range` should be a valid region of memory as validated by
- // `validate_swiotlb_info` and not used by any other rust code.
- unsafe {
- shared_pool.lock().init(range.start, range.len());
- }
+ shared_pool.lock().insert(range);
SHARED_POOL
.set(Box::new(shared_pool))
@@ -395,10 +389,8 @@
}
}
-/// Allocates a memory range of at least the given size that is shared with
-/// host. Returns a pointer to the buffer.
-///
-/// It will be aligned to the memory sharing granule size supported by the hypervisor.
+/// Allocates a memory range of at least the given size and alignment that is shared with the host.
+/// Returns a pointer to the buffer.
pub fn alloc_shared(layout: Layout) -> hyp::Result<NonNull<u8>> {
assert_ne!(layout.size(), 0);
let Some(buffer) = try_shared_alloc(layout) else {
@@ -412,11 +404,11 @@
fn try_shared_alloc(layout: Layout) -> Option<NonNull<u8>> {
let mut shared_pool = SHARED_POOL.get().unwrap().lock();
- if let Ok(buffer) = shared_pool.alloc(layout) {
- Some(buffer)
+ if let Some(buffer) = shared_pool.alloc_aligned(layout) {
+ Some(NonNull::new(buffer as _).unwrap())
} else if let Some(shared_memory) = SHARED_MEMORY.lock().as_mut() {
shared_memory.refill(&mut shared_pool, layout);
- shared_pool.alloc(layout).ok()
+ shared_pool.alloc_aligned(layout).map(|buffer| NonNull::new(buffer as _).unwrap())
} else {
None
}
@@ -424,14 +416,14 @@
/// Unshares and deallocates a memory range which was previously allocated by `alloc_shared`.
///
-/// The size passed in must be the size passed to the original `alloc_shared` call.
+/// The layout passed in must be the same layout passed to the original `alloc_shared` call.
///
/// # Safety
///
-/// The memory must have been allocated by `alloc_shared` with the same size, and not yet
+/// The memory must have been allocated by `alloc_shared` with the same layout, and not yet
/// deallocated.
pub unsafe fn dealloc_shared(vaddr: NonNull<u8>, layout: Layout) -> hyp::Result<()> {
- SHARED_POOL.get().unwrap().lock().dealloc(vaddr, layout);
+ SHARED_POOL.get().unwrap().lock().dealloc_aligned(vaddr.as_ptr() as usize, layout);
trace!("Deallocated shared buffer at {vaddr:?} with {layout:?}");
Ok(())
diff --git a/pvmfw/src/virtio/hal.rs b/pvmfw/src/virtio/hal.rs
index 51567cd..becc263 100644
--- a/pvmfw/src/virtio/hal.rs
+++ b/pvmfw/src/virtio/hal.rs
@@ -23,23 +23,25 @@
use log::trace;
use virtio_drivers::{BufferDirection, Hal, PhysAddr, PAGE_SIZE};
+/// The alignment to use for the temporary buffers allocated by `HalImpl::share`. There doesn't seem
+/// to be any particular alignment required by VirtIO for these, so 16 bytes should be enough to
+/// allow appropriate alignment for whatever fields are accessed. `alloc_shared` will increase the
+/// alignment to the memory sharing granule size anyway.
+const SHARED_BUFFER_ALIGNMENT: usize = size_of::<u128>();
+
pub struct HalImpl;
-/// Implements the `Hal` trait for `HalImpl`.
-///
/// # Safety
///
-/// Callers of this implementatation must follow the safety requirements documented in the `Hal`
-/// trait for the unsafe methods.
+/// See the 'Implementation Safety' comments on methods below for how they fulfill the safety
+/// requirements of the unsafe `Hal` trait.
unsafe impl Hal for HalImpl {
- /// Allocates the given number of contiguous physical pages of DMA memory for VirtIO use.
- ///
/// # Implementation Safety
///
/// `dma_alloc` ensures the returned DMA buffer is not aliased with any other allocation or
- /// reference in the program until it is deallocated by `dma_dealloc` by allocating a unique
- /// block of memory using `alloc_shared` and returning a non-null pointer to it that is
- /// aligned to `PAGE_SIZE`.
+ /// reference in the program until it is deallocated by `dma_dealloc` by allocating a unique
+ /// block of memory using `alloc_shared`, which is guaranteed to allocate valid and unique
+ /// memory. We request an alignment of at least `PAGE_SIZE` from `alloc_shared`.
fn dma_alloc(pages: usize, _direction: BufferDirection) -> (PhysAddr, NonNull<u8>) {
let vaddr = alloc_shared(dma_layout(pages))
.expect("Failed to allocate and share VirtIO DMA range with host");
@@ -51,18 +53,18 @@
}
unsafe fn dma_dealloc(_paddr: PhysAddr, vaddr: NonNull<u8>, pages: usize) -> i32 {
- // SAFETY - Memory was allocated by `dma_alloc` using `alloc_shared` with the same size.
+ // SAFETY - Memory was allocated by `dma_alloc` using `alloc_shared` with the same layout.
unsafe { dealloc_shared(vaddr, dma_layout(pages)) }
.expect("Failed to unshare VirtIO DMA range with host");
0
}
- /// Converts a physical address used for MMIO to a virtual address which the driver can access.
- ///
/// # Implementation Safety
///
- /// `mmio_phys_to_virt` satisfies the requirement by checking that the mapped memory region
- /// is within the PCI MMIO range.
+ /// The returned pointer must be valid because the `paddr` describes a valid MMIO region, we
+ /// check that it is within the PCI MMIO range, and we previously mapped the entire PCI MMIO
+ /// range. It can't alias any other allocations because we previously validated in
+ /// `map_mmio_range` that the PCI MMIO range didn't overlap with any other memory ranges.
unsafe fn mmio_phys_to_virt(paddr: PhysAddr, size: usize) -> NonNull<u8> {
let pci_info = PCI_INFO.get().expect("VirtIO HAL used before PCI_INFO was initialised");
let bar_range = {
@@ -109,7 +111,7 @@
unsafe { copy_nonoverlapping(bounce.as_ptr(), dest, size) };
}
- // SAFETY - Memory was allocated by `share` using `alloc_shared` with the same size.
+ // SAFETY - Memory was allocated by `share` using `alloc_shared` with the same layout.
unsafe { dealloc_shared(bounce, bb_layout(size)) }
.expect("Failed to unshare and deallocate VirtIO bounce buffer");
}
@@ -121,7 +123,5 @@
}
fn bb_layout(size: usize) -> Layout {
- // In theory, it would be legal to align to 1-byte but use a larger alignment for good measure.
- const VIRTIO_BOUNCE_BUFFER_ALIGN: usize = size_of::<u128>();
- Layout::from_size_align(size, VIRTIO_BOUNCE_BUFFER_ALIGN).unwrap()
+ Layout::from_size_align(size, SHARED_BUFFER_ALIGNMENT).unwrap()
}
diff --git a/tests/benchmark_hostside/Android.bp b/tests/benchmark_hostside/Android.bp
index e053406..b613a8a 100644
--- a/tests/benchmark_hostside/Android.bp
+++ b/tests/benchmark_hostside/Android.bp
@@ -13,6 +13,7 @@
static_libs: [
"MicrodroidHostTestHelper",
"MicrodroidTestHelper",
+ "MicrodroidTestPreparer",
],
test_suites: [
"general-tests",
diff --git a/vmclient/src/lib.rs b/vmclient/src/lib.rs
index d67d87e..8f25b99 100644
--- a/vmclient/src/lib.rs
+++ b/vmclient/src/lib.rs
@@ -112,7 +112,7 @@
// Wait for the child to signal that the RpcBinder server is ready
// by closing its end of the pipe.
- let _ = File::from(wait_fd).read(&mut [0]);
+ let _ignored = File::from(wait_fd).read(&mut [0]);
Ok(VirtualizationService { client_fd })
}