resources+pci: allocator rework (allocation tags)

AddressAllocator now maintains a HashMap<Alloc, (u64, u64, u64)>,
which uniquely maps a Allocation enum (e.g: PciBar(bus, dev, bar),
GpuRenderNode, etc...) to it's address, size, and human-readable tag
/ description.

The interface has also been modified to use Error instead of Option.

Aside from improving debugging, tracking allocations will have
numerous uses in the future. For example, when allocating guest memory
over VmControl sockets, it will be possible to restrict allocations to
pre-allocated slices of memory owned by the requesting device.

To plumb through PCI information to PCI devices, this CL necessitated
the addition of a PciDevice method called `assign_bus_dev`, which
notifies PCI devices of their uniquely assigned Bus and Device numbers.

BUG=chromium:936567
TEST=cargo test -p resources && cargo build --features="gpu gpu-forward"

Change-Id: I8b4b0e32c6f3168138739249ede53d03143ee5c3
Reviewed-on: https://chromium-review.googlesource.com/1536207
Commit-Ready: Daniel Prilik <prilik@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Zach Reizner <zachr@chromium.org>

12 files changed, 325 insertions, 74 deletions

diff --git a/arch/src/lib.rs b/arch/src/lib.rs
index 01c16a0..032170f 100644
--- a/arch/src/lib.rs
+++ b/arch/src/lib.rs
@@ -146,6 +146,9 @@ 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() {
+        // Only support one bus.
+        device.assign_bus_dev(0, dev_idx as u8);
+
         let mut keep_fds = device.keep_fds();
         syslog::push_fds(&mut keep_fds);
 
diff --git a/devices/src/pci/ac97.rs b/devices/src/pci/ac97.rs
index d883894..aeab1f1 100644
--- a/devices/src/pci/ac97.rs
+++ b/devices/src/pci/ac97.rs
@@ -5,7 +5,7 @@
 use std::os::unix::io::RawFd;
 
 use audio_streams::StreamSource;
-use resources::SystemAllocator;
+use resources::{Alloc, SystemAllocator};
 use sys_util::{error, EventFd, GuestMemory};
 
 use crate::pci::ac97_bus_master::Ac97BusMaster;
@@ -27,6 +27,7 @@ const PCI_DEVICE_ID_INTEL_82801AA_5: u16 = 0x2415;
 /// the audio backend.
 pub struct Ac97Dev {
     config_regs: PciConfiguration,
+    pci_bus_dev: Option<(u8, u8)>,
     // 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<EventFd>,
@@ -52,6 +53,7 @@ impl Ac97Dev {
 
         Ac97Dev {
             config_regs,
+            pci_bus_dev: None,
             irq_evt: None,
             irq_resample_evt: None,
             bus_master: Ac97BusMaster::new(mem, audio_server),
@@ -125,6 +127,10 @@ 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_irq(
         &mut self,
         irq_evt: EventFd,
@@ -138,10 +144,18 @@ impl PciDevice for Ac97Dev {
     }
 
     fn allocate_io_bars(&mut self, resources: &mut SystemAllocator) -> Result<Vec<(u64, u64)>> {
+        let (bus, dev) = self
+            .pci_bus_dev
+            .expect("assign_bus_dev must be called prior to allocate_io_bars");
         let mut ranges = Vec::new();
         let mixer_regs_addr = resources
-            .allocate_mmio_addresses(MIXER_REGS_SIZE)
-            .ok_or(pci_device::Error::IoAllocationFailed(MIXER_REGS_SIZE))?;
+            .mmio_allocator()
+            .allocate(
+                MIXER_REGS_SIZE,
+                Alloc::PciBar { bus, dev, bar: 0 },
+                "ac97-mixer_regs".to_string(),
+            )
+            .map_err(|e| pci_device::Error::IoAllocationFailed(MIXER_REGS_SIZE, e))?;
         let mixer_config = PciBarConfiguration::default()
             .set_register_index(0)
             .set_address(mixer_regs_addr)
@@ -152,8 +166,13 @@ impl PciDevice for Ac97Dev {
         ranges.push((mixer_regs_addr, MIXER_REGS_SIZE));
 
         let master_regs_addr = resources
-            .allocate_mmio_addresses(MASTER_REGS_SIZE)
-            .ok_or_else(|| pci_device::Error::IoAllocationFailed(MASTER_REGS_SIZE))?;
+            .mmio_allocator()
+            .allocate(
+                MASTER_REGS_SIZE,
+                Alloc::PciBar { bus, dev, bar: 1 },
+                "ac97-master_regs".to_string(),
+            )
+            .map_err(|e| pci_device::Error::IoAllocationFailed(MASTER_REGS_SIZE, e))?;
         let master_config = PciBarConfiguration::default()
             .set_register_index(1)
             .set_address(master_regs_addr)
@@ -228,6 +247,7 @@ mod tests {
             .add_device_addresses(0x3000_0000, 0x1000_0000)
             .create_allocator(5, false)
             .unwrap();
+        ac97_dev.assign_bus_dev(0, 0);
         assert!(ac97_dev.allocate_io_bars(&mut allocator).is_ok());
     }
 }
diff --git a/devices/src/pci/pci_configuration.rs b/devices/src/pci/pci_configuration.rs
index 1cfa6bb..ab969f7 100644
--- a/devices/src/pci/pci_configuration.rs
+++ b/devices/src/pci/pci_configuration.rs
@@ -490,6 +490,10 @@ impl PciBarConfiguration {
         self
     }
 
+    pub fn get_register_index(&self) -> usize {
+        self.reg_idx
+    }
+
     pub fn set_address(mut self, addr: u64) -> Self {
         self.addr = addr;
         self
diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs
index f8f249b..8ea3548 100644
--- a/devices/src/pci/pci_device.rs
+++ b/devices/src/pci/pci_device.rs
@@ -9,7 +9,7 @@ use std::fmt::{self, Display};
 use std::os::unix::io::RawFd;
 
 use kvm::Datamatch;
-use resources::SystemAllocator;
+use resources::{Error as SystemAllocatorFaliure, SystemAllocator};
 use sys_util::EventFd;
 
 use crate::pci::pci_configuration::{self, PciConfiguration};
@@ -21,7 +21,7 @@ pub enum Error {
     /// Setup of the device capabilities failed.
     CapabilitiesSetup(pci_configuration::Error),
     /// Allocating space for an IO BAR failed.
-    IoAllocationFailed(u64),
+    IoAllocationFailed(u64, SystemAllocatorFaliure),
     /// Registering an IO BAR failed.
     IoRegistrationFailed(u64, pci_configuration::Error),
 }
@@ -33,9 +33,11 @@ impl Display for Error {
 
         match self {
             CapabilitiesSetup(e) => write!(f, "failed to add capability {}", e),
-            IoAllocationFailed(size) => {
-                write!(f, "failed to allocate space for an IO BAR, size={}", size)
-            }
+            IoAllocationFailed(size, e) => write!(
+                f,
+                "failed to allocate space for an IO BAR, size={}: {}",
+                size, e
+            ),
             IoRegistrationFailed(addr, e) => {
                 write!(f, "failed to register an IO BAR, addr={} err={}", addr, e)
             }
@@ -46,6 +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*/) {}
     /// 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<RawFd>;
@@ -147,6 +151,9 @@ impl<T: PciDevice + ?Sized> PciDevice for Box<T> {
     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 keep_fds(&self) -> Vec<RawFd> {
         (**self).keep_fds()
     }
diff --git a/devices/src/usb/xhci/xhci_controller.rs b/devices/src/usb/xhci/xhci_controller.rs
index 1f58a5d..76e1d4a 100644
--- a/devices/src/usb/xhci/xhci_controller.rs
+++ b/devices/src/usb/xhci/xhci_controller.rs
@@ -12,7 +12,7 @@ use crate::usb::xhci::xhci::Xhci;
 use crate::usb::xhci::xhci_backend_device_provider::XhciBackendDeviceProvider;
 use crate::usb::xhci::xhci_regs::{init_xhci_mmio_space_and_regs, XhciRegs};
 use crate::utils::FailHandle;
-use resources::SystemAllocator;
+use resources::{Alloc, SystemAllocator};
 use std::mem;
 use std::os::unix::io::RawFd;
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -94,6 +94,7 @@ enum XhciControllerState {
 /// xHCI PCI interface implementation.
 pub struct XhciController {
     config_regs: PciConfiguration,
+    pci_bus_dev: Option<(u8, u8)>,
     mem: GuestMemory,
     bar0: u64, // bar0 in config_regs will be changed by guest. Not sure why.
     state: XhciControllerState,
@@ -114,6 +115,7 @@ impl XhciController {
         );
         XhciController {
             config_regs,
+            pci_bus_dev: None,
             mem,
             bar0: 0,
             state: XhciControllerState::Created {
@@ -167,6 +169,10 @@ 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 keep_fds(&self) -> Vec<RawFd> {
         match &self.state {
             XhciControllerState::Created { device_provider } => device_provider.keep_fds(),
@@ -204,10 +210,18 @@ impl PciDevice for XhciController {
         &mut self,
         resources: &mut SystemAllocator,
     ) -> std::result::Result<Vec<(u64, u64)>, PciDeviceError> {
+        let (bus, dev) = self
+            .pci_bus_dev
+            .expect("assign_bus_dev must be called prior to allocate_io_bars");
         // xHCI spec 5.2.1.
         let bar0_addr = resources
-            .allocate_mmio_addresses(XHCI_BAR0_SIZE)
-            .ok_or(PciDeviceError::IoAllocationFailed(XHCI_BAR0_SIZE))?;
+            .mmio_allocator()
+            .allocate(
+                XHCI_BAR0_SIZE,
+                Alloc::PciBar { bus, dev, bar: 0 },
+                "xhci_bar0".to_string(),
+            )
+            .map_err(|e| PciDeviceError::IoAllocationFailed(XHCI_BAR0_SIZE, e))?;
         let bar0_config = PciBarConfiguration::default()
             .set_register_index(0)
             .set_address(bar0_addr)
diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs
index 5b54de5..45bd423 100644
--- a/devices/src/virtio/virtio_pci_device.rs
+++ b/devices/src/virtio/virtio_pci_device.rs
@@ -9,7 +9,7 @@ use std::sync::Arc;
 
 use data_model::{DataInit, Le32};
 use kvm::Datamatch;
-use resources::SystemAllocator;
+use resources::{Alloc, SystemAllocator};
 use sys_util::{EventFd, GuestMemory, Result};
 
 use super::*;
@@ -148,6 +148,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)>,
 
     device: Box<dyn VirtioDevice>,
     device_activated: bool,
@@ -191,6 +192,7 @@ impl VirtioPciDevice {
 
         Ok(VirtioPciDevice {
             config_regs,
+            pci_bus_dev: None,
             device,
             device_activated: false,
             interrupt_status: Arc::new(AtomicUsize::new(0)),
@@ -300,6 +302,10 @@ 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 keep_fds(&self) -> Vec<RawFd> {
         let mut fds = self.device.keep_fds();
         if let Some(interrupt_evt) = &self.interrupt_evt {
@@ -327,11 +333,22 @@ impl PciDevice for VirtioPciDevice {
         &mut self,
         resources: &mut SystemAllocator,
     ) -> std::result::Result<Vec<(u64, u64)>, PciDeviceError> {
+        let (bus, dev) = self
+            .pci_bus_dev
+            .expect("assign_bus_dev 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
-            .allocate_mmio_addresses(CAPABILITY_BAR_SIZE)
-            .ok_or(PciDeviceError::IoAllocationFailed(CAPABILITY_BAR_SIZE))?;
+            .mmio_allocator()
+            .allocate(
+                CAPABILITY_BAR_SIZE,
+                Alloc::PciBar { bus, dev, bar: 0 },
+                format!(
+                    "virtio-{}-cap_bar",
+                    type_to_str(self.device.device_type()).unwrap_or("?")
+                ),
+            )
+            .map_err(|e| PciDeviceError::IoAllocationFailed(CAPABILITY_BAR_SIZE, e))?;
         let config = PciBarConfiguration::default()
             .set_register_index(0)
             .set_address(settings_config_addr)
@@ -353,12 +370,27 @@ impl PciDevice for VirtioPciDevice {
         &mut self,
         resources: &mut SystemAllocator,
     ) -> std::result::Result<Vec<(u64, u64)>, PciDeviceError> {
+        let (bus, dev) = self
+            .pci_bus_dev
+            .expect("assign_bus_dev must be called prior to allocate_device_bars");
         let mut ranges = Vec::new();
         for config in self.device.get_device_bars() {
             let device_addr = resources
-                .allocate_device_addresses(config.get_size())
-                .ok_or(PciDeviceError::IoAllocationFailed(config.get_size()))?;
-            config.set_address(device_addr);
+                .device_allocator()
+                .allocate(
+                    config.get_size(),
+                    Alloc::PciBar {
+                        bus,
+                        dev,
+                        bar: config.get_register_index() as u8,
+                    },
+                    format!(
+                        "virtio-{}-custom_bar",
+                        type_to_str(self.device.device_type()).unwrap_or("?")
+                    ),
+                )
+                .map_err(|e| PciDeviceError::IoAllocationFailed(config.get_size(), e))?;
+            let config = config.set_address(device_addr);
             let _device_bar = self
                 .config_regs
                 .add_pci_bar(&config)
diff --git a/resources/src/address_allocator.rs b/resources/src/address_allocator.rs
index 6c8513d..45927e0 100644
--- a/resources/src/address_allocator.rs
+++ b/resources/src/address_allocator.rs
@@ -2,16 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+use std::collections::HashMap;
+
+use crate::{Alloc, Error, Result};
+
 /// Manages allocating address ranges.
 /// Use `AddressAllocator` whenever an address range needs to be allocated to different users.
+/// Allocations must be uniquely tagged with an Alloc enum, which can be used for lookup.
+/// An human-readable tag String must also be provided for debugging / reference.
 ///
 /// # Examples
 ///
 /// ```
-/// # use resources::AddressAllocator;
+/// // Anon is used for brevity. Don't manually instantiate Anon allocs!
+/// # use resources::{Alloc, AddressAllocator};
 ///   AddressAllocator::new(0x1000, 0x10000, Some(0x100)).map(|mut pool| {
-///       assert_eq!(pool.allocate(0x110), Some(0x1000));
-///       assert_eq!(pool.allocate(0x100), Some(0x1200));
+///       assert_eq!(pool.allocate(0x110, Alloc::Anon(0), "caps".to_string()), Ok(0x1000));
+///       assert_eq!(pool.allocate(0x100, Alloc::Anon(1), "cache".to_string()), Ok(0x1200));
+///       assert_eq!(pool.allocate(0x100, Alloc::Anon(2), "etc".to_string()), Ok(0x1300));
+///       assert_eq!(pool.get(&Alloc::Anon(1)), Some(&(0x1200, 0x100, "cache".to_string())));
 ///   });
 /// ```
 #[derive(Debug, Eq, PartialEq)]
@@ -20,6 +29,7 @@ pub struct AddressAllocator {
     pool_end: u64,
     alignment: u64,
     next_addr: u64,
+    allocs: HashMap<Alloc, (u64, u64, String)>,
 }
 
 impl AddressAllocator {
@@ -30,43 +40,64 @@ impl AddressAllocator {
     /// * `pool_base` - The starting address of the range to manage.
     /// * `pool_size` - The size of the address range in bytes.
     /// * `align_size` - The minimum size of an address region to align to, defaults to four.
-    pub fn new(pool_base: u64, pool_size: u64, align_size: Option<u64>) -> Option<Self> {
+    pub fn new(pool_base: u64, pool_size: u64, align_size: Option<u64>) -> Result<Self> {
         if pool_size == 0 {
-            return None;
+            return Err(Error::PoolSizeZero);
         }
-        let pool_end = pool_base.checked_add(pool_size - 1)?;
+        let pool_end = pool_base
+            .checked_add(pool_size - 1)
+            .ok_or(Error::PoolOverflow {
+                base: pool_base,
+                size: pool_size,
+            })?;
         let alignment = align_size.unwrap_or(4);
         if !alignment.is_power_of_two() || alignment == 0 {
-            return None;
+            return Err(Error::BadAlignment);
         }
-        Some(AddressAllocator {
+        Ok(AddressAllocator {
             pool_base,
             pool_end,
             alignment,
             next_addr: pool_base,
+            allocs: HashMap::new(),
         })
     }
 
-    /// Allocates a range of addresses from the managed region. Returns `Some(allocated_address)`
-    /// when successful, or `None` if an area of `size` can't be allocated.
-    pub fn allocate(&mut self, size: u64) -> Option<u64> {
+    /// Allocates a range of addresses from the managed region with an optional tag.
+    /// Returns allocated_address. (allocated_address, size, tag) can be retrieved
+    /// through the `get` method.
+    pub fn allocate(&mut self, size: u64, alloc: Alloc, tag: String) -> Result<u64> {
+        if self.allocs.contains_key(&alloc) {
+            return Err(Error::ExistingAlloc(alloc));
+        }
         if size == 0 {
-            return None;
+            return Err(Error::AllocSizeZero);
         }
         let align_adjust = if self.next_addr % self.alignment != 0 {
             self.alignment - (self.next_addr % self.alignment)
         } else {
             0
         };
-        let addr = self.next_addr.checked_add(align_adjust)?;
-        let end_addr = addr.checked_add(size - 1)?;
+        let addr = self
+            .next_addr
+            .checked_add(align_adjust)
+            .ok_or(Error::OutOfSpace)?;
+        let end_addr = addr.checked_add(size - 1).ok_or(Error::OutOfSpace)?;
         if end_addr > self.pool_end {
-            return None;
+            return Err(Error::OutOfSpace);
         }
+
         // TODO(dgreid): Use a smarter allocation strategy. The current strategy is just
         // bumping this pointer, meaning it will eventually exhaust available addresses.
         self.next_addr = end_addr.saturating_add(1);
-        Some(addr)
+
+        self.allocs.insert(alloc, (addr, size, tag));
+        Ok(addr)
+    }
+
+    /// Returns allocation associated with `alloc`, or None if no such allocation exists.
+    pub fn get(&self, alloc: &Alloc) -> Option<&(u64, u64, String)> {
+        self.allocs.get(alloc)
     }
 }
 
@@ -76,36 +107,77 @@ mod tests {
 
     #[test]
     fn new_fails_overflow() {
-        assert_eq!(AddressAllocator::new(u64::max_value(), 0x100, None), None);
+        assert!(AddressAllocator::new(u64::max_value(), 0x100, None).is_err());
     }
 
     #[test]
     fn new_fails_size_zero() {
-        assert_eq!(AddressAllocator::new(0x1000, 0, None), None);
+        assert!(AddressAllocator::new(0x1000, 0, None).is_err());
     }
 
     #[test]
     fn new_fails_alignment_zero() {
-        assert_eq!(AddressAllocator::new(0x1000, 0x10000, Some(0)), None);
+        assert!(AddressAllocator::new(0x1000, 0x10000, Some(0)).is_err());
     }
 
     #[test]
     fn new_fails_alignment_non_power_of_two() {
-        assert_eq!(AddressAllocator::new(0x1000, 0x10000, Some(200)), None);
+        assert!(AddressAllocator::new(0x1000, 0x10000, Some(200)).is_err());
+    }
+
+    #[test]
+    fn allocate_fails_exising_alloc() {
+        let mut pool = AddressAllocator::new(0x1000, 0x1000, Some(0x100)).unwrap();
+        assert_eq!(
+            pool.allocate(0x800, Alloc::Anon(0), String::from("bar0")),
+            Ok(0x1000)
+        );
+        assert_eq!(
+            pool.allocate(0x800, Alloc::Anon(0), String::from("bar0")),
+            Err(Error::ExistingAlloc(Alloc::Anon(0)))
+        );
     }
 
     #[test]
     fn allocate_fails_not_enough_space() {
         let mut pool = AddressAllocator::new(0x1000, 0x1000, Some(0x100)).unwrap();
-        assert_eq!(pool.allocate(0x800), Some(0x1000));
-        assert_eq!(pool.allocate(0x900), None);
-        assert_eq!(pool.allocate(0x800), Some(0x1800));
+        assert_eq!(
+            pool.allocate(0x800, Alloc::Anon(0), String::from("bar0")),
+            Ok(0x1000)
+        );
+        assert_eq!(
+            pool.allocate(0x900, Alloc::Anon(1), String::from("bar1")),
+            Err(Error::OutOfSpace)
+        );
+        assert_eq!(
+            pool.allocate(0x800, Alloc::Anon(2), String::from("bar2")),
+            Ok(0x1800)
+        );
     }
 
     #[test]
     fn allocate_alignment() {
         let mut pool = AddressAllocator::new(0x1000, 0x10000, Some(0x100)).unwrap();
-        assert_eq!(pool.allocate(0x110), Some(0x1000));
-        assert_eq!(pool.allocate(0x100), Some(0x1200));
+        assert_eq!(
+            pool.allocate(0x110, Alloc::Anon(0), String::from("bar0")),
+            Ok(0x1000)
+        );
+        assert_eq!(
+            pool.allocate(0x100, Alloc::Anon(1), String::from("bar1")),
+            Ok(0x1200)
+        );
+    }
+
+    #[test]
+    fn allocate_retrieve_alloc() {
+        let mut pool = AddressAllocator::new(0x1000, 0x10000, Some(0x100)).unwrap();
+        assert_eq!(
+            pool.allocate(0x110, Alloc::Anon(0), String::from("bar0")),
+            Ok(0x1000)
+        );
+        assert_eq!(
+            pool.get(&Alloc::Anon(0)),
+            Some(&(0x1000, 0x110, String::from("bar0")))
+        );
     }
 }
diff --git a/resources/src/gpu_allocator.rs b/resources/src/gpu_allocator.rs
index 7b40a22..a44d3c8 100644
--- a/resources/src/gpu_allocator.rs
+++ b/resources/src/gpu_allocator.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::fmt::Debug;
 use std::fs::File;
 
 #[cfg(feature = "wl-dmabuf")]
@@ -13,7 +14,7 @@ use msg_socket::MsgOnSocket;
 use sys_util;
 
 #[allow(dead_code)]
-#[derive(Debug)]
+#[derive(Debug, Eq, PartialEq)]
 pub enum GpuAllocatorError {
     OpenGpuBufferDevice,
     CreateGpuBufferDevice,
@@ -35,7 +36,7 @@ pub struct GpuMemoryDesc {
 /// Trait that needs to be implemented in order to service GPU memory allocation
 /// requests. Implementations are expected to support some set of buffer sizes and
 /// formats but every possible combination is not required.
-pub trait GpuMemoryAllocator {
+pub trait GpuMemoryAllocator: Debug {
     /// Allocates GPU memory for a buffer of a specific size and format. The memory
     /// layout for the returned buffer must be linear. A file handle and the
     /// description of the planes for the buffer are returned on success.
@@ -58,6 +59,13 @@ pub struct GpuBufferDevice {
 }
 
 #[cfg(feature = "wl-dmabuf")]
+impl std::fmt::Debug for GpuBufferDevice {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(f, "GpuBufferDevice {{opaque}}")
+    }
+}
+
+#[cfg(feature = "wl-dmabuf")]
 impl GpuMemoryAllocator for GpuBufferDevice {
     fn allocate(
         &self,
diff --git a/resources/src/lib.rs b/resources/src/lib.rs
index b16b2da..a49f998 100644
--- a/resources/src/lib.rs
+++ b/resources/src/lib.rs
@@ -4,10 +4,67 @@
 
 //! Manages system resources that can be allocated to VMs and their devices.
 
+#[cfg(feature = "wl-dmabuf")]
+extern crate gpu_buffer;
+extern crate libc;
+extern crate msg_socket;
+extern crate sys_util;
+
+use std::fmt::Display;
+
+pub use crate::address_allocator::AddressAllocator;
+pub use crate::gpu_allocator::{
+    GpuAllocatorError, GpuMemoryAllocator, GpuMemoryDesc, GpuMemoryPlaneDesc,
+};
+pub use crate::system_allocator::SystemAllocator;
+
 mod address_allocator;
 mod gpu_allocator;
 mod system_allocator;
 
-pub use crate::address_allocator::AddressAllocator;
-pub use crate::gpu_allocator::{GpuMemoryAllocator, GpuMemoryDesc, GpuMemoryPlaneDesc};
-pub use crate::system_allocator::SystemAllocator;
+/// Used to tag SystemAllocator allocations.
+#[derive(Debug, Eq, PartialEq, Hash)]
+pub enum Alloc {
+    /// An anonymous resource allocation.
+    /// 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 },
+    /// GPU render node region.
+    GpuRenderNode,
+}
+
+#[derive(Debug, Eq, PartialEq)]
+pub enum Error {
+    AllocSizeZero,
+    BadAlignment,
+    CreateGpuAllocator(GpuAllocatorError),
+    ExistingAlloc(Alloc),
+    MissingDeviceAddresses,
+    MissingMMIOAddresses,
+    NoIoAllocator,
+    OutOfSpace,
+    PoolOverflow { base: u64, size: u64 },
+    PoolSizeZero,
+}
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+impl Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use self::Error::*;
+        match self {
+            AllocSizeZero => write!(f, "Allocation cannot have size of 0"),
+            BadAlignment => write!(f, "Pool alignment must be a power of 2"),
+            CreateGpuAllocator(e) => write!(f, "Failed to create GPU allocator: {:?}", e),
+            ExistingAlloc(tag) => write!(f, "Alloc already exists: {:?}", tag),
+            MissingDeviceAddresses => write!(f, "Device address range not specified"),
+            MissingMMIOAddresses => write!(f, "MMIO address range not specified"),
+            NoIoAllocator => write!(f, "No IO address range specified"),
+            OutOfSpace => write!(f, "Out of space"),
+            PoolOverflow { base, size } => write!(f, "base={} + size={} overflows", base, size),
+            PoolSizeZero => write!(f, "Pool cannot have size of 0"),
+        }
+    }
+}
diff --git a/resources/src/system_allocator.rs b/resources/src/system_allocator.rs
index 3cc564d..6cdb70e 100644
--- a/resources/src/system_allocator.rs
+++ b/resources/src/system_allocator.rs
@@ -6,29 +6,44 @@ use sys_util::pagesize;
 
 use crate::address_allocator::AddressAllocator;
 use crate::gpu_allocator::{self, GpuMemoryAllocator};
+use crate::{Alloc, Error, Result};
 
 /// Manages allocating system resources such as address space and interrupt numbers.
 ///
 /// # Example - Use the `SystemAddress` builder.
 ///
 /// ```
-/// # use resources::SystemAllocator;
-///   if let Some(mut a) = SystemAllocator::builder()
+/// # use resources::{Alloc, SystemAllocator};
+///   if let Ok(mut a) = SystemAllocator::builder()
 ///           .add_io_addresses(0x1000, 0x10000)
 ///           .add_device_addresses(0x10000000, 0x10000000)
 ///           .add_mmio_addresses(0x30000000, 0x10000)
 ///           .create_allocator(5, false) {
 ///       assert_eq!(a.allocate_irq(), Some(5));
 ///       assert_eq!(a.allocate_irq(), Some(6));
-///       assert_eq!(a.allocate_device_addresses(0x100), Some(0x10000000));
+///       assert_eq!(
+///           a.device_allocator()
+///              .allocate(
+///                  0x100,
+///                  Alloc::PciBar { bus: 0, dev: 0, bar: 0 },
+///                  "bar0".to_string()
+///              ),
+///           Ok(0x10000000)
+///       );
+///       assert_eq!(
+///           a.device_allocator().get(&Alloc::PciBar { bus: 0, dev: 0, bar: 0 }),
+///           Some(&(0x10000000, 0x100, "bar0".to_string()))
+///       );
 ///   }
 /// ```
+#[derive(Debug)]
 pub struct SystemAllocator {
     io_address_space: Option<AddressAllocator>,
     device_address_space: AddressAllocator,
     mmio_address_space: AddressAllocator,
     gpu_allocator: Option<Box<dyn GpuMemoryAllocator>>,
     next_irq: u32,
+    next_anon_id: usize,
 }
 
 impl SystemAllocator {
@@ -53,9 +68,9 @@ impl SystemAllocator {
         mmio_size: u64,
         create_gpu_allocator: bool,
         first_irq: u32,
-    ) -> Option<Self> {
+    ) -> Result<Self> {
         let page_size = pagesize() as u64;
-        Some(SystemAllocator {
+        Ok(SystemAllocator {
             io_address_space: if let (Some(b), Some(s)) = (io_base, io_size) {
                 Some(AddressAllocator::new(b, s, Some(0x400))?)
             } else {
@@ -64,11 +79,12 @@ impl SystemAllocator {
             device_address_space: AddressAllocator::new(dev_base, dev_size, Some(page_size))?,
             mmio_address_space: AddressAllocator::new(mmio_base, mmio_size, Some(page_size))?,
             gpu_allocator: if create_gpu_allocator {
-                gpu_allocator::create_gpu_memory_allocator().ok()?
+                gpu_allocator::create_gpu_memory_allocator().map_err(Error::CreateGpuAllocator)?
             } else {
                 None
             },
             next_irq: first_irq,
+            next_anon_id: 0,
         })
     }
 
@@ -87,25 +103,31 @@ impl SystemAllocator {
         }
     }
 
-    /// Reserves a section of `size` bytes of IO address space.
-    pub fn allocate_io_addresses(&mut self, size: u64) -> Option<u64> {
-        self.io_address_space.as_mut()?.allocate(size)
+    /// Gets an allocator to be used for IO memory.
+    pub fn io_allocator(&mut self) -> Option<&mut AddressAllocator> {
+        self.io_address_space.as_mut()
     }
 
-    /// Reserves a section of `size` bytes of device address space.
-    pub fn allocate_device_addresses(&mut self, size: u64) -> Option<u64> {
-        self.device_address_space.allocate(size)
+    /// Gets an allocator to be used for device memory.
+    pub fn device_allocator(&mut self) -> &mut AddressAllocator {
+        &mut self.device_address_space
     }
 
-    /// Reserves a section of `size` bytes of device address space.
-    pub fn allocate_mmio_addresses(&mut self, size: u64) -> Option<u64> {
-        self.mmio_address_space.allocate(size)
+    /// Gets an allocator to be used for MMIO memory.
+    pub fn mmio_allocator(&mut self) -> &mut AddressAllocator {
+        &mut self.mmio_address_space
     }
 
     /// Gets an allocator to be used for GPU memory.
     pub fn gpu_memory_allocator(&self) -> Option<&dyn GpuMemoryAllocator> {
         self.gpu_allocator.as_ref().map(|v| v.as_ref())
     }
+
+    /// Gets a unique anonymous allocation
+    pub fn get_anon_alloc(&mut self) -> Alloc {
+        self.next_anon_id += 1;
+        Alloc::Anon(self.next_anon_id)
+    }
 }
 
 /// Used to build a system address map for use in creating a `SystemAllocator`.
@@ -152,14 +174,14 @@ impl SystemAllocatorBuilder {
         &self,
         first_irq: u32,
         gpu_allocation: bool,
-    ) -> Option<SystemAllocator> {
+    ) -> Result<SystemAllocator> {
         SystemAllocator::new(
             self.io_base,
             self.io_size,
-            self.device_base?,
-            self.device_size?,
-            self.mmio_base?,
-            self.mmio_size?,
+            self.device_base.ok_or(Error::MissingDeviceAddresses)?,
+            self.device_size.ok_or(Error::MissingDeviceAddresses)?,
+            self.mmio_base.ok_or(Error::MissingMMIOAddresses)?,
+            self.mmio_size.ok_or(Error::MissingMMIOAddresses)?,
             gpu_allocation,
             first_irq,
         )
diff --git a/src/linux.rs b/src/linux.rs
index 19cdf09..6e2c2c9 100644
--- a/src/linux.rs
+++ b/src/linux.rs
@@ -34,6 +34,8 @@ use net_util::{Error as NetError, MacAddress, Tap};
 use qcow::{self, ImageType, QcowFile};
 use rand_ish::SimpleRng;
 use remain::sorted;
+#[cfg(feature = "gpu-forward")]
+use resources::Alloc;
 use sync::{Condvar, Mutex};
 use sys_util::net::{UnixSeqpacket, UnixSeqpacketListener, UnlinkUnixSeqpacketListener};
 use sys_util::{
@@ -1173,8 +1175,13 @@ pub fn run_config(cfg: Config) -> Result<()> {
         // guest address space.
         let gpu_addr = linux
             .resources
-            .allocate_device_addresses(RENDER_NODE_HOST_SIZE)
-            .ok_or(Error::AllocateGpuDeviceAddress)?;
+            .device_allocator()
+            .allocate(
+                RENDER_NODE_HOST_SIZE,
+                Alloc::GpuRenderNode,
+                "gpu_render_node".to_string(),
+            )
+            .map_err(|_| Error::AllocateGpuDeviceAddress)?;
 
         let host = RenderNodeHost::start(&gpu_mmap, gpu_addr, linux.vm.get_memory().clone());
 
diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs
index b197e9b..474380c 100644
--- a/vm_control/src/lib.rs
+++ b/vm_control/src/lib.rs
@@ -209,9 +209,14 @@ fn register_memory(
         Err(MmapError::SystemCallFailed(e)) => return Err(e),
         _ => return Err(SysError::new(EINVAL)),
     };
-    let addr = match allocator.allocate_device_addresses(size as u64) {
-        Some(a) => a,
-        None => return Err(SysError::new(EINVAL)),
+    let alloc = allocator.get_anon_alloc();
+    let addr = match allocator.device_allocator().allocate(
+        size as u64,
+        alloc,
+        "vmcontrol_register_memory".to_string(),
+    ) {
+        Ok(a) => a,
+        Err(_) => return Err(SysError::new(EINVAL)),
     };
     let slot = match vm.add_device_memory(GuestAddress(addr), mmap, false, false) {
         Ok(v) => v,