agent/pci: Extend Slot type to represent PCI function as well

pci::Slot represents a PCI slot.  However, in all cases where we use it, we
actually care about addressing a specific PCI function.  So, at the moment
we can only refer to function 0 in each slot.

Replace pci::Slot with pci::SlotFn to represent both the slot and function.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

src/agent/src/device.rs

@@ -71,7 +71,7 @@ pub fn pcipath_to_sysfs(root_bus_sysfs: &str, pcipath: &pci::Path) -> Result<Str
     let mut relpath = String::new();
 
     for i in 0..pcipath.len() {
-        let bdf = format!("{}:{}.0", bus, pcipath[i]);
+        let bdf = format!("{}:{}", bus, pcipath[i]);
 
         relpath = format!("{}/{}", relpath, bdf);
 

src/agent/src/pci.rs

@@ -9,51 +9,96 @@ use std::str::FromStr;
 
 use anyhow::anyhow;
 
-// The PCI spec reserves 5 bits for slot number (a.k.a. device
-// number), giving slots 0..31
+// The PCI spec reserves 5 bits (0..31) for slot number (a.k.a. device
+// number)
 const SLOT_BITS: u8 = 5;
 const SLOT_MAX: u8 = (1 << SLOT_BITS) - 1;
 
-// Represents a PCI function's slot number (a.k.a. device number),
-// giving its location on a single bus
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
-pub struct Slot(u8);
+// The PCI spec reserves 3 bits (0..7) for function number
+const FUNCTION_BITS: u8 = 3;
+const FUNCTION_MAX: u8 = (1 << FUNCTION_BITS) - 1;
 
-impl Slot {
-    pub fn new<T: TryInto<u8> + fmt::Display + Copy>(v: T) -> anyhow::Result<Self> {
-        if let Ok(v8) = v.try_into() {
-            if v8 <= SLOT_MAX {
-                return Ok(Slot(v8));
+// Represents a PCI function's slot (a.k.a. device) and function
+// numbers, giving its location on a single logical bus
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub struct SlotFn(u8);
+
+impl SlotFn {
+    pub fn new<T, U>(ss: T, f: U) -> anyhow::Result<Self>
+    where
+        T: TryInto<u8> + fmt::Display + Copy,
+        U: TryInto<u8> + fmt::Display + Copy,
+    {
+        let ss8 = match ss.try_into() {
+            Ok(ss8) if ss8 <= SLOT_MAX => ss8,
+            _ => {
+                return Err(anyhow!(
+                    "PCI slot {} should be in range [0..{:#x}]",
+                    ss,
+                    SLOT_MAX
+                ));
             }
-        }
-        Err(anyhow!(
-            "PCI slot {} should be in range [0..{:#x}]",
-            v,
-            SLOT_MAX
-        ))
+        };
+
+        let f8 = match f.try_into() {
+            Ok(f8) if f8 <= FUNCTION_MAX => f8,
+            _ => {
+                return Err(anyhow!(
+                    "PCI function {} should be in range [0..{:#x}]",
+                    f,
+                    FUNCTION_MAX
+                ));
+            }
+        };
+
+        Ok(SlotFn(ss8 << FUNCTION_BITS | f8))
+    }
+
+    pub fn slot(self) -> u8 {
+        self.0 >> FUNCTION_BITS
+    }
+
+    pub fn function(self) -> u8 {
+        self.0 & FUNCTION_MAX
     }
 }
 
-impl FromStr for Slot {
+impl FromStr for SlotFn {
     type Err = anyhow::Error;
 
     fn from_str(s: &str) -> anyhow::Result<Self> {
-        let v = isize::from_str_radix(s, 16)?;
-        Slot::new(v)
+        let mut tokens = s.split('.').fuse();
+        let slot = tokens.next();
+        let func = tokens.next();
+
+        if slot.is_none() || tokens.next().is_some() {
+            return Err(anyhow!(
+                "PCI slot/function {} should have the format SS.F",
+                s
+            ));
+        }
+
+        let slot = isize::from_str_radix(slot.unwrap(), 16)?;
+        let func = match func {
+            Some(func) => isize::from_str_radix(func, 16)?,
+            None => 0,
+        };
+
+        SlotFn::new(slot, func)
     }
 }
 
-impl fmt::Display for Slot {
+impl fmt::Display for SlotFn {
     fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        write!(f, "{:02x}", self.0)
+        write!(f, "{:02x}.{:01x}", self.slot(), self.function())
     }
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct Path(Vec<Slot>);
+pub struct Path(Vec<SlotFn>);
 
 impl Path {
-    pub fn new(slots: Vec<Slot>) -> anyhow::Result<Self> {
+    pub fn new(slots: Vec<SlotFn>) -> anyhow::Result<Self> {
         if slots.is_empty() {
             return Err(anyhow!("PCI path must have at least one element"));
         }
@@ -63,7 +108,7 @@ impl Path {
 
 // Let Path be treated as a slice of Slots
 impl Deref for Path {
-    type Target = [Slot];
+    type Target = [SlotFn];
 
     fn deref(&self) -> &Self::Target {
         &self.0
@@ -85,83 +130,131 @@ impl FromStr for Path {
     type Err = anyhow::Error;
 
     fn from_str(s: &str) -> anyhow::Result<Self> {
-        let rslots: anyhow::Result<Vec<Slot>> = s.split('/').map(Slot::from_str).collect();
+        let rslots: anyhow::Result<Vec<SlotFn>> = s.split('/').map(SlotFn::from_str).collect();
         Path::new(rslots?)
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::pci::{Path, Slot};
+    use super::*;
     use std::str::FromStr;
 
     #[test]
-    fn test_slot() {
+    fn test_slotfn() {
         // Valid slots
-        let slot = Slot::new(0x00).unwrap();
-        assert_eq!(format!("{}", slot), "00");
+        let sf = SlotFn::new(0x00, 0x0).unwrap();
+        assert_eq!(format!("{}", sf), "00.0");
 
-        let slot = Slot::from_str("00").unwrap();
-        assert_eq!(format!("{}", slot), "00");
+        let sf = SlotFn::from_str("00.0").unwrap();
+        assert_eq!(format!("{}", sf), "00.0");
 
-        let slot = Slot::new(31).unwrap();
-        let slot2 = Slot::from_str("1f").unwrap();
-        assert_eq!(slot, slot2);
+        let sf = SlotFn::from_str("00").unwrap();
+        assert_eq!(format!("{}", sf), "00.0");
+
+        let sf = SlotFn::new(31, 7).unwrap();
+        let sf2 = SlotFn::from_str("1f.7").unwrap();
+        assert_eq!(sf, sf2);
 
         // Bad slots
-        let slot = Slot::new(-1);
-        assert!(slot.is_err());
+        let sf = SlotFn::new(-1, 0);
+        assert!(sf.is_err());
 
-        let slot = Slot::new(32);
-        assert!(slot.is_err());
+        let sf = SlotFn::new(32, 0);
+        assert!(sf.is_err());
 
-        let slot = Slot::from_str("20");
-        assert!(slot.is_err());
+        let sf = SlotFn::from_str("20.0");
+        assert!(sf.is_err());
 
-        let slot = Slot::from_str("xy");
-        assert!(slot.is_err());
+        let sf = SlotFn::from_str("20");
+        assert!(sf.is_err());
 
-        let slot = Slot::from_str("00/");
-        assert!(slot.is_err());
+        let sf = SlotFn::from_str("xy.0");
+        assert!(sf.is_err());
 
-        let slot = Slot::from_str("");
-        assert!(slot.is_err());
+        let sf = SlotFn::from_str("xy");
+        assert!(sf.is_err());
+
+        // Bad functions
+        let sf = SlotFn::new(0, -1);
+        assert!(sf.is_err());
+
+        let sf = SlotFn::new(0, 8);
+        assert!(sf.is_err());
+
+        let sf = SlotFn::from_str("00.8");
+        assert!(sf.is_err());
+
+        let sf = SlotFn::from_str("00.x");
+        assert!(sf.is_err());
+
+        // Bad formats
+        let sf = SlotFn::from_str("");
+        assert!(sf.is_err());
+
+        let sf = SlotFn::from_str("00.0.0");
+        assert!(sf.is_err());
+
+        let sf = SlotFn::from_str("00.0/");
+        assert!(sf.is_err());
+
+        let sf = SlotFn::from_str("00/");
+        assert!(sf.is_err());
     }
 
     #[test]
     fn test_path() {
-        let slot3 = Slot::new(0x03).unwrap();
-        let slot4 = Slot::new(0x04).unwrap();
-        let slot5 = Slot::new(0x05).unwrap();
+        let sf3_0 = SlotFn::new(0x03, 0).unwrap();
+        let sf4_0 = SlotFn::new(0x04, 0).unwrap();
+        let sf5_0 = SlotFn::new(0x05, 0).unwrap();
+        let sfa_5 = SlotFn::new(0x0a, 5).unwrap();
+        let sfb_6 = SlotFn::new(0x0b, 6).unwrap();
+        let sfc_7 = SlotFn::new(0x0c, 7).unwrap();
 
         // Valid paths
-        let pcipath = Path::new(vec![slot3]).unwrap();
-        assert_eq!(format!("{}", pcipath), "03");
+        let pcipath = Path::new(vec![sf3_0]).unwrap();
+        assert_eq!(format!("{}", pcipath), "03.0");
+        let pcipath2 = Path::from_str("03.0").unwrap();
+        assert_eq!(pcipath, pcipath2);
         let pcipath2 = Path::from_str("03").unwrap();
         assert_eq!(pcipath, pcipath2);
         assert_eq!(pcipath.len(), 1);
-        assert_eq!(pcipath[0], slot3);
+        assert_eq!(pcipath[0], sf3_0);
 
-        let pcipath = Path::new(vec![slot3, slot4]).unwrap();
-        assert_eq!(format!("{}", pcipath), "03/04");
+        let pcipath = Path::new(vec![sf3_0, sf4_0]).unwrap();
+        assert_eq!(format!("{}", pcipath), "03.0/04.0");
+        let pcipath2 = Path::from_str("03.0/04.0").unwrap();
+        assert_eq!(pcipath, pcipath2);
         let pcipath2 = Path::from_str("03/04").unwrap();
         assert_eq!(pcipath, pcipath2);
         assert_eq!(pcipath.len(), 2);
-        assert_eq!(pcipath[0], slot3);
-        assert_eq!(pcipath[1], slot4);
+        assert_eq!(pcipath[0], sf3_0);
+        assert_eq!(pcipath[1], sf4_0);
 
-        let pcipath = Path::new(vec![slot3, slot4, slot5]).unwrap();
-        assert_eq!(format!("{}", pcipath), "03/04/05");
+        let pcipath = Path::new(vec![sf3_0, sf4_0, sf5_0]).unwrap();
+        assert_eq!(format!("{}", pcipath), "03.0/04.0/05.0");
+        let pcipath2 = Path::from_str("03.0/04.0/05.0").unwrap();
+        assert_eq!(pcipath, pcipath2);
         let pcipath2 = Path::from_str("03/04/05").unwrap();
         assert_eq!(pcipath, pcipath2);
         assert_eq!(pcipath.len(), 3);
-        assert_eq!(pcipath[0], slot3);
-        assert_eq!(pcipath[1], slot4);
-        assert_eq!(pcipath[2], slot5);
+        assert_eq!(pcipath[0], sf3_0);
+        assert_eq!(pcipath[1], sf4_0);
+        assert_eq!(pcipath[2], sf5_0);
+
+        let pcipath = Path::new(vec![sfa_5, sfb_6, sfc_7]).unwrap();
+        assert_eq!(format!("{}", pcipath), "0a.5/0b.6/0c.7");
+        let pcipath2 = Path::from_str("0a.5/0b.6/0c.7").unwrap();
+        assert_eq!(pcipath, pcipath2);
+        assert_eq!(pcipath.len(), 3);
+        assert_eq!(pcipath[0], sfa_5);
+        assert_eq!(pcipath[1], sfb_6);
+        assert_eq!(pcipath[2], sfc_7);
 
         // Bad paths
         assert!(Path::new(vec!()).is_err());
         assert!(Path::from_str("20").is_err());
+        assert!(Path::from_str("00.8").is_err());
         assert!(Path::from_str("//").is_err());
         assert!(Path::from_str("xyz").is_err());
     }

src/agent/src/rpc.rs

@@ -1606,7 +1606,7 @@ fn do_copy_file(req: &CopyFileRequest) -> Result<()> {
 async fn do_add_swap(sandbox: &Arc<Mutex<Sandbox>>, req: &AddSwapRequest) -> Result<()> {
     let mut slots = Vec::new();
     for slot in &req.PCIPath {
-        slots.push(pci::Slot::new(*slot)?);
+        slots.push(pci::SlotFn::new(*slot, 0)?);
     }
     let pcipath = pci::Path::new(slots)?;
     let dev_name = get_virtio_blk_pci_device_name(sandbox, &pcipath).await?;