devices: pci: don't reserve bridge window for hot added bridges

We do not need to reserve bridge window for hot added bridges
because guest will do that for us. This chunk of bridge window is
also reserved by pcie root port, so no extra MMIO allocation is
needed. So we config zeros sized window here for hot added bridges.

BUG=b:199986018
TEST=./tools/presubmit

Change-Id: I7e64b7596e1b5c060cf628a488e2399ae9c257fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3709794
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>

devices/src/pci/pcie/pci_bridge.rs

@@ -333,17 +333,22 @@ impl PciDevice for PciBridge {
         let mut window_size: u64 = 0;
         let mut pref_window_base: u64 = u64::MAX;
         let mut pref_window_size: u64 = 0;
+        let hotplug_implemented = self.device.lock().hotplug_implemented();
+        let hotplugged = self.device.lock().hotplugged();
 
-        if self.device.lock().hotplug_implemented() {
-            // Bridge for children hotplug, get desired bridge window size and reserve
-            // it for guest OS use
+        if hotplug_implemented || hotplugged {
+            // If bridge is for children hotplug, get desired bridge window size and reserve
+            // it for guest OS use.
+            // If bridge is hotplugged into the system, get the desired bridge window size
+            // from host.
             let (win_size, pref_win_size) = self.device.lock().get_bridge_window_size();
             window_size = win_size;
             pref_window_size = pref_win_size;
         } else {
+            // Bridge has children connected, get bridge window size from children
             let mut window_end: u64 = 0;
             let mut pref_window_end: u64 = 0;
-            // Bridge has children connected, get bridge window size from children
+
             for &BarRange {
                 addr,
                 size,
@@ -366,59 +371,21 @@ impl PciDevice for PciBridge {
             }
         }
 
-        if window_size == 0 {
-            // Allocate at least 2MB bridge winodw
-            window_size = BR_MEM_MINIMUM;
-        }
-        // if window_base isn't set, allocate a new one
-        if window_base == u64::MAX {
-            // align window_size to 1MB
-            if window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                window_size = (window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
+        if !hotplugged {
+            // Only static bridge needs to locate their window's position. Hotplugged bridge's
+            // window will be handled by guest kernel.
+            if window_size == 0 {
+                // Allocate at least 2MB bridge winodw
+                window_size = BR_MEM_MINIMUM;
             }
-            match resources.mmio_allocator(MmioType::Low).allocate_with_align(
-                window_size,
-                Alloc::PciBridgeWindow {
-                    bus: address.bus,
-                    dev: address.dev,
-                    func: address.func,
-                },
-                "pci_bridge_window".to_string(),
-                BR_WINDOW_ALIGNMENT,
-            ) {
-                Ok(addr) => window_base = addr,
-                Err(e) => warn!(
-                    "{} failed to allocate bridge window: {}",
-                    self.debug_label(),
-                    e
-                ),
-            }
-        } else {
-            // align window_base to 1MB
-            if window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                window_size += window_base - (window_base & BR_WINDOW_MASK);
+            // if window_base isn't set, allocate a new one
+            if window_base == u64::MAX {
                 // align window_size to 1MB
                 if window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
                     window_size = (window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
                 }
-                window_base &= BR_WINDOW_MASK;
-            }
-        }
-
-        if pref_window_size == 0 {
-            // Allocate at least 2MB prefetch bridge window
-            pref_window_size = BR_MEM_MINIMUM;
-        }
-        // if pref_window_base isn't set, allocate a new one
-        if pref_window_base == u64::MAX {
-            // align pref_window_size to 1MB
-            if pref_window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                pref_window_size = (pref_window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
-            }
-            match resources
-                .mmio_allocator(MmioType::High)
-                .allocate_with_align(
-                    pref_window_size,
+                match resources.mmio_allocator(MmioType::Low).allocate_with_align(
+                    window_size,
                     Alloc::PciBridgeWindow {
                         bus: address.bus,
                         dev: address.dev,
@@ -427,23 +394,74 @@ impl PciDevice for PciBridge {
                     "pci_bridge_window".to_string(),
                     BR_WINDOW_ALIGNMENT,
                 ) {
-                Ok(addr) => pref_window_base = addr,
-                Err(e) => warn!(
-                    "{} failed to allocate bridge window: {}",
-                    self.debug_label(),
-                    e
-                ),
+                    Ok(addr) => window_base = addr,
+                    Err(e) => warn!(
+                        "{} failed to allocate bridge window: {}",
+                        self.debug_label(),
+                        e
+                    ),
+                }
+            } else {
+                // align window_base to 1MB
+                if window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                    window_size += window_base - (window_base & BR_WINDOW_MASK);
+                    // align window_size to 1MB
+                    if window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                        window_size = (window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
+                    }
+                    window_base &= BR_WINDOW_MASK;
+                }
             }
-        } else {
-            // align pref_window_base to 1MB
-            if pref_window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
-                pref_window_size += pref_window_base - (pref_window_base & BR_WINDOW_MASK);
+
+            if pref_window_size == 0 {
+                // Allocate at least 2MB prefetch bridge window
+                pref_window_size = BR_MEM_MINIMUM;
+            }
+            // if pref_window_base isn't set, allocate a new one
+            if pref_window_base == u64::MAX {
                 // align pref_window_size to 1MB
                 if pref_window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
                     pref_window_size =
                         (pref_window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
                 }
-                pref_window_base &= BR_WINDOW_MASK;
+                match resources
+                    .mmio_allocator(MmioType::High)
+                    .allocate_with_align(
+                        pref_window_size,
+                        Alloc::PciBridgeWindow {
+                            bus: address.bus,
+                            dev: address.dev,
+                            func: address.func,
+                        },
+                        "pci_bridge_window".to_string(),
+                        BR_WINDOW_ALIGNMENT,
+                    ) {
+                    Ok(addr) => window_base = addr,
+                    Err(e) => warn!(
+                        "{} failed to allocate bridge window: {}",
+                        self.debug_label(),
+                        e
+                    ),
+                }
+            } else {
+                // align pref_window_base to 1MB
+                if pref_window_base & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                    pref_window_size += pref_window_base - (pref_window_base & BR_WINDOW_MASK);
+                    // align pref_window_size to 1MB
+                    if pref_window_size & (BR_WINDOW_ALIGNMENT - 1) != 0 {
+                        pref_window_size =
+                            (pref_window_size + BR_WINDOW_ALIGNMENT - 1) & BR_WINDOW_MASK;
+                    }
+                    pref_window_base &= BR_WINDOW_MASK;
+                }
+            }
+        } else {
+            // 0 is Ok here because guest will relocate the bridge window
+            if window_size > 0 {
+                window_base = 0;
+            }
+            if pref_window_size > 0 {
+                pref_window_base = 0;
             }
         }
 

devices/src/pci/pcie/pcie_device.rs

@@ -20,7 +20,7 @@ pub trait PcieDevice: Send {
     ) -> std::result::Result<PciAddress, PciDeviceError>;
     fn read_config(&self, reg_idx: usize, data: &mut u32);
     fn write_config(&mut self, reg_idx: usize, offset: u64, data: &[u8]);
-    fn clone_interrupt(&mut self, msix_config: Arc<Mutex<MsiConfig>>);
+    fn clone_interrupt(&mut self, msi_config: Arc<Mutex<MsiConfig>>);
     fn get_caps(&self) -> Vec<Box<dyn PciCapability>>;
     fn set_capability_reg_idx(&mut self, id: PciCapabilityID, reg_idx: usize);
     fn get_bus_range(&self) -> Option<PciBridgeBusRange> {
@@ -33,6 +33,9 @@ pub trait PcieDevice: Send {
     /// Return false, the children pci devices should be connected statically
     fn hotplug_implemented(&self) -> bool;
 
+    /// This function returns true if this pcie device is hotplugged into the system
+    fn hotplugged(&self) -> bool;
+
     /// Get bridge window size to cover children's mmio size
     /// (u64, u64) -> (non_prefetchable window size, prefetchable_window_size)
     fn get_bridge_window_size(&self) -> (u64, u64);

devices/src/pci/pcie/pcie_rp.rs

@@ -1,7 +1,6 @@
 // Copyright 2021 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-
 use std::collections::BTreeMap;
 use std::sync::Arc;
 use sync::Mutex;
@@ -115,6 +114,10 @@ impl PcieDevice for PcieRootPort {
         self.pcie_port.hotplug_implemented()
     }
 
+    fn hotplugged(&self) -> bool {
+        false
+    }
+
     fn get_bridge_window_size(&self) -> (u64, u64) {
         self.pcie_port.get_bridge_window_size()
     }

devices/src/pci/pcie/pcie_switch.rs

@@ -2,8 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+use std::sync::Arc;
+use sync::Mutex;
+
 use crate::pci::pci_configuration::PciCapabilityID;
-use crate::pci::{PciAddress, PciCapability, PciDeviceError};
+use crate::pci::{MsiConfig, PciAddress, PciCapability, PciDeviceError};
 
 use crate::pci::pcie::pci_bridge::PciBridgeBusRange;
 use crate::pci::pcie::pcie_device::{PciPmcCap, PcieCap, PcieDevice};
@@ -17,11 +20,12 @@ const PCIE_DP_DID: u16 = 0x3510;
 
 pub struct PcieUpstreamPort {
     pcie_port: PciePort,
+    hotplugged: bool,
 }
 
 impl PcieUpstreamPort {
     /// Constructs a new PCIE upstream port
-    pub fn new(primary_bus_num: u8, secondary_bus_num: u8) -> Self {
+    pub fn new(primary_bus_num: u8, secondary_bus_num: u8, hotplugged: bool) -> Self {
         PcieUpstreamPort {
             pcie_port: PciePort::new(
                 PCIE_UP_DID,
@@ -30,12 +34,14 @@ impl PcieUpstreamPort {
                 secondary_bus_num,
                 false,
             ),
+            hotplugged,
         }
     }
 
-    pub fn new_from_host(pcie_host: PcieHostPort) -> Self {
+    pub fn new_from_host(pcie_host: PcieHostPort, hotplugged: bool) -> Self {
         PcieUpstreamPort {
             pcie_port: PciePort::new_from_host(pcie_host, false),
+            hotplugged,
         }
     }
 }
@@ -56,6 +62,10 @@ impl PcieDevice for PcieUpstreamPort {
         self.pcie_port.allocate_address(resources)
     }
 
+    fn clone_interrupt(&mut self, msi_config: Arc<Mutex<MsiConfig>>) {
+        self.pcie_port.clone_interrupt(msi_config);
+    }
+
     fn read_config(&self, reg_idx: usize, data: &mut u32) {
         self.pcie_port.read_config(reg_idx, data);
     }
@@ -79,10 +89,18 @@ impl PcieDevice for PcieUpstreamPort {
         self.pcie_port.get_bus_range()
     }
 
+    fn get_removed_devices(&self) -> Vec<PciAddress> {
+        Vec::new()
+    }
+
     fn hotplug_implemented(&self) -> bool {
         false
     }
 
+    fn hotplugged(&self) -> bool {
+        self.hotplugged
+    }
+
     fn get_bridge_window_size(&self) -> (u64, u64) {
         self.pcie_port.get_bridge_window_size()
     }
@@ -90,11 +108,12 @@ impl PcieDevice for PcieUpstreamPort {
 
 pub struct PcieDownstreamPort {
     pcie_port: PciePort,
+    hotplugged: bool,
 }
 
 impl PcieDownstreamPort {
     /// Constructs a new PCIE downstream port
-    pub fn new(primary_bus_num: u8, secondary_bus_num: u8) -> Self {
+    pub fn new(primary_bus_num: u8, secondary_bus_num: u8, hotplugged: bool) -> Self {
         PcieDownstreamPort {
             pcie_port: PciePort::new(
                 PCIE_DP_DID,
@@ -103,12 +122,14 @@ impl PcieDownstreamPort {
                 secondary_bus_num,
                 false,
             ),
+            hotplugged,
         }
     }
 
-    pub fn new_from_host(pcie_host: PcieHostPort) -> Self {
+    pub fn new_from_host(pcie_host: PcieHostPort, hotplugged: bool) -> Self {
         PcieDownstreamPort {
             pcie_port: PciePort::new_from_host(pcie_host, false),
+            hotplugged,
         }
     }
 }
@@ -129,6 +150,10 @@ impl PcieDevice for PcieDownstreamPort {
         self.pcie_port.allocate_address(resources)
     }
 
+    fn clone_interrupt(&mut self, msi_config: Arc<Mutex<MsiConfig>>) {
+        self.pcie_port.clone_interrupt(msi_config);
+    }
+
     fn read_config(&self, reg_idx: usize, data: &mut u32) {
         self.pcie_port.read_config(reg_idx, data);
     }
@@ -152,10 +177,18 @@ impl PcieDevice for PcieDownstreamPort {
         self.pcie_port.get_bus_range()
     }
 
+    fn get_removed_devices(&self) -> Vec<PciAddress> {
+        Vec::new()
+    }
+
     fn hotplug_implemented(&self) -> bool {
         false
     }
 
+    fn hotplugged(&self) -> bool {
+        self.hotplugged
+    }
+
     fn get_bridge_window_size(&self) -> (u64, u64) {
         self.pcie_port.get_bridge_window_size()
     }