From ed22f6b611e2623f5836ccf1288adf6b46146088 Mon Sep 17 00:00:00 2001 From: Charles William Dick Date: Wed, 8 Apr 2020 11:05:24 +0900 Subject: crosvm balloon_stats command In preparation for moving balloon sizing logic from crosvm to concierge, expose a balloon_stats command in crosvm. This will allow concierge to query the actual balloon size and available memory of VMs. BUG=b:153134684 TEST=(chroot)$ tast run arc.Boot.vm; (vm)$ crosvm balloon_stats ; See stats are reported. Change-Id: I1f544526ee728a085d842035754a0c17cf41ed3f Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2141752 Tested-by: Charles Dueck Tested-by: kokoro Reviewed-by: Chirantan Ekbote Commit-Queue: Charles Dueck --- vm_control/src/lib.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 4 deletions(-) (limited to 'vm_control') diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs index 4035c1c..585b162 100644 --- a/vm_control/src/lib.rs +++ b/vm_control/src/lib.rs @@ -123,6 +123,43 @@ pub struct BalloonStats { pub hugetlb_failures: Option, } +impl Display for BalloonStats { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{{")?; + if let Some(swap_in) = self.swap_in { + write!(f, "\n swap_in: {}", swap_in)?; + } + if let Some(swap_out) = self.swap_out { + write!(f, "\n swap_out: {}", swap_out)?; + } + if let Some(major_faults) = self.major_faults { + write!(f, "\n major_faults: {}", major_faults)?; + } + if let Some(minor_faults) = self.minor_faults { + write!(f, "\n minor_faults: {}", minor_faults)?; + } + if let Some(free_memory) = self.free_memory { + write!(f, "\n free_memory: {}", free_memory)?; + } + if let Some(total_memory) = self.total_memory { + write!(f, "\n total_memory: {}", total_memory)?; + } + if let Some(available_memory) = self.available_memory { + write!(f, "\n available_memory: {}", available_memory)?; + } + if let Some(disk_caches) = self.disk_caches { + write!(f, "\n disk_caches: {}", disk_caches)?; + } + if let Some(hugetlb_allocations) = self.hugetlb_allocations { + write!(f, "\n hugetlb_allocations: {}", hugetlb_allocations)?; + } + if let Some(hugetlb_failures) = self.hugetlb_failures { + write!(f, "\n hugetlb_failures: {}", hugetlb_failures)?; + } + write!(f, "\n}}") + } +} + #[derive(MsgOnSocket, Debug)] pub enum BalloonControlResult { Stats { @@ -574,10 +611,30 @@ impl VmRequest { *run_mode = Some(VmRunMode::Running); VmResponse::Ok } - VmRequest::BalloonCommand(ref command) => match balloon_host_socket.send(command) { - Ok(_) => VmResponse::Ok, - Err(_) => VmResponse::Err(SysError::last()), - }, + VmRequest::BalloonCommand(BalloonControlCommand::Adjust { num_bytes }) => { + match balloon_host_socket.send(&BalloonControlCommand::Adjust { num_bytes }) { + Ok(_) => VmResponse::Ok, + Err(_) => VmResponse::Err(SysError::last()), + } + } + VmRequest::BalloonCommand(BalloonControlCommand::Stats) => { + match balloon_host_socket.send(&BalloonControlCommand::Stats {}) { + Ok(_) => match balloon_host_socket.recv() { + Ok(BalloonControlResult::Stats { + stats, + balloon_actual, + }) => VmResponse::BalloonStats { + stats, + balloon_actual, + }, + Err(e) => { + error!("balloon socket recv failed: {}", e); + VmResponse::Err(SysError::last()) + } + }, + Err(_) => VmResponse::Err(SysError::last()), + } + } VmRequest::DiskCommand { disk_index, ref command, @@ -639,6 +696,11 @@ pub enum VmResponse { slot: u32, desc: GpuMemoryDesc, }, + /// Results of balloon control commands. + BalloonStats { + stats: BalloonStats, + balloon_actual: u64, + }, /// Results of usb control commands. UsbResponse(UsbControlResult), } @@ -660,6 +722,14 @@ impl Display for VmResponse { "gpu memory allocated and registered to page frame number {:#x} and memory slot {}", pfn, slot ), + BalloonStats { + stats, + balloon_actual, + } => write!( + f, + "balloon size: {}\nballoon stats: {}", + balloon_actual, stats + ), UsbResponse(result) => write!(f, "usb control request get result {:?}", result), } } -- cgit 1.4.1 From 2d9dde9ee7ac78a9831d1e019e40107b5691f895 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Fri, 24 Apr 2020 19:37:04 +0900 Subject: Stop tracking sub-mappings in MemoryMappingArena The kernel already takes care of tracking all our memory mappings. Doing it again ourselves doesn't provide any benefit and also adds additional restrictions (like not being able to overlap with existing mappings or partially remove mappings). Additionally, the `MemoryMappingArena` will already unmap the entire memory mapped region so there is no need to individually unmap the sub-mappings. The kernel's mmap api doesn't have these restrictions and as far as I can tell there are no safety concerns with allowing this behavior so just stop tracking the sub-mappings. Safe use of MAP_FIXED only requires that the address is part of a previously mmaped region so allow any MemoryMapping to be converted into a MemoryMappedArena. BUG=b:147341783 TEST=unit tests Change-Id: Iaf944a971b8ba9333802aab73c1d184fe388af89 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2162542 Tested-by: kokoro Commit-Queue: Chirantan Ekbote Reviewed-by: Stephen Barber --- devices/src/virtio/pmem.rs | 11 ++- sys_util/src/mmap.rs | 200 ++++++++++++++++++++++----------------------- vm_control/src/lib.rs | 14 ++-- 3 files changed, 118 insertions(+), 107 deletions(-) (limited to 'vm_control') diff --git a/devices/src/virtio/pmem.rs b/devices/src/virtio/pmem.rs index 499e110..cbeecc3 100644 --- a/devices/src/virtio/pmem.rs +++ b/devices/src/virtio/pmem.rs @@ -8,8 +8,8 @@ use std::io; use std::os::unix::io::{AsRawFd, RawFd}; use std::thread; -use sys_util::Result as SysResult; use sys_util::{error, EventFd, GuestAddress, GuestMemory, PollContext, PollToken}; +use sys_util::{Error as SysError, Result as SysResult}; use data_model::{DataInit, Le32, Le64}; @@ -89,6 +89,7 @@ struct Worker { memory: GuestMemory, pmem_device_socket: VmMsyncRequestSocket, mapping_arena_slot: u32, + mapping_size: usize, } impl Worker { @@ -98,6 +99,7 @@ impl Worker { let request = VmMsyncRequest::MsyncArena { slot: self.mapping_arena_slot, offset: 0, // The pmem backing file is always at offset 0 in the arena. + size: self.mapping_size, }; if let Err(e) = self.pmem_device_socket.send(&request) { @@ -235,6 +237,10 @@ impl Pmem { mapping_size: u64, pmem_device_socket: Option, ) -> SysResult { + if mapping_size > usize::max_value() as u64 { + return Err(SysError::new(libc::EOVERFLOW)); + } + Ok(Pmem { kill_event: None, worker_thread: None, @@ -308,6 +314,8 @@ impl VirtioDevice for Pmem { let queue_event = queue_events.remove(0); let mapping_arena_slot = self.mapping_arena_slot; + // We checked that this fits in a usize in `Pmem::new`. + let mapping_size = self.mapping_size as usize; if let Some(pmem_device_socket) = self.pmem_device_socket.take() { let (self_kill_event, kill_event) = @@ -329,6 +337,7 @@ impl VirtioDevice for Pmem { queue, pmem_device_socket, mapping_arena_slot, + mapping_size, }; worker.run(queue_event, kill_event); }); diff --git a/sys_util/src/mmap.rs b/sys_util/src/mmap.rs index 006b0a8..c6a52ea 100644 --- a/sys_util/src/mmap.rs +++ b/sys_util/src/mmap.rs @@ -6,10 +6,9 @@ //! mmap object leaves scope. use std::cmp::min; -use std::collections::BTreeMap; use std::fmt::{self, Display}; use std::io; -use std::mem::{size_of, ManuallyDrop}; +use std::mem::size_of; use std::os::unix::io::AsRawFd; use std::ptr::{copy_nonoverlapping, null_mut, read_unaligned, write_unaligned}; @@ -28,8 +27,6 @@ pub enum Error { InvalidOffset, /// Requested mapping is not page aligned NotPageAligned, - /// Overlapping regions - Overlapping(usize, usize), /// Requested memory range spans past the end of the region. InvalidRange(usize, usize, usize), /// `mmap` returned the given error. @@ -49,11 +46,6 @@ impl Display for Error { InvalidAddress => write!(f, "requested memory out of range"), InvalidOffset => write!(f, "requested offset is out of range of off_t"), NotPageAligned => write!(f, "requested memory is not page aligned"), - Overlapping(offset, count) => write!( - f, - "requested memory range overlaps with existing region: offset={} size={}", - offset, count - ), InvalidRange(offset, count, region_size) => write!( f, "requested memory range spans past the end of the region: offset={} count={} region_size={}", @@ -643,13 +635,6 @@ impl Drop for MemoryMapping { pub struct MemoryMappingArena { addr: *mut u8, size: usize, - // When doing in-place swaps of MemoryMappings, the BTreeMap returns a owned - // instance of the old MemoryMapping. When the old MemoryMapping falls out - // of scope, it calls munmap on the same region as the new MemoryMapping - // that was just mapped in. To avoid accidentally munmapping the new, - // MemoryMapping, all mappings are wrapped in a ManuallyDrop, and then - // "forgotten" when removed from the BTreeMap - maps: BTreeMap>, } // Send and Sync aren't automatically inherited for the raw address pointer. @@ -666,17 +651,7 @@ impl MemoryMappingArena { /// * `size` - Size of memory region in bytes. pub fn new(size: usize) -> Result { // Reserve the arena's memory using an anonymous read-only mmap. - // The actual MemoryMapping object is forgotten, with - // MemoryMappingArena manually calling munmap on drop. - let mmap = MemoryMapping::new_protection(size, Protection::none().set_read())?; - let addr = mmap.as_ptr(); - let size = mmap.size(); - std::mem::forget(mmap); - Ok(MemoryMappingArena { - addr, - size, - maps: BTreeMap::new(), - }) + MemoryMapping::new_protection(size, Protection::none().set_read()).map(From::from) } /// Anonymously maps `size` bytes at `offset` bytes from the start of the arena. @@ -755,58 +730,43 @@ impl MemoryMappingArena { let mmap = unsafe { match fd { Some((fd, fd_offset)) => MemoryMapping::from_fd_offset_protection_fixed( - (self.addr as usize + offset) as *mut u8, + self.addr.add(offset), fd, size, fd_offset, prot, )?, - None => MemoryMapping::new_protection_fixed( - (self.addr as usize + offset) as *mut u8, - size, - prot, - )?, + None => MemoryMapping::new_protection_fixed(self.addr.add(offset), size, prot)?, } }; - self.maps.insert(offset, ManuallyDrop::new(mmap)); + // This mapping will get automatically removed when we drop the whole arena. + std::mem::forget(mmap); Ok(()) } - /// Removes a mapping at `offset` from the start of the arena. - /// Returns a boolean indicating if there was a mapping present at `offset`. - /// If none was present, this method is a noop. - pub fn remove(&mut self, offset: usize) -> Result { - if let Some(mmap) = self.maps.remove(&offset) { - // Instead of munmapping the memory map, leaving an unprotected hole - // in the arena, swap this mmap with an anonymous protection. - // This is safe since the memory mapping perfectly overlaps with an - // existing, known good memory mapping. - let mmap = unsafe { - MemoryMapping::new_protection_fixed( - mmap.as_ptr(), - mmap.size(), - Protection::none().set_read(), - )? - }; - self.maps.insert(offset, ManuallyDrop::new(mmap)); - Ok(true) - } else { - Ok(false) - } + /// Removes `size` bytes at `offset` bytes from the start of the arena. `offset` must be page + /// aligned. + /// + /// # Arguments + /// * `offset` - Page aligned offset into the arena in bytes. + /// * `size` - Size of memory region in bytes. + pub fn remove(&mut self, offset: usize, size: usize) -> Result<()> { + self.try_add(offset, size, Protection::read(), None) } - /// Calls msync with MS_SYNC on the mapping at `offset` from the start of - /// the arena. - /// Returns a boolean indicating if there was a mapping present at `offset`. - /// If none was present, this method is a noop. - pub fn msync(&self, offset: usize) -> Result { - if let Some(mmap) = self.maps.get(&offset) { - mmap.msync()?; - Ok(true) - } else { - Ok(false) + /// Calls msync with MS_SYNC on a mapping of `size` bytes starting at `offset` from the start of + /// the arena. `offset` must be page aligned. + pub fn msync(&self, offset: usize, size: usize) -> Result<()> { + self.validate_range(offset, size)?; + + // Safe because we've validated that this memory range is owned by this `MemoryMappingArena`. + let ret = + unsafe { libc::msync(self.addr as *mut libc::c_void, self.size(), libc::MS_SYNC) }; + if ret == -1 { + return Err(Error::SystemCallFailed(errno::Error::last())); } + Ok(()) } /// Returns a pointer to the beginning of the memory region. Should only be @@ -836,32 +796,26 @@ impl MemoryMappingArena { if end_offset > self.size { return Err(Error::InvalidAddress); } - // Ensure offset..offset+size doesn't overlap with existing regions - // Find the offset + size of the first mapping before the desired offset - let (prev_offset, prev_size) = match self.maps.range(..offset).rev().next() { - Some((offset, mmap)) => (*offset, mmap.size()), - None => { - // Empty map - return Ok(()); - } - }; - if offset == prev_offset { - // Perfectly overlapping regions are allowed - if size != prev_size { - return Err(Error::Overlapping(offset, size)); - } - } else if offset < (prev_offset + prev_size) { - return Err(Error::Overlapping(offset, size)); - } - Ok(()) } } +impl From for MemoryMappingArena { + fn from(mmap: MemoryMapping) -> Self { + let addr = mmap.as_ptr(); + let size = mmap.size(); + + // Forget the original mapping because the `MemoryMappingArena` will take care of calling + // `munmap` when it is dropped. + std::mem::forget(mmap); + MemoryMappingArena { addr, size } + } +} + impl Drop for MemoryMappingArena { fn drop(&mut self) { - // This is safe because we mmap the area at addr ourselves, and nobody - // else is holding a reference to it. + // This is safe because we own this memory range, and nobody else is holding a reference to + // it. unsafe { libc::munmap(self.addr as *mut libc::c_void, self.size); } @@ -977,22 +931,8 @@ mod tests { fn arena_remove() { let mut m = MemoryMappingArena::new(0x40000).unwrap(); assert!(m.add_anon(0, pagesize() * 4).is_ok()); - assert!(m.remove(0).unwrap(), true); - assert!(m.remove(0).unwrap(), false); - } - - #[test] - fn arena_add_overlap_error() { - let page = pagesize(); - let mut m = MemoryMappingArena::new(page * 4).unwrap(); - assert!(m.add_anon(0, page * 4).is_ok()); - let res = m.add_anon(page, page).unwrap_err(); - match res { - Error::Overlapping(a, o) => { - assert_eq!((a, o), (page, page)); - } - e => panic!("unexpected error: {}", e), - } + assert!(m.remove(0, pagesize()).is_ok()); + assert!(m.remove(0, pagesize() * 2).is_ok()); } #[test] @@ -1015,4 +955,62 @@ mod tests { e => panic!("unexpected error: {}", e), } } + + #[test] + fn arena_add_overlapping() { + let ps = pagesize(); + let mut m = + MemoryMappingArena::new(12 * ps).expect("failed to create `MemoryMappingArena`"); + m.add_anon(ps * 4, ps * 4) + .expect("failed to add sub-mapping"); + + // Overlap in the front. + m.add_anon(ps * 2, ps * 3) + .expect("failed to add front overlapping sub-mapping"); + + // Overlap in the back. + m.add_anon(ps * 7, ps * 3) + .expect("failed to add back overlapping sub-mapping"); + + // Overlap the back of the first mapping, all of the middle mapping, and the front of the + // last mapping. + m.add_anon(ps * 3, ps * 6) + .expect("failed to add mapping that overlaps several mappings"); + } + + #[test] + fn arena_remove_overlapping() { + let ps = pagesize(); + let mut m = + MemoryMappingArena::new(12 * ps).expect("failed to create `MemoryMappingArena`"); + m.add_anon(ps * 4, ps * 4) + .expect("failed to add sub-mapping"); + m.add_anon(ps * 2, ps * 2) + .expect("failed to add front overlapping sub-mapping"); + m.add_anon(ps * 8, ps * 2) + .expect("failed to add back overlapping sub-mapping"); + + // Remove the back of the first mapping and the front of the second. + m.remove(ps * 3, ps * 2) + .expect("failed to remove front overlapping mapping"); + + // Remove the back of the second mapping and the front of the third. + m.remove(ps * 7, ps * 2) + .expect("failed to remove back overlapping mapping"); + + // Remove a mapping that completely overlaps the middle mapping. + m.remove(ps * 5, ps * 2) + .expect("failed to remove fully overlapping mapping"); + } + + #[test] + fn arena_remove_unaligned() { + let ps = pagesize(); + let mut m = + MemoryMappingArena::new(12 * ps).expect("failed to create `MemoryMappingArena`"); + + m.add_anon(0, ps).expect("failed to add mapping"); + m.remove(0, ps - 1) + .expect("failed to remove unaligned mapping"); + } } diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs index 585b162..9e78be2 100644 --- a/vm_control/src/lib.rs +++ b/vm_control/src/lib.rs @@ -450,7 +450,12 @@ pub enum VmMsyncRequest { /// Flush the content of a memory mapping to its backing file. /// `slot` selects the arena (as returned by `Vm::add_mmap_arena`). /// `offset` is the offset of the mapping to sync within the arena. - MsyncArena { slot: u32, offset: usize }, + /// `size` is the size of the mapping to sync within the arena. + MsyncArena { + slot: u32, + offset: usize, + size: usize, + }, } #[derive(MsgOnSocket, Debug)] @@ -471,11 +476,10 @@ impl VmMsyncRequest { pub fn execute(&self, vm: &mut Vm) -> VmMsyncResponse { use self::VmMsyncRequest::*; match *self { - MsyncArena { slot, offset } => { + MsyncArena { slot, offset, size } => { if let Some(arena) = vm.get_mmap_arena(slot) { - match arena.msync(offset) { - Ok(true) => VmMsyncResponse::Ok, - Ok(false) => VmMsyncResponse::Err(SysError::new(EINVAL)), + match arena.msync(offset, size) { + Ok(()) => VmMsyncResponse::Ok, Err(e) => match e { MmapError::SystemCallFailed(errno) => VmMsyncResponse::Err(errno), _ => VmMsyncResponse::Err(SysError::new(EINVAL)), -- cgit 1.4.1 From da0e0f939b731d89d067fb9382bbdc05e47f4067 Mon Sep 17 00:00:00 2001 From: Tomasz Jeznach Date: Wed, 29 Apr 2020 12:58:11 -0700 Subject: devices: pci: refactor PCI devices to use PciAddress. Simple refactor of PCI device addressing to use PciAddress type providing bus:device.function number. BUG=None TEST=build_test & tast run crostini.Sanity Change-Id: I7755ad6b31aa8c882475cd8212630e1cc86ef49e Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2172766 Reviewed-by: Daniel Verkamp Tested-by: kokoro Commit-Queue: Tomasz Jeznach --- arch/src/lib.rs | 20 +++++++---------- devices/src/pci/ac97.rs | 36 ++++++++++++++++++++--------- devices/src/pci/pci_device.rs | 10 ++++----- devices/src/pci/vfio_pci.rs | 32 +++++++++++++------------- devices/src/usb/xhci/xhci_controller.rs | 25 ++++++++++++--------- devices/src/virtio/gpu/mod.rs | 11 +++++---- devices/src/virtio/virtio_device.rs | 4 ++-- devices/src/virtio/virtio_pci_device.rs | 40 +++++++++++++++++++-------------- resources/src/lib.rs | 4 ++-- resources/src/system_allocator.rs | 5 +++-- vm_control/src/lib.rs | 18 ++++++++++++--- 11 files changed, 122 insertions(+), 83 deletions(-) (limited to 'vm_control') diff --git a/arch/src/lib.rs b/arch/src/lib.rs index 3ef9bc1..6fedf9d 100644 --- a/arch/src/lib.rs +++ b/arch/src/lib.rs @@ -190,10 +190,13 @@ pub fn generate_pci_root( let mut pci_irqs = Vec::new(); let mut pid_labels = BTreeMap::new(); for (dev_idx, (mut device, jail)) in devices.into_iter().enumerate() { - // Auto assign PCI device numbers starting from 1. - let dev = 1 + dev_idx as u8; - // Only support one bus. - device.assign_bus_dev(0, dev); + // Auto assign PCI device numbers starting from 1 + let address = PciAddress { + bus: 0, + dev: 1 + dev_idx as u8, + func: 0, + }; + device.assign_address(address); let mut keep_fds = device.keep_fds(); syslog::push_fds(&mut keep_fds); @@ -253,14 +256,7 @@ pub fn generate_pci_root( device.on_sandboxed(); Arc::new(Mutex::new(device)) }; - root.add_device( - PciAddress { - bus: 0, - dev, - func: 0, - }, - arced_dev.clone(), - ); + root.add_device(address, arced_dev.clone()); for range in &ranges { mmio_bus .insert(arced_dev.clone(), range.0, range.1, true) diff --git a/devices/src/pci/ac97.rs b/devices/src/pci/ac97.rs index 228346d..8a6748d 100644 --- a/devices/src/pci/ac97.rs +++ b/devices/src/pci/ac97.rs @@ -23,7 +23,7 @@ use crate::pci::pci_configuration::{ PciBarConfiguration, PciClassCode, PciConfiguration, PciHeaderType, PciMultimediaSubclass, }; use crate::pci::pci_device::{self, PciDevice, Result}; -use crate::pci::PciInterruptPin; +use crate::pci::{PciAddress, PciInterruptPin}; // Use 82801AA because it's what qemu does. const PCI_DEVICE_ID_INTEL_82801AA_5: u16 = 0x2415; @@ -82,7 +82,7 @@ pub struct Ac97Parameters { pub struct Ac97Dev { config_regs: PciConfiguration, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, // The irq events are temporarily saved here. They need to be passed to the device after the // jail forks. This happens when the bus is first written. irq_evt: Option, @@ -108,7 +108,7 @@ impl Ac97Dev { Ac97Dev { config_regs, - pci_bus_dev: None, + pci_address: None, irq_evt: None, irq_resample_evt: None, bus_master: Ac97BusMaster::new(mem, audio_server), @@ -216,8 +216,8 @@ impl PciDevice for Ac97Dev { "AC97".to_owned() } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn assign_irq( @@ -233,15 +233,20 @@ impl PciDevice for Ac97Dev { } fn allocate_io_bars(&mut self, resources: &mut SystemAllocator) -> Result> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); let mut ranges = Vec::new(); let mixer_regs_addr = resources .mmio_allocator(MmioType::Low) .allocate_with_align( MIXER_REGS_SIZE, - Alloc::PciBar { bus, dev, bar: 0 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 0, + }, "ac97-mixer_regs".to_string(), MIXER_REGS_SIZE, ) @@ -259,7 +264,12 @@ impl PciDevice for Ac97Dev { .mmio_allocator(MmioType::Low) .allocate_with_align( MASTER_REGS_SIZE, - Alloc::PciBar { bus, dev, bar: 1 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 1, + }, "ac97-master_regs".to_string(), MASTER_REGS_SIZE, ) @@ -338,7 +348,11 @@ mod tests { .add_high_mmio_addresses(0x3000_0000, 0x1000_0000) .create_allocator(5, false) .unwrap(); - ac97_dev.assign_bus_dev(0, 0); + ac97_dev.assign_address(PciAddress { + bus: 0, + dev: 0, + func: 0, + }); assert!(ac97_dev.allocate_io_bars(&mut allocator).is_ok()); } } diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs index 2f74ea1..244edbd 100644 --- a/devices/src/pci/pci_device.rs +++ b/devices/src/pci/pci_device.rs @@ -10,7 +10,7 @@ use resources::{Error as SystemAllocatorFaliure, SystemAllocator}; use sys_util::EventFd; use crate::pci::pci_configuration; -use crate::pci::PciInterruptPin; +use crate::pci::{PciAddress, PciInterruptPin}; use crate::BusDevice; #[derive(Debug)] @@ -48,8 +48,8 @@ impl Display for Error { pub trait PciDevice: Send { /// Returns a label suitable for debug output. fn debug_label(&self) -> String; - /// Assign a unique bus and device number to this device. - fn assign_bus_dev(&mut self, _bus: u8, _device: u8 /*u5*/) {} + /// Assign a unique bus, device and function number to this device. + fn assign_address(&mut self, _address: PciAddress) {} /// A vector of device-specific file descriptors that must be kept open /// after jailing. Must be called before the process is jailed. fn keep_fds(&self) -> Vec; @@ -148,8 +148,8 @@ impl PciDevice for Box { fn debug_label(&self) -> String { (**self).debug_label() } - fn assign_bus_dev(&mut self, bus: u8, device: u8 /*u5*/) { - (**self).assign_bus_dev(bus, device) + fn assign_address(&mut self, address: PciAddress) { + (**self).assign_address(address) } fn keep_fds(&self) -> Vec { (**self).keep_fds() diff --git a/devices/src/pci/vfio_pci.rs b/devices/src/pci/vfio_pci.rs index d1b6f2b..7b5c233 100644 --- a/devices/src/pci/vfio_pci.rs +++ b/devices/src/pci/vfio_pci.rs @@ -22,7 +22,7 @@ use crate::pci::msix::{ }; use crate::pci::pci_device::{Error as PciDeviceError, PciDevice}; -use crate::pci::{PciClassCode, PciInterruptPin}; +use crate::pci::{PciAddress, PciClassCode, PciInterruptPin}; use crate::vfio::{VfioDevice, VfioIrqType}; @@ -457,7 +457,7 @@ enum DeviceData { pub struct VfioPciDevice { device: Arc, config: VfioPciConfig, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, interrupt_evt: Option, interrupt_resample_evt: Option, mmio_regions: Vec, @@ -522,7 +522,7 @@ impl VfioPciDevice { VfioPciDevice { device: dev, config, - pci_bus_dev: None, + pci_address: None, interrupt_evt: None, interrupt_resample_evt: None, mmio_regions: Vec::new(), @@ -776,8 +776,8 @@ impl PciDevice for VfioPciDevice { "vfio pci device".to_string() } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn keep_fds(&self) -> Vec { @@ -816,9 +816,9 @@ impl PciDevice for VfioPciDevice { ) -> Result, PciDeviceError> { let mut ranges = Vec::new(); let mut i = VFIO_PCI_BAR0_REGION_INDEX; - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); while i <= VFIO_PCI_ROM_REGION_INDEX { let mut low: u32 = 0xffffffff; @@ -857,8 +857,9 @@ impl PciDevice for VfioPciDevice { .allocate_with_align( size, Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: i as u8, }, "vfio_bar".to_string(), @@ -917,16 +918,17 @@ impl PciDevice for VfioPciDevice { VFIO_REGION_TYPE_PCI_VENDOR_TYPE | (INTEL_VENDOR_ID as u32), VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, ) { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_device_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_device_bars"); let bar_addr = resources .mmio_allocator(MmioType::Low) .allocate( size, Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: (index * 4) as u8, }, "vfio_bar".to_string(), diff --git a/devices/src/usb/xhci/xhci_controller.rs b/devices/src/usb/xhci/xhci_controller.rs index b77a73a..e357ae3 100644 --- a/devices/src/usb/xhci/xhci_controller.rs +++ b/devices/src/usb/xhci/xhci_controller.rs @@ -3,8 +3,8 @@ // found in the LICENSE file. use crate::pci::{ - PciBarConfiguration, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, - PciInterruptPin, PciProgrammingInterface, PciSerialBusSubClass, + PciAddress, PciBarConfiguration, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, + PciHeaderType, PciInterruptPin, PciProgrammingInterface, PciSerialBusSubClass, }; use crate::register_space::{Register, RegisterSpace}; use crate::usb::host_backend::host_backend_device_provider::HostBackendDeviceProvider; @@ -94,7 +94,7 @@ enum XhciControllerState { /// xHCI PCI interface implementation. pub struct XhciController { config_regs: PciConfiguration, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, mem: GuestMemory, state: XhciControllerState, } @@ -114,7 +114,7 @@ impl XhciController { ); XhciController { config_regs, - pci_bus_dev: None, + pci_address: None, mem, state: XhciControllerState::Created { device_provider: usb_provider, @@ -166,8 +166,8 @@ impl PciDevice for XhciController { "xhci controller".to_owned() } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn keep_fds(&self) -> Vec { @@ -206,15 +206,20 @@ impl PciDevice for XhciController { &mut self, resources: &mut SystemAllocator, ) -> std::result::Result, PciDeviceError> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); // xHCI spec 5.2.1. let bar0_addr = resources .mmio_allocator(MmioType::Low) .allocate_with_align( XHCI_BAR0_SIZE, - Alloc::PciBar { bus, dev, bar: 0 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 0, + }, "xhci_bar0".to_string(), XHCI_BAR0_SIZE, ) diff --git a/devices/src/virtio/gpu/mod.rs b/devices/src/virtio/gpu/mod.rs index e8e7f4a..7ba800e 100644 --- a/devices/src/virtio/gpu/mod.rs +++ b/devices/src/virtio/gpu/mod.rs @@ -42,7 +42,9 @@ use self::virtio_2d_backend::Virtio2DBackend; use self::virtio_3d_backend::Virtio3DBackend; #[cfg(feature = "gfxstream")] use self::virtio_gfxstream_backend::VirtioGfxStreamBackend; -use crate::pci::{PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability}; +use crate::pci::{ + PciAddress, PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability, +}; use vm_control::VmMemoryControlRequestSocket; @@ -1206,10 +1208,11 @@ impl VirtioDevice for Gpu { } // Require 1 BAR for mapping 3D buffers - fn get_device_bars(&mut self, bus: u8, dev: u8) -> Vec { + fn get_device_bars(&mut self, address: PciAddress) -> Vec { self.pci_bar = Some(Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: GPU_BAR_NUM, }); vec![PciBarConfiguration::new( diff --git a/devices/src/virtio/virtio_device.rs b/devices/src/virtio/virtio_device.rs index 6eb5548..7c07651 100644 --- a/devices/src/virtio/virtio_device.rs +++ b/devices/src/virtio/virtio_device.rs @@ -7,7 +7,7 @@ use std::os::unix::io::RawFd; use sys_util::{EventFd, GuestMemory}; use super::*; -use crate::pci::{MsixStatus, PciBarConfiguration, PciCapability}; +use crate::pci::{MsixStatus, PciAddress, PciBarConfiguration, PciCapability}; /// Trait for virtio devices to be driven by a virtio transport. /// @@ -75,7 +75,7 @@ pub trait VirtioDevice: Send { } /// Returns any additional BAR configuration required by the device. - fn get_device_bars(&mut self, _bus: u8, _dev: u8) -> Vec { + fn get_device_bars(&mut self, _address: PciAddress) -> Vec { Vec::new() } diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs index 61409f3..eb91e58 100644 --- a/devices/src/virtio/virtio_pci_device.rs +++ b/devices/src/virtio/virtio_pci_device.rs @@ -15,10 +15,10 @@ use sys_util::{warn, EventFd, GuestMemory, Result}; use super::*; use crate::pci::{ - MsixCap, MsixConfig, PciBarConfiguration, PciCapability, PciCapabilityID, PciClassCode, - PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciInterruptPin, PciSubclass, + MsixCap, MsixConfig, PciAddress, PciBarConfiguration, PciCapability, PciCapabilityID, + PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciInterruptPin, + PciSubclass, }; - use vm_control::VmIrqRequestSocket; use self::virtio_pci_common_config::VirtioPciCommonConfig; @@ -211,7 +211,7 @@ const VIRTIO_PCI_DEVICE_ID_BASE: u16 = 0x1040; // Add to device type to get devi /// transport for virtio devices. pub struct VirtioPciDevice { config_regs: PciConfiguration, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, device: Box, device_activated: bool, @@ -269,7 +269,7 @@ impl VirtioPciDevice { Ok(VirtioPciDevice { config_regs, - pci_bus_dev: None, + pci_address: None, device, device_activated: false, interrupt_status: Arc::new(AtomicUsize::new(0)), @@ -392,8 +392,8 @@ impl PciDevice for VirtioPciDevice { format!("virtio-pci ({})", self.device.debug_label()) } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn keep_fds(&self) -> Vec { @@ -425,16 +425,21 @@ impl PciDevice for VirtioPciDevice { &mut self, resources: &mut SystemAllocator, ) -> std::result::Result, PciDeviceError> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); // Allocate one bar for the structures pointed to by the capability structures. let mut ranges = Vec::new(); let settings_config_addr = resources .mmio_allocator(MmioType::Low) .allocate_with_align( CAPABILITY_BAR_SIZE, - Alloc::PciBar { bus, dev, bar: 0 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 0, + }, format!( "virtio-{}-cap_bar", type_to_str(self.device.device_type()).unwrap_or("?") @@ -463,18 +468,19 @@ impl PciDevice for VirtioPciDevice { &mut self, resources: &mut SystemAllocator, ) -> std::result::Result, PciDeviceError> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_device_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_device_bars"); let mut ranges = Vec::new(); - for config in self.device.get_device_bars(bus, dev) { + for config in self.device.get_device_bars(address) { let device_addr = resources .mmio_allocator(MmioType::High) .allocate_with_align( config.get_size(), Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: config.get_register_index() as u8, }, format!( diff --git a/resources/src/lib.rs b/resources/src/lib.rs index a0c6c98..17e8b72 100644 --- a/resources/src/lib.rs +++ b/resources/src/lib.rs @@ -30,8 +30,8 @@ pub enum Alloc { /// Should only be instantiated through `SystemAllocator::get_anon_alloc()`. /// Avoid using these. Instead, use / create a more descriptive Alloc variant. Anon(usize), - /// A PCI BAR region with associated bus, device, and bar numbers. - PciBar { bus: u8, dev: u8, bar: u8 }, + /// A PCI BAR region with associated bus, device, function and bar numbers. + PciBar { bus: u8, dev: u8, func: u8, bar: u8 }, /// GPU render node region. GpuRenderNode, /// Pmem device region with associated device index. diff --git a/resources/src/system_allocator.rs b/resources/src/system_allocator.rs index 984bc51..889fe01 100644 --- a/resources/src/system_allocator.rs +++ b/resources/src/system_allocator.rs @@ -25,13 +25,14 @@ use crate::{Alloc, Error, Result}; /// a.mmio_allocator(MmioType::High) /// .allocate( /// 0x100, -/// Alloc::PciBar { bus: 0, dev: 0, bar: 0 }, +/// Alloc::PciBar { bus: 0, dev: 0, func: 0, bar: 0 }, /// "bar0".to_string() /// ), /// Ok(0x10000000) /// ); /// assert_eq!( -/// a.mmio_allocator(MmioType::High).get(&Alloc::PciBar { bus: 0, dev: 0, bar: 0 }), +/// a.mmio_allocator(MmioType::High) +/// .get(&Alloc::PciBar { bus: 0, dev: 0, func: 0, bar: 0 }), /// Some(&(0x10000000, 0x100, "bar0".to_string())) /// ); /// } diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs index 9e78be2..a8535f8 100644 --- a/vm_control/src/lib.rs +++ b/vm_control/src/lib.rs @@ -550,11 +550,23 @@ fn register_memory( }; let addr = match allocation { - Some((Alloc::PciBar { bus, dev, bar }, offset)) => { + Some(( + Alloc::PciBar { + bus, + dev, + func, + bar, + }, + offset, + )) => { match allocator .mmio_allocator(MmioType::High) - .get(&Alloc::PciBar { bus, dev, bar }) - { + .get(&Alloc::PciBar { + bus, + dev, + func, + bar, + }) { Some((start_addr, length, _)) => { let address = *start_addr + offset; let range = *start_addr..*start_addr + *length; -- cgit 1.4.1