Skip to content

Stop virtio devices from blocking guest memory layout changes #519

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.

Sign up for GitHub

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

Draft
Lencerf wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Stop virtio devices from blocking guest memory layout changes #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion alioth/src/cpu/cpu_amd64/sev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ where
fn update_snp_desc(&self, desc: &SevMetadataDesc) -> Result<()> {
let mut cpuid_table = SnpCpuidInfo::new_zeroed();
let ram_bus = self.ctx.board.memory.ram_bus();
let ram = ram_bus.lock_layout();
let ram = ram_bus.load();
let page_type = match desc.type_ {
SevDescType::SNP_DESC_MEM => SnpPageType::UNMEASURED,
SevDescType::SNP_SECRETS => SnpPageType::SECRETS,
Expand Down
4 changes: 2 additions & 2 deletions alioth/src/mem/addressable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub trait SlotBackend {
fn size(&self) -> u64;
}

#[derive(Debug)]
#[derive(Debug, Clone)]
struct Slot<B>
where
B: SlotBackend,
Expand Down Expand Up @@ -52,7 +52,7 @@ where
}
}

#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct Addressable<B>
where
B: SlotBackend,
Expand Down
78 changes: 44 additions & 34 deletions alioth/src/mem/mapped.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use libc::{
MAP_ANONYMOUS, MAP_FAILED, MAP_PRIVATE, MAP_SHARED, MS_ASYNC, PROT_READ, PROT_WRITE, c_void,
madvise, mmap, msync, munmap,
};
use parking_lot::{RwLock, RwLockReadGuard};
use parking_lot::RwLock;
use snafu::ResultExt;
use zerocopy::{FromBytes, Immutable, IntoBytes};

Expand Down Expand Up @@ -180,14 +180,24 @@ impl ArcMemPages {
}
}

#[derive(Debug)]
/// A snapshot of the guest RAM layout.
///
/// A `Ram` does not change once a [`RamBus`] publishes it. Its slots hold
/// reference-counted [`ArcMemPages`], so a snapshot keeps the host mappings
/// it refers to alive, even after they are removed from the bus.
#[derive(Debug, Clone, Default)]
pub struct Ram {
inner: Addressable<ArcMemPages>,
}

#[derive(Debug)]
/// The guest RAM layout, updated in the read-copy-update (RCU) way.
///
/// A reader takes a snapshot with [`RamBus::load`] and keeps it as long as
/// it needs, while a writer publishes a new layout with [`RamBus::update`]
/// without waiting for readers.
#[derive(Debug, Default)]
pub struct RamBus {
ram: RwLock<Ram>,
ram: RwLock<Arc<Ram>>,
}

struct Iter<'m> {
Expand Down Expand Up @@ -397,74 +407,74 @@ impl Ram {
}
Ok(())
}
}

impl Default for RamBus {
fn default() -> Self {
Self::new()
pub(crate) fn add(&mut self, gpa: u64, user_mem: ArcMemPages) -> Result<()> {
self.inner.add(gpa, user_mem)?;
Ok(())
}
}

impl RamBus {
pub fn lock_layout(&self) -> RwLockReadGuard<'_, Ram> {
self.ram.read()
pub(crate) fn remove(&mut self, gpa: u64) -> Result<ArcMemPages> {
self.inner.remove(gpa)
}
}

impl RamBus {
pub fn new() -> Self {
Self {
ram: RwLock::new(Ram {
inner: Addressable::default(),
}),
}
Self::default()
}

pub(crate) fn add(&self, gpa: u64, user_mem: ArcMemPages) -> Result<(), Error> {
let mut ram = self.ram.write();
ram.inner.add(gpa, user_mem)?;
Ok(())
/// Returns a snapshot of the current layout.
///
/// The snapshot stays valid for as long as the caller holds it, but does
/// not see later updates.
pub fn load(&self) -> Arc<Ram> {
self.ram.read().clone()
}

pub(crate) fn remove(&self, gpa: u64) -> Result<ArcMemPages, Error> {
let mut ram = self.ram.write();
ram.inner.remove(gpa)
/// Copies the current layout, changes the copy with `f` and publishes it.
///
/// If `f` fails, nothing is published, so a change of several slots is
/// either fully visible or not visible at all.
pub(crate) fn update<T>(&self, f: impl FnOnce(&mut Ram) -> Result<T>) -> Result<T> {
let mut current = self.ram.write();
let mut ram = Ram::clone(&current);
let ret = f(&mut ram)?;
*current = Arc::new(ram);
Ok(ret)
}

pub fn read(&self, gpa: u64, buf: &mut [u8]) -> Result<()> {
let ram = self.ram.read();
ram.read(gpa, buf)
self.load().read(gpa, buf)
}

pub fn write(&self, gpa: u64, buf: &[u8]) -> Result<()> {
let ram = self.ram.read();
ram.write(gpa, buf)
self.load().write(gpa, buf)
}

pub fn read_t<T>(&self, gpa: u64) -> Result<T, Error>
where
T: FromBytes + IntoBytes,
{
let ram = self.ram.read();
ram.read_t(gpa)
self.load().read_t(gpa)
}

pub fn write_t<T>(&self, gpa: u64, val: &T) -> Result<(), Error>
where
T: IntoBytes + Immutable,
{
let ram = self.ram.read();
ram.write_t(gpa, val)
self.load().write_t(gpa, val)
}

pub fn read_range(&self, gpa: u64, len: u64, dst: &mut impl Write) -> Result<()> {
let ram = self.ram.read();
let ram = self.load();
for r in ram.slice_iter(gpa, len) {
dst.write_all(r?).context(error::Write)?;
}
Ok(())
}

pub fn write_range(&self, gpa: u64, len: u64, mut src: impl Read) -> Result<()> {
let ram = self.ram.read();
let ram = self.load();
for r in ram.slice_iter_mut(gpa, len) {
src.read_exact(r?).context(error::Read)?;
}
Expand Down
48 changes: 37 additions & 11 deletions alioth/src/mem/mapped_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ fn test_ram_bus_read() {
let mem2 = ArcMemPages::from_anonymous(PAGE_SIZE as usize, Some(prot), None).unwrap();

if mem1.addr > mem2.addr {
bus.add(0x0, mem1).unwrap();
bus.add(PAGE_SIZE, mem2).unwrap();
bus.update(|ram| ram.add(0x0, mem1)).unwrap();
bus.update(|ram| ram.add(PAGE_SIZE, mem2)).unwrap();
} else {
bus.add(0x0, mem2).unwrap();
bus.add(PAGE_SIZE, mem1).unwrap();
bus.update(|ram| ram.add(0x0, mem2)).unwrap();
bus.update(|ram| ram.add(PAGE_SIZE, mem1)).unwrap();
}

let data = MyStruct {
Expand Down Expand Up @@ -75,19 +75,45 @@ fn test_ram_bus_read() {

let guest_iov = [(0, 16), (PAGE_SIZE - 16, 32), (2 * PAGE_SIZE - 16, 16)];

let locked_bus = bus.lock_layout();
let mut iov = locked_bus.translate_iov_mut(&guest_iov).unwrap();
let ram = bus.load();
let mut iov = ram.translate_iov_mut(&guest_iov).unwrap();
assert_eq!(iov.len(), 4);
assert_matches!((&*data).read_vectored(&mut iov), Ok(64));
drop(iov);
let iov = locked_bus.translate_iov(&guest_iov).unwrap();
let iov = ram.translate_iov(&guest_iov).unwrap();
assert_eq!(iov.len(), 4);
let mut buf_read = Vec::new();
assert_matches!(buf_read.write_vectored(&iov), Ok(64));
assert_eq!(buf_read, data);
drop(iov);
drop(locked_bus);
bus.remove(0x0).unwrap();
drop(ram);
bus.update(|ram| ram.remove(0x0)).unwrap();
}

#[test]
fn test_ram_bus_snapshot() {
let bus = RamBus::new();
let empty = bus.load();

// Holding a snapshot does not block updates.
let mem = ArcMemPages::from_anonymous(PAGE_SIZE as usize, None, None).unwrap();
bus.update(|ram| ram.add(0x0, mem.clone())).unwrap();
assert_matches!(empty.read_t::<u32>(0x0), Err(_));
let snapshot = bus.load();
snapshot.write_t(0x0, &0xdeadbeef_u32).unwrap();

// A failed update publishes nothing.
let ret = bus.update(|ram| {
ram.add(PAGE_SIZE, mem.clone())?;
ram.add(0x0, mem.clone())
});
assert_matches!(ret, Err(_));
assert_matches!(bus.read_t::<u32>(PAGE_SIZE), Err(_));

// A removed slot stays accessible through older snapshots.
bus.update(|ram| ram.remove(0x0)).unwrap();
assert_matches!(bus.read_t::<u32>(0x0), Err(_));
assert_matches!(snapshot.read_t::<u32>(0x0), Ok(0xdeadbeef));
}

#[test]
Expand All @@ -97,9 +123,9 @@ fn test_ram_get_array_ptr() {
// Two adjacent guest pages, backed by separate host mappings.
for gpa in [0, PAGE_SIZE] {
let pages = ArcMemPages::from_anonymous(PAGE_SIZE as usize, Some(prot), None).unwrap();
bus.add(gpa, pages).unwrap();
bus.update(|ram| ram.add(gpa, pages)).unwrap();
}
let ram = bus.lock_layout();
let ram = bus.load();

let ptr = ram.get_array_ptr::<u64>(PAGE_SIZE - 16, 2).unwrap();
assert_eq!(ptr, ram.get_ptr::<u64>(PAGE_SIZE - 16).unwrap());
Expand Down
53 changes: 38 additions & 15 deletions alioth/src/mem/mem.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,19 @@ struct LayoutCallbacks {
updated: Vec<Box<dyn LayoutUpdated>>,
}

/// Iterates over the RAM-backed ranges of `region` placed at `addr`.
fn ram_ranges(addr: u64, region: &MemRegion) -> impl Iterator<Item = (u64, &ArcMemPages)> {
let ranges = region.ranges.iter().scan(addr, |gpa, range| {
let start = *gpa;
*gpa += range.size();
Some((start, range))
});
ranges.filter_map(|(gpa, range)| match range {
MemRange::Ram(pages) | MemRange::DevMem { pages, .. } => Some((gpa, pages)),
MemRange::Emulated(_) | MemRange::Span(_) => None,
})
}

// lock order: region -> callbacks -> bus
#[derive(Debug, Default)]
pub struct Memory {
Expand Down Expand Up @@ -308,7 +321,7 @@ impl Memory {
pub fn register_update_callback(&self, callback: Box<dyn LayoutUpdated>) -> Result<()> {
let _regions = self.regions.lock();
let mut callbacks = self.callbacks.lock();
let ram = self.ram_bus.lock_layout();
let ram = self.ram_bus.load();
callback.ram_updated(&ram)?;
callbacks.updated.push(callback);
Ok(())
Expand All @@ -332,15 +345,20 @@ impl Memory {
region.validate()?;
let mut regions = self.regions.lock();
regions.add(addr, region.clone())?;
let mut offset = 0;
let callbacks = self.callbacks.lock();
let mut ram_updated = false;
let ram_ranges: Vec<_> = ram_ranges(addr, &region).collect();
let ram_updated = !ram_ranges.is_empty();
if ram_updated {
self.ram_bus.update(|ram| {
for (gpa, pages) in ram_ranges {
ram.add(gpa, pages.clone())?;
}
Ok(())
})?;
}
let mut offset = 0;
for range in &region.ranges {
let gpa = addr + offset;
if let MemRange::Ram(pages) | MemRange::DevMem { pages, .. } = range {
self.ram_bus.add(gpa, pages.clone())?;
ram_updated = true;
}
match range {
MemRange::Emulated(r) => {
let mut mmio_bus = self.mmio_bus.write();
Expand All @@ -361,7 +379,7 @@ impl Memory {
offset += range.size();
}
if ram_updated {
let ram = self.ram_bus.lock_layout();
let ram = self.ram_bus.load();
for update_callback in &callbacks.updated {
update_callback.ram_updated(&ram)?;
}
Expand All @@ -374,15 +392,20 @@ impl Memory {
}

fn unmap_region(&self, addr: u64, region: &MemRegion) -> Result<()> {
let mut offset = 0;
let callbacks = self.callbacks.lock();
let mut ram_updated = false;
let ram_ranges: Vec<_> = ram_ranges(addr, region).collect();
let ram_updated = !ram_ranges.is_empty();
if ram_updated {
self.ram_bus.update(|ram| {
for (gpa, _) in ram_ranges {
ram.remove(gpa)?;
}
Ok(())
})?;
}
let mut offset = 0;
for range in &region.ranges {
let gpa = addr + offset;
if let MemRange::Ram(_) | MemRange::DevMem { .. } = range {
self.ram_bus.remove(gpa)?;
ram_updated = true;
}
match range {
MemRange::Emulated(_) => {
let mut mmio_bus = self.mmio_bus.write();
Expand All @@ -403,7 +426,7 @@ impl Memory {
offset += range.size();
}
if ram_updated {
let ram = self.ram_bus.lock_layout();
let ram = self.ram_bus.load();
for callback in &callbacks.updated {
callback.ram_updated(&ram)?;
}
Expand Down
5 changes: 3 additions & 2 deletions alioth/src/virtio/dev/dev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ where

fn run(&mut self, param: &StartParam<S>) -> Result<()> {
let memory = self.context.param.memory.clone();
let ram = memory.lock_layout();
let ram = memory.load();
let feature = param.feature & !VirtioFeature::ACCESS_PLATFORM.bits();
let queue_regs = self.context.param.queue_regs.clone();
let feature = VirtioFeature::from_bits_retain(feature);
Expand Down Expand Up @@ -656,7 +656,8 @@ where
if let Err(e) = self.loop_until_reset() {
log::error!("worker {}: {e:?}", self.context.dev.name());
}
// The memory layout lock is released by now.
// The worker has dropped the queues and its guest memory
// snapshot by now.
if let Some(seq) = self.context.reset_seq.take() {
self.context.ack_reset(seq);
}
Expand Down
Loading
Loading