devices: bus: Add BusType to Bus for better debugging.

With snapshot/restore, we are snapshotting and restoring all Bus devices
on crosvm (MMIO and IO). However there's no message differentiating
which bus was snapshotting/restoring, and the message only displays that
the action succeeded.
Add the type to Bus device and to the logs.

BUG=N/A
TEST=presubmit

Change-Id: Iec4f97f6f7502bfadc96026cc90710c97d351a83
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4936779
Commit-Queue: Elie Kheirallah <khei@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>

aarch64/src/lib.rs

@@ -28,6 +28,7 @@ use devices::vmwdt::VMWDT_DEFAULT_TIMEOUT_SEC;
 use devices::Bus;
 use devices::BusDeviceObj;
 use devices::BusError;
+use devices::BusType;
 use devices::IrqChip;
 use devices::IrqChipAArch64;
 use devices::IrqEventSource;
@@ -532,10 +533,10 @@ impl arch::LinuxArch for AArch64 {
             }
         }
 
-        let mmio_bus = Arc::new(devices::Bus::new());
+        let mmio_bus = Arc::new(devices::Bus::new(BusType::Mmio));
 
         // ARM doesn't really use the io bus like x86, so just create an empty bus.
-        let io_bus = Arc::new(devices::Bus::new());
+        let io_bus = Arc::new(devices::Bus::new(BusType::Io));
 
         // Event used by PMDevice to notify crosvm that
         // guest OS is trying to suspend.

devices/src/bus.rs

@@ -342,19 +342,26 @@ pub struct Bus {
     access_id: usize,
     #[cfg(feature = "stats")]
     pub stats: Arc<Mutex<BusStatistics>>,
+    bus_type: BusType,
 }
 
 impl Bus {
     /// Constructs an a bus with an empty address space.
-    pub fn new() -> Bus {
+    pub fn new(bus_type: BusType) -> Bus {
         Bus {
             devices: Arc::new(Mutex::new(BTreeMap::new())),
             access_id: 0,
             #[cfg(feature = "stats")]
             stats: Arc::new(Mutex::new(BusStatistics::new())),
+            bus_type,
         }
     }
 
+    /// Gets the bus type
+    pub fn get_bus_type(&self) -> BusType {
+        self.bus_type
+    }
+
     /// Sets the id that will be used for BusAccessInfo.
     pub fn set_access_id(&mut self, id: usize) {
         self.access_id = id;
@@ -702,7 +709,7 @@ impl Bus {
 
 impl Default for Bus {
     fn default() -> Self {
-        Self::new()
+        Self::new(BusType::Io)
     }
 }
 
@@ -808,7 +815,7 @@ mod tests {
 
     #[test]
     fn bus_insert() {
-        let bus = Bus::new();
+        let bus = Bus::new(BusType::Io);
         let dummy = Arc::new(Mutex::new(DummyDevice));
         assert!(bus.insert(dummy.clone(), 0x10, 0).is_err());
         assert!(bus.insert(dummy.clone(), 0x10, 0x10).is_ok());
@@ -825,7 +832,7 @@ mod tests {
 
     #[test]
     fn bus_insert_full_addr() {
-        let bus = Bus::new();
+        let bus = Bus::new(BusType::Io);
         let dummy = Arc::new(Mutex::new(DummyDevice));
         assert!(bus.insert(dummy.clone(), 0x10, 0).is_err());
         assert!(bus.insert(dummy.clone(), 0x10, 0x10).is_ok());
@@ -842,7 +849,7 @@ mod tests {
 
     #[test]
     fn bus_read_write() {
-        let bus = Bus::new();
+        let bus = Bus::new(BusType::Io);
         let dummy = Arc::new(Mutex::new(DummyDevice));
         assert!(bus.insert(dummy, 0x10, 0x10).is_ok());
         assert!(bus.read(0x10, &mut [0, 0, 0, 0]));
@@ -859,7 +866,7 @@ mod tests {
 
     #[test]
     fn bus_read_write_values() {
-        let bus = Bus::new();
+        let bus = Bus::new(BusType::Io);
         let dummy = Arc::new(Mutex::new(ConstantDevice {
             uses_full_addr: false,
         }));
@@ -876,7 +883,7 @@ mod tests {
 
     #[test]
     fn bus_read_write_full_addr_values() {
-        let bus = Bus::new();
+        let bus = Bus::new(BusType::Io);
         let dummy = Arc::new(Mutex::new(ConstantDevice {
             uses_full_addr: true,
         }));

devices/src/lib.rs

@@ -228,12 +228,13 @@ pub fn create_devices_worker_thread(
 fn sleep_devices(bus: &Bus) -> anyhow::Result<()> {
     match bus.sleep_devices() {
         Ok(_) => {
-            info!("Devices slept successfully");
+            info!("Devices slept successfully on {:?} Bus", bus.get_bus_type());
             Ok(())
         }
         Err(e) => Err(anyhow!(
-            "Failed to sleep all devices: {}. Waking up sleeping devices.",
-            e
+            "Failed to sleep all devices: {} on {:?} Bus. Waking up sleeping devices.",
+            e,
+            bus.get_bus_type(),
         )),
     }
 }
@@ -241,15 +242,19 @@ fn sleep_devices(bus: &Bus) -> anyhow::Result<()> {
 fn wake_devices(bus: &Bus) {
     match bus.wake_devices() {
         Ok(_) => {
-            info!("Devices awoken successfully");
+            info!(
+                "Devices awoken successfully on {:?} Bus",
+                bus.get_bus_type()
+            );
         }
         Err(e) => {
             // Some devices may have slept. Eternally.
             // Recovery - impossible.
             // Shut down VM.
             panic!(
-                "Failed to wake devices: {}. VM panicked to avoid unexpected behavior",
-                e
+                "Failed to wake devices: {} on {:?} Bus. VM panicked to avoid unexpected behavior",
+                e,
+                bus.get_bus_type(),
             )
         }
     }
@@ -296,12 +301,19 @@ fn snapshot_devices(
 ) -> anyhow::Result<()> {
     match bus.snapshot_devices(add_snapshot) {
         Ok(_) => {
-            info!("Devices snapshot successfully");
+            info!(
+                "Devices snapshot successfully for {:?} Bus",
+                bus.get_bus_type()
+            );
             Ok(())
         }
         Err(e) => {
             // If snapshot fails, wake devices and return error
-            error!("failed to snapshot devices: {}", e);
+            error!(
+                "failed to snapshot devices: {:#} on {:?} Bus",
+                e,
+                bus.get_bus_type()
+            );
             Err(e)
         }
     }
@@ -313,12 +325,19 @@ fn restore_devices(
 ) -> anyhow::Result<()> {
     match bus.restore_devices(devices_map) {
         Ok(_) => {
-            info!("Devices restore successfully");
+            info!(
+                "Devices restore successfully for {:?} Bus",
+                bus.get_bus_type()
+            );
             Ok(())
         }
         Err(e) => {
             // If restore fails, wake devices and return error
-            error!("failed to restore devices: {:#}", e);
+            error!(
+                "failed to restore devices: {:#} on {:?} Bus",
+                e,
+                bus.get_bus_type()
+            );
             Err(e)
         }
     }

devices/src/pci/pci_root.rs

@@ -758,8 +758,8 @@ mod tests {
     use crate::suspendable_tests;
 
     fn create_pci_root() -> Arc<Mutex<PciRoot>> {
-        let io_bus = Arc::new(Bus::new());
-        let mmio_bus = Arc::new(Bus::new());
+        let io_bus = Arc::new(Bus::new(BusType::Io));
+        let mmio_bus = Arc::new(Bus::new(BusType::Mmio));
         let root_bus = Arc::new(Mutex::new(PciBus::new(0, 0, false)));
 
         Arc::new(Mutex::new(PciRoot::new(

devices/tests/irqchip/kvm/x86_64.rs

@@ -7,6 +7,7 @@
 use base::EventWaitResult;
 use base::Tube;
 use devices::Bus;
+use devices::BusType;
 use devices::CrosvmDeviceId;
 use devices::DeviceId;
 use devices::IrqChip;
@@ -233,8 +234,8 @@ fn irq_event_tokens() {
 fn finalize_devices() {
     let mut chip = get_split_chip();
 
-    let mmio_bus = Bus::new();
-    let io_bus = Bus::new();
+    let mmio_bus = Bus::new(BusType::Mmio);
+    let io_bus = Bus::new(BusType::Io);
     let mut resources = SystemAllocator::new(
         SystemAllocatorConfig {
             io: Some(AddressRange {
@@ -372,8 +373,8 @@ fn get_external_interrupt() {
 fn broadcast_eoi() {
     let mut chip = get_split_chip();
 
-    let mmio_bus = Bus::new();
-    let io_bus = Bus::new();
+    let mmio_bus = Bus::new(BusType::Mmio);
+    let io_bus = Bus::new(BusType::Io);
     let mut resources = SystemAllocator::new(
         SystemAllocatorConfig {
             io: Some(AddressRange {

devices/tests/irqchip/userspace.rs

@@ -16,6 +16,7 @@ use base::Tube;
 use devices::Bus;
 use devices::BusAccessInfo;
 use devices::BusDeviceSync;
+use devices::BusType;
 use devices::CrosvmDeviceId;
 use devices::DestinationShorthand;
 use devices::DeviceId;
@@ -203,8 +204,8 @@ fn irq_event_tokens() {
 fn finalize_devices() {
     let mut chip = get_chip(1);
 
-    let mmio_bus = Bus::new();
-    let io_bus = Bus::new();
+    let mmio_bus = Bus::new(BusType::Mmio);
+    let io_bus = Bus::new(BusType::Io);
     let mut resources = SystemAllocator::new(
         SystemAllocatorConfig {
             io: Some(AddressRange {
@@ -459,8 +460,8 @@ fn lowest_priority_destination() {
 fn broadcast_eoi() {
     let mut chip = get_chip(1);
 
-    let mmio_bus = Bus::new();
-    let io_bus = Bus::new();
+    let mmio_bus = Bus::new(BusType::Mmio);
+    let io_bus = Bus::new(BusType::Io);
     let mut resources = SystemAllocator::new(
         SystemAllocatorConfig {
             io: Some(AddressRange {

riscv64/src/lib.rs

@@ -24,6 +24,7 @@ use devices::serial_device::SerialParameters;
 use devices::Bus;
 use devices::BusDeviceObj;
 use devices::BusError;
+use devices::BusType;
 use devices::IrqChipRiscv64;
 use devices::PciAddress;
 use devices::PciConfigMmio;
@@ -202,10 +203,10 @@ impl arch::LinuxArch for Riscv64 {
 
         let mem = vm.get_memory().clone();
 
-        let mmio_bus = Arc::new(Bus::new());
+        let mmio_bus = Arc::new(Bus::new(BusType::Mmio));
 
         // Riscv doesn't really use the io bus like x86, so just create an empty bus.
-        let io_bus = Arc::new(Bus::new());
+        let io_bus = Arc::new(Bus::new(BusType::Io));
 
         let com_evt_1_3 = Event::new().map_err(Error::CreateEvent)?;
         let com_evt_2_4 = Event::new().map_err(Error::CreateEvent)?;

src/crosvm/sys/linux.rs

@@ -98,6 +98,7 @@ use devices::virtio::VirtioDeviceType;
 use devices::virtio::VirtioTransportType;
 use devices::Bus;
 use devices::BusDeviceObj;
+use devices::BusType;
 use devices::CoIommuDev;
 #[cfg(feature = "usb")]
 use devices::DeviceProvider;
@@ -3558,11 +3559,17 @@ fn run_control<V: VmArch + 'static, Vcpu: VcpuArch + 'static>(
     // inside `linux`. If the checks below fail, then some other thread is probably still running
     // and needs to be explicitly stopped before dropping `linux` to ensure devices actually get
     // cleaned up.
-    match Arc::try_unwrap(std::mem::replace(&mut linux.mmio_bus, Arc::new(Bus::new()))) {
+    match Arc::try_unwrap(std::mem::replace(
+        &mut linux.mmio_bus,
+        Arc::new(Bus::new(BusType::Mmio)),
+    )) {
         Ok(_) => {}
         Err(_) => panic!("internal error: mmio_bus had more than one reference at shutdown"),
     }
-    match Arc::try_unwrap(std::mem::replace(&mut linux.io_bus, Arc::new(Bus::new()))) {
+    match Arc::try_unwrap(std::mem::replace(
+        &mut linux.io_bus,
+        Arc::new(Bus::new(BusType::Io)),
+    )) {
         Ok(_) => {}
         Err(_) => panic!("internal error: io_bus had more than one reference at shutdown"),
     }

x86_64/src/lib.rs

@@ -69,9 +69,11 @@ use base::TubeError;
 use chrono::Utc;
 pub use cpuid::adjust_cpuid;
 pub use cpuid::CpuIdContext;
+use devices::Bus;
 use devices::BusDevice;
 use devices::BusDeviceObj;
 use devices::BusResumeDevice;
+use devices::BusType;
 use devices::Debugcon;
 use devices::FwCfgParameters;
 use devices::IrqChip;
@@ -727,8 +729,8 @@ impl arch::LinuxArch for X8664arch {
         }
 
         let pcie_vcfg_range = Self::get_pcie_vcfg_mmio_range(&mem, &pcie_cfg_mmio_range);
-        let mmio_bus = Arc::new(devices::Bus::new());
-        let io_bus = Arc::new(devices::Bus::new());
+        let mmio_bus = Arc::new(Bus::new(BusType::Mmio));
+        let io_bus = Arc::new(Bus::new(BusType::Io));
 
         let (pci_devices, devs): (Vec<_>, Vec<_>) = devs
             .into_iter()
@@ -1519,7 +1521,7 @@ impl X8664arch {
         pflash_image: File,
         block_size: u32,
         bios_size: u64,
-        mmio_bus: &devices::Bus,
+        mmio_bus: &Bus,
         jail: Option<Minijail>,
         #[cfg(feature = "swap")] swap_controller: &mut Option<swap::SwapController>,
     ) -> Result<()> {
@@ -1705,7 +1707,7 @@ impl X8664arch {
     /// * - `fw_cfg_parameters` - command-line specified data to add to device. May contain
     /// all None fields if user did not specify data to add to the device
     fn setup_fw_cfg_device(
-        io_bus: &devices::Bus,
+        io_bus: &Bus,
         fw_cfg_parameters: Vec<FwCfgParameters>,
         bootorder_fw_cfg_blob: Vec<u8>,
         fw_cfg_jail: Option<Minijail>,
@@ -1775,7 +1777,7 @@ impl X8664arch {
     /// * - `pit_uses_speaker_port` - does the PIT use port 0x61 for the PC speaker
     /// * - `vm_evt_wrtube` - the event object which should receive exit events
     pub fn setup_legacy_i8042_device(
-        io_bus: &devices::Bus,
+        io_bus: &Bus,
         pit_uses_speaker_port: bool,
         vm_evt_wrtube: SendTube,
     ) -> Result<()> {
@@ -1798,7 +1800,7 @@ impl X8664arch {
     /// * - `io_bus` - the IO bus object
     /// * - `mem_size` - the size in bytes of physical ram for the guest
     pub fn setup_legacy_cmos_device(
-        io_bus: &devices::Bus,
+        io_bus: &Bus,
         irq_chip: &mut dyn IrqChipX86_64,
         vm_control: Tube,
         mem_size: u64,
@@ -1860,7 +1862,7 @@ impl X8664arch {
     pub fn setup_acpi_devices(
         pci_root: Arc<Mutex<PciRoot>>,
         mem: &GuestMemory,
-        io_bus: &devices::Bus,
+        io_bus: &Bus,
         resources: &mut SystemAllocator,
         suspend_evt: Event,
         vm_evt_wrtube: SendTube,
@@ -1868,7 +1870,7 @@ impl X8664arch {
         irq_chip: &mut dyn IrqChip,
         sci_irq: u32,
         battery: (Option<BatteryType>, Option<Minijail>),
-        #[cfg_attr(windows, allow(unused_variables))] mmio_bus: &devices::Bus,
+        #[cfg_attr(windows, allow(unused_variables))] mmio_bus: &Bus,
         max_bus: u8,
         resume_notify_devices: &mut Vec<Arc<Mutex<dyn BusResumeDevice>>>,
         #[cfg(feature = "swap")] swap_controller: &mut Option<swap::SwapController>,
@@ -2128,7 +2130,7 @@ impl X8664arch {
     pub fn setup_serial_devices(
         protection_type: ProtectionType,
         irq_chip: &mut dyn IrqChip,
-        io_bus: &devices::Bus,
+        io_bus: &Bus,
         serial_parameters: &BTreeMap<(SerialHardware, u8), SerialParameters>,
         serial_jail: Option<Minijail>,
         #[cfg(feature = "swap")] swap_controller: &mut Option<swap::SwapController>,
@@ -2165,7 +2167,7 @@ impl X8664arch {
 
     fn setup_debugcon_devices(
         protection_type: ProtectionType,
-        io_bus: &devices::Bus,
+        io_bus: &Bus,
         serial_parameters: &BTreeMap<(SerialHardware, u8), SerialParameters>,
         debugcon_jail: Option<Minijail>,
         #[cfg(feature = "swap")] swap_controller: &mut Option<swap::SwapController>,

x86_64/tests/integration/main.rs

@@ -16,6 +16,8 @@ use arch::LinuxArch;
 use arch::SmbiosOptions;
 use base::Event;
 use base::Tube;
+use devices::Bus;
+use devices::BusType;
 use devices::IrqChipX86_64;
 use devices::PciConfigIo;
 use hypervisor::CpuConfigX86_64;
@@ -107,8 +109,8 @@ where
 
     let mut irq_chip = create_irq_chip(vm.try_clone().expect("failed to clone vm"), 1, device_tube);
 
-    let mmio_bus = Arc::new(devices::Bus::new());
-    let io_bus = Arc::new(devices::Bus::new());
+    let mmio_bus = Arc::new(Bus::new(BusType::Mmio));
+    let io_bus = Arc::new(Bus::new(BusType::Io));
     let (exit_evt_wrtube, _) = Tube::directional_pair().unwrap();
 
     let mut control_tubes = vec![TaggedControlTube::VmIrq(irqchip_tube)];