Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate initial virtio console implementation #217

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 57 additions & 2 deletions src/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ impl Cli {
.required(false)
.takes_value(true)
.help("Block device configuration. \n\tFormat: \"path=<string>\"")
)
.arg(
Arg::with_name("console")
.long("console")
.required(false)
.takes_value(true)
.help("Console configuration. \n\tFormat: \"type=<string>\" \
\nPossible values for \"type\":\
\n\t * uart - Use the serial UART console.\
\n\t * virtio - Use the virtio console.\
\nUses the UART console by default if the \"--console\" \
option is not present.")
);

// Save the usage beforehand as a string, because `get_matches` consumes the `App`.
Expand All @@ -69,6 +81,7 @@ impl Cli {
.vcpu_config(matches.value_of("vcpu"))
.net_config(matches.value_of("net"))
.block_config(matches.value_of("block"))
.console_config(matches.value_of("console"))
.build()
.map_err(|e| format!("{:?}", e))
}
Expand All @@ -82,7 +95,10 @@ mod tests {

use linux_loader::cmdline::Cmdline;

use vmm::{KernelConfig, MemoryConfig, VcpuConfig, DEFAULT_KERNEL_LOAD_ADDR};
use vmm::{
ConsoleConfig, ConsoleType, KernelConfig, MemoryConfig, VcpuConfig,
DEFAULT_KERNEL_LOAD_ADDR,
};

#[test]
fn test_launch() {
Expand Down Expand Up @@ -214,7 +230,40 @@ mod tests {
let mut foo_cmdline = Cmdline::new(4096);
foo_cmdline.insert_str("\"foo=bar bar=foo\"").unwrap();

// OK.
// OK. Virtio console.
assert_eq!(
Cli::launch(vec![
"foobar",
"--memory",
"size_mib=128",
"--vcpu",
"num=1",
"--kernel",
"path=/foo/bar,cmdline=\"foo=bar bar=foo\",kernel_load_addr=42",
"--console",
"type=virtio",
])
.unwrap(),
VMMConfig {
kernel_config: KernelConfig {
path: PathBuf::from("/foo/bar"),
cmdline: foo_cmdline,
load_addr: 42,
},
memory_config: MemoryConfig { size_mib: 128 },
vcpu_config: VcpuConfig { num: 1 },
block_config: None,
net_config: None,
console_config: Some(ConsoleConfig {
console_type: ConsoleType::Virtio
}),
}
);

let mut foo_cmdline = Cmdline::new(4096);
foo_cmdline.insert_str("\"foo=bar bar=foo\"").unwrap();

// OK. UART console.
assert_eq!(
Cli::launch(vec![
"foobar",
Expand All @@ -224,6 +273,8 @@ mod tests {
"num=1",
"--kernel",
"path=/foo/bar,cmdline=\"foo=bar bar=foo\",kernel_load_addr=42",
"--console",
"type=uart",
])
.unwrap(),
VMMConfig {
Expand All @@ -236,6 +287,9 @@ mod tests {
vcpu_config: VcpuConfig { num: 1 },
block_config: None,
net_config: None,
console_config: Some(ConsoleConfig {
console_type: ConsoleType::Uart
}),
}
);

Expand All @@ -252,6 +306,7 @@ mod tests {
vcpu_config: VcpuConfig { num: 1 },
block_config: None,
net_config: None,
console_config: None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be uart?

}
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/arch/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ edition = "2018"

[dependencies]
vm-fdt = "0.2.0"
vm-memory = "0.7.0"
vm-memory = "0.8.0"
11 changes: 6 additions & 5 deletions src/devices/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@ kvm-ioctls = "0.11.0"
libc = "0.2.76"
linux-loader = "0.4.0"
log = "0.4.6"
vm-memory = "0.7.0"
vm-memory = "0.8.0"
vm-superio = "0.5.0"
vmm-sys-util = "0.8.0"
vm-device = "0.1.0"

virtio-blk = { git = "https://github.com/rust-vmm/vm-virtio.git", features = ["backend-stdio"] }
virtio-device = { git = "https://github.com/rust-vmm/vm-virtio.git"}
virtio-queue = { git = "https://github.com/rust-vmm/vm-virtio.git"}
virtio-blk = { git = "https://github.com/rust-vmm/vm-virtio.git", rev = "1cde0af5c80aead5e44018029b5ecc0be83dea50", features = ["backend-stdio"] }
virtio-device = { git = "https://github.com/rust-vmm/vm-virtio.git", rev = "1cde0af5c80aead5e44018029b5ecc0be83dea50"}
virtio-queue = { git = "https://github.com/rust-vmm/vm-virtio.git", rev = "1cde0af5c80aead5e44018029b5ecc0be83dea50"}
virtio-console = { git = "https://github.com/rust-vmm/vm-virtio.git", rev = "1cde0af5c80aead5e44018029b5ecc0be83dea50"}

utils = { path = "../utils" }

[dev-dependencies]
vm-memory = { version = "0.7.0", features = ["backend-mmap"] }
vm-memory = { version = "0.8.0", features = ["backend-mmap"] }
122 changes: 122 additions & 0 deletions src/devices/src/virtio/console/device.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause

use crate::virtio::console::CONSOLE_DEVICE_ID;
use crate::virtio::features::{VIRTIO_F_IN_ORDER, VIRTIO_F_RING_EVENT_IDX, VIRTIO_F_VERSION_1};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we group together the crate imports, same comment for the external crates (see use virtio_console::console below)?


use std::borrow::{Borrow, BorrowMut};
use std::io::stdout;
use std::ops::DerefMut;
use std::sync::{Arc, Mutex};
use virtio_console::console;

use super::inorder_handler::InOrderQueueHandler;
use crate::virtio::console::queue_handler::QueueHandler;
use crate::virtio::{CommonConfig, Env, SingleFdSignalQueue, QUEUE_MAX_SIZE};
use virtio_device::{VirtioConfig, VirtioDeviceActions, VirtioDeviceType, VirtioMmioDevice};
use virtio_queue::Queue;
use vm_device::bus::MmioAddress;
use vm_device::device_manager::MmioManager;
use vm_device::{DeviceMmio, MutDeviceMmio};
use vm_memory::GuestAddressSpace;

use super::{Error, Result};

pub struct Console<M: GuestAddressSpace> {
cfg: CommonConfig<M>,
}

impl<M> Console<M>
where
M: GuestAddressSpace + Clone + Send + 'static,
{
pub fn new<B>(env: &mut Env<M, B>) -> Result<Arc<Mutex<Self>>>
where
// We're using this (more convoluted) bound so we can pass both references and smart
// pointers such as mutex guards here.
B: DerefMut,
B::Target: MmioManager<D = Arc<dyn DeviceMmio + Send + Sync>>,
{
let device_features =
(1 << VIRTIO_F_VERSION_1) | (1 << VIRTIO_F_IN_ORDER) | (1 << VIRTIO_F_RING_EVENT_IDX);
let queues = vec![
Queue::new(env.mem.clone(), QUEUE_MAX_SIZE),
Queue::new(env.mem.clone(), QUEUE_MAX_SIZE),
];
// TODO: Add a config space to implement the optional features of the console.
// For basic operation it can be left empty.
let config_space = Vec::new();
lauralt marked this conversation as resolved.
Show resolved Hide resolved
let virtio_cfg = VirtioConfig::new(device_features, queues, config_space);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: too many whitespaces. Why do we need them?

let common_cfg = CommonConfig::new(virtio_cfg, env).map_err(Error::Virtio)?;
let console = Arc::new(Mutex::new(Console { cfg: common_cfg }));

env.register_mmio_device(console.clone())
.map_err(Error::Virtio)?;

Ok(console)
}
}

impl<M: GuestAddressSpace + Clone + Send + 'static> VirtioDeviceType for Console<M> {
fn device_type(&self) -> u32 {
CONSOLE_DEVICE_ID
}
}

impl<M: GuestAddressSpace + Clone + Send + 'static> Borrow<VirtioConfig<M>> for Console<M> {
fn borrow(&self) -> &VirtioConfig<M> {
&self.cfg.virtio
}
}

impl<M: GuestAddressSpace + Clone + Send + 'static> BorrowMut<VirtioConfig<M>> for Console<M> {
fn borrow_mut(&mut self) -> &mut VirtioConfig<M> {
&mut self.cfg.virtio
}
}

impl<M: GuestAddressSpace + Clone + Send + 'static> VirtioDeviceActions for Console<M> {
type E = Error;

fn activate(&mut self) -> Result<()> {
let driver_notify = SingleFdSignalQueue {
irqfd: self.cfg.irqfd.clone(),
interrupt_status: self.cfg.virtio.interrupt_status.clone(),
};

let mut ioevents = self.cfg.prepare_activate().map_err(Error::Virtio)?;

let inner = InOrderQueueHandler {
driver_notify,
receiveq: self.cfg.virtio.queues.remove(0),
transmitq: self.cfg.virtio.queues.remove(0),
console: console::Console::new_with_capacity(console::DEFAULT_CAPACITY, stdout())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use here the new constructor or even the Default implementation.

.map_err(Error::Console)?,
};

let handler = Arc::new(Mutex::new(QueueHandler {
inner,
receiveqfd: ioevents.remove(0),
transmitqfd: ioevents.remove(0),
}));

self.cfg.finalize_activate(handler).map_err(Error::Virtio)
}

fn reset(&mut self) -> Result<()> {
// Not implemented for now.
Ok(())
}
}

impl<M: GuestAddressSpace + Clone + Send + 'static> VirtioMmioDevice<M> for Console<M> {}

impl<M: GuestAddressSpace + Clone + Send + 'static> MutDeviceMmio for Console<M> {
fn mmio_read(&mut self, _base: MmioAddress, offset: u64, data: &mut [u8]) {
self.read(offset, data);
}

fn mmio_write(&mut self, _base: MmioAddress, offset: u64, data: &[u8]) {
self.write(offset, data);
}
}
Loading