Merge with upstream 2022-06-27

1414622073 tools: Set host as default target
64af01caed seccomp: add arm policy for Goldfish battery
2d2eec381b hypervisor: update CpuIdEntry uses for haxm and whpx
44fd232dbc crosvm: rename DevicesCommand to DeviceCommand
8b9fc10192 vm_control: use Protection in mapping requests
6e06b9f0e3 tools: Make run_tests python3.8 compatible
e12a9aad61 base: Upstream stream_channel
2bb4f97051 base: add nanoseconds to log timestamps.
5cf9a8fb87 devices: virtio: snd: Support num_{output,input}_devices
d220083da9 crosvm: Add documentation for crosvm_control
8297d745c6 devices: virtio: console: use ReadNotifier trait for polling
947754f011 devices: virtio: console: add async console device shared with vhost-user
3b2d0d6cfc base: Add safety comments in net.rs
9fde8f499a examples/baremetal: exit instead of hanging
bf7d3bd38f examples/baremetal: make paddr == vaddr
b4244d3952 kernel_loader: load ELF kernels at the right paddr
102d03b380 x86_64: return kernel load address from load_kernel
333bf60e91 kernel_loader: return entry point of loaded kernel
b5dbe329be x86_64: pass initial registers in VcpuInit
569a96bed8 hypervisor: x86_64: impl Default for Regs
066276676b infra: Add crosvm_windows builder
3349a67660 crosvm: Add a first filter_cpuid test.
6af7ff8540 crosvm: Pass down CpuidResult instead.
982c92bb98 ci: kokoro: Add presubmit configs for x86_64-direct
ab004ddc01 irqchip: Upstream tests
ff3a722691 devices: Upstream virtio console Windows implementation
b6ef09ecc5 devices: pci: don't reserve bridge window for hot added bridges
4d854e80b4 devices: pci: support hotplugged pci bus to be removed from tree
0e1faf9898 devices: pcie: make pcie root port use PciePort
61052012a4 devices: pcie: add pcie upstream and downstream port
0d8eef2074 broker_ipc: fix child process logging init order.
972ed6d094 base: log with local timestamps instead of UTC

918bdde54f..1414622073

BUG=b:236924546
BUG=b:213149158
BUG=b:201745804
BUG=b:188858559
BUG=b:199986018
BUG=b:234155022
BUG=b:213149162
BUG=b:233914170
BUG=b:237004396
BUG=236850894
BUG=b:236004673
BUG=b:234173142
BUG=b:214124318
BUG=b:228912920
BUG=b:218223240
BUG=b:220292205

Change-Id: I945e6e190fe8271f0f9c94bd8c3c37846320958c

aarch64/src/lib.rs

@@ -276,7 +276,7 @@ impl arch::LinuxArch for AArch64 {
                 } else {
                     let loaded_kernel = elf_result.map_err(Error::LoadElfKernel)?;
                     kernel_size = loaded_kernel.size as usize;
-                    kernel_end = loaded_kernel.end.offset();
+                    kernel_end = loaded_kernel.address_range.end;
                 }
                 initrd = match components.initrd_image {
                     Some(initrd_file) => {

arch/src/lib.rs

@@ -640,7 +640,7 @@ pub fn generate_pci_root(
 
     let mut device_ranges = BTreeMap::new();
     let mut io_ranges = BTreeMap::new();
-    let root_bus = Arc::new(Mutex::new(PciBus::new(0, 0)));
+    let root_bus = Arc::new(Mutex::new(PciBus::new(0, 0, false)));
 
     generate_pci_topology(
         root_bus.clone(),

base/src/lib.rs

@@ -101,13 +101,14 @@ cfg_if::cfg_if! {
             DuplicateHandleRequest, DuplicateHandleResponse, DuplicateHandleTube,
         };
         pub use platform::{set_audio_thread_priorities, thread};
-        pub use platform::{BlockingMode, FramingMode, StreamChannel};
         pub use platform::gmtime_secure;
     } else {
         compile_error!("Unsupported platform");
     }
 }
 
+pub use platform::{BlockingMode, FramingMode, StreamChannel};
+
 pub use platform::{
     deserialize_with_descriptors, EventContext, FileAllocate, FileGetLen, FileSerdeWrapper,
     SerializeDescriptors, UnsyncMarker,

base/src/notifiers.rs

@@ -10,6 +10,12 @@ pub trait ReadNotifier {
     fn get_read_notifier(&self) -> &dyn AsRawDescriptor;
 }
 
+impl ReadNotifier for std::fs::File {
+    fn get_read_notifier(&self) -> &dyn AsRawDescriptor {
+        self
+    }
+}
+
 pub trait CloseNotifier {
     /// Gets a descriptor that can be used in EventContext to wait for the closed event.
     fn get_close_notifier(&self) -> &dyn AsRawDescriptor;

base/src/sys/unix/mmap.rs

@@ -20,6 +20,7 @@ use libc::{self, c_int, c_void, read, write};
 use remain::sorted;
 
 use data_model::{volatile_memory::*, DataInit};
+use serde::{Deserialize, Serialize};
 
 use super::{pagesize, Error as ErrnoError};
 
@@ -50,7 +51,7 @@ pub enum Error {
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// Memory access type for anonymous shared memory mapping.
-#[derive(Copy, Clone, Eq, PartialEq)]
+#[derive(Copy, Clone, Eq, PartialEq, Serialize, Deserialize, Debug)]
 pub struct Protection(c_int);
 impl Protection {
     /// Returns Protection allowing no access.

base/src/sys/unix/mod.rs

@@ -39,6 +39,7 @@ mod shm;
 pub mod signal;
 mod signalfd;
 mod sock_ctrl_msg;
+mod stream_channel;
 mod terminal;
 mod timer;
 pub mod vsock;
@@ -69,6 +70,7 @@ pub use shm::{kernel_has_memfd, MemfdSeals, SharedMemory, Unix as SharedMemoryUn
 pub use signal::*;
 pub use signalfd::*;
 pub use sock_ctrl_msg::*;
+pub use stream_channel::*;
 pub use terminal::*;
 pub use timer::*;
 

base/src/sys/unix/net.rs

@@ -598,6 +598,8 @@ impl UnixSeqpacket {
     /// Sets the blocking mode for this socket.
     pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> {
         let mut nonblocking = nonblocking as libc::c_int;
+        // Safe because the return value is checked, and this ioctl call sets the nonblocking mode
+        // and does not continue holding the file descriptor after the call.
         let ret = unsafe { libc::ioctl(self.fd, libc::FIONBIO, &mut nonblocking) };
         if ret < 0 {
             Err(io::Error::last_os_error())
@@ -802,6 +804,8 @@ impl UnixSeqpacketListener {
     /// Sets the blocking mode for this socket.
     pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> {
         let mut nonblocking = nonblocking as libc::c_int;
+        // Safe because the return value is checked, and this ioctl call sets the nonblocking mode
+        // and does not continue holding the file descriptor after the call.
         let ret = unsafe { libc::ioctl(self.fd, libc::FIONBIO, &mut nonblocking) };
         if ret < 0 {
             Err(io::Error::last_os_error())

base/src/sys/unix/stream_channel.rs

@@ -0,0 +1,251 @@
+// Copyright 2022 The ChromiumOS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use super::super::{net::UnixSeqpacket, Result};
+use super::RawDescriptor;
+use crate::{descriptor::AsRawDescriptor, ReadNotifier};
+use libc::{
+    c_void, {self},
+};
+use std::{
+    io::{
+        Read, {self},
+    },
+    os::unix::net::UnixStream,
+};
+
+#[derive(Copy, Clone)]
+pub enum FramingMode {
+    Message,
+    Byte,
+}
+
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub enum BlockingMode {
+    Blocking,
+    Nonblocking,
+}
+
+impl io::Read for StreamChannel {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        self.inner_read(buf)
+    }
+}
+
+impl io::Read for &StreamChannel {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        self.inner_read(buf)
+    }
+}
+
+impl AsRawDescriptor for StreamChannel {
+    fn as_raw_descriptor(&self) -> RawDescriptor {
+        (&self).as_raw_descriptor()
+    }
+}
+
+enum SocketType {
+    Message(UnixSeqpacket),
+    Byte(UnixStream),
+}
+
+/// An abstraction over named pipes and unix socketpairs. This abstraction can be used in a blocking
+/// and non blocking mode.
+pub struct StreamChannel {
+    stream: SocketType,
+}
+
+impl StreamChannel {
+    pub fn set_nonblocking(&mut self, nonblocking: bool) -> io::Result<()> {
+        match &mut self.stream {
+            SocketType::Byte(sock) => sock.set_nonblocking(nonblocking),
+            SocketType::Message(sock) => sock.set_nonblocking(nonblocking),
+        }
+    }
+
+    pub(super) fn inner_read(&self, buf: &mut [u8]) -> io::Result<usize> {
+        match &self.stream {
+            SocketType::Byte(sock) => (&mut &*sock).read(buf),
+
+            // On Windows, reading from SOCK_SEQPACKET with a buffer that is too small is an error,
+            // but on Linux will silently truncate unless MSG_TRUNC is passed. Here, we emulate
+            // Windows behavior on POSIX.
+            //
+            // Note that Rust translates ERROR_MORE_DATA into io::ErrorKind::Other
+            // (see sys::decode_error_kind) on Windows, so we preserve this behavior on POSIX even
+            // though one could argue ErrorKind::UnexpectedEof is a closer match to the true error.
+            SocketType::Message(sock) => {
+                // Safe because buf is valid, we pass buf's size to recv to bound the return
+                // length, and we check the return code.
+                let retval = unsafe {
+                    // TODO(nkgold|b/152067913): Move this into the UnixSeqpacket struct as a
+                    // recv_with_flags method once that struct's tests are working.
+                    libc::recv(
+                        sock.as_raw_descriptor(),
+                        buf.as_mut_ptr() as *mut c_void,
+                        buf.len(),
+                        libc::MSG_TRUNC,
+                    )
+                };
+                let receive_len = if retval < 0 {
+                    Err(std::io::Error::last_os_error())
+                } else {
+                    Ok(retval)
+                }? as usize;
+
+                if receive_len > buf.len() {
+                    Err(std::io::Error::new(
+                        std::io::ErrorKind::Other,
+                        format!(
+                            "packet size {:?} encountered, but buffer was only of size {:?}",
+                            receive_len,
+                            buf.len()
+                        ),
+                    ))
+                } else {
+                    Ok(receive_len)
+                }
+            }
+        }
+    }
+
+    /// Creates a cross platform stream pair.
+    pub fn pair(
+        blocking_mode: BlockingMode,
+        framing_mode: FramingMode,
+    ) -> Result<(StreamChannel, StreamChannel)> {
+        let (pipe_a, pipe_b) = match framing_mode {
+            FramingMode::Byte => {
+                let (pipe_a, pipe_b) = UnixStream::pair()?;
+                (SocketType::Byte(pipe_a), SocketType::Byte(pipe_b))
+            }
+            FramingMode::Message => {
+                let (pipe_a, pipe_b) = UnixSeqpacket::pair()?;
+                (SocketType::Message(pipe_a), SocketType::Message(pipe_b))
+            }
+        };
+        let mut stream_a = StreamChannel { stream: pipe_a };
+        let mut stream_b = StreamChannel { stream: pipe_b };
+        let is_non_blocking = blocking_mode == BlockingMode::Nonblocking;
+        stream_a.set_nonblocking(is_non_blocking)?;
+        stream_b.set_nonblocking(is_non_blocking)?;
+        Ok((stream_a, stream_b))
+    }
+}
+
+impl io::Write for StreamChannel {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        match &mut self.stream {
+            SocketType::Byte(sock) => sock.write(buf),
+            SocketType::Message(sock) => sock.send(buf),
+        }
+    }
+    fn flush(&mut self) -> io::Result<()> {
+        match &mut self.stream {
+            SocketType::Byte(sock) => sock.flush(),
+            SocketType::Message(_) => Ok(()),
+        }
+    }
+}
+
+impl AsRawDescriptor for &StreamChannel {
+    fn as_raw_descriptor(&self) -> RawDescriptor {
+        match &self.stream {
+            SocketType::Byte(sock) => sock.as_raw_descriptor(),
+            SocketType::Message(sock) => sock.as_raw_descriptor(),
+        }
+    }
+}
+
+impl ReadNotifier for StreamChannel {
+    /// Returns a RawDescriptor that can be polled for reads using PollContext.
+    fn get_read_notifier(&self) -> &dyn AsRawDescriptor {
+        self
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::{EventContext, EventToken, ReadNotifier};
+    use std::io::{Read, Write};
+
+    #[derive(EventToken, Debug, Eq, PartialEq, Copy, Clone)]
+    enum Token {
+        ReceivedData,
+    }
+
+    #[test]
+    fn test_non_blocking_pair() {
+        let (mut sender, mut receiver) =
+            StreamChannel::pair(BlockingMode::Nonblocking, FramingMode::Byte).unwrap();
+
+        sender.write_all(&[75, 77, 54, 82, 76, 65]).unwrap();
+
+        // Wait for the data to arrive.
+        let event_ctx: EventContext<Token> =
+            EventContext::build_with(&[(receiver.get_read_notifier(), Token::ReceivedData)])
+                .unwrap();
+        let events = event_ctx.wait().unwrap();
+        let tokens: Vec<Token> = events
+            .iter()
+            .filter(|e| e.is_readable)
+            .map(|e| e.token)
+            .collect();
+        assert_eq!(tokens, vec! {Token::ReceivedData});
+
+        // Smaller than what we sent so we get multiple chunks
+        let mut recv_buffer: [u8; 4] = [0; 4];
+
+        let mut size = receiver.read(&mut recv_buffer).unwrap();
+        assert_eq!(size, 4);
+        assert_eq!(recv_buffer, [75, 77, 54, 82]);
+
+        size = receiver.read(&mut recv_buffer).unwrap();
+        assert_eq!(size, 2);
+        assert_eq!(recv_buffer[0..2], [76, 65]);
+
+        // Now that we've polled for & received all data, polling again should show no events.
+        assert_eq!(
+            event_ctx
+                .wait_timeout(std::time::Duration::new(0, 0))
+                .unwrap()
+                .len(),
+            0
+        );
+    }
+
+    #[test]
+    fn test_non_blocking_pair_error_no_data() {
+        let (mut sender, mut receiver) =
+            StreamChannel::pair(BlockingMode::Nonblocking, FramingMode::Byte).unwrap();
+        receiver
+            .set_nonblocking(true)
+            .expect("Failed to set receiver to nonblocking mode.");
+
+        sender.write_all(&[75, 77]).unwrap();
+
+        // Wait for the data to arrive.
+        let event_ctx: EventContext<Token> =
+            EventContext::build_with(&[(receiver.get_read_notifier(), Token::ReceivedData)])
+                .unwrap();
+        let events = event_ctx.wait().unwrap();
+        let tokens: Vec<Token> = events
+            .iter()
+            .filter(|e| e.is_readable)
+            .map(|e| e.token)
+            .collect();
+        assert_eq!(tokens, vec! {Token::ReceivedData});
+
+        // We only read 2 bytes, even though we requested 4 bytes.
+        let mut recv_buffer: [u8; 4] = [0; 4];
+        let size = receiver.read(&mut recv_buffer).unwrap();
+        assert_eq!(size, 2);
+        assert_eq!(recv_buffer, [75, 77, 00, 00]);
+
+        // Further reads should encounter an error since there is no available data and this is a
+        // non blocking pipe.
+        assert!(receiver.read(&mut recv_buffer).is_err());
+    }
+}

base/src/sys/windows/console.rs

@@ -2,16 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-use std::io::{stdin, Error, Read, Result};
+use std::io::{Error, Read, Result};
 
 use winapi::{
     shared::{minwindef::LPVOID, ntdef::NULL},
     um::{fileapi::ReadFile, minwinbase::LPOVERLAPPED},
 };
 
-use crate::{AsRawDescriptor, RawDescriptor};
+use crate::{AsRawDescriptor, ReadNotifier};
 
-pub struct Console;
+pub struct Console(std::io::Stdin);
+
+impl Console {
+    pub fn new() -> Self {
+        Self(std::io::stdin())
+    }
+}
 
 impl Read for Console {
     fn read(&mut self, out: &mut [u8]) -> Result<usize> {
@@ -20,7 +26,7 @@ impl Read for Console {
         // and `num_of_bytes_read` is a valid u32.
         let res = unsafe {
             ReadFile(
-                stdin().as_raw_descriptor(),
+                self.0.as_raw_descriptor(),
                 out.as_mut_ptr() as LPVOID,
                 out.len() as u32,
                 &mut num_of_bytes_read,
@@ -36,8 +42,8 @@ impl Read for Console {
     }
 }
 
-impl AsRawDescriptor for Console {
-    fn as_raw_descriptor(&self) -> RawDescriptor {
-        stdin().as_raw_descriptor()
+impl ReadNotifier for Console {
+    fn get_read_notifier(&self) -> &dyn AsRawDescriptor {
+        &self.0
     }
 }

base/src/sys/windows/mmap.rs

@@ -12,6 +12,7 @@ use std::{
 
 use crate::descriptor::{FromRawDescriptor, SafeDescriptor};
 use data_model::{volatile_memory::*, DataInit};
+use serde::{Deserialize, Serialize};
 use win_util::create_file_mapping;
 use win_util::duplicate_handle;
 use winapi::um::winnt::PAGE_READWRITE;
@@ -56,7 +57,7 @@ pub enum Error {
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// Memory access type for anonymous shared memory mapping.
-#[derive(Copy, Clone, Eq, PartialEq)]
+#[derive(Copy, Clone, Eq, PartialEq, Serialize, Deserialize, Debug)]
 pub struct Protection(c_uint);
 
 impl Protection {

base/src/syslog.rs

@@ -49,8 +49,9 @@
 //!
 //! [log-crate-url]: https://docs.rs/log/
 
-use std::{fmt::Display, io};
+use std::{fmt::Display, io, io::Write};
 
+use chrono::Local;
 use once_cell::sync::OnceCell;
 use remain::sorted;
 use serde::{Deserialize, Serialize};
@@ -247,8 +248,25 @@ impl State {
         builder.parse(cfg.filter);
         let filter = builder.build();
 
-        if cfg.stderr {
+        let create_formatted_builder = || {
             let mut builder = env_logger::Builder::new();
+
+            // Output log lines w/ local ISO 8601 timestamps.
+            builder.format(|buf, record| {
+                writeln!(
+                    buf,
+                    "[{} {:5} {}] {}",
+                    Local::now().format("%Y-%m-%dT%H:%M:%S%.9f%:z"),
+                    record.level(),
+                    record.module_path().unwrap_or("<missing module path>"),
+                    record.args()
+                )
+            });
+            builder
+        };
+
+        if cfg.stderr {
+            let mut builder = create_formatted_builder();
             builder.filter_level(log::LevelFilter::Trace);
             builder.target(env_logger::Target::Stderr);
             loggers.push(Box::new(builder.build()));
@@ -259,7 +277,7 @@ impl State {
         }
 
         if let Some(file) = cfg.pipe {
-            let mut builder = env_logger::Builder::new();
+            let mut builder = create_formatted_builder();
             builder.filter_level(log::LevelFilter::Trace);
             builder.target(env_logger::Target::Pipe(Box::new(file)));
             // https://github.com/env-logger-rs/env_logger/issues/208

broker_ipc/src/lib.rs

@@ -35,9 +35,7 @@ pub struct ChildLifecycleCleanup {
 ///
 /// Returns a value that should be dropped when the process exits.
 pub fn common_child_setup(args: CommonChildStartupArgs) -> anyhow::Result<ChildLifecycleCleanup> {
-    // Crash reporting should start as early as possible, in case other startup tasks fail.
-    init_child_crash_reporting(&args.product_attrs);
-
+    // Logging must initialize first in case there are other startup errors.
     let mut cfg = syslog::LogConfig::default();
     if let Some(log_file_descriptor) = args.syslog_file {
         // Safe because we are taking ownership of a SafeDescriptor.
@@ -50,6 +48,9 @@ pub fn common_child_setup(args: CommonChildStartupArgs) -> anyhow::Result<ChildL
     }
     syslog::init_with(cfg)?;
 
+    // Crash reporting should start as early as possible, in case other startup tasks fail.
+    init_child_crash_reporting(&args.product_attrs);
+
     // Initialize anything product specific.
     product_child_setup(&args.product_attrs)?;
 

ci/kokoro/presubmit-cq-x86_64-direct.cfg

@@ -0,0 +1,3 @@
+# Format: //devtools/kokoro/config/proto/build.proto
+
+build_file: "crosvm/ci/kokoro/build-x86_64-direct.sh"

ci/kokoro/presubmit-cr-x86_64-direct.cfg

@@ -0,0 +1,3 @@
+# Format: //devtools/kokoro/config/proto/build.proto
+
+build_file: "crosvm/ci/kokoro/build-x86_64-direct.sh"

devices/src/irqchip/apic.rs

@@ -13,11 +13,6 @@
 //   * cluster-mode logical addressing
 //   * external interrupts -- these are handled by querying `Pic` separately in
 //     `UserspaceIrqChip::inject_interrupts`
-//
-
-// TODO(b/213149158): this code will be used once the rest of the module
-// upstreaming is done.
-#![allow(dead_code)]
 
 use std::convert::{TryFrom, TryInto};
 use std::time::{Duration, Instant};

devices/src/irqchip/kvm/x86_64.rs

@@ -841,6 +841,12 @@ mod tests {
         chip
     }
 
+    #[test]
+    fn kernel_irqchip_pit_uses_speaker_port() {
+        let chip = get_kernel_chip();
+        assert!(!chip.pit_uses_speaker_port());
+    }
+
     #[test]
     fn kernel_irqchip_get_pic() {
         test_get_pic(get_kernel_chip());
@@ -921,6 +927,12 @@ mod tests {
         test_route_irq(get_split_chip());
     }
 
+    #[test]
+    fn split_irqchip_pit_uses_speaker_port() {
+        let chip = get_split_chip();
+        assert!(chip.pit_uses_speaker_port());
+    }
+
     #[test]
     fn split_irqchip_routes_conflict() {
         let mut chip = get_split_chip();

devices/src/pci/coiommu.rs

@@ -27,7 +27,7 @@ use std::{fmt, mem, thread};
 use anyhow::{anyhow, bail, ensure, Context, Result};
 use base::{
     error, info, AsRawDescriptor, Event, EventToken, MemoryMapping, MemoryMappingBuilder,
-    RawDescriptor, SafeDescriptor, SharedMemory, Timer, Tube, TubeError, WaitContext,
+    Protection, RawDescriptor, SafeDescriptor, SharedMemory, Timer, Tube, TubeError, WaitContext,
 };
 use data_model::DataInit;
 use hypervisor::Datamatch;
@@ -1072,7 +1072,7 @@ impl CoIommuDev {
         size: usize,
         offset: u64,
         gpa: u64,
-        read_only: bool,
+        prot: Protection,
     ) -> Result<()> {
         let request = VmMemoryRequest::RegisterMemory {
             source: VmMemorySource::Descriptor {
@@ -1081,7 +1081,7 @@ impl CoIommuDev {
                 size: size as u64,
             },
             dest: VmMemoryDestination::GuestPhysicalAddress(gpa),
-            read_only,
+            prot,
         };
         self.send_msg(&request)
     }
@@ -1097,7 +1097,7 @@ impl CoIommuDev {
                 COIOMMU_NOTIFYMAP_SIZE,
                 0,
                 gpa,
-                false,
+                Protection::read_write(),
             ) {
                 Ok(_) => {}
                 Err(e) => {
@@ -1112,7 +1112,7 @@ impl CoIommuDev {
                 COIOMMU_TOPOLOGYMAP_SIZE,
                 0,
                 gpa,
-                true,
+                Protection::read(),
             ) {
                 Ok(_) => {}
                 Err(e) => {

devices/src/pci/pci_device.rs

@@ -31,8 +31,8 @@ use super::PciId;
 #[sorted]
 #[derive(Error, Debug)]
 pub enum Error {
-    /// Device does not located on this bus
-    #[error("pci device {0} does not located on bus {1}")]
+    /// Added pci device's parent bus does not belong to this bus
+    #[error("pci device {0}'s parent bus does not belong to bus {1}")]
     AddedDeviceBusNotExist(PciAddress, u8),
     /// Invalid alignment encountered.
     #[error("Alignment must be a power of 2")]
@@ -40,6 +40,9 @@ pub enum Error {
     /// The new bus has already been added to this bus
     #[error("Added bus {0} already existed on bus {1}")]
     BusAlreadyExist(u8, u8),
+    /// Target bus not exists on this bus
+    #[error("pci bus {0} does not exist on bus {1}")]
+    BusNotExist(u8, u8),
     /// Setup of the device capabilities failed.
     #[error("failed to add capability {0}")]
     CapabilitiesSetup(pci_configuration::Error),
@@ -54,7 +57,9 @@ pub enum Error {
     /// Device is already on this bus
     #[error("pci device {0} has already been added to bus {1}")]
     DeviceAlreadyExist(PciAddress, u8),
-
+    /// Device not exist on this bus
+    #[error("pci device {0} does not located on bus {1}")]
+    DeviceNotExist(PciAddress, u8),
     /// Allocating space for an IO BAR failed.
     #[error("failed to allocate space for an IO BAR, size={0}: {1}")]
     IoAllocationFailed(u64, SystemAllocatorFaliure),
@@ -112,20 +117,27 @@ pub struct PciBus {
     // Hash map that stores all direct child buses of this bus.
     // It maps from child bus number to its pci bus structure.
     child_buses: HashMap<u8, Arc<Mutex<PciBus>>>,
+    // Is hotplug bus
+    hotplug_bus: bool,
 }
 
 impl PciBus {
     // Creates a new pci bus
-    pub fn new(bus_num: u8, parent_bus_num: u8) -> Self {
+    pub fn new(bus_num: u8, parent_bus_num: u8, hotplug_bus: bool) -> Self {
         PciBus {
             bus_num,
             parent_bus_num,
             child_devices: HashSet::new(),
             child_buses: HashMap::new(),
+            hotplug_bus,
         }
     }
 
-    // Add a new child device to this pci bus tree recursively.
+    pub fn get_bus_num(&self) -> u8 {
+        self.bus_num
+    }
+
+    // Add a new child device to this pci bus tree.
     pub fn add_child_device(&mut self, add_device: PciAddress) -> Result<()> {
         if self.bus_num == add_device.bus {
             if !self.child_devices.insert(add_device) {
@@ -147,7 +159,20 @@ impl PciBus {
         Err(Error::AddedDeviceBusNotExist(add_device, self.bus_num))
     }
 
-    // Add a new child bus to this pci bus tree recursively.
+    // Remove one child device from this pci bus tree
+    pub fn remove_child_device(&mut self, device: PciAddress) -> Result<()> {
+        if self.child_devices.remove(&device) {
+            return Ok(());
+        }
+        for child_bus in self.child_buses.values() {
+            if child_bus.lock().remove_child_device(device).is_ok() {
+                return Ok(());
+            }
+        }
+        Err(Error::DeviceNotExist(device, self.bus_num))
+    }
+
+    // Add a new child bus to this pci bus tree.
     pub fn add_child_bus(&mut self, add_bus: Arc<Mutex<PciBus>>) -> Result<()> {
         let add_bus_num = add_bus.lock().bus_num;
         let add_bus_parent = add_bus.lock().parent_bus_num;
@@ -172,6 +197,34 @@ impl PciBus {
         Err(Error::ParentBusNotExist(add_bus_num, self.bus_num))
     }
 
+    // Remove one child bus from this pci bus tree.
+    pub fn remove_child_bus(&mut self, bus_no: u8) -> Result<()> {
+        if self.child_buses.remove(&bus_no).is_some() {
+            return Ok(());
+        }
+        for (_, child_bus) in self.child_buses.iter() {
+            if child_bus.lock().remove_child_bus(bus_no).is_ok() {
+                return Ok(());
+            }
+        }
+        Err(Error::BusNotExist(bus_no, self.bus_num))
+    }
+
+    // Find all downstream devices under the given bus
+    pub fn find_downstream_devices(&self, bus_no: u8) -> Vec<PciAddress> {
+        if self.bus_num == bus_no {
+            return self.get_downstream_devices();
+        }
+        for (_, child_bus) in self.child_buses.iter() {
+            let res = child_bus.lock().find_downstream_devices(bus_no);
+            if !res.is_empty() {
+                return res;
+            }
+        }
+
+        Vec::new()
+    }
+
     // Get all devices in this pci bus tree
     pub fn get_downstream_devices(&self) -> Vec<PciAddress> {
         let mut devices = Vec::new();
@@ -182,8 +235,33 @@ impl PciBus {
         devices
     }
 
-    pub fn get_bus_num(&self) -> u8 {
-        self.bus_num
+    // Check if given device is located in the device tree
+    pub fn contains(&self, device: PciAddress) -> bool {
+        if self.child_devices.contains(&device) {
+            return true;
+        }
+
+        for (_, child_bus) in self.child_buses.iter() {
+            if child_bus.lock().contains(device) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    // Returns the hotplug bus that this device is on.
+    pub fn get_hotplug_bus(&self, device: PciAddress) -> Option<u8> {
+        if self.hotplug_bus && self.contains(device) {
+            return Some(self.bus_num);
+        }
+        for (_, child_bus) in self.child_buses.iter() {
+            let hotplug_bus = child_bus.lock().get_hotplug_bus(device);
+            if hotplug_bus.is_some() {
+                return hotplug_bus;
+            }
+        }
+        return None;
     }
 }
 

devices/src/pci/pcie/mod.rs

@@ -5,6 +5,7 @@
 mod pci_bridge;
 mod pcie_device;
 mod pcie_host;
+mod pcie_port;
 mod pcie_rp;
 
 pub use pci_bridge::PciBridge;

devices/src/pci/pcie/pci_bridge.rs

@@ -92,6 +92,7 @@ impl PciBridge {
             .lock()
             .get_bus_range()
             .expect("PciBridge's backend device must implement get_bus_range()");
+
         let data = [
             bus_range.primary,
             bus_range.secondary,
@@ -99,15 +100,17 @@ impl PciBridge {
             0,
         ];
         config.write_reg(BR_BUS_NUMBER_REG, 0, &data[..]);
+        let pci_bus = Arc::new(Mutex::new(PciBus::new(
+            bus_range.secondary,
+            bus_range.primary,
+            device.lock().hotplug_implemented(),
+        )));
 
         PciBridge {
             device,
             config,
             pci_address: None,
-            pci_bus: Arc::new(Mutex::new(PciBus::new(
-                bus_range.secondary,
-                bus_range.primary,
-            ))),
+            pci_bus,
             bus_range,
             msi_config,
             msi_cap_offset,
@@ -330,17 +333,22 @@ impl PciDevice for PciBridge {
         let mut window_size: u64 = 0;
         let mut pref_window_base: u64 = u64::MAX;
         let mut pref_window_size: u64 = 0;
+        let hotplug_implemented = self.device.lock().hotplug_implemented();
+        let hotplugged = self.device.lock().hotplugged();
 
-        if self.device.lock().hotplug_implemented() {
-            // Bridge for children hotplug, get desired bridge window size and reserve
-            // it for guest OS use
+        if hotplug_implemented || hotplugged {
+            // If bridge is for children hotplug, get desired bridge window size and reserve
+            // it for guest OS use.
+            // If bridge is hotplugged into the system, get the desired bridge window size
+            // from host.
             let (win_size, pref_win_size) = self.device.lock().get_bridge_window_size();
             window_size = win_size;
             pref_window_size = pref_win_size;
         } else {
+            // Bridge has children connected, get bridge window size from children
             let mut window_end: u64 = 0;
             let mut pref_window_end: u64 = 0;
-            // Bridge has children connected, get bridge window size from children
+
             for &BarRange {
                 addr,
                 size,
@@ -363,59 +371,21 @@ impl PciDevice for PciBridge {
             }
         }
 
-        if window_size == 0 {
-            // Allocate at least 2MB bridge winodw
-            window_size = BR_MEM_MINIMUM;
-        }
-        // if window_base isn't set, allocate a new one
-        if window_base == u64::MAX {
-            // align window_size to 1MB
-            if window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                window_size = (window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
+        if !hotplugged {
+            // Only static bridge needs to locate their window's position. Hotplugged bridge's
+            // window will be handled by guest kernel.
+            if window_size == 0 {
+                // Allocate at least 2MB bridge winodw
+                window_size = BR_MEM_MINIMUM;
             }
-            match resources.mmio_allocator(MmioType::Low).allocate_with_align(
-                window_size,
-                Alloc::PciBridgeWindow {
-                    bus: address.bus,
-                    dev: address.dev,
-                    func: address.func,
-                },
-                "pci_bridge_window".to_string(),
-                BR_WINDOW_ALIGNMENT,
-            ) {
-                Ok(addr) => window_base = addr,
-                Err(e) => warn!(
-                    "{} failed to allocate bridge window: {}",
-                    self.debug_label(),
-                    e
-                ),
-            }
-        } else {
-            // align window_base to 1MB
-            if window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                window_size += window_base - (window_base & BR_WINDOW_MASK);
+            // if window_base isn't set, allocate a new one
+            if window_base == u64::MAX {
                 // align window_size to 1MB
                 if window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
                     window_size = (window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
                 }
-                window_base &= BR_WINDOW_MASK;
-            }
-        }
-
-        if pref_window_size == 0 {
-            // Allocate at least 2MB prefetch bridge window
-            pref_window_size = BR_MEM_MINIMUM;
-        }
-        // if pref_window_base isn't set, allocate a new one
-        if pref_window_base == u64::MAX {
-            // align pref_window_size to 1MB
-            if pref_window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                pref_window_size = (pref_window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
-            }
-            match resources
-                .mmio_allocator(MmioType::High)
-                .allocate_with_align(
-                    pref_window_size,
+                match resources.mmio_allocator(MmioType::Low).allocate_with_align(
+                    window_size,
                     Alloc::PciBridgeWindow {
                         bus: address.bus,
                         dev: address.dev,
@@ -424,23 +394,74 @@ impl PciDevice for PciBridge {
                     "pci_bridge_window".to_string(),
                     BR_WINDOW_ALIGNMENT,
                 ) {
-                Ok(addr) => pref_window_base = addr,
-                Err(e) => warn!(
-                    "{} failed to allocate bridge window: {}",
-                    self.debug_label(),
-                    e
-                ),
+                    Ok(addr) => window_base = addr,
+                    Err(e) => warn!(
+                        "{} failed to allocate bridge window: {}",
+                        self.debug_label(),
+                        e
+                    ),
+                }
+            } else {
+                // align window_base to 1MB
+                if window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                    window_size += window_base - (window_base & BR_WINDOW_MASK);
+                    // align window_size to 1MB
+                    if window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                        window_size = (window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
+                    }
+                    window_base &= BR_WINDOW_MASK;
+                }
             }
-        } else {
-            // align pref_window_base to 1MB
-            if pref_window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                pref_window_size += pref_window_base - (pref_window_base & BR_WINDOW_MASK);
+
+            if pref_window_size == 0 {
+                // Allocate at least 2MB prefetch bridge window
+                pref_window_size = BR_MEM_MINIMUM;
+            }
+            // if pref_window_base isn't set, allocate a new one
+            if pref_window_base == u64::MAX {
                 // align pref_window_size to 1MB
                 if pref_window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
                     pref_window_size =
                         (pref_window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
                 }
-                pref_window_base &= BR_WINDOW_MASK;
+                match resources
+                    .mmio_allocator(MmioType::High)
+                    .allocate_with_align(
+                        pref_window_size,
+                        Alloc::PciBridgeWindow {
+                            bus: address.bus,
+                            dev: address.dev,
+                            func: address.func,
+                        },
+                        "pci_bridge_window".to_string(),
+                        BR_WINDOW_ALIGNMENT,
+                    ) {
+                    Ok(addr) => window_base = addr,
+                    Err(e) => warn!(
+                        "{} failed to allocate bridge window: {}",
+                        self.debug_label(),
+                        e
+                    ),
+                }
+            } else {
+                // align pref_window_base to 1MB
+                if pref_window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                    pref_window_size += pref_window_base - (pref_window_base & BR_WINDOW_MASK);
+                    // align pref_window_size to 1MB
+                    if pref_window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                        pref_window_size =
+                            (pref_window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
+                    }
+                    pref_window_base &= BR_WINDOW_MASK;
+                }
+            }
+        } else {
+            // 0 is Ok here because guest will relocate the bridge window
+            if window_size > 0 {
+                window_base = 0;
+            }
+            if pref_window_size > 0 {
+                pref_window_base = 0;
             }
         }
 

devices/src/pci/pcie/pcie_device.rs

@@ -20,7 +20,7 @@ pub trait PcieDevice: Send {
     ) -> std::result::Result<PciAddress, PciDeviceError>;
     fn read_config(&self, reg_idx: usize, data: &mut u32);
     fn write_config(&mut self, reg_idx: usize, offset: u64, data: &[u8]);
-    fn clone_interrupt(&mut self, msix_config: Arc<Mutex<MsiConfig>>);
+    fn clone_interrupt(&mut self, msi_config: Arc<Mutex<MsiConfig>>);
     fn get_caps(&self) -> Vec<Box<dyn PciCapability>>;
     fn set_capability_reg_idx(&mut self, id: PciCapabilityID, reg_idx: usize);
     fn get_bus_range(&self) -> Option<PciBridgeBusRange> {
@@ -33,6 +33,9 @@ pub trait PcieDevice: Send {
     /// Return false, the children pci devices should be connected statically
     fn hotplug_implemented(&self) -> bool;
 
+    /// This function returns true if this pcie device is hotplugged into the system
+    fn hotplugged(&self) -> bool;
+
     /// Get bridge window size to cover children's mmio size
     /// (u64, u64) -> (non_prefetchable window size, prefetchable_window_size)
     fn get_bridge_window_size(&self) -> (u64, u64);

devices/src/pci/pcie/pcie_rp.rs

@@ -1,9 +1,7 @@
 // Copyright 2021 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-
 use std::collections::BTreeMap;
-use std::str::FromStr;
 use std::sync::Arc;
 use sync::Mutex;
 
@@ -12,349 +10,76 @@ use crate::pci::pci_configuration::PciCapabilityID;
 use crate::pci::{MsiConfig, PciAddress, PciCapability, PciDeviceError};
 
 use crate::pci::pcie::pci_bridge::PciBridgeBusRange;
-use crate::pci::pcie::pcie_device::{PciPmcCap, PcieCap, PcieDevice, PmcConfig};
+use crate::pci::pcie::pcie_device::{PciPmcCap, PcieCap, PcieDevice};
 use crate::pci::pcie::pcie_host::PcieHostPort;
+use crate::pci::pcie::pcie_port::PciePort;
 use crate::pci::pcie::*;
 
-use anyhow::{anyhow, Result};
-use base::warn;
-use data_model::DataInit;
-use resources::{Alloc, SystemAllocator};
+use anyhow::Result;
+use resources::SystemAllocator;
 use vm_control::GpeNotify;
 
-// reserve 8MB memory window
-const PCIE_RP_BR_MEM_SIZE: u64 = 0x80_0000;
-// reserve 64MB prefetch window
-const PCIE_RP_BR_PREF_MEM_SIZE: u64 = 0x400_0000;
-
 const PCIE_RP_DID: u16 = 0x3420;
 pub struct PcieRootPort {
-    pcie_cap_reg_idx: Option<usize>,
-    msi_config: Option<Arc<Mutex<MsiConfig>>>,
-    pmc_config: PmcConfig,
-    pmc_cap_reg_idx: Option<usize>,
-    pci_address: Option<PciAddress>,
-    slot_control: Option<u16>,
-    slot_status: u16,
-    root_control: u16,
-    root_status: u32,
-    hp_interrupt_pending: bool,
-    pme_pending_request_id: Option<PciAddress>,
-    bus_range: PciBridgeBusRange,
+    pcie_port: PciePort,
     downstream_devices: BTreeMap<PciAddress, HostHotPlugKey>,
     hotplug_out_begin: bool,
     removed_downstream: Vec<PciAddress>,
-    removed_downstream_valid: bool,
-    pcie_host: Option<PcieHostPort>,
-    prepare_hotplug: bool,
 }
 
 impl PcieRootPort {
     /// Constructs a new PCIE root port
     pub fn new(secondary_bus_num: u8, slot_implemented: bool) -> Self {
-        let bus_range = PciBridgeBusRange {
-            primary: 0,
-            secondary: secondary_bus_num,
-            subordinate: secondary_bus_num,
-        };
         PcieRootPort {
-            pcie_cap_reg_idx: None,
-            msi_config: None,
-            pmc_config: PmcConfig::new(),
-            pmc_cap_reg_idx: None,
-            pci_address: None,
-            slot_control: if slot_implemented {
-                Some(PCIE_SLTCTL_PIC_OFF | PCIE_SLTCTL_AIC_OFF)
-            } else {
-                None
-            },
-            slot_status: 0,
-            root_control: 0,
-            root_status: 0,
-            hp_interrupt_pending: false,
-            pme_pending_request_id: None,
-            bus_range,
+            pcie_port: PciePort::new(
+                PCIE_RP_DID,
+                "PcieRootPort".to_string(),
+                0,
+                secondary_bus_num,
+                slot_implemented,
+            ),
             downstream_devices: BTreeMap::new(),
             hotplug_out_begin: false,
             removed_downstream: Vec::new(),
-            removed_downstream_valid: false,
-            pcie_host: None,
-            prepare_hotplug: false,
         }
     }
 
     /// Constructs a new PCIE root port which associated with the host physical pcie RP
     pub fn new_from_host(pcie_host: PcieHostPort, slot_implemented: bool) -> Result<Self> {
-        let bus_range = pcie_host.get_bus_range();
-        // if physical pcie root port isn't on bus 0, ignore this physical pcie root port.
-        if bus_range.primary != 0 {
-            return Err(anyhow!(
-                "physical pcie RP isn't on bus 0: {}",
-                bus_range.primary
-            ));
-        }
-
         Ok(PcieRootPort {
-            pcie_cap_reg_idx: None,
-            msi_config: None,
-            pmc_config: PmcConfig::new(),
-            pmc_cap_reg_idx: None,
-            pci_address: None,
-            slot_control: if slot_implemented {
-                Some(PCIE_SLTCTL_PIC_OFF | PCIE_SLTCTL_AIC_OFF)
-            } else {
-                None
-            },
-            slot_status: 0,
-            root_control: 0,
-            root_status: 0,
-            hp_interrupt_pending: false,
-            pme_pending_request_id: None,
-            bus_range,
+            pcie_port: PciePort::new_from_host(pcie_host, slot_implemented),
             downstream_devices: BTreeMap::new(),
             hotplug_out_begin: false,
             removed_downstream: Vec::new(),
-            removed_downstream_valid: false,
-            pcie_host: Some(pcie_host),
-            prepare_hotplug: false,
         })
     }
-
-    fn get_slot_control(&self) -> u16 {
-        if let Some(slot_control) = self.slot_control {
-            return slot_control;
-        }
-        0
-    }
-
-    fn read_pcie_cap(&self, offset: usize, data: &mut u32) {
-        if offset == PCIE_SLTCTL_OFFSET {
-            *data = ((self.slot_status as u32) << 16) | (self.get_slot_control() as u32);
-        } else if offset == PCIE_ROOTCTL_OFFSET {
-            *data = self.root_control as u32;
-        } else if offset == PCIE_ROOTSTA_OFFSET {
-            *data = self.root_status;
-        }
-    }
-
-    fn write_pcie_cap(&mut self, offset: usize, data: &[u8]) {
-        self.removed_downstream_valid = false;
-        match offset {
-            PCIE_SLTCTL_OFFSET => {
-                let value = match u16::from_slice(data) {
-                    Some(&v) => v,
-                    None => {
-                        warn!("write SLTCTL isn't word, len: {}", data.len());
-                        return;
-                    }
-                };
-
-                // if slot is populated, power indicator is off,
-                // it will detach devices
-                let old_control = self.get_slot_control();
-                match self.slot_control.as_mut() {
-                    Some(v) => *v = value,
-                    None => return,
-                }
-                if (self.slot_status & PCIE_SLTSTA_PDS != 0)
-                    && (value & PCIE_SLTCTL_PIC_OFF == PCIE_SLTCTL_PIC_OFF)
-                    && (old_control & PCIE_SLTCTL_PIC_OFF != PCIE_SLTCTL_PIC_OFF)
-                {
-                    self.removed_downstream_valid = true;
-                    self.slot_status &= !PCIE_SLTSTA_PDS;
-                    self.slot_status |= PCIE_SLTSTA_PDC;
-                    self.trigger_hp_interrupt();
-                }
-
-                if old_control != value {
-                    // send Command completed events
-                    self.slot_status |= PCIE_SLTSTA_CC;
-                    self.trigger_cc_interrupt();
-                }
-            }
-            PCIE_SLTSTA_OFFSET => {
-                if self.slot_control.is_none() {
-                    return;
-                }
-                let value = match u16::from_slice(data) {
-                    Some(v) => *v,
-                    None => {
-                        warn!("write SLTSTA isn't word, len: {}", data.len());
-                        return;
-                    }
-                };
-                if value & PCIE_SLTSTA_ABP != 0 {
-                    self.slot_status &= !PCIE_SLTSTA_ABP;
-                }
-                if value & PCIE_SLTSTA_PFD != 0 {
-                    self.slot_status &= !PCIE_SLTSTA_PFD;
-                }
-                if value & PCIE_SLTSTA_PDC != 0 {
-                    self.slot_status &= !PCIE_SLTSTA_PDC;
-                }
-                if value & PCIE_SLTSTA_CC != 0 {
-                    self.slot_status &= !PCIE_SLTSTA_CC;
-                }
-                if value & PCIE_SLTSTA_DLLSC != 0 {
-                    self.slot_status &= !PCIE_SLTSTA_DLLSC;
-                }
-            }
-            PCIE_ROOTCTL_OFFSET => match u16::from_slice(data) {
-                Some(v) => self.root_control = *v,
-                None => warn!("write root control isn't word, len: {}", data.len()),
-            },
-            PCIE_ROOTSTA_OFFSET => match u32::from_slice(data) {
-                Some(v) => {
-                    if *v & PCIE_ROOTSTA_PME_STATUS != 0 {
-                        if let Some(request_id) = self.pme_pending_request_id {
-                            self.root_status &= !PCIE_ROOTSTA_PME_PENDING;
-                            let req_id = ((request_id.bus as u32) << 8)
-                                | ((request_id.dev as u32) << 3)
-                                | (request_id.func as u32);
-                            self.root_status &= !PCIE_ROOTSTA_PME_REQ_ID_MASK;
-                            self.root_status |= req_id;
-                            self.root_status |= PCIE_ROOTSTA_PME_STATUS;
-                            self.pme_pending_request_id = None;
-                            self.trigger_pme_interrupt();
-                        } else {
-                            self.root_status &= !PCIE_ROOTSTA_PME_STATUS;
-                            if self.hp_interrupt_pending {
-                                self.hp_interrupt_pending = false;
-                                self.trigger_hp_interrupt();
-                            }
-                        }
-                    }
-                }
-                None => warn!("write root status isn't dword, len: {}", data.len()),
-            },
-            _ => (),
-        }
-    }
-
-    fn trigger_interrupt(&self) {
-        if let Some(msi_config) = &self.msi_config {
-            let msi_config = msi_config.lock();
-            if msi_config.is_msi_enabled() {
-                msi_config.trigger()
-            }
-        }
-    }
-
-    fn trigger_cc_interrupt(&self) {
-        if (self.get_slot_control() & PCIE_SLTCTL_CCIE) != 0
-            && (self.slot_status & PCIE_SLTSTA_CC) != 0
-        {
-            self.trigger_interrupt()
-        }
-    }
-
-    fn trigger_hp_interrupt(&self) {
-        let slot_control = self.get_slot_control();
-        if (slot_control & PCIE_SLTCTL_HPIE) != 0
-            && (self.slot_status & slot_control & (PCIE_SLTCTL_ABPE | PCIE_SLTCTL_PDCE)) != 0
-        {
-            self.trigger_interrupt()
-        }
-    }
-
-    fn trigger_pme_interrupt(&self) {
-        if (self.root_control & PCIE_ROOTCTL_PME_ENABLE) != 0
-            && (self.root_status & PCIE_ROOTSTA_PME_STATUS) != 0
-        {
-            self.trigger_interrupt()
-        }
-    }
-
-    fn inject_pme(&mut self) {
-        if (self.root_status & PCIE_ROOTSTA_PME_STATUS) != 0 {
-            self.root_status |= PCIE_ROOTSTA_PME_PENDING;
-            self.pme_pending_request_id = self.pci_address;
-        } else {
-            let request_id = self.pci_address.unwrap();
-            let req_id = ((request_id.bus as u32) << 8)
-                | ((request_id.dev as u32) << 3)
-                | (request_id.func as u32);
-            self.root_status &= !PCIE_ROOTSTA_PME_REQ_ID_MASK;
-            self.root_status |= req_id;
-            self.pme_pending_request_id = None;
-            self.root_status |= PCIE_ROOTSTA_PME_STATUS;
-            self.trigger_pme_interrupt();
-        }
-    }
-
-    // when RP is D3, HP interrupt is disabled by pcie driver, so inject a PME to wakeup
-    // RP first, then inject HP interrupt.
-    fn trigger_hp_or_pme_interrupt(&mut self) {
-        if self.pmc_config.should_trigger_pme() {
-            self.hp_interrupt_pending = true;
-            self.inject_pme();
-        } else {
-            self.trigger_hp_interrupt();
-        }
-    }
 }
 
 impl PcieDevice for PcieRootPort {
     fn get_device_id(&self) -> u16 {
-        match &self.pcie_host {
-            Some(host) => host.read_device_id(),
-            None => PCIE_RP_DID,
-        }
+        self.pcie_port.get_device_id()
     }
 
     fn debug_label(&self) -> String {
-        match &self.pcie_host {
-            Some(host) => host.host_name(),
-            None => "PcieRootPort".to_string(),
-        }
+        self.pcie_port.debug_label()
     }
 
     fn allocate_address(
         &mut self,
         resources: &mut SystemAllocator,
     ) -> std::result::Result<PciAddress, PciDeviceError> {
-        if self.pci_address.is_none() {
-            match &self.pcie_host {
-                Some(host) => {
-                    let address = PciAddress::from_str(&host.host_name())
-                        .map_err(|e| PciDeviceError::PciAddressParseFailure(host.host_name(), e))?;
-                    if resources.reserve_pci(
-                        Alloc::PciBar {
-                            bus: address.bus,
-                            dev: address.dev,
-                            func: address.func,
-                            bar: 0,
-                        },
-                        host.host_name(),
-                    ) {
-                        self.pci_address = Some(address);
-                    } else {
-                        self.pci_address = None;
-                    }
-                }
-                None => match resources.allocate_pci(self.bus_range.primary, self.debug_label()) {
-                    Some(Alloc::PciBar {
-                        bus,
-                        dev,
-                        func,
-                        bar: _,
-                    }) => self.pci_address = Some(PciAddress { bus, dev, func }),
-                    _ => self.pci_address = None,
-                },
-            }
-        }
-        self.pci_address.ok_or(PciDeviceError::PciAllocationFailed)
+        self.pcie_port.allocate_address(resources)
     }
 
     fn clone_interrupt(&mut self, msi_config: Arc<Mutex<MsiConfig>>) {
-        self.msi_config = Some(msi_config);
+        self.pcie_port.clone_interrupt(msi_config);
     }
 
     fn get_caps(&self) -> Vec<Box<dyn PciCapability>> {
         vec![
             Box::new(PcieCap::new(
                 PcieDevicePortType::RootPort,
-                self.slot_control.is_some(),
+                self.pcie_port.hotplug_implemented(),
                 0,
             )),
             Box::new(PciPmcCap::new()),
@@ -362,66 +87,23 @@ impl PcieDevice for PcieRootPort {
     }
 
     fn set_capability_reg_idx(&mut self, id: PciCapabilityID, reg_idx: usize) {
-        match id {
-            PciCapabilityID::PciExpress => self.pcie_cap_reg_idx = Some(reg_idx),
-            PciCapabilityID::PowerManagement => self.pmc_cap_reg_idx = Some(reg_idx),
-            _ => (),
-        }
+        self.pcie_port.set_capability_reg_idx(id, reg_idx);
     }
 
     fn read_config(&self, reg_idx: usize, data: &mut u32) {
-        if let Some(pcie_cap_reg_idx) = self.pcie_cap_reg_idx {
-            if reg_idx >= pcie_cap_reg_idx && reg_idx < pcie_cap_reg_idx + (PCIE_CAP_LEN / 4) {
-                let offset = (reg_idx - pcie_cap_reg_idx) * 4;
-                self.read_pcie_cap(offset, data);
-            }
-        }
-        if let Some(pmc_cap_reg_idx) = self.pmc_cap_reg_idx {
-            if reg_idx == pmc_cap_reg_idx + PMC_CAP_CONTROL_STATE_OFFSET {
-                self.pmc_config.read(data);
-            }
-        }
-        if let Some(host) = &self.pcie_host {
-            // pcie host may override some config registers
-            host.read_config(reg_idx, data);
-        }
+        self.pcie_port.read_config(reg_idx, data);
     }
 
     fn write_config(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
-        if let Some(pcie_cap_reg_idx) = self.pcie_cap_reg_idx {
-            if reg_idx >= pcie_cap_reg_idx && reg_idx < pcie_cap_reg_idx + (PCIE_CAP_LEN / 4) {
-                let delta = ((reg_idx - pcie_cap_reg_idx) * 4) + offset as usize;
-                self.write_pcie_cap(delta, data);
-            }
-        }
-        if let Some(pmc_cap_reg_idx) = self.pmc_cap_reg_idx {
-            if reg_idx == pmc_cap_reg_idx + PMC_CAP_CONTROL_STATE_OFFSET {
-                let old_status = self.pmc_config.get_power_status();
-                self.pmc_config.write(offset, data);
-                let new_status = self.pmc_config.get_power_status();
-                if old_status == PciDevicePower::D3
-                    && new_status == PciDevicePower::D0
-                    && self.prepare_hotplug
-                {
-                    if let Some(host) = self.pcie_host.as_mut() {
-                        host.hotplug_probe();
-                        self.prepare_hotplug = false;
-                    }
-                }
-            }
-        }
-        if let Some(host) = self.pcie_host.as_mut() {
-            // device may write data to host, or do something at specific register write
-            host.write_config(reg_idx, offset, data);
-        }
+        self.pcie_port.write_config(reg_idx, offset, data);
     }
 
     fn get_bus_range(&self) -> Option<PciBridgeBusRange> {
-        Some(self.bus_range)
+        self.pcie_port.get_bus_range()
     }
 
     fn get_removed_devices(&self) -> Vec<PciAddress> {
-        if self.removed_downstream_valid {
+        if self.pcie_port.removed_downstream_valid() {
             self.removed_downstream.clone()
         } else {
             Vec::new()
@@ -429,15 +111,15 @@ impl PcieDevice for PcieRootPort {
     }
 
     fn hotplug_implemented(&self) -> bool {
-        self.slot_control.is_some()
+        self.pcie_port.hotplug_implemented()
+    }
+
+    fn hotplugged(&self) -> bool {
+        false
     }
 
     fn get_bridge_window_size(&self) -> (u64, u64) {
-        if let Some(host) = &self.pcie_host {
-            host.get_bridge_window_size()
-        } else {
-            (PCIE_RP_BR_MEM_SIZE, PCIE_RP_BR_PREF_MEM_SIZE)
-        }
+        self.pcie_port.get_bridge_window_size()
     }
 }
 
@@ -447,8 +129,9 @@ impl HotPlugBus for PcieRootPort {
             return;
         }
 
-        self.slot_status = self.slot_status | PCIE_SLTSTA_PDS | PCIE_SLTSTA_PDC | PCIE_SLTSTA_ABP;
-        self.trigger_hp_or_pme_interrupt();
+        self.pcie_port
+            .set_slot_status(PCIE_SLTSTA_PDS | PCIE_SLTSTA_PDC | PCIE_SLTSTA_ABP);
+        self.pcie_port.trigger_hp_or_pme_interrupt();
     }
 
     fn hot_unplug(&mut self, addr: PciAddress) {
@@ -464,11 +147,12 @@ impl HotPlugBus for PcieRootPort {
                 self.removed_downstream.push(*guest_pci_addr);
             }
 
-            self.slot_status = self.slot_status | PCIE_SLTSTA_PDC | PCIE_SLTSTA_ABP;
-            self.trigger_hp_or_pme_interrupt();
+            self.pcie_port
+                .set_slot_status(PCIE_SLTSTA_PDC | PCIE_SLTSTA_ABP);
+            self.pcie_port.trigger_hp_or_pme_interrupt();
 
-            if let Some(host) = self.pcie_host.as_mut() {
-                host.hot_unplug();
+            if self.pcie_port.is_host() {
+                self.pcie_port.hot_unplug()
             }
         }
 
@@ -476,20 +160,11 @@ impl HotPlugBus for PcieRootPort {
     }
 
     fn is_match(&self, host_addr: PciAddress) -> Option<u8> {
-        let _ = self.slot_control?;
-
-        if (host_addr.bus >= self.bus_range.secondary
-            && host_addr.bus <= self.bus_range.subordinate)
-            || self.pcie_host.is_none()
-        {
-            Some(self.bus_range.secondary)
-        } else {
-            None
-        }
+        self.pcie_port.is_match(host_addr)
     }
 
     fn add_hotplug_device(&mut self, host_key: HostHotPlugKey, guest_addr: PciAddress) {
-        if self.slot_control.is_none() {
+        if !self.pcie_port.hotplug_implemented() {
             return;
         }
 
@@ -525,16 +200,16 @@ impl HotPlugBus for PcieRootPort {
 
 impl GpeNotify for PcieRootPort {
     fn notify(&mut self) {
-        if self.slot_control.is_none() {
+        if !self.pcie_port.hotplug_implemented() {
             return;
         }
 
-        if self.pcie_host.is_some() {
-            self.prepare_hotplug = true;
+        if self.pcie_port.is_host() {
+            self.pcie_port.prepare_hotplug();
         }
 
-        if self.pmc_config.should_trigger_pme() {
-            self.inject_pme();
+        if self.pcie_port.should_trigger_pme() {
+            self.pcie_port.inject_pme();
         }
     }
 }

devices/src/pci/pcie/pcie_switch.rs

@@ -0,0 +1,195 @@
+// Copyright 2022 The ChromiumOS Authors.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use std::sync::Arc;
+use sync::Mutex;
+
+use crate::pci::pci_configuration::PciCapabilityID;
+use crate::pci::{MsiConfig, PciAddress, PciCapability, PciDeviceError};
+
+use crate::pci::pcie::pci_bridge::PciBridgeBusRange;
+use crate::pci::pcie::pcie_device::{PciPmcCap, PcieCap, PcieDevice};
+use crate::pci::pcie::pcie_port::PciePort;
+use crate::pci::pcie::*;
+
+use resources::SystemAllocator;
+
+const PCIE_UP_DID: u16 = 0x3500;
+const PCIE_DP_DID: u16 = 0x3510;
+
+pub struct PcieUpstreamPort {
+    pcie_port: PciePort,
+    hotplugged: bool,
+}
+
+impl PcieUpstreamPort {
+    /// Constructs a new PCIE upstream port
+    pub fn new(primary_bus_num: u8, secondary_bus_num: u8, hotplugged: bool) -> Self {
+        PcieUpstreamPort {
+            pcie_port: PciePort::new(
+                PCIE_UP_DID,
+                "PcieUpstreamPort".to_string(),
+                primary_bus_num,
+                secondary_bus_num,
+                false,
+            ),
+            hotplugged,
+        }
+    }
+
+    pub fn new_from_host(pcie_host: PcieHostPort, hotplugged: bool) -> Self {
+        PcieUpstreamPort {
+            pcie_port: PciePort::new_from_host(pcie_host, false),
+            hotplugged,
+        }
+    }
+}
+
+impl PcieDevice for PcieUpstreamPort {
+    fn get_device_id(&self) -> u16 {
+        self.pcie_port.get_device_id()
+    }
+
+    fn debug_label(&self) -> String {
+        self.pcie_port.debug_label()
+    }
+
+    fn allocate_address(
+        &mut self,
+        resources: &mut SystemAllocator,
+    ) -> std::result::Result<PciAddress, PciDeviceError> {
+        self.pcie_port.allocate_address(resources)
+    }
+
+    fn clone_interrupt(&mut self, msi_config: Arc<Mutex<MsiConfig>>) {
+        self.pcie_port.clone_interrupt(msi_config);
+    }
+
+    fn read_config(&self, reg_idx: usize, data: &mut u32) {
+        self.pcie_port.read_config(reg_idx, data);
+    }
+
+    fn write_config(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        self.pcie_port.write_config(reg_idx, offset, data);
+    }
+
+    fn get_caps(&self) -> Vec<Box<dyn PciCapability>> {
+        vec![
+            Box::new(PcieCap::new(PcieDevicePortType::UpstreamPort, false, 0)),
+            Box::new(PciPmcCap::new()),
+        ]
+    }
+
+    fn set_capability_reg_idx(&mut self, id: PciCapabilityID, reg_idx: usize) {
+        self.pcie_port.set_capability_reg_idx(id, reg_idx);
+    }
+
+    fn get_bus_range(&self) -> Option<PciBridgeBusRange> {
+        self.pcie_port.get_bus_range()
+    }
+
+    fn get_removed_devices(&self) -> Vec<PciAddress> {
+        Vec::new()
+    }
+
+    fn hotplug_implemented(&self) -> bool {
+        false
+    }
+
+    fn hotplugged(&self) -> bool {
+        self.hotplugged
+    }
+
+    fn get_bridge_window_size(&self) -> (u64, u64) {
+        self.pcie_port.get_bridge_window_size()
+    }
+}
+
+pub struct PcieDownstreamPort {
+    pcie_port: PciePort,
+    hotplugged: bool,
+}
+
+impl PcieDownstreamPort {
+    /// Constructs a new PCIE downstream port
+    pub fn new(primary_bus_num: u8, secondary_bus_num: u8, hotplugged: bool) -> Self {
+        PcieDownstreamPort {
+            pcie_port: PciePort::new(
+                PCIE_DP_DID,
+                "PcieDownstreamPort".to_string(),
+                primary_bus_num,
+                secondary_bus_num,
+                false,
+            ),
+            hotplugged,
+        }
+    }
+
+    pub fn new_from_host(pcie_host: PcieHostPort, hotplugged: bool) -> Self {
+        PcieDownstreamPort {
+            pcie_port: PciePort::new_from_host(pcie_host, false),
+            hotplugged,
+        }
+    }
+}
+
+impl PcieDevice for PcieDownstreamPort {
+    fn get_device_id(&self) -> u16 {
+        self.pcie_port.get_device_id()
+    }
+
+    fn debug_label(&self) -> String {
+        self.pcie_port.debug_label()
+    }
+
+    fn allocate_address(
+        &mut self,
+        resources: &mut SystemAllocator,
+    ) -> std::result::Result<PciAddress, PciDeviceError> {
+        self.pcie_port.allocate_address(resources)
+    }
+
+    fn clone_interrupt(&mut self, msi_config: Arc<Mutex<MsiConfig>>) {
+        self.pcie_port.clone_interrupt(msi_config);
+    }
+
+    fn read_config(&self, reg_idx: usize, data: &mut u32) {
+        self.pcie_port.read_config(reg_idx, data);
+    }
+
+    fn write_config(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        self.pcie_port.write_config(reg_idx, offset, data);
+    }
+
+    fn get_caps(&self) -> Vec<Box<dyn PciCapability>> {
+        vec![
+            Box::new(PcieCap::new(PcieDevicePortType::DownstreamPort, false, 0)),
+            Box::new(PciPmcCap::new()),
+        ]
+    }
+
+    fn set_capability_reg_idx(&mut self, id: PciCapabilityID, reg_idx: usize) {
+        self.pcie_port.set_capability_reg_idx(id, reg_idx);
+    }
+
+    fn get_bus_range(&self) -> Option<PciBridgeBusRange> {
+        self.pcie_port.get_bus_range()
+    }
+
+    fn get_removed_devices(&self) -> Vec<PciAddress> {
+        Vec::new()
+    }
+
+    fn hotplug_implemented(&self) -> bool {
+        false
+    }
+
+    fn hotplugged(&self) -> bool {
+        self.hotplugged
+    }
+
+    fn get_bridge_window_size(&self) -> (u64, u64) {
+        self.pcie_port.get_bridge_window_size()
+    }
+}

devices/src/pci/vfio_pci.rs

@@ -17,8 +17,8 @@ use std::u32;
 use sync::Mutex;
 
 use base::{
-    error, pagesize, warn, AsRawDescriptor, AsRawDescriptors, Event, EventToken, RawDescriptor,
-    Tube, WaitContext,
+    error, pagesize, warn, AsRawDescriptor, AsRawDescriptors, Event, EventToken, Protection,
+    RawDescriptor, Tube, WaitContext,
 };
 use hypervisor::{Datamatch, MemSlot};
 
@@ -915,7 +915,7 @@ impl VfioPciDevice {
                             size: mmap_size,
                         },
                         dest: VmMemoryDestination::GuestPhysicalAddress(guest_map_start),
-                        read_only: false,
+                        prot: Protection::read_write(),
                     })
                     .is_err()
                 {

devices/src/platform/vfio_platform.rs

@@ -7,7 +7,7 @@ use crate::{BusAccessInfo, BusDevice, BusDeviceObj, DeviceId, IrqEdgeEvent, IrqL
 use anyhow::{bail, Context, Result};
 use base::{
     error, pagesize, AsRawDescriptor, AsRawDescriptors, Event, MappedRegion, MemoryMapping,
-    MemoryMappingBuilder, RawDescriptor, Tube,
+    MemoryMappingBuilder, Protection, RawDescriptor, Tube,
 };
 use resources::SystemAllocator;
 use std::fs::File;
@@ -185,7 +185,7 @@ impl VfioPlatformDevice {
                             size: mmap_size,
                         },
                         dest: VmMemoryDestination::GuestPhysicalAddress(guest_map_start),
-                        read_only: false,
+                        prot: Protection::read_write(),
                     })
                     .is_err()
                 {

devices/src/serial.rs

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+pub(crate) mod sys;
+
 use std::collections::VecDeque;
 use std::io::{self, Write};
 use std::sync::atomic::{AtomicU8, Ordering};

devices/src/serial/sys.rs

@@ -0,0 +1,15 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cfg_if::cfg_if! {
+    if #[cfg(unix)] {
+        mod unix;
+        use unix as platform;
+    } else if #[cfg(windows)] {
+        mod windows;
+        use windows as platform;
+    }
+}
+
+pub(crate) use platform::InStreamType;

devices/src/serial/sys/unix.rs

@@ -0,0 +1,9 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use crate::serial_device::SerialInput;
+
+// TODO(b/234469655): Remove type alias once ReadNotifier is implemented for
+// PipeConnection.
+pub(crate) type InStreamType = Box<dyn SerialInput>;

devices/src/serial/sys/windows.rs

@@ -0,0 +1,9 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use base::named_pipes::PipeConnection;
+
+// TODO(b/234469655): Remove type alias once ReadNotifier is implemented for
+// PipeConnection.
+pub(crate) type InStreamType = Box<PipeConnection>;

devices/src/serial_device.rs

@@ -9,7 +9,9 @@ use std::path::PathBuf;
 
 #[cfg(windows)]
 use base::platform::Console as WinConsole;
-use base::{error, open_file, syslog, AsRawDescriptor, Event, FileSync, RawDescriptor};
+use base::{
+    error, open_file, syslog, AsRawDescriptor, Event, FileSync, RawDescriptor, ReadNotifier,
+};
 use hypervisor::ProtectionType;
 use remain::sorted;
 use serde::{Deserialize, Serialize};
@@ -42,7 +44,7 @@ pub enum Error {
 }
 
 /// Trait for types that can be used as input for a serial device.
-pub trait SerialInput: io::Read + AsRawDescriptor + Send {}
+pub trait SerialInput: io::Read + ReadNotifier + Send {}
 impl SerialInput for File {}
 #[cfg(windows)]
 impl SerialInput for WinConsole {}
@@ -159,7 +161,7 @@ impl SerialParameters {
             Some(Box::new(input_file))
         } else if self.stdin {
             keep_rds.push(stdin().as_raw_descriptor());
-            Some(Box::new(ConsoleInput))
+            Some(Box::new(ConsoleInput::new()))
         } else {
             None
         };

devices/src/sys/unix/serial_device.rs

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-use base::{error, AsRawDescriptor, Event, FileSync, RawDescriptor};
+use base::{error, AsRawDescriptor, Event, FileSync, RawDescriptor, ReadNotifier};
 use base::{info, read_raw_stdin};
 use hypervisor::ProtectionType;
 use std::borrow::Cow;
@@ -20,16 +20,23 @@ pub const SYSTEM_SERIAL_TYPE_NAME: &str = "UnixSocket";
 
 // This wrapper is used in place of the libstd native version because we don't want
 // buffering for stdin.
-pub struct ConsoleInput;
+pub struct ConsoleInput(std::io::Stdin);
+
+impl ConsoleInput {
+    pub fn new() -> Self {
+        Self(std::io::stdin())
+    }
+}
+
 impl io::Read for ConsoleInput {
     fn read(&mut self, out: &mut [u8]) -> io::Result<usize> {
         read_raw_stdin(out).map_err(|e| e.into())
     }
 }
 
-impl AsRawDescriptor for ConsoleInput {
-    fn as_raw_descriptor(&self) -> RawDescriptor {
-        std::io::stdin().as_raw_descriptor()
+impl ReadNotifier for ConsoleInput {
+    fn get_read_notifier(&self) -> &dyn AsRawDescriptor {
+        &self.0
     }
 }
 

devices/src/tsc/cpuid.rs

@@ -28,21 +28,17 @@ pub fn tsc_frequency_cpuid(cpuid_count: CpuidCountFn) -> Option<hypervisor::CpuI
         function: 0x15,
         index: 0,
         flags: 0,
-        eax: 0,
-        ebx: 0,
-        ecx: 0,
-        edx: 0,
+        cpuid: CpuidResult {
+            eax: 0,
+            ebx: 0,
+            ecx: 0,
+            edx: 0,
+        },
     };
     // Safe because we pass 0 and 0 for this call and the host supports the `cpuid` instruction.
-    unsafe {
-        let result = cpuid_count(tsc_freq.function, tsc_freq.index);
-        tsc_freq.eax = result.eax;
-        tsc_freq.ebx = result.ebx;
-        tsc_freq.ecx = result.ecx;
-        tsc_freq.edx = result.edx;
-    }
+    tsc_freq.cpuid = unsafe { cpuid_count(tsc_freq.function, tsc_freq.index) };
 
-    if tsc_freq.ecx != 0 {
+    if tsc_freq.cpuid.ecx != 0 {
         Some(tsc_freq)
     } else {
         // The core crystal frequency is missing. Old kernels (<5.3) don't try to derive it from the
@@ -63,8 +59,8 @@ pub fn tsc_frequency_cpuid(cpuid_count: CpuidCountFn) -> Option<hypervisor::CpuI
             // base_mhz = cpu_clock.eax
             // tsc_to_base_ratio = tsc_freq.eax / tsc_freq.ebx
             // crystal_hz = base_mhz * tsc_base_to_clock_ratio * 10^6
-            tsc_freq.ecx = (cpu_clock.eax as f64 * tsc_freq.eax as f64 * 1_000_000_f64
-                / tsc_freq.ebx as f64)
+            tsc_freq.cpuid.ecx = (cpu_clock.eax as f64 * tsc_freq.cpuid.eax as f64 * 1_000_000_f64
+                / tsc_freq.cpuid.ebx as f64)
                 .round() as u32;
             Some(tsc_freq)
         } else {
@@ -86,24 +82,27 @@ pub fn fake_tsc_frequency_cpuid(tsc_hz: u64, bus_hz: u32) -> hypervisor::CpuIdEn
         function: 0x15,
         index: 0,
         flags: 0,
-        eax: crystal_clock_ratio_denominator,
-        ebx: crystal_clock_ratio_numerator,
-        ecx: bus_hz,
-        edx: 0,
+        cpuid: CpuidResult {
+            eax: crystal_clock_ratio_denominator,
+            ebx: crystal_clock_ratio_numerator,
+            ecx: bus_hz,
+            edx: 0,
+        },
     }
 }
 
 /// Returns the Bus frequency in Hz, based on reading Intel-specific cpuids, or None
 /// if the frequency can't be determined from cpuids.
 pub fn bus_freq_hz(cpuid_count: CpuidCountFn) -> Option<u32> {
-    tsc_frequency_cpuid(cpuid_count).map(|cpuid| cpuid.ecx)
+    tsc_frequency_cpuid(cpuid_count).map(|cpuid| cpuid.cpuid.ecx)
 }
 
 /// Returns the TSC frequency in Hz, based on reading Intel-specific cpuids, or None
 /// if the frequency can't be determined from cpuids.
 pub fn tsc_freq_hz(cpuid_count: CpuidCountFn) -> Option<u32> {
-    tsc_frequency_cpuid(cpuid_count)
-        .map(|cpuid| (cpuid.ecx as u64 * cpuid.ebx as u64 / cpuid.eax as u64) as u32)
+    tsc_frequency_cpuid(cpuid_count).map(|cpuid| {
+        (cpuid.cpuid.ecx as u64 * cpuid.cpuid.ebx as u64 / cpuid.cpuid.eax as u64) as u32
+    })
 }
 
 #[cfg(test)]

devices/src/virtio/console.rs

@@ -2,6 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+//! Legacy console device that uses a polling thread. This is kept because it is still used by
+//! Windows ; outside of this use-case, please use [[asynchronous::AsyncConsole]] instead.
+
+pub mod asynchronous;
+mod sys;
+
 use std::collections::VecDeque;
 use std::io::{self, Read, Write};
 use std::ops::DerefMut;
@@ -9,7 +15,7 @@ use std::result;
 use std::sync::Arc;
 use std::thread;
 
-use base::{error, Event, EventToken, FileSync, RawDescriptor, WaitContext};
+use base::{error, AsRawDescriptor, Descriptor, Event, EventToken, RawDescriptor, WaitContext};
 use data_model::{DataInit, Le16, Le32};
 use hypervisor::ProtectionType;
 use remain::sorted;
@@ -17,12 +23,10 @@ use sync::Mutex;
 use thiserror::Error as ThisError;
 use vm_memory::GuestMemory;
 
-use super::{
+use crate::virtio::{
     base_features, copy_config, DeviceType, Interrupt, Queue, Reader, SignalableInterrupt,
     VirtioDevice, Writer,
 };
-use crate::serial_device::SerialInput;
-use crate::SerialDevice;
 
 pub(crate) const QUEUE_SIZE: u16 = 256;
 
@@ -58,7 +62,7 @@ unsafe impl DataInit for virtio_console_config {}
 /// * `interrupt` - SignalableInterrupt used to signal that the queue has been used
 /// * `buffer` - Ring buffer providing data to put into the guest
 /// * `receive_queue` - The receive virtio Queue
-pub fn handle_input<I: SignalableInterrupt>(
+fn handle_input<I: SignalableInterrupt>(
     mem: &GuestMemory,
     interrupt: &I,
     buffer: &mut VecDeque<u8>,
@@ -109,7 +113,7 @@ pub fn handle_input<I: SignalableInterrupt>(
 ///
 /// * `reader` - The Reader with the data we want to write.
 /// * `output` - The output sink we are going to write the data to.
-pub fn process_transmit_request(mut reader: Reader, output: &mut dyn io::Write) -> io::Result<()> {
+fn process_transmit_request(mut reader: Reader, output: &mut dyn io::Write) -> io::Result<()> {
     let len = reader.available_bytes();
     let mut data = vec![0u8; len];
     reader.read_exact(&mut data)?;
@@ -126,7 +130,7 @@ pub fn process_transmit_request(mut reader: Reader, output: &mut dyn io::Write)
 /// * `interrupt` - SignalableInterrupt used to signal (if required) that the queue has been used
 /// * `transmit_queue` - The transmit virtio Queue
 /// * `output` - The output sink we are going to write the data into
-pub fn process_transmit_queue<I: SignalableInterrupt>(
+fn process_transmit_queue<I: SignalableInterrupt>(
     mem: &GuestMemory,
     interrupt: &I,
     transmit_queue: &mut Queue,
@@ -176,8 +180,8 @@ struct Worker {
 ///
 /// * `rx` - Data source that the reader thread will wait on to send data back to the buffer
 /// * `in_avail_evt` - Event triggered by the thread when new input is available on the buffer
-pub fn spawn_input_thread(
-    mut rx: Box<dyn SerialInput>,
+fn spawn_input_thread(
+    mut rx: crate::serial::sys::InStreamType,
     in_avail_evt: &Event,
 ) -> Option<Arc<Mutex<VecDeque<u8>>>> {
     let buffer = Arc::new(Mutex::new(VecDeque::<u8>::new()));
@@ -205,7 +209,7 @@ pub fn spawn_input_thread(
                     }
                     Err(e) => {
                         // Being interrupted is not an error, but everything else is.
-                        if e.kind() != io::ErrorKind::Interrupted {
+                        if sys::is_a_fatal_input_error(&e) {
                             error!(
                                 "failed to read for bytes to queue into console device: {}",
                                 e
@@ -214,6 +218,9 @@ pub fn spawn_input_thread(
                         }
                     }
                 }
+
+                // Depending on the platform, a short sleep is needed here (ie. Windows).
+                sys::read_delay_if_needed();
             }
         });
     if let Err(e) = res {
@@ -330,7 +337,7 @@ impl Worker {
 }
 
 enum ConsoleInput {
-    FromRead(Box<dyn SerialInput>),
+    FromRead(crate::serial::sys::InStreamType),
     FromThread(Arc<Mutex<VecDeque<u8>>>),
 }
 
@@ -342,17 +349,14 @@ pub struct Console {
     worker_thread: Option<thread::JoinHandle<Worker>>,
     input: Option<ConsoleInput>,
     output: Option<Box<dyn io::Write + Send>>,
-    keep_rds: Vec<RawDescriptor>,
+    keep_descriptors: Vec<Descriptor>,
 }
 
-impl SerialDevice for Console {
+impl Console {
     fn new(
         protected_vm: ProtectionType,
-        _evt: Event,
-        input: Option<Box<dyn SerialInput>>,
+        input: Option<ConsoleInput>,
         output: Option<Box<dyn io::Write + Send>>,
-        _sync: Option<Box<dyn FileSync + Send>>,
-        _out_timestamp: bool,
         keep_rds: Vec<RawDescriptor>,
     ) -> Console {
         Console {
@@ -360,9 +364,9 @@ impl SerialDevice for Console {
             in_avail_evt: None,
             kill_evt: None,
             worker_thread: None,
-            input: input.map(ConsoleInput::FromRead),
+            input,
             output,
-            keep_rds,
+            keep_descriptors: keep_rds.iter().map(|rd| Descriptor(*rd)).collect(),
         }
     }
 }
@@ -382,7 +386,11 @@ impl Drop for Console {
 
 impl VirtioDevice for Console {
     fn keep_rds(&self) -> Vec<RawDescriptor> {
-        self.keep_rds.clone()
+        // return the raw descriptors as opposed to descriptor.
+        self.keep_descriptors
+            .iter()
+            .map(|descr| descr.as_raw_descriptor())
+            .collect()
     }
 
     fn features(&self) -> u64 {

devices/src/virtio/console/asynchronous.rs

@@ -0,0 +1,417 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+//! Asynchronous console device which implementation can be shared by VMM and vhost-user.
+
+use std::{collections::VecDeque, io, sync::Arc, thread};
+
+use anyhow::Context;
+use base::{error, warn, AsRawDescriptor, Event, FileSync, RawDescriptor};
+use cros_async::{select2, AsyncResult, EventAsync, Executor, IntoAsync, IoSourceExt};
+use data_model::DataInit;
+use futures::FutureExt;
+use hypervisor::ProtectionType;
+use sync::Mutex;
+use vm_memory::GuestMemory;
+use vmm_vhost::message::VhostUserVirtioFeatures;
+
+use crate::{
+    serial_device::SerialInput,
+    virtio::{
+        self, async_device::AsyncQueueState, async_utils, base_features, copy_config,
+        virtio_console_config, ConsoleError, DeviceType, Interrupt, Queue, SignalableInterrupt,
+        VirtioDevice,
+    },
+    SerialDevice,
+};
+
+use super::{handle_input, process_transmit_queue, QUEUE_SIZES};
+
+/// Wrapper that makes any `SerialInput` usable as an async source by providing an implementation of
+/// `IntoAsync`.
+struct AsyncSerialInput(Box<dyn SerialInput>);
+impl AsRawDescriptor for AsyncSerialInput {
+    fn as_raw_descriptor(&self) -> RawDescriptor {
+        self.0.get_read_notifier().as_raw_descriptor()
+    }
+}
+impl IntoAsync for AsyncSerialInput {}
+
+async fn run_tx_queue<I: SignalableInterrupt>(
+    mut queue: virtio::Queue,
+    mem: GuestMemory,
+    doorbell: I,
+    kick_evt: EventAsync,
+    output: &mut Box<dyn io::Write + Send>,
+) {
+    loop {
+        if let Err(e) = kick_evt.next_val().await {
+            error!("Failed to read kick event for tx queue: {}", e);
+            break;
+        }
+        process_transmit_queue(&mem, &doorbell, &mut queue, output.as_mut());
+    }
+}
+
+async fn run_rx_queue<I: SignalableInterrupt>(
+    mut queue: virtio::Queue,
+    mem: GuestMemory,
+    doorbell: I,
+    kick_evt: EventAsync,
+    input: &dyn IoSourceExt<AsyncSerialInput>,
+) {
+    // Staging buffer, required because of `handle_input`'s API. We can probably remove this once
+    // the regular virtio device is switched to async.
+    let mut in_buffer = VecDeque::<u8>::new();
+    let mut rx_buf = vec![0u8; 4096];
+    let mut input_offset = 0u64;
+
+    loop {
+        match input.read_to_vec(Some(input_offset), rx_buf).await {
+            // Input source has closed.
+            Ok((0, _)) => break,
+            Ok((size, v)) => {
+                in_buffer.extend(&v[0..size]);
+                input_offset += size as u64;
+                rx_buf = v;
+            }
+            Err(e) => {
+                error!("Failed to read console input: {}", e);
+                return;
+            }
+        }
+
+        // Submit all the data obtained during this read.
+        while !in_buffer.is_empty() {
+            match handle_input(&mem, &doorbell, &mut in_buffer, &mut queue) {
+                Ok(()) => {}
+                Err(ConsoleError::RxDescriptorsExhausted) => {
+                    // Wait until a descriptor becomes available and try again.
+                    if let Err(e) = kick_evt.next_val().await {
+                        error!("Failed to read kick event for rx queue: {}", e);
+                        return;
+                    }
+                }
+            }
+        }
+    }
+}
+
+pub struct ConsoleDevice {
+    input: Option<AsyncQueueState<AsyncSerialInput>>,
+    output: Option<AsyncQueueState<Box<dyn io::Write + Send>>>,
+    avail_features: u64,
+}
+
+impl ConsoleDevice {
+    pub fn avail_features(&self) -> u64 {
+        self.avail_features
+    }
+
+    pub fn start_receive_queue<I: SignalableInterrupt + 'static>(
+        &mut self,
+        ex: &Executor,
+        mem: GuestMemory,
+        queue: virtio::Queue,
+        doorbell: I,
+        kick_evt: Event,
+    ) -> anyhow::Result<()> {
+        let input_queue = match self.input.as_mut() {
+            Some(input_queue) => input_queue,
+            None => return Ok(()),
+        };
+
+        let kick_evt =
+            EventAsync::new(kick_evt, ex).context("Failed to create EventAsync for kick_evt")?;
+
+        let closure_ex = ex.clone();
+        let rx_future = move |input, abort| {
+            let async_input = closure_ex
+                .async_from(input)
+                .context("failed to create async input")?;
+
+            Ok(async move {
+                select2(
+                    run_rx_queue(queue, mem, doorbell, kick_evt, async_input.as_ref())
+                        .boxed_local(),
+                    abort,
+                )
+                .await;
+
+                async_input.into_source()
+            })
+        };
+
+        input_queue.start(ex, rx_future)
+    }
+
+    pub fn stop_receive_queue(&mut self) -> AsyncResult<bool> {
+        if let Some(queue) = self.input.as_mut() {
+            queue.stop()
+        } else {
+            Ok(false)
+        }
+    }
+
+    pub fn start_transmit_queue<I: SignalableInterrupt + 'static>(
+        &mut self,
+        ex: &Executor,
+        mem: GuestMemory,
+        queue: virtio::Queue,
+        doorbell: I,
+        kick_evt: Event,
+    ) -> anyhow::Result<()> {
+        let output_queue = match self.output.as_mut() {
+            Some(output_queue) => output_queue,
+            None => return Ok(()),
+        };
+
+        let kick_evt =
+            EventAsync::new(kick_evt, ex).context("Failed to create EventAsync for kick_evt")?;
+
+        let tx_future = |mut output, abort| {
+            Ok(async move {
+                select2(
+                    run_tx_queue(queue, mem, doorbell, kick_evt, &mut output).boxed_local(),
+                    abort,
+                )
+                .await;
+
+                output
+            })
+        };
+
+        output_queue.start(ex, tx_future)
+    }
+
+    pub fn stop_transmit_queue(&mut self) -> AsyncResult<bool> {
+        if let Some(queue) = self.output.as_mut() {
+            queue.stop()?;
+            Ok(true)
+        } else {
+            Ok(false)
+        }
+    }
+}
+
+impl SerialDevice for ConsoleDevice {
+    fn new(
+        protected_vm: ProtectionType,
+        _evt: Event,
+        input: Option<Box<dyn SerialInput>>,
+        output: Option<Box<dyn io::Write + Send>>,
+        _sync: Option<Box<dyn FileSync + Send>>,
+        _out_timestamp: bool,
+        _keep_rds: Vec<RawDescriptor>,
+    ) -> ConsoleDevice {
+        let avail_features =
+            virtio::base_features(protected_vm) | VhostUserVirtioFeatures::PROTOCOL_FEATURES.bits();
+        ConsoleDevice {
+            input: input.map(AsyncSerialInput).map(AsyncQueueState::Stopped),
+            output: output.map(AsyncQueueState::Stopped),
+            avail_features,
+        }
+    }
+}
+
+enum VirtioConsoleState {
+    Stopped(ConsoleDevice),
+    Running {
+        kill_evt: Event,
+        worker_thread: thread::JoinHandle<anyhow::Result<ConsoleDevice>>,
+    },
+    Broken,
+}
+
+/// Virtio console device.
+pub struct AsyncConsole {
+    state: VirtioConsoleState,
+    base_features: u64,
+    keep_rds: Vec<RawDescriptor>,
+}
+
+impl SerialDevice for AsyncConsole {
+    fn new(
+        protected_vm: ProtectionType,
+        evt: Event,
+        input: Option<Box<dyn SerialInput>>,
+        output: Option<Box<dyn io::Write + Send>>,
+        sync: Option<Box<dyn FileSync + Send>>,
+        out_timestamp: bool,
+        keep_rds: Vec<RawDescriptor>,
+    ) -> AsyncConsole {
+        AsyncConsole {
+            state: VirtioConsoleState::Stopped(ConsoleDevice::new(
+                protected_vm,
+                evt,
+                input,
+                output,
+                sync,
+                out_timestamp,
+                Default::default(),
+            )),
+            base_features: base_features(protected_vm),
+            keep_rds,
+        }
+    }
+}
+
+impl Drop for AsyncConsole {
+    fn drop(&mut self) {
+        let _ = self.reset();
+    }
+}
+
+impl VirtioDevice for AsyncConsole {
+    fn keep_rds(&self) -> Vec<RawDescriptor> {
+        self.keep_rds.clone()
+    }
+
+    fn features(&self) -> u64 {
+        self.base_features
+    }
+
+    fn device_type(&self) -> DeviceType {
+        DeviceType::Console
+    }
+
+    fn queue_max_sizes(&self) -> &[u16] {
+        QUEUE_SIZES
+    }
+
+    fn read_config(&self, offset: u64, data: &mut [u8]) {
+        let config = virtio_console_config {
+            max_nr_ports: 1.into(),
+            ..Default::default()
+        };
+        copy_config(data, 0, config.as_slice(), offset);
+    }
+
+    fn activate(
+        &mut self,
+        mem: GuestMemory,
+        interrupt: Interrupt,
+        mut queues: Vec<Queue>,
+        mut queue_evts: Vec<Event>,
+    ) {
+        if queues.len() < 2 || queue_evts.len() < 2 {
+            return;
+        }
+
+        // Reset the device if it was already running.
+        if matches!(self.state, VirtioConsoleState::Running { .. }) {
+            self.reset();
+        }
+
+        let state = std::mem::replace(&mut self.state, VirtioConsoleState::Broken);
+        let console = match state {
+            VirtioConsoleState::Running { .. } => {
+                error!("device should not be running here. This is a bug.");
+                return;
+            }
+            VirtioConsoleState::Stopped(console) => console,
+            VirtioConsoleState::Broken => {
+                warn!("device is broken and cannot be activated");
+                return;
+            }
+        };
+
+        let (self_kill_evt, kill_evt) = match Event::new().and_then(|e| Ok((e.try_clone()?, e))) {
+            Ok(v) => v,
+            Err(e) => {
+                error!("failed creating kill Event pair: {}", e);
+                return;
+            }
+        };
+
+        let ex = Executor::new().expect("failed to create an executor");
+        let receive_queue = queues.remove(0);
+        let receive_evt = queue_evts.remove(0);
+        let transmit_queue = queues.remove(0);
+        let transmit_evt = queue_evts.remove(0);
+
+        let worker_result = thread::Builder::new()
+            .name("virtio_console".to_string())
+            .spawn(move || {
+                let mut console = console;
+                let interrupt = Arc::new(Mutex::new(interrupt));
+
+                console.start_receive_queue(
+                    &ex,
+                    mem.clone(),
+                    receive_queue,
+                    interrupt.clone(),
+                    receive_evt,
+                )?;
+
+                console.start_transmit_queue(&ex, mem, transmit_queue, interrupt, transmit_evt)?;
+
+                // Run until the kill event is signaled and cancel all tasks.
+                ex.run_until(async {
+                    async_utils::await_and_exit(&ex, kill_evt).await?;
+                    if let Some(input) = console.input.as_mut() {
+                        input.stop().context("failed to stop rx queue")?;
+                    }
+                    if let Some(output) = console.output.as_mut() {
+                        output.stop().context("failed to stop tx queue")?;
+                    }
+
+                    Ok(console)
+                })?
+            });
+
+        match worker_result {
+            Err(e) => error!("failed to spawn virtio_console worker: {}", e),
+            Ok(join_handle) => {
+                self.state = VirtioConsoleState::Running {
+                    kill_evt: self_kill_evt,
+                    worker_thread: join_handle,
+                };
+            }
+        }
+    }
+
+    fn reset(&mut self) -> bool {
+        match std::mem::replace(&mut self.state, VirtioConsoleState::Broken) {
+            // Stopped console is already in reset state.
+            state @ VirtioConsoleState::Stopped(_) => {
+                self.state = state;
+                true
+            }
+            // Stop the worker thread and go back to `Stopped` state.
+            VirtioConsoleState::Running {
+                kill_evt,
+                worker_thread,
+            } => match kill_evt.write(1) {
+                Ok(_) => {
+                    let thread_res = match worker_thread.join() {
+                        Ok(thread_res) => thread_res,
+                        Err(_) => {
+                            error!("worker thread has panicked");
+                            return false;
+                        }
+                    };
+
+                    match thread_res {
+                        Ok(console) => {
+                            self.state = VirtioConsoleState::Stopped(console);
+                            true
+                        }
+                        Err(e) => {
+                            error!("worker thread returned an error: {}", e);
+                            false
+                        }
+                    }
+                }
+                Err(e) => {
+                    error!("error while requesting worker thread to stop: {}", e);
+                    error!("the worker thread will keep running");
+                    false
+                }
+            },
+            // We are broken and cannot reset properly.
+            VirtioConsoleState::Broken => false,
+        }
+    }
+}

devices/src/virtio/console/sys.rs

@@ -0,0 +1,15 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cfg_if::cfg_if! {
+    if #[cfg(unix)] {
+        mod unix;
+        use unix as platform;
+    } else if #[cfg(windows)] {
+        mod windows;
+        use windows as platform;
+    }
+}
+
+pub(in crate::virtio::console) use platform::{is_a_fatal_input_error, read_delay_if_needed};

devices/src/virtio/console/sys/unix.rs

@@ -0,0 +1,41 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use std::io;
+
+use base::{Event, FileSync, RawDescriptor};
+
+use crate::serial_device::SerialInput;
+use crate::virtio::console::{Console, ConsoleInput};
+use crate::virtio::ProtectionType;
+use crate::SerialDevice;
+
+impl SerialDevice for Console {
+    fn new(
+        protected_vm: ProtectionType,
+        _event: Event,
+        input: Option<Box<dyn SerialInput>>,
+        out: Option<Box<dyn io::Write + Send>>,
+        // TODO(b/171331752): connect filesync functionality.
+        _sync: Option<Box<dyn FileSync + Send>>,
+        _out_timestamp: bool,
+        keep_rds: Vec<RawDescriptor>,
+    ) -> Console {
+        Console::new(
+            protected_vm,
+            input.map(ConsoleInput::FromRead),
+            out,
+            keep_rds,
+        )
+    }
+}
+
+/// Platform-specific function to add a delay for reading rx.
+///
+/// On Unix, this function does nothing.
+pub(crate) fn read_delay_if_needed() {}
+
+pub(in crate::virtio::console) fn is_a_fatal_input_error(e: &io::Error) -> bool {
+    e.kind() != io::ErrorKind::Interrupted
+}

devices/src/virtio/console/sys/windows.rs

@@ -0,0 +1,68 @@
+// Copyright 2022 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use std::io;
+use std::thread;
+use std::time::Duration;
+
+use base::{named_pipes, Event, RawDescriptor};
+
+use crate::serial_device::SerialInput;
+use crate::virtio::console::{Console, ConsoleInput};
+use crate::virtio::ProtectionType;
+use crate::SerialDevice;
+
+impl SerialDevice for Console {
+    fn new(
+        protected_vm: ProtectionType,
+        _event: Event,
+        _input: Option<Box<dyn SerialInput>>,
+        out: Option<Box<dyn io::Write + Send>>,
+        // TODO(b/171331752): connect filesync functionality.
+        _sync: Option<Box<dyn FileSync + Send>>,
+        _out_timestamp: bool,
+        keep_rds: Vec<RawDescriptor>,
+    ) -> Console {
+        Console::new(protected_vm, None, out, keep_rds)
+    }
+
+    /// Constructs a console with named pipe as input/output connections.
+    fn new_with_pipe(
+        protected_vm: ProtectionType,
+        _interrupt_evt: Event,
+        pipe_in: named_pipes::PipeConnection,
+        pipe_out: named_pipes::PipeConnection,
+        keep_rds: Vec<RawDescriptor>,
+    ) -> Console {
+        Console::new(
+            protected_vm,
+            Some(ConsoleInput::FromRead(Box::new(pipe_in))),
+            Some(Box::new(pipe_out)),
+            keep_rds,
+        )
+    }
+}
+
+/// Platform-specific function to add a delay for reading rx.
+///
+/// We can't issue blocking reads here and overlapped I/O is
+/// incompatible with the call site where writes to this pipe are being
+/// made, so instead we issue a small wait to prevent us from hogging
+/// the CPU. This 20ms delay while typing doesn't seem to be noticeable.
+pub(in crate::virtio::console) fn read_delay_if_needed() {
+    thread::sleep(Duration::from_millis(20));
+}
+
+pub(in crate::virtio::console) fn is_a_fatal_input_error(e: &io::Error) -> bool {
+    match e.kind() {
+        // Being interrupted is not an error.
+        io::ErrorKind::Interrupted => false,
+        // Ignore two kinds of errors on Windows.
+        //   - ErrorKind::Other when reading a named pipe before a client connects.
+        //   - ErrorKind::WouldBlock when reading a named pipe (we don't use non-blocking I/O).
+        io::ErrorKind::Other | io::ErrorKind::WouldBlock => false,
+        // Everything else is a fatal input error.
+        _ => true,
+    }
+}

devices/src/virtio/fs/worker.rs

@@ -8,7 +8,7 @@ use std::io;
 use std::os::unix::io::AsRawFd;
 use std::sync::Arc;
 
-use base::{error, syscall, Event, EventToken, SafeDescriptor, Tube, WaitContext};
+use base::{error, syscall, Event, EventToken, Protection, SafeDescriptor, Tube, WaitContext};
 use fuse::filesystem::{FileSystem, ZeroCopyReader, ZeroCopyWriter};
 use sync::Mutex;
 use vm_control::{FsMappingRequest, VmResponse};
@@ -97,7 +97,7 @@ impl fuse::Mapper for Mapper {
             fd,
             size,
             file_offset,
-            prot,
+            prot: Protection::from(prot as libc::c_int),
             mem_offset,
         };
 

devices/src/virtio/gpu/virtio_gpu.rs

@@ -11,7 +11,7 @@ use std::sync::Arc;
 
 use crate::virtio::gpu::GpuDisplayParameters;
 use crate::virtio::resource_bridge::{BufferInfo, PlaneInfo, ResourceInfo, ResourceResponse};
-use base::{error, ExternalMapping, SafeDescriptor, Tube};
+use base::{error, ExternalMapping, Protection, SafeDescriptor, Tube};
 
 use data_model::VolatileSlice;
 
@@ -746,7 +746,7 @@ impl VirtioGpu {
                 allocation: self.pci_bar,
                 offset,
             },
-            read_only: false,
+            prot: Protection::read_write(),
         };
         self.gpu_device_tube.send(&request)?;
         let response = self.gpu_device_tube.recv()?;

devices/src/virtio/snd/cras_backend/mod.rs

@@ -124,6 +124,8 @@ pub struct Parameters {
     pub capture: bool,
     pub client_type: CrasClientType,
     pub socket_type: CrasSocketType,
+    pub num_output_devices: u32,
+    pub num_input_devices: u32,
     pub num_output_streams: u32,
     pub num_input_streams: u32,
 }
@@ -134,6 +136,8 @@ impl Default for Parameters {
             capture: false,
             client_type: CrasClientType::CRAS_CLIENT_TYPE_CROSVM,
             socket_type: CrasSocketType::Unified,
+            num_output_devices: 1,
+            num_input_devices: 1,
             num_output_streams: 1,
             num_input_streams: 1,
         }
@@ -164,6 +168,12 @@ impl FromStr for Parameters {
                         Error::InvalidParameterValue(v.to_string(), e.to_string())
                     })?;
                 }
+                "num_output_devices" => {
+                    params.num_output_devices = v.parse::<u32>().map_err(Error::InvalidIntValue)?;
+                }
+                "num_input_devices" => {
+                    params.num_input_devices = v.parse::<u32>().map_err(Error::InvalidIntValue)?;
+                }
                 "num_output_streams" => {
                     params.num_output_streams = v.parse::<u32>().map_err(Error::InvalidIntValue)?;
                 }
@@ -465,8 +475,10 @@ impl VirtioSndCras {
 pub fn hardcoded_virtio_snd_config(params: &Parameters) -> virtio_snd_config {
     virtio_snd_config {
         jacks: 0.into(),
-        streams: (params.num_output_streams + params.num_input_streams).into(),
-        chmaps: 4.into(),
+        streams: (params.num_output_devices * params.num_output_streams
+            + params.num_input_devices * params.num_input_streams)
+            .into(),
+        chmaps: (params.num_output_devices * 3 + params.num_input_devices).into(),
     }
 }
 
@@ -476,78 +488,88 @@ pub fn hardcoded_snd_data(params: &Parameters) -> SndData {
     let mut pcm_info: Vec<virtio_snd_pcm_info> = Vec::new();
     let mut chmap_info: Vec<virtio_snd_chmap_info> = Vec::new();
 
-    for _ in 0..params.num_output_streams {
-        pcm_info.push(virtio_snd_pcm_info {
-            hdr: virtio_snd_info {
-                hda_fn_nid: 0.into(),
-            },
-            features: 0.into(), /* 1 << VIRTIO_SND_PCM_F_XXX */
-            formats: SUPPORTED_FORMATS.into(),
-            rates: SUPPORTED_FRAME_RATES.into(),
-            direction: VIRTIO_SND_D_OUTPUT,
-            channels_min: 1,
-            channels_max: 6,
-            padding: [0; 5],
-        });
+    for dev in 0..params.num_output_devices {
+        for _ in 0..params.num_output_streams {
+            pcm_info.push(virtio_snd_pcm_info {
+                hdr: virtio_snd_info {
+                    hda_fn_nid: dev.into(),
+                },
+                features: 0.into(), /* 1 << VIRTIO_SND_PCM_F_XXX */
+                formats: SUPPORTED_FORMATS.into(),
+                rates: SUPPORTED_FRAME_RATES.into(),
+                direction: VIRTIO_SND_D_OUTPUT,
+                channels_min: 1,
+                channels_max: 6,
+                padding: [0; 5],
+            });
+        }
     }
-    for _ in 0..params.num_input_streams {
-        pcm_info.push(virtio_snd_pcm_info {
-            hdr: virtio_snd_info {
-                hda_fn_nid: 0.into(),
-            },
-            features: 0.into(), /* 1 << VIRTIO_SND_PCM_F_XXX */
-            formats: SUPPORTED_FORMATS.into(),
-            rates: SUPPORTED_FRAME_RATES.into(),
-            direction: VIRTIO_SND_D_INPUT,
-            channels_min: 1,
-            channels_max: 2,
-            padding: [0; 5],
-        });
+    for dev in 0..params.num_input_devices {
+        for _ in 0..params.num_input_streams {
+            pcm_info.push(virtio_snd_pcm_info {
+                hdr: virtio_snd_info {
+                    hda_fn_nid: dev.into(),
+                },
+                features: 0.into(), /* 1 << VIRTIO_SND_PCM_F_XXX */
+                formats: SUPPORTED_FORMATS.into(),
+                rates: SUPPORTED_FRAME_RATES.into(),
+                direction: VIRTIO_SND_D_INPUT,
+                channels_min: 1,
+                channels_max: 2,
+                padding: [0; 5],
+            });
+        }
     }
-
     // Use stereo channel map.
     let mut positions = [VIRTIO_SND_CHMAP_NONE; VIRTIO_SND_CHMAP_MAX_SIZE];
     positions[0] = VIRTIO_SND_CHMAP_FL;
     positions[1] = VIRTIO_SND_CHMAP_FR;
-
-    chmap_info.push(virtio_snd_chmap_info {
-        hdr: virtio_snd_info {
-            hda_fn_nid: 0.into(),
-        },
-        direction: VIRTIO_SND_D_OUTPUT,
-        channels: 2,
-        positions,
-    });
-    chmap_info.push(virtio_snd_chmap_info {
-        hdr: virtio_snd_info {
-            hda_fn_nid: 0.into(),
-        },
-        direction: VIRTIO_SND_D_INPUT,
-        channels: 2,
-        positions,
-    });
+    for dev in 0..params.num_output_devices {
+        chmap_info.push(virtio_snd_chmap_info {
+            hdr: virtio_snd_info {
+                hda_fn_nid: dev.into(),
+            },
+            direction: VIRTIO_SND_D_OUTPUT,
+            channels: 2,
+            positions,
+        });
+    }
+    for dev in 0..params.num_input_devices {
+        chmap_info.push(virtio_snd_chmap_info {
+            hdr: virtio_snd_info {
+                hda_fn_nid: dev.into(),
+            },
+            direction: VIRTIO_SND_D_INPUT,
+            channels: 2,
+            positions,
+        });
+    }
     positions[2] = VIRTIO_SND_CHMAP_RL;
     positions[3] = VIRTIO_SND_CHMAP_RR;
-    chmap_info.push(virtio_snd_chmap_info {
-        hdr: virtio_snd_info {
-            hda_fn_nid: 0.into(),
-        },
-        direction: VIRTIO_SND_D_OUTPUT,
-        channels: 4,
-        positions,
-    });
+    for dev in 0..params.num_output_devices {
+        chmap_info.push(virtio_snd_chmap_info {
+            hdr: virtio_snd_info {
+                hda_fn_nid: dev.into(),
+            },
+            direction: VIRTIO_SND_D_OUTPUT,
+            channels: 4,
+            positions,
+        });
+    }
     positions[2] = VIRTIO_SND_CHMAP_FC;
     positions[3] = VIRTIO_SND_CHMAP_LFE;
     positions[4] = VIRTIO_SND_CHMAP_RL;
     positions[5] = VIRTIO_SND_CHMAP_RR;
-    chmap_info.push(virtio_snd_chmap_info {
-        hdr: virtio_snd_info {
-            hda_fn_nid: 0.into(),
-        },
-        direction: VIRTIO_SND_D_OUTPUT,
-        channels: 6,
-        positions,
-    });
+    for dev in 0..params.num_output_devices {
+        chmap_info.push(virtio_snd_chmap_info {
+            hdr: virtio_snd_info {
+                hda_fn_nid: dev.into(),
+            },
+            direction: VIRTIO_SND_D_OUTPUT,
+            channels: 6,
+            positions,
+        });
+    }
 
     SndData {
         jack_info,
@@ -775,6 +797,8 @@ mod tests {
             capture: bool,
             client_type: CrasClientType,
             socket_type: CrasSocketType,
+            num_output_devices: u32,
+            num_input_devices: u32,
             num_output_streams: u32,
             num_input_streams: u32,
         ) {
@@ -782,6 +806,8 @@ mod tests {
             assert_eq!(params.capture, capture);
             assert_eq!(params.client_type, client_type);
             assert_eq!(params.socket_type, socket_type);
+            assert_eq!(params.num_output_devices, num_output_devices);
+            assert_eq!(params.num_input_devices, num_input_devices);
             assert_eq!(params.num_output_streams, num_output_streams);
             assert_eq!(params.num_input_streams, num_input_streams);
         }
@@ -797,6 +823,8 @@ mod tests {
             CrasSocketType::Unified,
             1,
             1,
+            1,
+            1,
         );
         check_success(
             "capture=true,client_type=crosvm",
@@ -805,6 +833,8 @@ mod tests {
             CrasSocketType::Unified,
             1,
             1,
+            1,
+            1,
         );
         check_success(
             "capture=true,client_type=arcvm",
@@ -813,6 +843,8 @@ mod tests {
             CrasSocketType::Unified,
             1,
             1,
+            1,
+            1,
         );
         check_failure("capture=true,client_type=none");
         check_success(
@@ -822,6 +854,8 @@ mod tests {
             CrasSocketType::Legacy,
             1,
             1,
+            1,
+            1,
         );
         check_success(
             "socket_type=unified",
@@ -830,14 +864,39 @@ mod tests {
             CrasSocketType::Unified,
             1,
             1,
+            1,
+            1,
         );
         check_success(
             "capture=true,client_type=arcvm,num_output_streams=2,num_input_streams=3",
             true,
             CrasClientType::CRAS_CLIENT_TYPE_ARCVM,
             CrasSocketType::Unified,
+            1,
+            1,
             2,
             3,
         );
+        check_success(
+            "capture=true,client_type=arcvm,num_output_devices=3,num_input_devices=2",
+            true,
+            CrasClientType::CRAS_CLIENT_TYPE_ARCVM,
+            CrasSocketType::Unified,
+            3,
+            2,
+            1,
+            1,
+        );
+        check_success(
+            "capture=true,client_type=arcvm,num_output_devices=2,num_input_devices=3,\
+            num_output_streams=3,num_input_streams=2",
+            true,
+            CrasClientType::CRAS_CLIENT_TYPE_ARCVM,
+            CrasSocketType::Unified,
+            2,
+            3,
+            3,
+            2,
+        );
     }
 }

devices/src/virtio/vhost/user/device/console.rs

@@ -2,222 +2,33 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-use std::collections::VecDeque;
-use std::io::{self, stdin};
-use std::path::PathBuf;
-use std::sync::Arc;
+use std::{io::stdin, path::PathBuf, sync::Arc};
 
 use anyhow::{anyhow, bail, Context};
-use base::{error, AsRawDescriptor, Event, FileSync, RawDescriptor, Terminal};
-use cros_async::{select2, AsyncResult, EventAsync, Executor, IntoAsync, IoSourceExt};
+use base::{error, Event, Terminal};
+use cros_async::Executor;
 use data_model::DataInit;
 
 use argh::FromArgs;
-use futures::FutureExt;
 use hypervisor::ProtectionType;
 use sync::Mutex;
 use vm_memory::GuestMemory;
 use vmm_vhost::message::{VhostUserProtocolFeatures, VhostUserVirtioFeatures};
 
-use crate::serial_device::{
-    SerialDevice, SerialHardware, SerialInput, SerialParameters, SerialType,
+use crate::{
+    virtio::{
+        self,
+        console::{asynchronous::ConsoleDevice, virtio_console_config},
+        copy_config,
+        vhost::user::device::handler::{DeviceRequestHandler, Doorbell, VhostUserBackend},
+        vhost::user::device::vvu::pci::VvuPciDevice,
+    },
+    SerialHardware, SerialParameters, SerialType,
 };
-use crate::virtio::async_device::AsyncQueueState;
-use crate::virtio::console::{
-    handle_input, process_transmit_queue, virtio_console_config, ConsoleError,
-};
-use crate::virtio::vhost::user::device::handler::{
-    DeviceRequestHandler, Doorbell, VhostUserBackend,
-};
-use crate::virtio::vhost::user::device::vvu::pci::VvuPciDevice;
-use crate::virtio::{self, copy_config, SignalableInterrupt};
 
 const MAX_QUEUE_NUM: usize = 2 /* transmit and receive queues */;
 const MAX_VRING_LEN: u16 = 256;
 
-/// Wrapper that makes any `SerialInput` usable as an async source by providing an implementation of
-/// `IntoAsync`.
-struct AsyncSerialInput(Box<dyn SerialInput>);
-impl AsRawDescriptor for AsyncSerialInput {
-    fn as_raw_descriptor(&self) -> RawDescriptor {
-        self.0.as_raw_descriptor()
-    }
-}
-impl IntoAsync for AsyncSerialInput {}
-
-async fn run_tx_queue<I: SignalableInterrupt>(
-    mut queue: virtio::Queue,
-    mem: GuestMemory,
-    doorbell: I,
-    kick_evt: EventAsync,
-    output: &mut Box<dyn io::Write + Send>,
-) {
-    loop {
-        if let Err(e) = kick_evt.next_val().await {
-            error!("Failed to read kick event for tx queue: {}", e);
-            break;
-        }
-        process_transmit_queue(&mem, &doorbell, &mut queue, output.as_mut());
-    }
-}
-
-async fn run_rx_queue<I: SignalableInterrupt>(
-    mut queue: virtio::Queue,
-    mem: GuestMemory,
-    doorbell: I,
-    kick_evt: EventAsync,
-    input: &dyn IoSourceExt<AsyncSerialInput>,
-) {
-    // Staging buffer, required because of `handle_input`'s API. We can probably remove this once
-    // the regular virtio device is switched to async.
-    let mut in_buffer = VecDeque::<u8>::new();
-    let mut rx_buf = vec![0u8; 4096];
-    let mut input_offset = 0u64;
-
-    loop {
-        match input.read_to_vec(Some(input_offset), rx_buf).await {
-            // Input source has closed.
-            Ok((0, _)) => break,
-            Ok((size, v)) => {
-                in_buffer.extend(&v[0..size]);
-                input_offset += size as u64;
-                rx_buf = v;
-            }
-            Err(e) => {
-                error!("Failed to read console input: {}", e);
-                return;
-            }
-        }
-
-        // Submit all the data obtained during this read.
-        while !in_buffer.is_empty() {
-            match handle_input(&mem, &doorbell, &mut in_buffer, &mut queue) {
-                Ok(()) => {}
-                Err(ConsoleError::RxDescriptorsExhausted) => {
-                    // Wait until a descriptor becomes available and try again.
-                    if let Err(e) = kick_evt.next_val().await {
-                        error!("Failed to read kick event for rx queue: {}", e);
-                        return;
-                    }
-                }
-            }
-        }
-    }
-}
-
-struct ConsoleDevice {
-    input: Option<AsyncQueueState<AsyncSerialInput>>,
-    output: Option<AsyncQueueState<Box<dyn io::Write + Send>>>,
-    avail_features: u64,
-}
-
-impl ConsoleDevice {
-    fn start_receive_queue<I: SignalableInterrupt + 'static>(
-        &mut self,
-        ex: &Executor,
-        mem: GuestMemory,
-        queue: virtio::Queue,
-        doorbell: I,
-        kick_evt: Event,
-    ) -> anyhow::Result<()> {
-        let input_queue = match self.input.as_mut() {
-            Some(input_queue) => input_queue,
-            None => return Ok(()),
-        };
-
-        let kick_evt =
-            EventAsync::new(kick_evt, ex).context("Failed to create EventAsync for kick_evt")?;
-
-        let closure_ex = ex.clone();
-        let rx_future = move |input, abort| {
-            let async_input = closure_ex
-                .async_from(input)
-                .context("failed to create async input")?;
-
-            Ok(async move {
-                select2(
-                    run_rx_queue(queue, mem, doorbell, kick_evt, async_input.as_ref())
-                        .boxed_local(),
-                    abort,
-                )
-                .await;
-
-                async_input.into_source()
-            })
-        };
-
-        input_queue.start(ex, rx_future)
-    }
-
-    fn stop_receive_queue(&mut self) -> AsyncResult<bool> {
-        if let Some(queue) = self.input.as_mut() {
-            queue.stop()
-        } else {
-            Ok(false)
-        }
-    }
-
-    fn start_transmit_queue<I: SignalableInterrupt + 'static>(
-        &mut self,
-        ex: &Executor,
-        mem: GuestMemory,
-        queue: virtio::Queue,
-        doorbell: I,
-        kick_evt: Event,
-    ) -> anyhow::Result<()> {
-        let output_queue = match self.output.as_mut() {
-            Some(output_queue) => output_queue,
-            None => return Ok(()),
-        };
-
-        let kick_evt =
-            EventAsync::new(kick_evt, ex).context("Failed to create EventAsync for kick_evt")?;
-
-        let tx_future = |mut output, abort| {
-            Ok(async move {
-                select2(
-                    run_tx_queue(queue, mem, doorbell, kick_evt, &mut output).boxed_local(),
-                    abort,
-                )
-                .await;
-
-                output
-            })
-        };
-
-        output_queue.start(ex, tx_future)
-    }
-
-    fn stop_transmit_queue(&mut self) -> AsyncResult<bool> {
-        if let Some(queue) = self.output.as_mut() {
-            queue.stop()?;
-            Ok(true)
-        } else {
-            Ok(false)
-        }
-    }
-}
-
-impl SerialDevice for ConsoleDevice {
-    fn new(
-        protected_vm: ProtectionType,
-        _evt: Event,
-        input: Option<Box<dyn SerialInput>>,
-        output: Option<Box<dyn io::Write + Send>>,
-        _sync: Option<Box<dyn FileSync + Send>>,
-        _out_timestamp: bool,
-        _keep_rds: Vec<RawDescriptor>,
-    ) -> ConsoleDevice {
-        let avail_features =
-            virtio::base_features(protected_vm) | VhostUserVirtioFeatures::PROTOCOL_FEATURES.bits();
-        ConsoleDevice {
-            input: input.map(AsyncSerialInput).map(AsyncQueueState::Stopped),
-            output: output.map(AsyncQueueState::Stopped),
-            avail_features,
-        }
-    }
-}
-
 struct ConsoleBackend {
     ex: Executor,
     device: ConsoleDevice,
@@ -246,7 +57,7 @@ impl VhostUserBackend for ConsoleBackend {
     }
 
     fn features(&self) -> u64 {
-        self.device.avail_features
+        self.device.avail_features() | VhostUserVirtioFeatures::PROTOCOL_FEATURES.bits()
     }
 
     fn ack_features(&mut self, value: u64) -> anyhow::Result<()> {

devices/src/virtio/vhost/user/device/cras_snd.rs

@@ -249,9 +249,12 @@ pub struct Options {
     /// Possible key values:
     /// capture - Enable audio capture. Default to false.
     /// client_type - Set specific client type for cras backend.
-    /// num_output_streams - Set number of output PCM streams.
-    /// num_input_streams - Set number of input PCM streams.
-    /// Example: [capture=true,client=crosvm,socket=unified,num_output_streams=1,num_input_streams=1]
+    /// num_output_devices - Set number of output PCM devices.
+    /// num_input_devices - Set number of input PCM devices.
+    /// num_output_streams - Set number of output PCM streams per device.
+    /// num_input_streams - Set number of input PCM streams per device.
+    /// Example: [capture=true,client=crosvm,socket=unified,
+    /// num_output_devices=1,num_input_devices=1,num_output_streams=1,num_input_streams=1]
     config: Option<String>,
 }
 

devices/src/virtio/vhost/user/proxy.rs

@@ -20,7 +20,7 @@ use std::thread;
 use anyhow::{anyhow, bail, Context};
 use base::{
     error, info, AsRawDescriptor, Event, EventToken, EventType, FromRawDescriptor,
-    IntoRawDescriptor, RawDescriptor, SafeDescriptor, Tube, WaitContext,
+    IntoRawDescriptor, Protection, RawDescriptor, SafeDescriptor, Tube, WaitContext,
 };
 use data_model::{DataInit, Le32};
 use libc::{recv, MSG_DONTWAIT, MSG_PEEK};
@@ -674,7 +674,7 @@ impl Worker {
         let request = VmMemoryRequest::RegisterMemory {
             source,
             dest,
-            read_only: false,
+            prot: Protection::read_write(),
         };
         self.send_memory_request(&request)?;
         Ok(())

devices/src/virtio/wl.rs

@@ -54,8 +54,8 @@ use data_model::*;
 use base::ioctl_iow_nr;
 use base::{
     error, ioctl_iowr_nr, ioctl_with_ref, pipe, round_up_to_page_size, warn, AsRawDescriptor,
-    Error, Event, EventToken, EventType, FileFlags, FromRawDescriptor, RawDescriptor, Result,
-    ScmSocket, SharedMemory, SharedMemoryUnix, Tube, TubeError, WaitContext,
+    Error, Event, EventToken, EventType, FileFlags, FromRawDescriptor, Protection, RawDescriptor,
+    Result, ScmSocket, SharedMemory, SharedMemoryUnix, Tube, TubeError, WaitContext,
 };
 #[cfg(feature = "gpu")]
 use base::{IntoRawDescriptor, SafeDescriptor};
@@ -348,7 +348,7 @@ impl VmRequester {
         let request = VmMemoryRequest::RegisterMemory {
             source: VmMemorySource::SharedMemory(shm),
             dest: VmMemoryDestination::NewAllocation,
-            read_only: false,
+            prot: Protection::read_write(),
         };
         let response = self.request(&request)?;
         match request {

docs/book/src/SUMMARY.md

@@ -8,6 +8,7 @@
   - [Custom Kernel / Rootfs](./running_crosvm/custom_kernel_rootfs.md)
   - [System Requirements](./running_crosvm/requirements.md)
   - [Features](./running_crosvm/features.md)
+  - [Programmatic Interaction](./running_crosvm/programmatic_interaction.md)
 - [Devices](./devices/index.md)
   - [Block](./devices/block.md)
   - [Network](./devices/net.md)
@@ -34,4 +35,4 @@ ______________________________________________________________________
 
 ______________________________________________________________________
 
-[API Documentation](./api.md)
+[Package Documentation](./package_documentation.md)

docs/book/src/api.md

@@ -1,4 +0,0 @@
-# API Document
-
-The API documentation generated by `cargo doc` is available
-[here](https://google.github.io/crosvm/doc/crosvm/).

docs/book/src/package_documentation.md

@@ -0,0 +1,4 @@
+# Package Documentation
+
+The package documentation generated by `cargo doc` is available
+[here](https://google.github.io/crosvm/doc/crosvm/).

docs/book/src/running_crosvm/programmatic_interaction.md

@@ -0,0 +1,36 @@
+# Programmatic Interaction Using the `crosvm_control` Library
+
+## Usage
+
+[`crosvm_control`](https://chromium.googlesource.com/chromiumos/platform/crosvm/+/refs/heads/main/crosvm_control/src/lib.rs)
+provides a programmatic way to interface with crosvm as a substitute to the CLI.
+
+The library itself is written in Rust, but a C/C++ compatible header (`crosvm_control.h`) is
+generated during the crosvm build and emitted to the Rust `OUT_DIR`.
+([See the `build.rs`](https://chromium.googlesource.com/chromiumos/platform/crosvm/+/refs/heads/main/crosvm_control/build.rs)
+script for more information).
+
+The best practice for using `crosvm_control` from your project is to exclusively use the
+`crosvm_control.h` generated by the crosvm build. This ensures that there will never be a runtime
+version mismatch between your project and crosvm. Additionally, this will allow for build-time
+checks against the crosvm API.
+
+During your project's build step, when building the crosvm dependency, the emitted
+`crosvm_control.h` should be installed to your project's include dir - overwriting the old version
+if present.
+
+## Changes
+
+As `crosvm_control` is a externally facing interface to crosvm, great care must be taken when
+updating the API surface. Any breaking change to a `crosvm_control` entrypoint must be handled the
+same way as a breaking change to the crosvm CLI.
+
+As a general rule, additive changes (such as adding new fields to the end of a struct, or adding a
+new API) are fine and should be integrated correctly with downstream projects so long as those
+projects follow the usage best practices. Changes that change the signature of any existing
+`crosvm_control` function will cause problems downstream and should be considered a breaking change.
+
+### (ChromeOS Developers Only)
+
+For ChromeOS, it is possible to integrate a breaking change from upstream crosvm, but it should be
+avoided if at all possible. [See here](../integration/chromeos.md#cq-depend) for more information.

hypervisor/src/haxm.rs

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-use std::arch::x86_64::__cpuid;
+use std::arch::x86_64::{CpuidResult, __cpuid};
 use std::sync::atomic::{AtomicBool, Ordering};
 
 use base::{AsRawDescriptor, RawDescriptor, Result, SafeDescriptor};
@@ -155,46 +155,56 @@ impl HypervisorX86_64 for Haxm {
                     function: 0x1,
                     index: 0,
                     flags: 0,
-                    eax: 0,
-                    ebx: 0,
-                    ecx: supported_features_1_ecx,
-                    edx: supported_features_1_edx,
+                    cpuid: CpuidResult {
+                        eax: 0,
+                        ebx: 0,
+                        ecx: supported_features_1_ecx,
+                        edx: supported_features_1_edx,
+                    },
                 },
                 CpuIdEntry {
                     function: 0x7,
                     index: 0,
                     flags: 0,
-                    eax: cpuid_7.eax,
-                    ebx: cpuid_7.ebx,
-                    ecx: cpuid_7.ecx,
-                    edx: cpuid_7.edx,
+                    cpuid: CpuidResult {
+                        eax: cpuid_7.eax,
+                        ebx: cpuid_7.ebx,
+                        ecx: cpuid_7.ecx,
+                        edx: cpuid_7.edx,
+                    },
                 },
                 CpuIdEntry {
                     function: 0x15,
                     index: 0,
                     flags: 0,
-                    eax: cpuid_15.eax,
-                    ebx: cpuid_15.ebx,
-                    ecx: cpuid_15.ecx,
-                    edx: cpuid_15.edx,
+                    cpuid: CpuidResult {
+                        eax: cpuid_15.eax,
+                        ebx: cpuid_15.ebx,
+                        ecx: cpuid_15.ecx,
+                        edx: cpuid_15.edx,
+                    },
                 },
                 CpuIdEntry {
                     function: 0x16,
                     index: 0,
                     flags: 0,
-                    eax: cpuid_16.eax,
-                    ebx: cpuid_16.ebx,
-                    ecx: cpuid_16.ecx,
-                    edx: cpuid_16.edx,
+                    cpuid: CpuidResult {
+                        eax: cpuid_16.eax,
+                        ebx: cpuid_16.ebx,
+                        ecx: cpuid_16.ecx,
+                        edx: cpuid_16.edx,
+                    },
                 },
                 CpuIdEntry {
                     function: 0x80000001,
                     index: 0,
                     flags: 0,
-                    eax: 0,
-                    ebx: 0,
-                    ecx: supported_features_80000001_ecx,
-                    edx: supported_features_80000001_edx,
+                    cpuid: CpuidResult {
+                        eax: 0,
+                        ebx: 0,
+                        ecx: supported_features_80000001_ecx,
+                        edx: supported_features_80000001_edx,
+                    },
                 },
             ],
         })

hypervisor/src/haxm/vcpu.rs

@@ -4,6 +4,7 @@
 
 use core::ffi::c_void;
 use libc::{EINVAL, ENOENT, ENOSPC, ENXIO};
+use std::arch::x86_64::CpuidResult;
 use std::cmp::min;
 use std::intrinsics::copy_nonoverlapping;
 use std::mem::{size_of, ManuallyDrop};
@@ -934,10 +935,12 @@ impl From<&hax_cpuid_entry> for CpuIdEntry {
             function: item.function,
             index: item.index,
             flags: item.flags,
-            eax: item.eax,
-            ebx: item.ebx,
-            ecx: item.ecx,
-            edx: item.edx,
+            cpuid: CpuidResult {
+                eax: item.eax,
+                ebx: item.ebx,
+                ecx: item.ecx,
+                edx: item.edx,
+            },
         }
     }
 }
@@ -948,10 +951,10 @@ impl From<&CpuIdEntry> for hax_cpuid_entry {
             function: item.function,
             index: item.index,
             flags: item.flags,
-            eax: item.eax,
-            ebx: item.ebx,
-            ecx: item.ecx,
-            edx: item.edx,
+            eax: item.cpuid.eax,
+            ebx: item.cpuid.ebx,
+            ecx: item.cpuid.ecx,
+            edx: item.cpuid.edx,
             pad: Default::default(),
         }
     }

hypervisor/src/kvm/x86_64.rs

@@ -5,6 +5,7 @@
 use base::IoctlNr;
 
 use libc::{E2BIG, ENXIO};
+use std::arch::x86_64::CpuidResult;
 
 use base::{
     errno_result, error, ioctl, ioctl_with_mut_ptr, ioctl_with_mut_ref, ioctl_with_ptr,
@@ -811,10 +812,12 @@ impl<'a> From<&'a KvmCpuId> for CpuId {
                 function: entry.function,
                 index: entry.index,
                 flags: entry.flags,
-                eax: entry.eax,
-                ebx: entry.ebx,
-                ecx: entry.ecx,
-                edx: entry.edx,
+                cpuid: CpuidResult {
+                    eax: entry.eax,
+                    ebx: entry.ebx,
+                    ecx: entry.ecx,
+                    edx: entry.edx,
+                },
             };
             cpu_id_entries.push(cpu_id_entry)
         }
@@ -831,10 +834,10 @@ impl From<&CpuId> for KvmCpuId {
                 function: e.function,
                 index: e.index,
                 flags: e.flags,
-                eax: e.eax,
-                ebx: e.ebx,
-                ecx: e.ecx,
-                edx: e.edx,
+                eax: e.cpuid.eax,
+                ebx: e.cpuid.ebx,
+                ecx: e.cpuid.ecx,
+                edx: e.cpuid.edx,
                 ..Default::default()
             };
         }

hypervisor/src/whpx.rs

@@ -190,10 +190,7 @@ fn cpuid_entry_from_host(function: u32, index: u32) -> CpuIdEntry {
         function,
         index,
         flags: 0,
-        eax: result.eax,
-        ebx: result.ebx,
-        ecx: result.ecx,
-        edx: result.edx,
+        cpuid: result,
     }
 }
 

hypervisor/src/whpx/types.rs

@@ -720,10 +720,10 @@ impl From<&CpuIdEntry> for WHV_X64_CPUID_RESULT {
     fn from(entry: &CpuIdEntry) -> Self {
         WHV_X64_CPUID_RESULT {
             Function: entry.function,
-            Eax: entry.eax,
-            Ebx: entry.ebx,
-            Ecx: entry.ecx,
-            Edx: entry.edx,
+            Eax: entry.cpuid.eax,
+            Ebx: entry.cpuid.ebx,
+            Ecx: entry.cpuid.ecx,
+            Edx: entry.cpuid.edx,
             ..Default::default()
         }
     }

hypervisor/src/whpx/vcpu.rs

@@ -4,6 +4,7 @@
 
 use core::ffi::c_void;
 use libc::{EBUSY, EINVAL, ENOENT, ENXIO};
+use std::arch::x86_64::CpuidResult;
 use std::cell::RefCell;
 use std::convert::TryInto;
 use std::mem::size_of;
@@ -786,26 +787,28 @@ impl Vcpu for WhpxVcpu {
                         function: self.last_exit_context.__bindgen_anon_1.CpuidAccess.Rax as u32,
                         index: self.last_exit_context.__bindgen_anon_1.CpuidAccess.Rcx as u32,
                         flags: 0,
-                        eax: self
-                            .last_exit_context
-                            .__bindgen_anon_1
-                            .CpuidAccess
-                            .DefaultResultRax as u32,
-                        ebx: self
-                            .last_exit_context
-                            .__bindgen_anon_1
-                            .CpuidAccess
-                            .DefaultResultRbx as u32,
-                        ecx: self
-                            .last_exit_context
-                            .__bindgen_anon_1
-                            .CpuidAccess
-                            .DefaultResultRcx as u32,
-                        edx: self
-                            .last_exit_context
-                            .__bindgen_anon_1
-                            .CpuidAccess
-                            .DefaultResultRdx as u32,
+                        cpuid: CpuidResult {
+                            eax: self
+                                .last_exit_context
+                                .__bindgen_anon_1
+                                .CpuidAccess
+                                .DefaultResultRax as u32,
+                            ebx: self
+                                .last_exit_context
+                                .__bindgen_anon_1
+                                .CpuidAccess
+                                .DefaultResultRbx as u32,
+                            ecx: self
+                                .last_exit_context
+                                .__bindgen_anon_1
+                                .CpuidAccess
+                                .DefaultResultRcx as u32,
+                            edx: self
+                                .last_exit_context
+                                .__bindgen_anon_1
+                                .CpuidAccess
+                                .DefaultResultRdx as u32,
+                        },
                     }
                 };
                 Ok(VcpuExit::Cpuid { entry })
@@ -1199,16 +1202,16 @@ impl VcpuX86_64 for WhpxVcpu {
         let values = vec![
             WHV_REGISTER_VALUE { Reg64: rip },
             WHV_REGISTER_VALUE {
-                Reg64: entry.eax as u64,
+                Reg64: entry.cpuid.eax as u64,
             },
             WHV_REGISTER_VALUE {
-                Reg64: entry.ebx as u64,
+                Reg64: entry.cpuid.ebx as u64,
             },
             WHV_REGISTER_VALUE {
-                Reg64: entry.ecx as u64,
+                Reg64: entry.cpuid.ecx as u64,
             },
             WHV_REGISTER_VALUE {
-                Reg64: entry.edx as u64,
+                Reg64: entry.cpuid.edx as u64,
             },
         ];
 

hypervisor/src/x86_64.rs

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+use std::arch::x86_64::CpuidResult;
 #[cfg(any(unix, feature = "haxm", feature = "whpx"))]
 use std::arch::x86_64::{__cpuid, _rdtsc};
 
@@ -171,8 +172,11 @@ pub(crate) fn host_phys_addr_bits() -> u8 {
 }
 
 /// Initial state for x86_64 VCPUs.
-#[derive(Copy, Clone)]
-pub struct VcpuInitX86_64 {}
+#[derive(Copy, Clone, Default)]
+pub struct VcpuInitX86_64 {
+    /// General-purpose registers.
+    pub regs: Regs,
+}
 
 /// A CpuId Entry contains supported feature information for the given processor.
 /// This can be modified by the hypervisor to pass additional information to the guest kernel
@@ -180,17 +184,14 @@ pub struct VcpuInitX86_64 {}
 /// by the cpu for a given function and index/subfunction (passed into the cpu via the eax and ecx
 /// register respectively).
 #[repr(C)]
-#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct CpuIdEntry {
     pub function: u32,
     pub index: u32,
     // flags is needed for KVM.  We store it on CpuIdEntry to preserve the flags across
     // get_supported_cpuids() -> kvm_cpuid2 -> CpuId -> kvm_cpuid2 -> set_cpuid().
     pub flags: u32,
-    pub eax: u32,
-    pub ebx: u32,
-    pub ecx: u32,
-    pub edx: u32,
+    pub cpuid: CpuidResult,
 }
 
 /// A container for the list of cpu id entries for the hypervisor and underlying cpu.
@@ -549,7 +550,7 @@ impl IrqRoute {
 
 /// State of a VCPU's general purpose registers.
 #[repr(C)]
-#[derive(Debug, Default, Copy, Clone)]
+#[derive(Debug, Copy, Clone)]
 pub struct Regs {
     pub rax: u64,
     pub rbx: u64,
@@ -571,6 +572,31 @@ pub struct Regs {
     pub rflags: u64,
 }
 
+impl Default for Regs {
+    fn default() -> Self {
+        Regs {
+            rax: 0,
+            rbx: 0,
+            rcx: 0,
+            rdx: 0,
+            rsi: 0,
+            rdi: 0,
+            rsp: 0,
+            rbp: 0,
+            r8: 0,
+            r9: 0,
+            r10: 0,
+            r11: 0,
+            r12: 0,
+            r13: 0,
+            r14: 0,
+            r15: 0,
+            rip: 0,
+            rflags: 0x2, // Bit 1 (0x2) is always 1.
+        }
+    }
+}
+
 /// State of a memory segment.
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]

infra/config/generated/cr-buildbucket.cfg

@@ -165,6 +165,27 @@ buckets {
         value: 100
       }
     }
+    builders {
+      name: "crosvm_windows"
+      swarming_host: "chromium-swarm.appspot.com"
+      dimensions: "cpu:x86-64"
+      dimensions: "os:Windows"
+      dimensions: "pool:luci.crosvm.ci"
+      exe {
+        cipd_package: "infra/recipe_bundles/chromium.googlesource.com/crosvm/crosvm"
+        cipd_version: "refs/heads/main"
+        cmd: "luciexe"
+      }
+      properties:
+        '{'
+        '  "recipe": "build_windows"'
+        '}'
+      service_account: "crosvm-luci-ci-builder@crosvm-infra.iam.gserviceaccount.com"
+      experiments {
+        key: "luci.recipes.use_python3"
+        value: 100
+      }
+    }
   }
 }
 buckets {

infra/config/generated/luci-milo.cfg

@@ -26,6 +26,10 @@ consoles {
     name: "buildbucket/luci.crosvm.ci/crosvm_chromeos_amd64-generic"
     category: "linux"
   }
+  builders {
+    name: "buildbucket/luci.crosvm.ci/crosvm_windows"
+    category: "windows"
+  }
   builders {
     name: "buildbucket/luci.crosvm.ci/crosvm_health_check"
     category: "linux"

infra/config/generated/luci-scheduler.cfg

@@ -75,6 +75,16 @@ job {
     builder: "crosvm_update_chromeos_merges"
   }
 }
+job {
+  id: "crosvm_windows"
+  realm: "ci"
+  acl_sets: "ci"
+  buildbucket {
+    server: "cr-buildbucket.appspot.com"
+    bucket: "ci"
+    builder: "crosvm_windows"
+  }
+}
 trigger {
   id: "main source"
   realm: "ci"
@@ -85,6 +95,7 @@ trigger {
   triggers: "crosvm_linux_armhf"
   triggers: "crosvm_linux_x86_64"
   triggers: "crosvm_push_to_github"
+  triggers: "crosvm_windows"
   gitiles {
     repo: "https://chromium.googlesource.com/crosvm/crosvm"
     refs: "regexp:refs/heads/main"

infra/config/main.star

@@ -137,7 +137,7 @@ luci.list_view(
     name = "Infra",
 )
 
-def verify_builder(name, dimensions, presubmit = True, postsubmit = True, **args):
+def verify_builder(name, dimensions, presubmit = True, postsubmit = True, category = "generic", **args):
     """Creates both a CI and try builder with the same properties.
 
     The CI builder is attached to the gitlies poller and console view, and the try builder
@@ -147,7 +147,8 @@ def verify_builder(name, dimensions, presubmit = True, postsubmit = True, **args
         name: Name of the builder
         dimensions: Passed to luci.builder
         presubmit: Create a presubmit builder (defaults to True)
-        postsubmit: Creaet a postsubmit builder (defaults to True)
+        postsubmit: Create a postsubmit builder (defaults to True)
+        category: Category of this builder in the concole view
         **args: Passed to luci.builder
     """
 
@@ -169,7 +170,7 @@ def verify_builder(name, dimensions, presubmit = True, postsubmit = True, **args
         luci.console_view_entry(
             console_view = "Postsubmit",
             builder = "ci/%s" % name,
-            category = "linux",
+            category = category,
         )
 
     # Try builder
@@ -207,6 +208,7 @@ def verify_linux_builder(arch, **kwargs):
         properties = {
             "test_arch": arch,
         },
+        category = "linux",
         **kwargs
     )
 
@@ -229,6 +231,7 @@ def verify_chromeos_builder(board, **kwargs):
         properties = {
             "board": board,
         },
+        category = "linux",
         **kwargs
     )
 
@@ -271,6 +274,19 @@ verify_linux_builder("armhf")
 
 verify_chromeos_builder("amd64-generic", presubmit = False)
 
+verify_builder(
+    name = "crosvm_windows",
+    dimensions = {
+        "os": "Windows",
+        "cpu": "x86-64",
+    },
+    executable = luci.recipe(
+        name = "build_windows",
+    ),
+    presubmit = False,
+    category = "windows",
+)
+
 verify_builder(
     name = "crosvm_health_check",
     dimensions = {
@@ -280,6 +296,7 @@ verify_builder(
     executable = luci.recipe(
         name = "health_check",
     ),
+    category = "linux",
 )
 
 infra_builder(

kernel_loader/Cargo.toml

@@ -8,6 +8,7 @@ data_model = { path = "../common/data_model" }
 libc = "*"
 base = { path = "../base" }
 remain = "*"
+resources = { path = "../resources" }
 thiserror = "*"
 vm_memory = { path = "../vm_memory" }
 

kernel_loader/src/lib.rs

@@ -11,6 +11,7 @@ use std::mem;
 use base::AsRawDescriptor;
 use data_model::DataInit;
 use remain::sorted;
+use resources::AddressRange;
 use thiserror::Error;
 use vm_memory::{GuestAddress, GuestMemory};
 
@@ -38,6 +39,8 @@ pub enum Error {
     CommandLineOverflow,
     #[error("invalid Elf magic number")]
     InvalidElfMagicNumber,
+    #[error("invalid entry point")]
+    InvalidEntryPoint,
     #[error("invalid Program Header Address")]
     InvalidProgramHeaderAddress,
     #[error("invalid Program Header memory size")]
@@ -46,6 +49,10 @@ pub enum Error {
     InvalidProgramHeaderOffset,
     #[error("invalid program header size")]
     InvalidProgramHeaderSize,
+    #[error("no loadable program headers found")]
+    NoLoadableProgramHeaders,
+    #[error("program header address out of allowed address range")]
+    ProgramHeaderAddressOutOfRange,
     #[error("unable to read elf header")]
     ReadElfHeader,
     #[error("unable to read kernel image")]
@@ -64,19 +71,27 @@ pub type Result<T> = std::result::Result<T, Error>;
 #[derive(Debug, Copy, Clone, PartialEq)]
 /// Information about a kernel loaded with the [`load_kernel`] function.
 pub struct LoadedKernel {
-    /// End address (exclusive) of the kernel.
-    pub end: GuestAddress,
+    /// Address range containg the bounds of the loaded program headers.
+    /// `address_range.start` is the start of the lowest loaded program header.
+    /// `address_range.end` is the end of the highest loaded program header.
+    pub address_range: AddressRange,
 
     /// Size of the kernel image in bytes.
     pub size: u64,
+
+    /// Entry point address of the kernel.
+    pub entry: GuestAddress,
 }
 
-/// Loads a kernel from a vmlinux elf image to a slice
+/// Loads a kernel from an ELF image into memory.
+///
+/// The ELF file will be loaded at the physical address specified by the `p_paddr` fields of its
+/// program headers.
 ///
 /// # Arguments
 ///
 /// * `guest_mem` - The guest memory region the kernel is written to.
-/// * `kernel_start` - The offset into `guest_mem` at which to load the kernel.
+/// * `kernel_start` - The minimum guest address to allow when loading program headers.
 /// * `kernel_image` - Input vmlinux image.
 pub fn load_kernel<F>(
     guest_mem: &GuestMemory,
@@ -120,11 +135,29 @@ where
         })
         .collect::<Result<Vec<elf::Elf64_Phdr>>>()?;
 
-    let mut kernel_end = 0;
+    let mut start = None;
+    let mut end = 0;
 
     // Read in each section pointed to by the program headers.
     for phdr in &phdrs {
-        if phdr.p_type != elf::PT_LOAD || phdr.p_filesz == 0 {
+        if phdr.p_type != elf::PT_LOAD {
+            continue;
+        }
+
+        if phdr.p_paddr < kernel_start.offset() {
+            return Err(Error::ProgramHeaderAddressOutOfRange);
+        }
+
+        if start.is_none() {
+            start = Some(phdr.p_paddr);
+        }
+
+        end = phdr
+            .p_paddr
+            .checked_add(phdr.p_memsz)
+            .ok_or(Error::InvalidProgramHeaderMemSize)?;
+
+        if phdr.p_filesz == 0 {
             continue;
         }
 
@@ -132,24 +165,35 @@ where
             .seek(SeekFrom::Start(phdr.p_offset))
             .map_err(|_| Error::SeekKernelStart)?;
 
-        let mem_offset = kernel_start
-            .checked_add(phdr.p_paddr)
-            .ok_or(Error::InvalidProgramHeaderAddress)?;
         guest_mem
-            .read_to_memory(mem_offset, kernel_image, phdr.p_filesz as usize)
+            .read_to_memory(
+                GuestAddress(phdr.p_paddr),
+                kernel_image,
+                phdr.p_filesz as usize,
+            )
             .map_err(|_| Error::ReadKernelImage)?;
-
-        kernel_end = mem_offset
-            .offset()
-            .checked_add(phdr.p_memsz)
-            .ok_or(Error::InvalidProgramHeaderMemSize)?;
     }
 
-    let size = kernel_end - kernel_start.offset();
+    // We should have found at least one PT_LOAD program header. If not, `start` will not be set.
+    let start = start.ok_or(Error::NoLoadableProgramHeaders)?;
+
+    let size = end
+        .checked_sub(start)
+        .ok_or(Error::InvalidProgramHeaderSize)?;
+
+    let address_range = AddressRange { start, end };
+
+    // `e_entry` of 0 means there is no entry point, which we do not want to allow.
+    // The entry point address must also fall within one of the loaded sections.
+    // We approximate this by checking whether it within the bounds of the first and last sections.
+    if ehdr.e_entry == 0 || !address_range.contains(ehdr.e_entry) {
+        return Err(Error::InvalidEntryPoint);
+    }
 
     Ok(LoadedKernel {
-        end: GuestAddress(kernel_end),
+        address_range,
         size,
+        entry: GuestAddress(ehdr.e_entry),
     })
 }
 
@@ -262,8 +306,10 @@ mod test {
         let kernel_addr = GuestAddress(0x0);
         let mut image = make_elf_bin();
         let kernel = load_kernel(&gm, kernel_addr, &mut image).expect("failed to load ELF");
-        assert_eq!(kernel.end, GuestAddress(0x20_0035));
-        assert_eq!(kernel.size, 0x20_0035);
+        assert_eq!(kernel.address_range.start, 0x20_0000);
+        assert_eq!(kernel.address_range.end, 0x20_0035);
+        assert_eq!(kernel.size, 0x35);
+        assert_eq!(kernel.entry, GuestAddress(0x20_000e));
     }
 
     #[test]
@@ -303,4 +349,14 @@ mod test {
             load_kernel(&gm, kernel_addr, &mut bad_image)
         );
     }
+
+    #[test]
+    fn paddr_below_start() {
+        let gm = create_guest_mem();
+        // test_elf.bin loads a phdr at 0x20_0000, so this will fail due to an out-of-bounds address
+        let kernel_addr = GuestAddress(0x30_0000);
+        let mut image = make_elf_bin();
+        let res = load_kernel(&gm, kernel_addr, &mut image);
+        assert_eq!(res, Err(Error::ProgramHeaderAddressOutOfRange));
+    }
 }

seccomp/arm/battery.policy

@@ -3,4 +3,9 @@
 # found in the LICENSE file.
 
 @include /usr/share/policy/crosvm/common_device.policy
+connect: 1
+geteuid32: 1
+getsockname: 1
 prctl: arg0 == PR_SET_NAME
+send: 1
+socket: arg0 == AF_UNIX

src/crosvm/cmdline.rs

@@ -66,7 +66,7 @@ pub enum Command {
     #[cfg(feature = "composite-disk")]
     CreateComposite(CreateCompositeCommand),
     CreateQcow2(CreateQcow2Command),
-    Device(DevicesCommand),
+    Device(DeviceCommand),
     Disk(DiskCommand),
     MakeRT(MakeRTCommand),
     Resume(ResumeCommand),
@@ -301,9 +301,9 @@ pub struct VfioCrosvmCommand {
 #[derive(FromArgs)]
 #[argh(subcommand, name = "device")]
 /// Start a device process
-pub struct DevicesCommand {
+pub struct DeviceCommand {
     #[argh(subcommand)]
-    pub command: DevicesSubcommand,
+    pub command: DeviceSubcommand,
 }
 
 #[derive(FromArgs)]
@@ -315,7 +315,7 @@ pub enum CrossPlatformDevicesCommands {
 }
 
 #[derive(argh_helpers::FlattenSubcommand)]
-pub enum DevicesSubcommand {
+pub enum DeviceSubcommand {
     CrossPlatform(CrossPlatformDevicesCommands),
     Sys(super::sys::cmdline::DevicesSubcommand),
 }
@@ -454,7 +454,8 @@ pub struct RunCommand {
     #[cfg(feature = "audio_cras")]
     #[argh(
         option,
-        arg_name = "[capture=true,client=crosvm,socket=unified,num_output_streams=1,num_input_streams=1]",
+        arg_name = "[capture=true,client=crosvm,socket=unified,\
+        num_output_devices=1,num_input_devices=1,num_output_streams=1,num_input_streams=1]",
         long = "cras-snd"
     )]
     /// comma separated key=value pairs for setting up cras snd
@@ -462,8 +463,12 @@ pub struct RunCommand {
     /// Possible key values:
     ///     capture - Enable audio capture. Default to false.
     ///     client_type - Set specific client type for cras backend.
+    ///     num_output_devices - Set number of output PCM devices.
+    ///     num_input_devices - Set number of input PCM devices.
     ///     num_output_streams - Set number of output PCM streams
+    ///         per device.
     ///     num_input_streams - Set number of input PCM streams
+    ///         per device.
     pub cras_snds: Vec<CrasSndParameters>,
     #[argh(switch)]
     /// don't set VCPUs real-time until make-rt command is run

src/crosvm/linux/device_helpers.rs

@@ -22,6 +22,7 @@ use base::*;
 use devices::serial_device::{SerialParameters, SerialType};
 use devices::vfio::{VfioCommonSetup, VfioCommonTrait};
 use devices::virtio::block::block::DiskOption;
+use devices::virtio::console::asynchronous::AsyncConsole;
 use devices::virtio::ipc_memory_mapper::{create_ipc_mapper, CreateIpcMapperRet};
 use devices::virtio::memory_mapper::{BasicMemoryMapper, MemoryMapperTrait};
 #[cfg(feature = "audio_cras")]
@@ -38,7 +39,7 @@ use devices::virtio::vhost::user::vmm::{
 use devices::virtio::vhost::vsock::VhostVsockConfig;
 #[cfg(any(feature = "video-decoder", feature = "video-encoder"))]
 use devices::virtio::VideoBackendType;
-use devices::virtio::{self, BalloonMode, Console, VirtioDevice};
+use devices::virtio::{self, BalloonMode, VirtioDevice};
 use devices::IommuDevType;
 #[cfg(feature = "tpm")]
 use devices::SoftwareTpm;
@@ -1182,7 +1183,7 @@ pub fn create_console_device(
     let mut keep_rds = Vec::new();
     let evt = Event::new().context("failed to create event")?;
     let dev = param
-        .create_serial_device::<Console>(protected_vm, &evt, &mut keep_rds)
+        .create_serial_device::<AsyncConsole>(protected_vm, &evt, &mut keep_rds)
         .context("failed to create console device")?;
 
     let jail = match simple_jail(jail_config, "serial_device")? {

src/main.rs

@@ -333,13 +333,13 @@ fn create_qcow2(cmd: cmdline::CreateQcow2Command) -> std::result::Result<(), ()>
     Ok(())
 }
 
-fn start_device(opts: cmdline::DevicesCommand) -> std::result::Result<(), ()> {
+fn start_device(opts: cmdline::DeviceCommand) -> std::result::Result<(), ()> {
     let result = match opts.command {
-        cmdline::DevicesSubcommand::CrossPlatform(command) => match command {
+        cmdline::DeviceSubcommand::CrossPlatform(command) => match command {
             cmdline::CrossPlatformDevicesCommands::Block(cfg) => run_block_device(cfg),
             cmdline::CrossPlatformDevicesCommands::Net(cfg) => run_net_device(cfg),
         },
-        cmdline::DevicesSubcommand::Sys(command) => sys::start_device(command),
+        cmdline::DeviceSubcommand::Sys(command) => sys::start_device(command),
     };
 
     result.map_err(|e| {

tools/examples/baremetal/layout.ld

@@ -3,7 +3,6 @@ ENTRY(_start)
 /* Crosvm will load our code into 0x200000 + paddr */
 MEMORY {
     VMA : ORIGIN = 0x00200000, LENGTH = 0x200000
-    RAM : ORIGIN = 0x00000000, LENGTH = 0x200000
 }
 
 SECTIONS {
@@ -13,7 +12,7 @@ SECTIONS {
   {
       _stack_end = . ;
       *(.boot)
-  }> VMA AT>RAM
+  }> VMA
 
   .kernel :
   {
@@ -22,7 +21,7 @@ SECTIONS {
     *(.rodata .rodata.*)
     *(.data .data.*)
     *(.bss .bss.*)
-  }> VMA AT>RAM
+  }> VMA
 
   DISCARD/ :
   {

tools/examples/baremetal/src/boot.asm

@@ -1,12 +1,13 @@
+/* Copyright 2022 The ChromiumOS Authors.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
 .section .boot, "awx"
 .global _start
 .code64
-# crosvm starts execution at 0x200 offset from the beginning
-.fill 0x200
 
 _start:
     lea rsp, _stack_end
 
     jmp main
-
-

tools/examples/baremetal/src/main.rs

@@ -5,7 +5,7 @@
 #![no_std] // don't link the Rust standard library
 #![no_main] // disable all Rust-level entry points
 
-use core::arch::global_asm;
+use core::arch::{asm, global_asm};
 use core::panic::PanicInfo;
 
 use log::*;
@@ -15,6 +15,12 @@ global_asm!(include_str!("../src/boot.asm"));
 /// This function is called on panic.
 #[panic_handler]
 fn panic(_info: &PanicInfo) -> ! {
+    // Execute a debug breakpoint instruction to cause a VMEXIT.
+    // SAFETY: This instruction will exit the hosting VM, so no further Rust code will execute.
+    unsafe {
+        asm!("int3");
+    }
+    // Just in case we are still running somehow, spin forever.
     loop {}
 }
 

tools/impl/test_config.py

@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 import enum
+from typing import List, Dict
 
 
 class TestOption(enum.Enum):
@@ -69,7 +70,7 @@ WIN64_DISABLED_CRATES = [
     "x86_64",
 ]
 
-CRATE_OPTIONS: dict[str, list[TestOption]] = {
+CRATE_OPTIONS: Dict[str, List[TestOption]] = {
     "base": [TestOption.SINGLE_THREADED, TestOption.LARGE],
     "cros_async": [TestOption.LARGE],
     "crosvm": [TestOption.SINGLE_THREADED],
@@ -116,7 +117,7 @@ CRATE_OPTIONS: dict[str, list[TestOption]] = {
 for name in WIN64_DISABLED_CRATES:
     CRATE_OPTIONS[name] = CRATE_OPTIONS.get(name, []) + [TestOption.DO_NOT_BUILD_WIN64]
 
-BUILD_FEATURES: dict[str, str] = {
+BUILD_FEATURES: Dict[str, str] = {
     "x86_64": "linux-x86_64",
     "aarch64": "linux-aarch64",
     "armhf": "linux-armhf",

tools/impl/test_runner.py

@@ -138,8 +138,8 @@ def exclude_crosvm(target_arch: Arch):
 def cargo(
     cargo_command: str,
     cwd: Path,
-    flags: list[str],
-    env: dict[str, str],
+    flags: List[str],
+    env: Dict[str, str],
     build_arch: Arch,
 ) -> Iterable[Executable]:
     """
@@ -207,7 +207,7 @@ def cargo(
 
 
 def cargo_build_executables(
-    flags: list[str],
+    flags: List[str],
     build_arch: Arch,
     cwd: Path = Path("."),
     env: Dict[str, str] = {},
@@ -221,7 +221,7 @@ def cargo_build_executables(
     yield from cargo("test", cwd, ["--no-run", *flags], env, build_arch)
 
 
-def build_common_crate(build_env: dict[str, str], build_arch: Arch, crate: Crate):
+def build_common_crate(build_env: Dict[str, str], build_arch: Arch, crate: Crate):
     print(f"Building tests for: common/{crate.name}")
     return list(cargo_build_executables([], build_arch, env=build_env, cwd=crate.path))
 
@@ -282,7 +282,7 @@ def execute_test(target: TestTarget, executable: Executable):
     Test output is hidden unless the test fails or VERBOSE mode is enabled.
     """
     options = CRATE_OPTIONS.get(executable.crate_name, [])
-    args: list[str] = []
+    args: List[str] = []
     if TestOption.SINGLE_THREADED in options:
         args += ["--test-threads=1"]
 
@@ -324,7 +324,7 @@ def execute_test(target: TestTarget, executable: Executable):
 
 
 def execute_all(
-    executables: list[Executable],
+    executables: List[Executable],
     target: test_target.TestTarget,
     repeat: int,
 ):
@@ -353,7 +353,7 @@ def execute_all(
     print()
 
 
-def find_crosvm_binary(executables: list[Executable]):
+def find_crosvm_binary(executables: List[Executable]):
     for executable in executables:
         if not executable.is_test and executable.cargo_target == "crosvm":
             return executable
@@ -371,6 +371,7 @@ def main():
     )
     parser.add_argument(
         "--target",
+        default="host",
         help="Execute tests on the selected target. See ./tools/set_test_target",
     )
     parser.add_argument(

tools/impl/test_target.py

@@ -5,7 +5,7 @@ import argparse
 import platform
 import subprocess
 from pathlib import Path
-from typing import Any, Literal, Optional, cast
+from typing import Any, Literal, Optional, cast, List, Dict
 import typing
 import sys
 from . import testvm
@@ -63,9 +63,9 @@ class Ssh:
     """Wrapper around subprocess to execute commands remotely via SSH."""
 
     hostname: str
-    opts: list[str]
+    opts: List[str]
 
-    def __init__(self, hostname: str, opts: list[str] = []):
+    def __init__(self, hostname: str, opts: List[str] = []):
         self.hostname = hostname
         self.opts = opts
 
@@ -95,9 +95,9 @@ class Ssh:
             check=True,
         ).stdout
 
-    def upload_files(self, files: list[Path], remote_dir: str = "", quiet: bool = False):
+    def upload_files(self, files: List[Path], remote_dir: str = "", quiet: bool = False):
         """Wrapper around SCP."""
-        flags: list[str] = []
+        flags: List[str] = []
         if quiet:
             flags.append("-q")
         scp_cmd = [
@@ -114,7 +114,7 @@ class TestTarget(object):
     """A test target can be the local host, a VM or a remote devica via SSH."""
 
     target_str: str
-    is_host: bool = False
+    is_host: bool = True
     vm: Optional[testvm.Arch] = None
     ssh: Optional[Ssh] = None
     __arch: Optional[Arch] = None
@@ -131,10 +131,12 @@ class TestTarget(object):
             arch: testvm.Arch = parts[1]  # type: ignore
             self.vm = arch
             self.ssh = Ssh("localhost", testvm.ssh_cmd_args(arch))
+            self.is_host = False
         elif len(parts) == 2 and parts[0] == "ssh":
             self.ssh = Ssh(parts[1])
+            self.is_host = False
         elif len(parts) == 1 and parts[0] == "host":
-            self.is_host = True
+            pass
         else:
             raise Exception(f"Invalid target {target_str}")
         if build_arch:
@@ -171,13 +173,13 @@ def find_rust_libs():
     yield from lib_dir.glob("libtest-*")
 
 
-def prepare_remote(ssh: Ssh, extra_files: list[Path] = []):
+def prepare_remote(ssh: Ssh, extra_files: List[Path] = []):
     print("Preparing remote")
     ssh.upload_files(list(find_rust_libs()) + extra_files)
     pass
 
 
-def prepare_target(target: TestTarget, extra_files: list[Path] = []):
+def prepare_target(target: TestTarget, extra_files: List[Path] = []):
     if target.vm:
         testvm.build_if_needed(target.vm)
         testvm.wait(target.vm)
@@ -204,7 +206,7 @@ def get_cargo_build_target(arch: Arch):
 
 def get_cargo_env(target: TestTarget, build_arch: Arch):
     """Environment variables to make cargo use the test target."""
-    env: dict[str, str] = BUILD_ENV.copy()
+    env: Dict[str, str] = BUILD_ENV.copy()
     cargo_target = get_cargo_build_target(build_arch)
     upper_target = cargo_target.upper().replace("-", "_")
     if build_arch != platform.machine():
@@ -215,7 +217,7 @@ def get_cargo_env(target: TestTarget, build_arch: Arch):
     return env
 
 
-def write_envrc(values: dict[str, str]):
+def write_envrc(values: Dict[str, str]):
     with open(ENVRC_PATH, "w") as file:
         for key, value in values.items():
             file.write(f'export {key}="{value}"\n')
@@ -234,8 +236,8 @@ def exec_file_on_target(
     target: TestTarget,
     filepath: Path,
     timeout: int,
-    args: list[str] = [],
-    extra_files: list[Path] = [],
+    args: List[str] = [],
+    extra_files: List[Path] = [],
     **kwargs: Any,
 ):
     """Executes a file on the test target.
@@ -290,9 +292,9 @@ def exec_file_on_target(
 def exec_file(
     target: TestTarget,
     filepath: Path,
-    args: list[str] = [],
+    args: List[str] = [],
     timeout: int = 60,
-    extra_files: list[Path] = [],
+    extra_files: List[Path] = [],
 ):
     if not filepath.exists():
         raise Exception(f"File does not exist: {filepath}")

tools/impl/testvm.py

@@ -3,7 +3,7 @@
 # found in the LICENSE file.
 
 from pathlib import Path
-from typing import Iterable, Optional, Literal
+from typing import Iterable, Optional, Literal, Dict, List, Tuple
 import argparse
 import itertools
 import os
@@ -108,13 +108,13 @@ def rootfs_img_path(arch: Arch):
 
 
 # List of ports to use for SSH for each architecture
-SSH_PORTS: dict[Arch, int] = {
+SSH_PORTS: Dict[Arch, int] = {
     "x86_64": 9000,
     "aarch64": 9001,
 }
 
 # QEMU arguments shared by all architectures
-SHARED_ARGS: list[tuple[str, str]] = [
+SHARED_ARGS: List[Tuple[str, str]] = [
     ("-display", "none"),
     ("-device", "virtio-net-pci,netdev=net0"),
     ("-smp", "8"),
@@ -122,7 +122,7 @@ SHARED_ARGS: list[tuple[str, str]] = [
 ]
 
 # Arguments to QEMU for each architecture
-ARCH_TO_QEMU: dict[Arch, tuple[str, list[Iterable[str]]]] = {
+ARCH_TO_QEMU: Dict[Arch, Tuple[str, List[Iterable[str]]]] = {
     # arch: (qemu-binary, [(param, value), ...])
     "x86_64": (
         "qemu-system-x86_64",
@@ -150,7 +150,7 @@ ARCH_TO_QEMU: dict[Arch, tuple[str, list[Iterable[str]]]] = {
 }
 
 
-def ssh_opts(arch: Arch) -> dict[str, str]:
+def ssh_opts(arch: Arch) -> Dict[str, str]:
     return {
         "Port": str(SSH_PORTS[arch]),
         "User": "crosvm",
@@ -359,7 +359,7 @@ def clean(arch: Arch):
     shutil.rmtree(data_dir(arch))
 
 
-def main(arch: Arch, argv: list[str]):
+def main(arch: Arch, argv: List[str]):
     COMMANDS = [
         "build",
         "up",

vm_control/src/lib.rs

@@ -276,16 +276,16 @@ impl VmMemorySource {
         self,
         map_request: Arc<Mutex<Option<ExternalMapping>>>,
         gralloc: &mut RutabagaGralloc,
-        read_only: bool,
+        prot: Protection,
     ) -> Result<(Box<dyn MappedRegion>, u64)> {
         let (mem_region, size) = match self {
             VmMemorySource::Descriptor {
                 descriptor,
                 offset,
                 size,
-            } => (map_descriptor(&descriptor, offset, size, read_only)?, size),
+            } => (map_descriptor(&descriptor, offset, size, prot)?, size),
             VmMemorySource::SharedMemory(shm) => {
-                (map_descriptor(&shm, 0, shm.size(), read_only)?, shm.size())
+                (map_descriptor(&shm, 0, shm.size(), prot)?, shm.size())
             }
             VmMemorySource::Vulkan {
                 descriptor,
@@ -367,7 +367,7 @@ pub enum VmMemoryRequest {
         /// Where to map the memory in the guest.
         dest: VmMemoryDestination,
         /// Whether to map the memory read only (true) or read-write (false).
-        read_only: bool,
+        prot: Protection,
     },
     /// Allocate GPU buffer of a given size/format and register the memory into guest address space.
     /// The response variant is `VmResponse::AllocateAndRegisterGpuMemory`
@@ -411,14 +411,10 @@ impl VmMemoryRequest {
     ) -> VmMemoryResponse {
         use self::VmMemoryRequest::*;
         match self {
-            RegisterMemory {
-                source,
-                dest,
-                read_only,
-            } => {
+            RegisterMemory { source, dest, prot } => {
                 // Correct on Windows because callers of this IPC guarantee descriptor is a mapping
                 // handle.
-                let (mapped_region, size) = match source.map(map_request, gralloc, read_only) {
+                let (mapped_region, size) = match source.map(map_request, gralloc, prot) {
                     Ok((region, size)) => (region, size),
                     Err(e) => return VmMemoryResponse::Err(e),
                 };
@@ -428,7 +424,12 @@ impl VmMemoryRequest {
                     Err(e) => return VmMemoryResponse::Err(e),
                 };
 
-                let slot = match vm.add_memory_region(guest_addr, mapped_region, read_only, false) {
+                let slot = match vm.add_memory_region(
+                    guest_addr,
+                    mapped_region,
+                    prot == Protection::read(),
+                    false,
+                ) {
                     Ok(slot) => slot,
                     Err(e) => return VmMemoryResponse::Err(e),
                 };
@@ -534,7 +535,7 @@ impl VmMemoryRequest {
         let descriptor =
             unsafe { SafeDescriptor::from_raw_descriptor(handle.os_handle.into_raw_descriptor()) };
 
-        let mapped_region = map_descriptor(&descriptor, 0, reqs.size, false)?;
+        let mapped_region = map_descriptor(&descriptor, 0, reqs.size, Protection::read_write())?;
         Ok((mapped_region, reqs.size, descriptor, desc))
     }
 }
@@ -914,14 +915,9 @@ fn map_descriptor(
     descriptor: &dyn AsRawDescriptor,
     offset: u64,
     size: u64,
-    read_only: bool,
+    prot: Protection,
 ) -> Result<Box<dyn MappedRegion>> {
     let size: usize = size.try_into().map_err(|_e| SysError::new(ERANGE))?;
-    let prot = if read_only {
-        Protection::read()
-    } else {
-        Protection::read_write()
-    };
     match MemoryMappingBuilder::new(size)
         .from_descriptor(descriptor)
         .offset(offset)

vm_control/src/sys/unix.rs

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 use libc::{EINVAL, ERANGE};
-use std::{os::raw::c_int, path::Path, thread::JoinHandle};
+use std::{path::Path, thread::JoinHandle};
 
 use base::{
     error, AsRawDescriptor, Descriptor, Error as SysError, Killable, MemoryMappingArena, MmapError,
@@ -104,7 +104,7 @@ pub enum FsMappingRequest {
         file_offset: u64,
         /// The memory protection to be used for the mapping.  Protections other than readable and
         /// writable will be silently dropped.
-        prot: u32,
+        prot: Protection,
         /// The offset into the shared memory region where the mapping should be placed.
         mem_offset: usize,
     },
@@ -174,14 +174,7 @@ impl FsMappingRequest {
             } => {
                 let raw_fd: Descriptor = Descriptor(fd.as_raw_descriptor());
 
-                match vm.add_fd_mapping(
-                    slot,
-                    mem_offset,
-                    size,
-                    &raw_fd,
-                    file_offset,
-                    Protection::from(prot as c_int & (libc::PROT_READ | libc::PROT_WRITE)),
-                ) {
+                match vm.add_fd_mapping(slot, mem_offset, size, &raw_fd, file_offset, prot) {
                     Ok(()) => VmResponse::Ok,
                     Err(e) => VmResponse::Err(e),
                 }

x86_64/src/cpuid.rs

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-use std::arch::x86_64::{__cpuid, __cpuid_count};
+use std::arch::x86_64::{CpuidResult, __cpuid, __cpuid_count};
 use std::cmp;
 use std::result;
 
@@ -47,7 +47,7 @@ const EAX_CORE_TEMP: u32 = 0; // Core Temperature
 const EAX_PKG_TEMP: u32 = 6; // Package Temperature
 
 /// All of the context required to emulate the CPUID instruction.
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct CpuIdContext {
     /// Id of the Vcpu associated with this context.
     vcpu_id: usize,
@@ -68,6 +68,10 @@ pub struct CpuIdContext {
     enable_pnp_data: bool,
     /// Enable Intel Turbo Boost Max Technology 3.0.
     itmt: bool,
+    /// __cpuid_count or a fake function for test.
+    cpuid_count: unsafe fn(u32, u32) -> CpuidResult,
+    /// __cpuid or a fake function for test.
+    cpuid: unsafe fn(u32) -> CpuidResult,
 }
 
 impl CpuIdContext {
@@ -79,6 +83,8 @@ impl CpuIdContext {
         irq_chip: Option<&dyn IrqChipX86_64>,
         enable_pnp_data: bool,
         itmt: bool,
+        cpuid_count: unsafe fn(u32, u32) -> CpuidResult,
+        cpuid: unsafe fn(u32) -> CpuidResult,
     ) -> CpuIdContext {
         CpuIdContext {
             vcpu_id,
@@ -93,6 +99,8 @@ impl CpuIdContext {
             host_cpu_topology,
             enable_pnp_data,
             itmt,
+            cpuid_count,
+            cpuid,
         }
     }
 }
@@ -108,67 +116,55 @@ pub fn adjust_cpuid(entry: &mut CpuIdEntry, ctx: &CpuIdContext) {
         0 => {
             if ctx.tsc_frequency.is_some() {
                 // We add leaf 0x15 for the TSC frequency if it is available.
-                entry.eax = cmp::max(0x15, entry.eax);
+                entry.cpuid.eax = cmp::max(0x15, entry.cpuid.eax);
             }
         }
         1 => {
             // X86 hypervisor feature
             if entry.index == 0 {
-                entry.ecx |= 1 << ECX_HYPERVISOR_SHIFT;
+                entry.cpuid.ecx |= 1 << ECX_HYPERVISOR_SHIFT;
             }
             if ctx.x2apic {
-                entry.ecx |= 1 << ECX_X2APIC_SHIFT;
+                entry.cpuid.ecx |= 1 << ECX_X2APIC_SHIFT;
             } else {
-                entry.ecx &= !(1 << ECX_X2APIC_SHIFT);
+                entry.cpuid.ecx &= !(1 << ECX_X2APIC_SHIFT);
             }
             if ctx.tsc_deadline_timer {
-                entry.ecx |= 1 << ECX_TSC_DEADLINE_TIMER_SHIFT;
+                entry.cpuid.ecx |= 1 << ECX_TSC_DEADLINE_TIMER_SHIFT;
             }
 
             if ctx.host_cpu_topology {
-                entry.ebx |= EBX_CLFLUSH_CACHELINE << EBX_CLFLUSH_SIZE_SHIFT;
+                entry.cpuid.ebx |= EBX_CLFLUSH_CACHELINE << EBX_CLFLUSH_SIZE_SHIFT;
 
                 // Expose HT flag to Guest.
-                let result = unsafe { __cpuid(entry.function) };
-                entry.edx |= result.edx & (1 << EDX_HTT_SHIFT);
+                let result = unsafe { (ctx.cpuid)(entry.function) };
+                entry.cpuid.edx |= result.edx & (1 << EDX_HTT_SHIFT);
                 return;
             }
 
-            entry.ebx = (ctx.vcpu_id << EBX_CPUID_SHIFT) as u32
+            entry.cpuid.ebx = (ctx.vcpu_id << EBX_CPUID_SHIFT) as u32
                 | (EBX_CLFLUSH_CACHELINE << EBX_CLFLUSH_SIZE_SHIFT);
             if ctx.cpu_count > 1 {
                 // This field is only valid if CPUID.1.EDX.HTT[bit 28]= 1.
-                entry.ebx |= (ctx.cpu_count as u32) << EBX_CPU_COUNT_SHIFT;
+                entry.cpuid.ebx |= (ctx.cpu_count as u32) << EBX_CPU_COUNT_SHIFT;
                 // A value of 0 for HTT indicates there is only a single logical
                 // processor in the package and software should assume only a
                 // single APIC ID is reserved.
-                entry.edx |= 1 << EDX_HTT_SHIFT;
+                entry.cpuid.edx |= 1 << EDX_HTT_SHIFT;
             }
         }
         2 | // Cache and TLB Descriptor information
         0x80000002 | 0x80000003 | 0x80000004 | // Processor Brand String
         0x80000005 | 0x80000006 // L1 and L2 cache information
-            => unsafe {
-            let result = __cpuid(entry.function);
-            entry.eax = result.eax;
-            entry.ebx = result.ebx;
-            entry.ecx = result.ecx;
-            entry.edx = result.edx;
-        },
+            => entry.cpuid = unsafe { (ctx.cpuid)(entry.function) },
         4 => {
-            unsafe {
-                let result = __cpuid_count(entry.function, entry.index);
-                entry.eax = result.eax;
-                entry.ebx = result.ebx;
-                entry.ecx = result.ecx;
-                entry.edx = result.edx;
-            }
+            entry.cpuid = unsafe { (ctx.cpuid_count)(entry.function, entry.index) };
 
             if ctx.host_cpu_topology {
                 return;
             }
 
-            entry.eax &= !0xFC000000;
+            entry.cpuid.eax &= !0xFC000000;
             if ctx.cpu_count > 1 {
                 let cpu_cores = if ctx.no_smt {
                     ctx.cpu_count as u32
@@ -177,31 +173,31 @@ pub fn adjust_cpuid(entry: &mut CpuIdEntry, ctx: &CpuIdContext) {
                 } else {
                     1
                 };
-                entry.eax |= (cpu_cores - 1) << EAX_CPU_CORES_SHIFT;
+                entry.cpuid.eax |= (cpu_cores - 1) << EAX_CPU_CORES_SHIFT;
             }
         }
         6 => {
             // Clear X86 EPB feature.  No frequency selection in the hypervisor.
-            entry.ecx &= !(1 << ECX_EPB_SHIFT);
+            entry.cpuid.ecx &= !(1 << ECX_EPB_SHIFT);
 
             // Set ITMT related features.
             if ctx.itmt || ctx.enable_pnp_data {
                 // Safe because we pass 6 for this call and the host
                 // supports the `cpuid` instruction
-                let result = unsafe { __cpuid(entry.function) };
+                let result = unsafe { (ctx.cpuid)(entry.function) };
                 if ctx.itmt {
                     // Expose ITMT to guest.
-                    entry.eax |= result.eax & (1 << EAX_ITMT_SHIFT);
+                    entry.cpuid.eax |= result.eax & (1 << EAX_ITMT_SHIFT);
                     // Expose HWP and HWP_EPP to guest.
-                    entry.eax |= result.eax & (1 << EAX_HWP_SHIFT);
-                    entry.eax |= result.eax & (1 << EAX_HWP_EPP_SHIFT);
+                    entry.cpuid.eax |= result.eax & (1 << EAX_HWP_SHIFT);
+                    entry.cpuid.eax |= result.eax & (1 << EAX_HWP_EPP_SHIFT);
                 }
                 if ctx.enable_pnp_data {
                     // Expose core temperature, package temperature
                     // and APEF/MPERF to guest
-                    entry.eax |= result.eax & (1 << EAX_CORE_TEMP);
-                    entry.eax |= result.eax & (1 << EAX_PKG_TEMP);
-                    entry.ecx |= result.ecx & (1 << ECX_HCFC_PERF_SHIFT);
+                    entry.cpuid.eax |= result.eax & (1 << EAX_CORE_TEMP);
+                    entry.cpuid.eax |= result.eax & (1 << EAX_PKG_TEMP);
+                    entry.cpuid.ecx |= result.ecx & (1 << ECX_HCFC_PERF_SHIFT);
                 }
             }
         }
@@ -209,20 +205,16 @@ pub fn adjust_cpuid(entry: &mut CpuIdEntry, ctx: &CpuIdContext) {
             if ctx.host_cpu_topology && entry.index == 0 {
                 // Safe because we pass 7 and 0 for this call and the host supports the
                 // `cpuid` instruction
-                let result = unsafe { __cpuid_count(entry.function, entry.index) };
-                entry.edx |= result.edx & (1 << EDX_HYBRID_CPU_SHIFT);
+                let result = unsafe { (ctx.cpuid_count)(entry.function, entry.index) };
+                entry.cpuid.edx |= result.edx & (1 << EDX_HYBRID_CPU_SHIFT);
             }
         }
         0x15 => {
             if ctx.enable_pnp_data {
+                // Expose TSC frequency to guest
                 // Safe because we pass 0x15 for this call and the host
                 // supports the `cpuid` instruction
-                let result = unsafe { __cpuid(entry.function) };
-                // Expose TSC frequency to guest
-                entry.eax = result.eax;
-                entry.ebx = result.ebx;
-                entry.ecx = result.ecx;
-                entry.edx = result.edx;
+                entry.cpuid = unsafe { (ctx.cpuid)(entry.function) };
             }
         }
         0x1A => {
@@ -230,11 +222,7 @@ pub fn adjust_cpuid(entry: &mut CpuIdEntry, ctx: &CpuIdContext) {
             if ctx.host_cpu_topology {
                 // Safe because we pass 0x1A for this call and the host supports the
                 // `cpuid` instruction
-                let result = unsafe { __cpuid(entry.function) };
-                entry.eax = result.eax;
-                entry.ebx = result.ebx;
-                entry.ecx = result.ecx;
-                entry.edx = result.edx;
+                entry.cpuid = unsafe { (ctx.cpuid)(entry.function) };
             }
         }
         0xB | 0x1F => {
@@ -245,33 +233,34 @@ pub fn adjust_cpuid(entry: &mut CpuIdEntry, ctx: &CpuIdContext) {
             // NOTE: these will need to be split if any of the fields that differ between
             // the two versions are to be set.
             // On AMD, these leaves are not used, so it is currently safe to leave in.
-            entry.edx = ctx.vcpu_id as u32; // x2APIC ID
+            entry.cpuid.edx = ctx.vcpu_id as u32; // x2APIC ID
             if entry.index == 0 {
                 if ctx.no_smt || (ctx.cpu_count == 1) {
                     // Make it so that all VCPUs appear as different,
                     // non-hyperthreaded cores on the same package.
-                    entry.eax = 0; // Shift to get id of next level
-                    entry.ebx = 1; // Number of logical cpus at this level
+                    entry.cpuid.eax = 0; // Shift to get id of next level
+                    entry.cpuid.ebx = 1; // Number of logical cpus at this level
                 } else if ctx.cpu_count % 2 == 0 {
                     // Each core has 2 hyperthreads
-                    entry.eax = 1; // Shift to get id of next level
-                    entry.ebx = 2; // Number of logical cpus at this level
+                    entry.cpuid.eax = 1; // Shift to get id of next level
+                    entry.cpuid.ebx = 2; // Number of logical cpus at this level
                 } else {
                     // One core contain all the cpu_count hyperthreads
                     let cpu_bits: u32 = 32 - ((ctx.cpu_count - 1) as u32).leading_zeros();
-                    entry.eax = cpu_bits; // Shift to get id of next level
-                    entry.ebx = ctx.cpu_count as u32; // Number of logical cpus at this level
+                    entry.cpuid.eax = cpu_bits; // Shift to get id of next level
+                    entry.cpuid.ebx = ctx.cpu_count as u32; // Number of logical cpus at this level
                 }
-                entry.ecx = (ECX_TOPO_SMT_TYPE << ECX_TOPO_TYPE_SHIFT) | entry.index;
+                entry.cpuid.ecx = (ECX_TOPO_SMT_TYPE << ECX_TOPO_TYPE_SHIFT) | entry.index;
             } else if entry.index == 1 {
                 let cpu_bits: u32 = 32 - ((ctx.cpu_count - 1) as u32).leading_zeros();
-                entry.eax = cpu_bits;
-                entry.ebx = (ctx.cpu_count as u32) & 0xffff; // Number of logical cpus at this level
-                entry.ecx = (ECX_TOPO_CORE_TYPE << ECX_TOPO_TYPE_SHIFT) | entry.index;
+                entry.cpuid.eax = cpu_bits;
+                // Number of logical cpus at this level
+                entry.cpuid.ebx = (ctx.cpu_count as u32) & 0xffff;
+                entry.cpuid.ecx = (ECX_TOPO_CORE_TYPE << ECX_TOPO_TYPE_SHIFT) | entry.index;
             } else {
-                entry.eax = 0;
-                entry.ebx = 0;
-                entry.ecx = 0;
+                entry.cpuid.eax = 0;
+                entry.cpuid.ebx = 0;
+                entry.cpuid.ecx = 0;
             }
         }
         _ => (),
@@ -291,7 +280,14 @@ fn filter_cpuid(cpuid: &mut hypervisor::CpuId, ctx: &CpuIdContext) {
     {
         cpuid.cpu_id_entries.push(CpuIdEntry {
             function: 0x15,
-            ..Default::default()
+            index: 0,
+            flags: 0,
+            cpuid: CpuidResult {
+                eax: 0,
+                ebx: 0,
+                ecx: 0,
+                edx: 0,
+            },
         })
     }
 
@@ -340,6 +336,8 @@ pub fn setup_cpuid(
             Some(irq_chip),
             enable_pnp_data,
             itmt,
+            __cpuid_count,
+            __cpuid,
         ),
     );
 
@@ -393,28 +391,96 @@ mod tests {
         let entries = &mut cpuid.cpu_id_entries;
         entries.push(hypervisor::CpuIdEntry {
             function: 0,
-            ..Default::default()
+            index: 0,
+            flags: 0,
+            cpuid: CpuidResult {
+                eax: 0,
+                ebx: 0,
+                ecx: 0,
+                edx: 0,
+            },
         });
         entries.push(hypervisor::CpuIdEntry {
             function: 1,
-            ecx: 0x10,
-            edx: 0,
-            ..Default::default()
+            index: 0,
+            flags: 0,
+            cpuid: CpuidResult {
+                eax: 0,
+                ebx: 0,
+                ecx: 0x10,
+                edx: 0,
+            },
         });
         filter_cpuid(
             &mut cpuid,
-            &CpuIdContext::new(1, 2, false, false, Some(&irq_chip), false, false),
+            &CpuIdContext::new(
+                1,
+                2,
+                false,
+                false,
+                Some(&irq_chip),
+                false,
+                false,
+                __cpuid_count,
+                __cpuid,
+            ),
         );
 
         let entries = &mut cpuid.cpu_id_entries;
         assert_eq!(entries[0].function, 0);
-        assert_eq!(1, (entries[1].ebx >> EBX_CPUID_SHIFT) & 0x000000ff);
-        assert_eq!(2, (entries[1].ebx >> EBX_CPU_COUNT_SHIFT) & 0x000000ff);
+        assert_eq!(1, (entries[1].cpuid.ebx >> EBX_CPUID_SHIFT) & 0x000000ff);
+        assert_eq!(
+            2,
+            (entries[1].cpuid.ebx >> EBX_CPU_COUNT_SHIFT) & 0x000000ff
+        );
         assert_eq!(
             EBX_CLFLUSH_CACHELINE,
-            (entries[1].ebx >> EBX_CLFLUSH_SIZE_SHIFT) & 0x000000ff
+            (entries[1].cpuid.ebx >> EBX_CLFLUSH_SIZE_SHIFT) & 0x000000ff
         );
-        assert_ne!(0, entries[1].ecx & (1 << ECX_HYPERVISOR_SHIFT));
-        assert_ne!(0, entries[1].edx & (1 << EDX_HTT_SHIFT));
+        assert_ne!(0, entries[1].cpuid.ecx & (1 << ECX_HYPERVISOR_SHIFT));
+        assert_ne!(0, entries[1].cpuid.edx & (1 << EDX_HTT_SHIFT));
+    }
+
+    #[test]
+    fn cpuid_copies_register() {
+        let fake_cpuid_count = |_function: u32, _index: u32| CpuidResult {
+            eax: 27,
+            ebx: 18,
+            ecx: 28,
+            edx: 18,
+        };
+        let fake_cpuid = |_function: u32| CpuidResult {
+            eax: 0,
+            ebx: 0,
+            ecx: 0,
+            edx: 0,
+        };
+        let ctx = CpuIdContext {
+            vcpu_id: 0,
+            cpu_count: 0,
+            no_smt: false,
+            x2apic: false,
+            tsc_deadline_timer: false,
+            apic_frequency: 0,
+            tsc_frequency: None,
+            host_cpu_topology: true,
+            enable_pnp_data: false,
+            itmt: false,
+            cpuid_count: fake_cpuid_count,
+            cpuid: fake_cpuid,
+        };
+        let mut cpu_id_entry = CpuIdEntry {
+            function: 0x4,
+            index: 0,
+            flags: 0,
+            cpuid: CpuidResult {
+                eax: 31,
+                ebx: 41,
+                ecx: 59,
+                edx: 26,
+            },
+        };
+        adjust_cpuid(&mut cpu_id_entry, &ctx);
+        assert_eq!(cpu_id_entry.cpuid.eax, 27)
     }
 }

x86_64/src/lib.rs

@@ -698,7 +698,7 @@ impl arch::LinuxArch for X8664arch {
                 .map_err(Error::Cmdline)?;
         }
 
-        let vcpu_init = VcpuInitX86_64 {};
+        let mut vcpu_init = VcpuInitX86_64::default();
 
         match components.vm_image {
             VmImage::Bios(ref mut bios) => {
@@ -710,11 +710,10 @@ impl arch::LinuxArch for X8664arch {
                 )
                 .map_err(Error::LoadCmdline)?;
                 Self::load_bios(&mem, bios)?
+                // RIP and CS will be configured by `set_reset_vector()` later.
             }
             VmImage::Kernel(ref mut kernel_image) => {
-                // separate out load_kernel from other setup to get a specific error for
-                // kernel loading
-                let (params, kernel_end) = Self::load_kernel(&mem, kernel_image)?;
+                let (params, kernel_end, kernel_entry) = Self::load_kernel(&mem, kernel_image)?;
 
                 Self::setup_system_memory(
                     &mem,
@@ -724,6 +723,12 @@ impl arch::LinuxArch for X8664arch {
                     kernel_end,
                     params,
                 )?;
+
+                // Configure the VCPU for the Linux/x86 64-bit boot protocol.
+                // <https://www.kernel.org/doc/html/latest/x86/boot.html>
+                vcpu_init.regs.rip = kernel_entry.offset();
+                vcpu_init.regs.rsp = BOOT_STACK_POINTER;
+                vcpu_init.regs.rsi = ZERO_PAGE_OFFSET;
             }
         }
 
@@ -757,7 +762,7 @@ impl arch::LinuxArch for X8664arch {
         hypervisor: &dyn HypervisorX86_64,
         irq_chip: &mut dyn IrqChipX86_64,
         vcpu: &mut dyn VcpuX86_64,
-        _vcpu_init: &VcpuInitX86_64,
+        vcpu_init: &VcpuInitX86_64,
         vcpu_id: usize,
         num_cpus: usize,
         has_bios: bool,
@@ -788,18 +793,8 @@ impl arch::LinuxArch for X8664arch {
         }
 
         let guest_mem = vm.get_memory();
-        let kernel_load_addr = GuestAddress(KERNEL_START_OFFSET);
         regs::setup_msrs(vm, vcpu, read_pci_mmio_before_32bit().start).map_err(Error::SetupMsrs)?;
-        let kernel_end = guest_mem
-            .checked_offset(kernel_load_addr, KERNEL_64BIT_ENTRY_OFFSET)
-            .ok_or(Error::KernelOffsetPastEnd)?;
-        regs::setup_regs(
-            vcpu,
-            (kernel_end).offset() as u64,
-            BOOT_STACK_POINTER as u64,
-            ZERO_PAGE_OFFSET as u64,
-        )
-        .map_err(Error::SetupRegs)?;
+        vcpu.set_regs(&vcpu_init.regs).map_err(Error::WriteRegs)?;
         regs::setup_fpu(vcpu).map_err(Error::SetupFpu)?;
         regs::setup_sregs(guest_mem, vcpu).map_err(Error::SetupSregs)?;
         interrupts::set_lint(vcpu_id, irq_chip).map_err(Error::SetLint)?;
@@ -1218,15 +1213,37 @@ impl X8664arch {
     ///
     /// * `mem` - The memory to be used by the guest.
     /// * `kernel_image` - the File object for the specified kernel.
-    fn load_kernel(mem: &GuestMemory, kernel_image: &mut File) -> Result<(boot_params, u64)> {
-        let elf_result =
-            kernel_loader::load_kernel(mem, GuestAddress(KERNEL_START_OFFSET), kernel_image);
-        if elf_result == Err(kernel_loader::Error::InvalidElfMagicNumber) {
-            bzimage::load_bzimage(mem, GuestAddress(KERNEL_START_OFFSET), kernel_image)
-                .map_err(Error::LoadBzImage)
-        } else {
-            let loaded_kernel = elf_result.map_err(Error::LoadKernel)?;
-            Ok((Default::default(), loaded_kernel.end.offset()))
+    ///
+    /// # Returns
+    ///
+    /// On success, returns the Linux x86_64 boot protocol parameters, the first address past the
+    /// end of the kernel, and the entry point (initial `RIP` value).
+    fn load_kernel(
+        mem: &GuestMemory,
+        kernel_image: &mut File,
+    ) -> Result<(boot_params, u64, GuestAddress)> {
+        let kernel_start = GuestAddress(KERNEL_START_OFFSET);
+        match kernel_loader::load_kernel(mem, kernel_start, kernel_image) {
+            Ok(loaded_kernel) => {
+                // ELF kernels don't contain a `boot_params` structure, so synthesize a default one.
+                let boot_params = Default::default();
+                Ok((
+                    boot_params,
+                    loaded_kernel.address_range.end,
+                    loaded_kernel.entry,
+                ))
+            }
+            Err(kernel_loader::Error::InvalidElfMagicNumber) => {
+                // The image failed to parse as ELF, so try to load it as a bzImage.
+                let (boot_params, bzimage_end) =
+                    bzimage::load_bzimage(mem, kernel_start, kernel_image)
+                        .map_err(Error::LoadBzImage)?;
+                let bzimage_entry = mem
+                    .checked_offset(kernel_start, KERNEL_64BIT_ENTRY_OFFSET)
+                    .ok_or(Error::KernelOffsetPastEnd)?;
+                Ok((boot_params, bzimage_end, bzimage_entry))
+            }
+            Err(e) => Err(Error::LoadKernel(e)),
         }
     }
 

x86_64/src/regs.rs

@@ -5,7 +5,7 @@
 use std::{mem, result};
 
 use base::{self, warn};
-use hypervisor::{Fpu, Register, Regs, Sregs, VcpuX86_64, Vm};
+use hypervisor::{Fpu, Register, Sregs, VcpuX86_64, Vm};
 use remain::sorted;
 use thiserror::Error;
 use vm_memory::{GuestAddress, GuestMemory};
@@ -219,27 +219,6 @@ pub fn setup_fpu(vcpu: &dyn VcpuX86_64) -> Result<()> {
     vcpu.set_fpu(&fpu).map_err(Error::FpuIoctlFailed)
 }
 
-/// Configure base registers for x86
-///
-/// # Arguments
-///
-/// * `vcpu` - Structure for the vcpu that holds the vcpu descriptor.
-/// * `boot_ip` - Starting instruction pointer.
-/// * `boot_sp` - Starting stack pointer.
-/// * `boot_si` - Must point to zero page address per Linux ABI.
-pub fn setup_regs(vcpu: &dyn VcpuX86_64, boot_ip: u64, boot_sp: u64, boot_si: u64) -> Result<()> {
-    let regs = Regs {
-        rflags: 0x0000000000000002u64,
-        rip: boot_ip,
-        rsp: boot_sp,
-        rbp: boot_sp,
-        rsi: boot_si,
-        ..Default::default()
-    };
-
-    vcpu.set_regs(&regs).map_err(Error::SettingRegistersIoctl)
-}
-
 const X86_CR0_PE: u64 = 0x1;
 const X86_CR0_PG: u64 = 0x80000000;
 const X86_CR4_PAE: u64 = 0x20;

x86_64/src/test_integration.rs

@@ -13,14 +13,15 @@ mod sys;
 use arch::LinuxArch;
 use devices::IrqChipX86_64;
 use hypervisor::{
-    HypervisorX86_64, IoOperation, IoParams, ProtectionType, VcpuExit, VcpuX86_64, VmCap, VmX86_64,
+    HypervisorX86_64, IoOperation, IoParams, ProtectionType, Regs, VcpuExit, VcpuX86_64, VmCap,
+    VmX86_64,
 };
 use resources::{AddressRange, SystemAllocator};
 use vm_memory::{GuestAddress, GuestMemory};
 
 use super::cpuid::setup_cpuid;
 use super::interrupts::set_lint;
-use super::regs::{setup_fpu, setup_msrs, setup_regs, setup_sregs};
+use super::regs::{setup_fpu, setup_msrs, setup_sregs};
 use super::X8664arch;
 use super::{
     acpi, arch_memory_regions, bootparam, init_low_memory_layout, mptable,
@@ -243,15 +244,13 @@ where
             }
             setup_msrs(&vm, &vcpu, read_pci_mmio_before_32bit().start).unwrap();
 
-            setup_regs(
-                &vcpu,
-                start_addr.offset() as u64,
-                BOOT_STACK_POINTER as u64,
-                ZERO_PAGE_OFFSET as u64,
-            )
-            .unwrap();
+            let mut vcpu_regs = Regs {
+                rip: start_addr.offset(),
+                rsp: BOOT_STACK_POINTER,
+                rsi: ZERO_PAGE_OFFSET,
+                ..Default::default()
+            };
 
-            let mut vcpu_regs = vcpu.get_regs().unwrap();
             // instruction is
             // mov [eax],ebx
             // so we're writing 0x12 (the contents of ebx) to the address