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>

4 files changed, 209 insertions, 50 deletions

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,
         )